Merge lp:~afrantzis/mir/fix-1347053 into lp:mir

[Alexandros Frantzis]

mesa,android: Don't propagate exceptions to graphics driver code

mesa,android: Don't propagate exceptions to graphics driver code

Report the error in a way the driver expects, in our specific cases by returning an error code.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1811
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2290/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1170
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1158
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/812
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/812/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/813
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/813/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/93
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/93/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2280
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10853

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2290/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

Sometimes these errors are useful to see when we get logs back... for the sake of debuggability, could we dump the message to stderr?

(this also reminds me that a native window logger would be pretty useful for android anyways...)

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

If possible, please replace:
   catch (...)
with:
   catch (std::exception)
so that we may in future define fatal error exception base classes that don't get accidentally caught by blanket catches like this.

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

> could we dump the message to stderr?

Done. Just printing to stderr for now, I didn't introduce any special reporting or logging mechanism.

> catch (std::exception)

Done.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1812
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2302/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1189
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1171
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/824
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/824/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/825
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/825/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/106
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/106/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2292
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10937

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2302/rebuild

review: Approve (continuous-integration)

[Robert Carr (robertcarr)]

Ok!

review: Approve

[Kevin DuBois (kdub)]

okay, lgtm, thanks

review: Approve

[Daniel van Vugt (vanvugt)]

Minor issue: The same string and expression is duplicated multiple times in a couple of source files. It could easily be un-duplicated on a per-file basis at least:

    std::cerr << "Caught exception at Mir/EGL driver boundary: "
              << boost::diagnostic_information(e) << std::endl;

Although I appreciate the benefit of seeing what's being emitted in-situ. You would lose that with a global or a function...

Think about it and optionally implement it. Then Approved for me.

[Alexandros Frantzis (afrantzis)]

> It could easily be un-duplicated on a per-file basis at least:

Done.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1813
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2309/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1194
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1200
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1182
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/831
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/831/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/832
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/832/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/117
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/117/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2300
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10976

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2309/rebuild

review: Approve (continuous-integration)

=== modified file 'src/client/mesa/native_surface.cpp'
--- src/client/mesa/native_surface.cpp	2014-03-06 06:05:17 +0000
+++ src/client/mesa/native_surface.cpp	2014-08-05 07:09:25 +0000
@@ -20,6 +20,9 @@
 #include "../client_buffer.h"
 #include "native_surface.h"
 
+#include <iostream>
+#include <boost/exception/diagnostic_information.hpp> 
+
 namespace mclm=mir::client::mesa;
 
 namespace
@@ -43,6 +46,12 @@
     auto s = static_cast<mclm::NativeSurface*>(surface);
     return s->set_swapinterval(interval);
 }
+
+void report_exception_at_driver_boundary(std::exception const& e)
+{
+    std::cerr << "Caught exception at Mir/EGL driver boundary: "
+              << boost::diagnostic_information(e) << std::endl;
+}
 }
 
 mclm::NativeSurface::NativeSurface(ClientSurface& surface)
@@ -54,6 +63,7 @@
 }
 
 int mclm::NativeSurface::advance_buffer(MirBufferPackage* buffer_package)
+try
 {
     /*
      * At present dri2_create_mir_window_surface will trigger
@@ -72,15 +82,27 @@
     memcpy(buffer_package, buffer_to_driver.get(), sizeof(MirBufferPackage));
     return MIR_MESA_TRUE;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return MIR_MESA_FALSE;
+}
 
 int mclm::NativeSurface::get_parameters(MirSurfaceParameters* surface_parameters)
+try
 {
     auto params = surface.get_parameters();
     memcpy(surface_parameters, &params, sizeof(MirSurfaceParameters));
     return MIR_MESA_TRUE;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return MIR_MESA_FALSE;
+}
 
 int mclm::NativeSurface::set_swapinterval(int interval)
+try
 {
     if ((interval < 0) || (interval > 1))
         return MIR_MESA_FALSE;
@@ -88,3 +110,8 @@
     surface.request_and_wait_for_configure(mir_surface_attrib_swapinterval, interval);
     return MIR_MESA_TRUE;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return MIR_MESA_FALSE;
+}

=== modified file 'src/shared/graphics/android/mir_native_window.cpp'
--- src/shared/graphics/android/mir_native_window.cpp	2014-06-17 18:24:48 +0000
+++ src/shared/graphics/android/mir_native_window.cpp	2014-08-05 07:09:25 +0000
@@ -20,13 +20,15 @@
 #include "mir/graphics/android/android_driver_interpreter.h"
 #include "mir/graphics/android/sync_fence.h"
 
+#include <iostream>
+#include <boost/exception/diagnostic_information.hpp> 
+
 namespace mg=mir::graphics;
 namespace mga=mir::graphics::android;
 
 namespace
 {
 
-
 static int query_static(const ANativeWindow* anw, int key, int* value);
 static int perform_static(ANativeWindow* anw, int key, ...);
 static int setSwapInterval_static (struct ANativeWindow* window, int interval);
@@ -121,6 +123,12 @@
     return self->cancelBuffer(buffer, fence_fd);
 }
 
+void report_exception_at_driver_boundary(std::exception const& e)
+{
+    std::cerr << "Caught exception at Mir/EGL driver boundary: "
+              << boost::diagnostic_information(e) << std::endl;
+}
+
 }
 
 mga::MirNativeWindow::MirNativeWindow(std::shared_ptr<AndroidDriverInterpreter> const& interpreter)
@@ -158,42 +166,73 @@
 }
 
 int mga::MirNativeWindow::dequeueBuffer(struct ANativeWindowBuffer** buffer_to_driver, int* fence_fd)
+try
 {
     auto buffer = driver_interpreter->driver_requests_buffer();
 
-    //driver is responsible for closing this native handle
+    //EGL driver is responsible for closing this native handle
     *fence_fd = buffer->copy_fence();
     *buffer_to_driver = buffer->anwb();
     return 0;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}
 
 int mga::MirNativeWindow::dequeueBufferAndWait(struct ANativeWindowBuffer** buffer_to_driver)
+try
 {
     auto buffer = driver_interpreter->driver_requests_buffer();
     *buffer_to_driver = buffer->anwb();
     buffer->ensure_available_for(mga::BufferAccess::write);
     return 0;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}
 
 int mga::MirNativeWindow::queueBuffer(struct ANativeWindowBuffer* buffer, int fence)
+try
 {
     driver_interpreter->driver_returns_buffer(buffer, fence);
     return 0;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}
 
 int mga::MirNativeWindow::cancelBuffer(struct ANativeWindowBuffer* buffer, int fence)
+try
 {
     driver_interpreter->driver_returns_buffer(buffer, fence);
     return 0;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}
 
 int mga::MirNativeWindow::query(int key, int* value) const
+try
 {
     *value = driver_interpreter->driver_requests_info(key);
     return 0;
 }
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}
 
 int mga::MirNativeWindow::perform(int key, va_list arg_list )
+try
 {
     int ret = 0;
     va_list args;
@@ -213,4 +252,8 @@
     va_end(args);
     return ret;
 }
-
+catch (std::exception const& e)
+{
+    report_exception_at_driver_boundary(e);
+    return -1;
+}

=== modified file 'tests/unit-tests/client/android/test_android_native_window.cpp'
--- tests/unit-tests/client/android/test_android_native_window.cpp	2014-07-11 13:20:14 +0000
+++ tests/unit-tests/client/android/test_android_native_window.cpp	2014-08-05 07:09:25 +0000
@@ -49,41 +49,37 @@
 
     std::shared_ptr<mir::graphics::NativeBuffer> buffer;
 };
-}
 
 class AndroidNativeWindowTest : public ::testing::Test
 {
 protected:
-    virtual void SetUp()
-    {
-        using namespace testing;
-
-        mock_driver_interpreter = std::make_shared<NiceMock<MockAndroidDriverInterpreter>>();
-    }
-
-    std::shared_ptr<MockAndroidDriverInterpreter> mock_driver_interpreter;
+    std::shared_ptr<MockAndroidDriverInterpreter> const mock_driver_interpreter =
+        std::make_shared<testing::NiceMock<MockAndroidDriverInterpreter>>();
+    mga::MirNativeWindow mir_native_window{mock_driver_interpreter};
+    ANativeWindow& window = mir_native_window;
+    int const failure_code{-1};
 };
 
+}
+
 TEST_F(AndroidNativeWindowTest, native_window_swapinterval)
 {
     using namespace testing;
 
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-
-    ASSERT_NE(nullptr, window->setSwapInterval);
-    EXPECT_CALL(*mock_driver_interpreter, sync_to_display(true))
-        .Times(1);
-    window->setSwapInterval(window.get(), 1);
-    Mock::VerifyAndClearExpectations(window.get());
-
-    EXPECT_CALL(*mock_driver_interpreter, sync_to_display(true))
-        .Times(1);
-    window->setSwapInterval(window.get(), 2);
-    Mock::VerifyAndClearExpectations(window.get());
+    ASSERT_NE(nullptr, window.setSwapInterval);
+    EXPECT_CALL(*mock_driver_interpreter, sync_to_display(true))
+        .Times(1);
+    window.setSwapInterval(&window, 1);
+    Mock::VerifyAndClearExpectations(mock_driver_interpreter.get());
+
+    EXPECT_CALL(*mock_driver_interpreter, sync_to_display(true))
+        .Times(1);
+    window.setSwapInterval(&window, 2);
+    Mock::VerifyAndClearExpectations(mock_driver_interpreter.get());
 
     EXPECT_CALL(*mock_driver_interpreter, sync_to_display(false))
         .Times(1);
-    window->setSwapInterval(window.get(), 0);
+    window.setSwapInterval(&window, 0);
 }
 
 /* Query hook tests */
@@ -91,15 +87,15 @@
 {
     using namespace testing;
 
-    int returned_width, width = 271828;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
+    int returned_width;
+    int const width = 271828;
 
-    ASSERT_NE(nullptr, window->query);
+    ASSERT_NE(nullptr, window.query);
     EXPECT_CALL(*mock_driver_interpreter, driver_requests_info(NATIVE_WINDOW_WIDTH))
         .Times(1)
         .WillOnce(Return(width));
 
-    window->query(window.get(), NATIVE_WINDOW_WIDTH ,&returned_width);
+    window.query(&window, NATIVE_WINDOW_WIDTH ,&returned_width);
 
     EXPECT_EQ(width, returned_width);
 }
@@ -107,24 +103,22 @@
 /* perform hook tests */
 TEST_F(AndroidNativeWindowTest, native_window_perform_hook_callable)
 {
-    int format = 4;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
+    int const format = 4;
 
     EXPECT_CALL(*mock_driver_interpreter, dispatch_driver_request_format(format))
         .Times(1);
 
-    ASSERT_NE(nullptr, window->perform);
-    window->perform(window.get(), NATIVE_WINDOW_SET_BUFFERS_FORMAT, format);
+    ASSERT_NE(nullptr, window.perform);
+    window.perform(&window, NATIVE_WINDOW_SET_BUFFERS_FORMAT, format);
 }
 
 /* setSwapInterval hook tests */
 TEST_F(AndroidNativeWindowTest, native_window_setswapinterval_hook_callable)
 {
-    int swap = 2;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
+    int const swap = 2;
 
-    ASSERT_NE(nullptr, window->setSwapInterval);
-    window->setSwapInterval(window.get(), swap);
+    ASSERT_NE(nullptr, window.setSwapInterval);
+    window.setSwapInterval(&window, swap);
 }
 
 /* dequeue hook tests */
@@ -132,10 +126,9 @@
 {
     ANativeWindowBuffer* returned_buffer;
     int fence_fd;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->dequeueBuffer);
-    window->dequeueBuffer(window.get(), &returned_buffer, &fence_fd);
+    ASSERT_NE(nullptr, window.dequeueBuffer);
+    window.dequeueBuffer(&window, &returned_buffer, &fence_fd);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_dequeue_returns_right_buffer)
@@ -151,11 +144,9 @@
         .Times(1)
         .WillOnce(Return(mock_buffer.get()));
 
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-
     int fence_fd;
     ANativeWindowBuffer* returned_buffer;
-    window->dequeueBuffer(window.get(), &returned_buffer, &fence_fd);
+    window.dequeueBuffer(&window, &returned_buffer, &fence_fd);
 
     EXPECT_EQ(mock_buffer->anwb(), returned_buffer);
     EXPECT_EQ(fake_fd, fence_fd);
@@ -164,10 +155,9 @@
 TEST_F(AndroidNativeWindowTest, native_window_dequeue_deprecated_hook_callable)
 {
     ANativeWindowBuffer* tmp;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->dequeueBuffer_DEPRECATED);
-    window->dequeueBuffer_DEPRECATED(window.get(), &tmp);
+    ASSERT_NE(nullptr, window.dequeueBuffer_DEPRECATED);
+    window.dequeueBuffer_DEPRECATED(&window, &tmp);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_dequeue_deprecated_returns_right_buffer)
@@ -176,7 +166,6 @@
 
     ANativeWindowBuffer* returned_buffer;
     auto mock_buffer = std::make_shared<NiceMock<mtd::MockAndroidNativeBuffer>>();
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
     EXPECT_CALL(*mock_driver_interpreter, driver_requests_buffer())
         .Times(1)
@@ -186,7 +175,7 @@
     EXPECT_CALL(*mock_buffer, copy_fence())
         .Times(0);
 
-    window->dequeueBuffer_DEPRECATED(window.get(), &returned_buffer);
+    window.dequeueBuffer_DEPRECATED(&window, &returned_buffer);
     EXPECT_EQ(mock_buffer->anwb(), returned_buffer);
 }
 
@@ -194,10 +183,9 @@
 TEST_F(AndroidNativeWindowTest, native_window_queue_hook_callable)
 {
     ANativeWindowBuffer* tmp = nullptr;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->queueBuffer);
-    window->queueBuffer(window.get(), tmp, -1);
+    ASSERT_NE(nullptr, window.queueBuffer);
+    window.queueBuffer(&window, tmp, -1);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_queue_passes_buffer_back)
@@ -205,79 +193,69 @@
     using namespace testing;
     ANativeWindowBuffer buffer;
     int fence_fd = 33;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
     EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, fence_fd))
         .Times(1);
 
-    window->queueBuffer(window.get(), &buffer, fence_fd);
+    window.queueBuffer(&window, &buffer, fence_fd);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_hook_callable)
 {
     ANativeWindowBuffer* tmp = nullptr;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->queueBuffer_DEPRECATED);
-    window->queueBuffer_DEPRECATED(window.get(), tmp);
+    ASSERT_NE(nullptr, window.queueBuffer_DEPRECATED);
+    window.queueBuffer_DEPRECATED(&window, tmp);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_passes_buffer_back)
 {
     using namespace testing;
     ANativeWindowBuffer buffer;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
     EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer,_))
         .Times(1);
 
-    window->queueBuffer_DEPRECATED(window.get(), &buffer);
+    window.queueBuffer_DEPRECATED(&window, &buffer);
 }
 
 /* cancel hook tests */
 TEST_F(AndroidNativeWindowTest, native_window_cancel_hooks_callable)
 {
     ANativeWindowBuffer* tmp = nullptr;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->cancelBuffer_DEPRECATED);
-    ASSERT_NE(nullptr, window->cancelBuffer);
-    window->cancelBuffer_DEPRECATED(window.get(), tmp);
-    window->cancelBuffer(window.get(), tmp, -1);
+    ASSERT_NE(nullptr, window.cancelBuffer_DEPRECATED);
+    ASSERT_NE(nullptr, window.cancelBuffer);
+    window.cancelBuffer_DEPRECATED(&window, tmp);
+    window.cancelBuffer(&window, tmp, -1);
 }
 
 /* lock hook tests */
 TEST_F(AndroidNativeWindowTest, native_window_lock_hook_callable)
 {
     ANativeWindowBuffer* tmp = 0x0;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    ASSERT_NE(nullptr, window->lockBuffer_DEPRECATED);
-    window->lockBuffer_DEPRECATED(window.get(), tmp);
+    ASSERT_NE(nullptr, window.lockBuffer_DEPRECATED);
+    window.lockBuffer_DEPRECATED(&window, tmp);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_incref_hook_callable)
 {
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-
-    ASSERT_NE(nullptr, window->common.incRef);
-    window->common.incRef(NULL);
+    ASSERT_NE(nullptr, window.common.incRef);
+    window.common.incRef(NULL);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_decref_hook_callable)
 {
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-
-    ASSERT_NE(nullptr, window->common.decRef);
-    window->common.decRef(NULL);
+    ASSERT_NE(nullptr, window.common.decRef);
+    window.common.decRef(NULL);
 }
 
 TEST_F(AndroidNativeWindowTest, native_window_dequeue_deprecated_has_proper_rc)
 {
     ANativeWindowBuffer* tmp;
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
 
-    auto ret = window->dequeueBuffer_DEPRECATED(window.get(), &tmp);
+    auto ret = window.dequeueBuffer_DEPRECATED(&window, &tmp);
     EXPECT_EQ(0, ret);
 }
 
@@ -286,9 +264,7 @@
     ANativeWindowBuffer* tmp;
     int fencefd;
 
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-
-    auto ret = window->dequeueBuffer(window.get(), &tmp, &fencefd);
+    auto ret = window.dequeueBuffer(&window, &tmp, &fencefd);
     EXPECT_EQ(0, ret);
 }
 
@@ -301,7 +277,69 @@
     EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, _))
         .Times(1);
 
-    std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
-    auto rc = window->cancelBuffer(window.get(), &buffer, fence_fd);
+    auto rc = window.cancelBuffer(&window, &buffer, fence_fd);
     EXPECT_EQ(0, rc);
 }
+
+TEST_F(AndroidNativeWindowTest, returns_error_on_dequeue_buffer_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_driver_interpreter, driver_requests_buffer())
+        .WillOnce(Throw(std::runtime_error("")))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    EXPECT_THAT(window.dequeueBuffer(&window, nullptr, nullptr), Eq(failure_code));
+    EXPECT_THAT(window.dequeueBuffer_DEPRECATED(&window, nullptr), Eq(failure_code));
+}
+
+TEST_F(AndroidNativeWindowTest, returns_error_on_queue_buffer_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(_, _))
+        .WillOnce(Throw(std::runtime_error("")))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    EXPECT_THAT(window.queueBuffer(&window, nullptr, 0), Eq(failure_code));
+    EXPECT_THAT(window.queueBuffer_DEPRECATED(&window, nullptr), Eq(failure_code));
+}
+
+TEST_F(AndroidNativeWindowTest, returns_error_on_cancel_buffer_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(_, _))
+        .WillOnce(Throw(std::runtime_error("")))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    EXPECT_THAT(window.cancelBuffer(&window, nullptr, 0), Eq(failure_code));
+    EXPECT_THAT(window.cancelBuffer_DEPRECATED(&window, nullptr), Eq(failure_code));
+}
+
+TEST_F(AndroidNativeWindowTest, returns_error_on_query_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_driver_interpreter, driver_requests_info(_))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    EXPECT_THAT(window.query(&window, 0, nullptr), Eq(failure_code));
+}
+
+TEST_F(AndroidNativeWindowTest, returns_error_on_perform_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(*mock_driver_interpreter, dispatch_driver_request_format(_))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    auto perform = [this] (int key, ...)
+    {
+        va_list args;
+        va_start(args, key);
+        EXPECT_THAT(window.perform(&window, key, args), Eq(failure_code));
+        va_end(args);
+    };
+    perform(NATIVE_WINDOW_SET_BUFFERS_FORMAT, 0);
+}

=== modified file 'tests/unit-tests/client/mesa/test_native_surface.cpp'
--- tests/unit-tests/client/mesa/test_native_surface.cpp	2014-07-11 03:33:03 +0000
+++ tests/unit-tests/client/mesa/test_native_surface.cpp	2014-08-05 07:09:25 +0000
@@ -74,14 +74,13 @@
 
     MirSurfaceParameters surf_params;
     testing::NiceMock<mtd::MockClientSurface> mock_surface;
+    mclg::NativeSurface native_surface{mock_surface};
 };
 
 TEST_F(MesaClientNativeSurfaceTest, basic_parameters)
 {
-    mclg::NativeSurface interpreter(mock_surface);
-
     MirSurfaceParameters params;
-    interpreter.surface_get_parameters(&interpreter, &params);
+    native_surface.surface_get_parameters(&native_surface, &params);
     EXPECT_EQ(surf_params.width, params.width);
     EXPECT_EQ(surf_params.height, params.height);
     EXPECT_EQ(surf_params.pixel_format, params.pixel_format);
@@ -98,8 +97,7 @@
     EXPECT_CALL(mock_surface, get_current_buffer())
         .Times(1);
 
-    mclg::NativeSurface interpreter(mock_surface);
-    interpreter.surface_advance_buffer(&interpreter, &buffer_package);
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
 }
 
 TEST_F(MesaClientNativeSurfaceTest, basic_advance)
@@ -115,9 +113,8 @@
     EXPECT_CALL(mock_surface, get_current_buffer())
         .Times(1);
 
-    mclg::NativeSurface interpreter(mock_surface);
-    interpreter.surface_advance_buffer(&interpreter, &buffer_package);
-    interpreter.surface_advance_buffer(&interpreter, &buffer_package);
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
 }
 
 TEST_F(MesaClientNativeSurfaceTest, swapinterval_request)
@@ -130,16 +127,46 @@
     EXPECT_CALL(mock_surface, request_and_wait_for_configure(mir_surface_attrib_swapinterval,1))
         .InSequence(seq);
 
-    mclg::NativeSurface interpreter(mock_surface);
-    interpreter.set_swapinterval(0);
-    interpreter.set_swapinterval(1);
+    native_surface.set_swapinterval(0);
+    native_surface.set_swapinterval(1);
 }
 
 TEST_F(MesaClientNativeSurfaceTest, swapinterval_unsupported_request)
 {
-    mclg::NativeSurface interpreter(mock_surface);
-    EXPECT_EQ(MIR_MESA_FALSE, interpreter.set_swapinterval(-1));
-    EXPECT_EQ(MIR_MESA_TRUE, interpreter.set_swapinterval(0));
-    EXPECT_EQ(MIR_MESA_TRUE, interpreter.set_swapinterval(1));
-    EXPECT_EQ(MIR_MESA_FALSE, interpreter.set_swapinterval(2));
+    EXPECT_EQ(MIR_MESA_FALSE, native_surface.set_swapinterval(-1));
+    EXPECT_EQ(MIR_MESA_TRUE, native_surface.set_swapinterval(0));
+    EXPECT_EQ(MIR_MESA_TRUE, native_surface.set_swapinterval(1));
+    EXPECT_EQ(MIR_MESA_FALSE, native_surface.set_swapinterval(2));
+}
+
+TEST_F(MesaClientNativeSurfaceTest, returns_error_on_advance_buffer_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(mock_surface, get_current_buffer())
+        .WillOnce(Throw(std::runtime_error("")));
+
+    MirBufferPackage buffer_package;
+    EXPECT_THAT(native_surface.advance_buffer(&buffer_package), Eq(MIR_MESA_FALSE));
+}
+
+TEST_F(MesaClientNativeSurfaceTest, returns_error_on_get_parameters_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(mock_surface, get_parameters())
+        .WillOnce(Throw(std::runtime_error("")));
+
+    MirSurfaceParameters surface_params;
+    EXPECT_THAT(native_surface.get_parameters(&surface_params), Eq(MIR_MESA_FALSE));
+}
+
+TEST_F(MesaClientNativeSurfaceTest, returns_error_on_set_swap_interval_failure)
+{
+    using namespace testing;
+
+    EXPECT_CALL(mock_surface, request_and_wait_for_configure(_,_))
+        .WillOnce(Throw(std::runtime_error("")));
+
+    EXPECT_THAT(native_surface.set_swapinterval(0), Eq(MIR_MESA_FALSE));
 }