Merge lp:~kdub/mir/fix-1239577 into lp:mir

[Kevin DuBois]

fix: lp 1239577

TestClientIPCRender (an android-only gfx driver test) was hanging due to changes in signal handling. refactor the test, changing the cross-process sync mechanism so it doesn't use sigcont.

fix: lp 1239577

TestClientIPCRender (an android-only gfx driver test) was hanging due to changes in signal handling. refactor the test, changing the cross-process sync mechanism so it doesn't use sigcont.

this test was messy to begin with. It now passes reliably, and is cleaner after the refactor, although I'll admit there still is room to improve it.

also, I rm-ed the fragile check this test was doing for surfaceflinger. Now that mir is on the system images, it would compete for resources with unity8 (and detecting the running processes from /proc is fragile anyways). Just better to remove

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1134
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/164/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2354
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2239/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/161/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/164/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1135
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/165/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2358
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2243
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/162/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/165/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

1. I would have recommended keeping share_ptr's as regular pointers at least, to keep the diff down. But not important now that it's done.

2. This should have been an if statement: switch (i % 2)

3. Using braces{
    like this}; // is a minor style violation, I think

4. CI is showing some test failures on amd64 (see above).

I'm seeing some other newish failures, but they're on development-branch too (!?)

What's important though is bug 1239577 and that indeed seems to be fixed. Only bug 1239955 is holding up integration-tests on Nexus 4 now.

So #4 needs fixing. But otherwise approved.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1138
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/186/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2397
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2282
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/183/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/186/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1139
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/187/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2398
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2283
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/184
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/184/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/187/rebuild

review: Approve (continuous-integration)

=== modified file 'include/test/mir_test/draw/android_graphics.h'
--- include/test/mir_test/draw/android_graphics.h	2013-10-15 02:59:59 +0000
+++ include/test/mir_test/draw/android_graphics.h	2013-10-16 23:18:17 +0000
@@ -53,8 +53,6 @@
     alloc_device_t* alloc_dev;
 };
 
-bool is_surface_flinger_running();
-
 }
 }
 }

=== modified file 'include/test/mir_test/draw/draw_pattern_checkered-inl.h'
--- include/test/mir_test/draw/draw_pattern_checkered-inl.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test/draw/draw_pattern_checkered-inl.h	2013-10-16 23:18:17 +0000
@@ -24,37 +24,37 @@
 }
 
 template<size_t Rows, size_t Cols>
-void DrawPatternCheckered<Rows,Cols>::draw(std::shared_ptr<MirGraphicsRegion>& region) const
+void DrawPatternCheckered<Rows,Cols>::draw(MirGraphicsRegion const& region) const
 {
-    if (region->pixel_format != mir_pixel_format_abgr_8888 )
+    if (region.pixel_format != mir_pixel_format_abgr_8888)
         throw(std::runtime_error("cannot draw region, incorrect format"));
 
-    uint32_t *pixel = (uint32_t*) region->vaddr;
-    for(int i=0; i< region->width; i++)
+    uint32_t *pixel = (uint32_t*) region.vaddr;
+    for(int i=0; i< region.width; i++)
     {
-        for(int j=0; j<region->height; j++)
+        for(int j=0; j<region.height; j++)
         {
             int key_row = i % Rows;
             int key_col = j % Cols;
-            pixel[j*region->stride + i] = color_pattern[key_row][key_col];
+            pixel[j*region.stride + i] = color_pattern[key_row][key_col];
         }
     }
 }
 
 template<size_t Rows, size_t Cols>
-bool DrawPatternCheckered<Rows, Cols>::check(const std::shared_ptr<MirGraphicsRegion>& region) const
+bool DrawPatternCheckered<Rows, Cols>::check(MirGraphicsRegion const& region) const
 {
-    if (region->pixel_format != mir_pixel_format_abgr_8888 )
+    if (region.pixel_format != mir_pixel_format_abgr_8888)
         throw(std::runtime_error("cannot check region, incorrect format"));
 
-    uint32_t *pixel = (uint32_t*) region->vaddr;
-    for(int i=0; i< region->width; i++)
+    uint32_t *pixel = (uint32_t*) region.vaddr;
+    for(int i=0; i< region.width; i++)
     {
-        for(int j=0; j<region->height; j++)
+        for(int j=0; j<region.height; j++)
         {
             int key_row = i % Rows;
             int key_col = j % Cols;
-            if (pixel[j*region->stride + i] != color_pattern[key_row][key_col])
+            if (pixel[j*region.stride + i] != color_pattern[key_row][key_col])
             {
                 return false;
             }

=== modified file 'include/test/mir_test/draw/patterns.h'
--- include/test/mir_test/draw/patterns.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test/draw/patterns.h	2013-10-16 23:18:17 +0000
@@ -37,8 +37,8 @@
 {
 public:
     virtual ~DrawPattern() {};
-    virtual void draw(std::shared_ptr<MirGraphicsRegion>& region) const = 0;
-    virtual bool check(const std::shared_ptr<MirGraphicsRegion>& region) const = 0;
+    virtual void draw(MirGraphicsRegion const& region) const = 0;
+    virtual bool check(MirGraphicsRegion const& region) const = 0;
 
 protected:
     DrawPattern() = default;
@@ -52,8 +52,8 @@
     /* todo: should construct with a color value type, not an uint32 */
     DrawPatternSolid(uint32_t color_value);
 
-    void draw(std::shared_ptr<MirGraphicsRegion>& region) const;
-    bool check(const std::shared_ptr<MirGraphicsRegion>& region) const;
+    void draw(MirGraphicsRegion const& region) const;
+    bool check(MirGraphicsRegion const& region) const;
 
 private:
     const uint32_t color_value;
@@ -66,8 +66,8 @@
     /* todo: should construct with a color value type, not an uint32 */
     DrawPatternCheckered(uint32_t (&pattern) [Rows][Cols]);
 
-    void draw(std::shared_ptr<MirGraphicsRegion>& region) const;
-    bool check(const std::shared_ptr<MirGraphicsRegion>& region) const;
+    void draw(MirGraphicsRegion const& region) const;
+    bool check(MirGraphicsRegion const& region) const;
 
 private:
     uint32_t color_pattern [Rows][Cols];

=== modified file 'include/test/mir_test/stub_server_tool.h'
--- include/test/mir_test/stub_server_tool.h	2013-08-28 03:41:48 +0000
+++ include/test/mir_test/stub_server_tool.h	2013-10-16 23:18:17 +0000
@@ -80,10 +80,11 @@
     virtual void connect(
         ::google::protobuf::RpcController*,
                          const ::mir::protobuf::ConnectParameters* request,
-                         ::mir::protobuf::Connection*,
+                         ::mir::protobuf::Connection* connect_msg,
                          ::google::protobuf::Closure* done) override
     {
         app_name = request->application_name();
+        connect_msg->set_error("");
         done->Run();
     }
 

=== modified file 'tests/draw/android_graphics.cpp'
--- tests/draw/android_graphics.cpp	2013-10-15 02:59:59 +0000
+++ tests/draw/android_graphics.cpp	2013-10-16 23:18:17 +0000
@@ -113,9 +113,3 @@
 
     return std::shared_ptr<MirGraphicsRegion>(region, del);
 }
-
-bool mtd::is_surface_flinger_running()
-{
-    struct dirent **namelist;
-    return 0 < scandir(proc_dir, &namelist, surface_flinger_filter, 0);
-}

=== modified file 'tests/draw/patterns.cpp'
--- tests/draw/patterns.cpp	2013-02-04 19:24:07 +0000
+++ tests/draw/patterns.cpp	2013-10-16 23:18:17 +0000
@@ -25,34 +25,34 @@
 {
 }
 
-void mtd::DrawPatternSolid::draw(std::shared_ptr<MirGraphicsRegion>& region) const
+void mtd::DrawPatternSolid::draw(MirGraphicsRegion const& region) const
 {
-    if (region->pixel_format != mir_pixel_format_abgr_8888 )
+    if (region.pixel_format != mir_pixel_format_abgr_8888 )
         throw(std::runtime_error("cannot draw region, incorrect format"));
 
-    uint32_t *pixel = (uint32_t*) region->vaddr;
+    uint32_t *pixel = (uint32_t*) region.vaddr;
     int i,j;
-    for(i=0; i<region->height; i++)
+    for(i=0; i<region.height; i++)
     {
-        for(j=0; j< region->width; j++)
+        for(j=0; j< region.width; j++)
         {
-            pixel[i*region->stride + j] = color_value;
+            pixel[i*region.stride + j] = color_value;
         }
     }
 }
 
-bool mtd::DrawPatternSolid::check(const std::shared_ptr<MirGraphicsRegion>& region) const
+bool mtd::DrawPatternSolid::check(MirGraphicsRegion const& region) const
 {
-    if (region->pixel_format != mir_pixel_format_abgr_8888 )
+    if (region.pixel_format != mir_pixel_format_abgr_8888 )
         throw(std::runtime_error("cannot check region, incorrect format"));
 
-    uint32_t *pixel = (uint32_t*) region->vaddr;
+    uint32_t *pixel = (uint32_t*) region.vaddr;
     int i,j;
-    for(i=0; i< region->width; i++)
+    for(i=0; i< region.width; i++)
     {
-        for(j=0; j<region->height; j++)
+        for(j=0; j<region.height; j++)
         {
-            if (pixel[j*region->stride + i] != color_value)
+            if (pixel[j*region.stride + i] != color_value)
             {
                 return false;
             }

=== modified file 'tests/integration-tests/client/test_client_render.cpp'
--- tests/integration-tests/client/test_client_render.cpp	2013-10-15 02:59:59 +0000
+++ tests/integration-tests/client/test_client_render.cpp	2013-10-16 23:18:17 +0000
@@ -20,9 +20,11 @@
 
 #include "mir/graphics/buffer_properties.h"
 #include "mir/graphics/buffer_initializer.h"
+#include "src/server/graphics/android/buffer.h"
 #include "mir/graphics/android/native_buffer.h"
 #include "src/server/graphics/android/android_graphic_buffer_allocator.h"
 
+#include "mir_test_framework/cross_process_sync.h"
 #include "mir_test/draw/android_graphics.h"
 #include "mir_test/draw/patterns.h"
 #include "mir_test/stub_server_tool.h"
@@ -48,156 +50,62 @@
 {
 static int test_width  = 300;
 static int test_height = 200;
-}
-
-namespace mir
-{
-namespace test
-{
-/* client code */
-static void connected_callback(MirConnection *connection, void* context)
-{
-    MirConnection** tmp = (MirConnection**) context;
-    *tmp = connection;
-}
-static void create_callback(MirSurface *surface, void*context)
-{
-    MirSurface** surf = (MirSurface**) context;
-    *surf = surface;
-}
-
-static void next_callback(MirSurface *, void*)
-{
-}
 
 static uint32_t pattern0 [2][2] = {{0x12345678, 0x23456789},
                                    {0x34567890, 0x45678901}};
-
 static uint32_t pattern1 [2][2] = {{0xFFFFFFFF, 0xFFFF0000},
                                    {0xFF00FF00, 0xFF0000FF}};
+static mtd::DrawPatternCheckered<2,2> draw_pattern0(pattern0);
+static mtd::DrawPatternCheckered<2,2> draw_pattern1(pattern1);
+static const char socket_file[] = "./test_client_ipc_render_socket";
+
 struct TestClient
 {
-
-    static void sig_handle(int)
-    {
-    }
-
-    static int render_single()
-    {
-        if (signal(SIGCONT, sig_handle) == SIG_ERR)
-            return -1;
-        pause();
-
-        MirConnection* connection = NULL;
-        MirSurface* surface;
-        MirSurfaceParameters surface_parameters;
-
-         /* establish connection. wait for server to come up */
-        while (connection == NULL)
-        {
-            mir_wait_for(mir_connect("./test_socket_surface", "test_renderer",
-                                         &connected_callback, &connection));
-            std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        }
-        /* make surface */
-        surface_parameters.name = "testsurface";
-        surface_parameters.width = test_width;
-        surface_parameters.height = test_height;
-        surface_parameters.pixel_format = mir_pixel_format_abgr_8888;
-        mir_wait_for(mir_connection_create_surface(connection,
-                                                   &surface_parameters,
-                                                   &create_callback,
-                                                   &surface));
-
-        auto graphics_region = std::make_shared<MirGraphicsRegion>();
-        /* grab a buffer*/
-        mir_surface_get_graphics_region(surface, graphics_region.get());
-
-        /* render pattern */
-        mtd::DrawPatternCheckered<2,2> draw_pattern0(mt::pattern0);
-        draw_pattern0.draw(graphics_region);
-
-        mir_wait_for(mir_surface_release(surface, &create_callback, &surface));
-
-        /* release */
-        mir_connection_release(connection);
-        return 0;
-    }
-
-    static int render_double()
-    {
-        if (signal(SIGCONT, sig_handle) == SIG_ERR)
-            return -1;
-        pause();
-
-        MirConnection* connection = NULL;
-        MirSurface* surface;
-        MirSurfaceParameters surface_parameters;
-
-         /* establish connection. wait for server to come up */
-        while (connection == NULL)
-        {
-            mir_wait_for(mir_connect("./test_socket_surface", "test_renderer",
-                                         &connected_callback, &connection));
-            std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        }
-        /* make surface */
-        surface_parameters.name = "testsurface";
-        surface_parameters.width = test_width;
-        surface_parameters.height = test_height;
-        surface_parameters.pixel_format = mir_pixel_format_abgr_8888;
-
-        mir_wait_for(mir_connection_create_surface(connection,
-                                                   &surface_parameters,
-                                                   &create_callback,
-                                                   &surface));
-
-        auto graphics_region = std::make_shared<MirGraphicsRegion>();
-        mir_surface_get_graphics_region( surface, graphics_region.get());
-        mtd::DrawPatternCheckered<2,2> draw_pattern0(mt::pattern0);
-        draw_pattern0.draw(graphics_region);
-
-        mir_wait_for(mir_surface_swap_buffers(surface, &next_callback, (void*) NULL));
-        mir_surface_get_graphics_region( surface, graphics_region.get());
-        mtd::DrawPatternCheckered<2,2> draw_pattern1(mt::pattern1);
-        draw_pattern1.draw(graphics_region);
-
-        mir_wait_for(mir_surface_release(surface, &create_callback, &surface));
-
-        /* release */
-        mir_connection_release(connection);
-        return 0;
-    }
-
-    static int render_accelerated()
-    {
-        if (signal(SIGCONT, sig_handle) == SIG_ERR)
-            return -1;
-        pause();
-
-        /* only use C api */
-        MirConnection* connection = NULL;
-        MirSurface* surface;
-        MirSurfaceParameters surface_parameters;
-
-         /* establish connection. wait for server to come up */
-        while (connection == NULL)
-        {
-            mir_wait_for(mir_connect("./test_socket_surface", "test_renderer",
-                                         &connected_callback, &connection));
-            std::this_thread::sleep_for(std::chrono::milliseconds(10));
-        }
-        /* make surface */
-        surface_parameters.name = "testsurface";
-        surface_parameters.width = test_width;
-        surface_parameters.height = test_height;
-        surface_parameters.pixel_format = mir_pixel_format_abgr_8888;
-
-        mir_wait_for(mir_connection_create_surface(connection,
-                                                   &surface_parameters,
-                                                   &create_callback,
-                                                   &surface));
-
+    static int render_cpu_pattern(mtf::CrossProcessSync& process_sync, int num_frames)
+    {
+        process_sync.wait_for_signal_ready_for();
+
+        MirSurfaceParameters surface_parameters
+        {
+            "testsurface", test_width, test_height, mir_pixel_format_abgr_8888,
+            mir_buffer_usage_software, mir_display_output_id_invalid
+        };
+        auto connection = mir_connect_sync(socket_file, "test_renderer");
+        auto surface = mir_connection_create_surface_sync(connection, &surface_parameters);
+        MirGraphicsRegion graphics_region;
+        for(int i=0u; i < num_frames; i++)
+        {
+            mir_surface_get_graphics_region(surface, &graphics_region);
+            if (i % 2)
+            {
+                draw_pattern1.draw(graphics_region);
+            }
+            else
+            {
+                draw_pattern0.draw(graphics_region);
+            }
+            mir_surface_swap_buffers_sync(surface);
+        }
+
+        mir_surface_release_sync(surface);
+        mir_connection_release(connection);
+        return 0;
+    }
+
+    //performs num_frames renders, in red, green, blue repeating pattern
+    static int render_rgb_with_gl(mtf::CrossProcessSync& process_sync, int num_frames)
+    {
+        process_sync.wait_for_signal_ready_for();
+
+        MirSurfaceParameters surface_parameters
+        {
+            "testsurface", test_width, test_height, mir_pixel_format_abgr_8888,
+            mir_buffer_usage_hardware, mir_display_output_id_invalid
+        };
+        auto connection = mir_connect_sync(socket_file, "test_renderer");
+        auto surface = mir_connection_create_surface_sync(connection, &surface_parameters);
+
+        /* set up egl context */
         int major, minor, n;
         EGLDisplay disp;
         EGLContext context;
@@ -221,83 +129,27 @@
         context = eglCreateContext(disp, egl_config, EGL_NO_CONTEXT, context_attribs);
         eglMakeCurrent(disp, egl_surface, egl_surface, context);
 
-        glClearColor(1.0, 0.0, 0.0, 1.0);
-        glClear(GL_COLOR_BUFFER_BIT);
-
-        eglSwapBuffers(disp, egl_surface);
-        mir_wait_for(mir_surface_release(surface, &create_callback, &surface));
-
-        /* release */
-        mir_connection_release(connection);
-        return 0;
-
-    }
-
-    static int render_accelerated_double()
-    {
-        if (signal(SIGCONT, sig_handle) == SIG_ERR)
-            return -1;
-        pause();
-
-        /* only use C api */
-        MirConnection* connection = NULL;
-        MirSurface* surface;
-        MirSurfaceParameters surface_parameters;
-
-         /* establish connection. wait for server to come up */
-        while (connection == NULL)
+        for (auto i=0; i < num_frames; i++)
         {
-            mir_wait_for(mir_connect("./test_socket_surface", "test_renderer",
-                                         &connected_callback, &connection));
-            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+            switch (i % 3)
+            {
+                case 0: //red
+                    glClearColor(1.0, 0.0, 0.0, 1.0);
+                    break;
+                case 1: //green
+                    glClearColor(0.0, 1.0, 0.0, 1.0);
+                    break;
+                case 2: //blue
+                default:
+                    glClearColor(0.0, 0.0, 1.0, 1.0);
+                    break;
+            }
+            glClear(GL_COLOR_BUFFER_BIT);
+
+            eglSwapBuffers(disp, egl_surface);
         }
-        /* make surface */
-        surface_parameters.name = "testsurface";
-        surface_parameters.width = test_width;
-        surface_parameters.height = test_height;
-        surface_parameters.pixel_format = mir_pixel_format_abgr_8888;
-
-        mir_wait_for(mir_connection_create_surface(connection,
-                                                   &surface_parameters,
-                                                   &create_callback,
-                                                   &surface));
-
-        int major, minor, n;
-        EGLDisplay disp;
-        EGLContext context;
-        EGLSurface egl_surface;
-        EGLConfig egl_config;
-        EGLint attribs[] = {
-            EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
-            EGL_GREEN_SIZE, 8,
-            EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
-            EGL_NONE };
-        EGLint context_attribs[] = { EGL_CONTEXT_CLIENT_VERSION, 2, EGL_NONE };
-
-        EGLNativeDisplayType native_display = (EGLNativeDisplayType)mir_connection_get_egl_native_display(connection);
-        EGLNativeWindowType native_window = (EGLNativeWindowType)mir_surface_get_egl_native_window(surface);
-
-        disp = eglGetDisplay(native_display);
-        eglInitialize(disp, &major, &minor);
-
-        eglChooseConfig(disp, attribs, &egl_config, 1, &n);
-        egl_surface = eglCreateWindowSurface(disp, egl_config, native_window, NULL);
-        context = eglCreateContext(disp, egl_config, EGL_NO_CONTEXT, context_attribs);
-        eglMakeCurrent(disp, egl_surface, egl_surface, context);
-
-        glClearColor(1.0, 0.0, 0.0, 1.0);
-        glClear(GL_COLOR_BUFFER_BIT);
-
-        eglSwapBuffers(disp, egl_surface);
-
-        glClearColor(0.0, 1.0, 0.0, 1.0);
-        glClear(GL_COLOR_BUFFER_BIT);
-
-        eglSwapBuffers(disp, egl_surface);
-
-        mir_wait_for(mir_surface_release(surface, &create_callback, &surface));
-
-        /* release */
+
+        mir_surface_release_sync(surface);
         mir_connection_release(connection);
         return 0;
 
@@ -312,13 +164,16 @@
 /* server code */
 struct StubServerGenerator : public mt::StubServerTool
 {
-    StubServerGenerator(std::shared_ptr<mg::NativeBuffer> const& handle, int id)
-     : handle(handle),
-       next_received(false),
-       next_allowed(false),
-       handle_id(id)
+    StubServerGenerator()
+     : next_received(false),
+       next_allowed(false)
     {
-
+        auto initializer = std::make_shared<mg::NullBufferInitializer>();
+        allocator = std::make_shared<mga::AndroidGraphicBufferAllocator> (initializer);
+        auto size = geom::Size{test_width, test_height};
+        auto pf = geom::PixelFormat::abgr_8888;
+        last_posted = allocator->alloc_buffer_platform(size, pf, mga::BufferUsage::use_hardware);
+        client_buffer = allocator->alloc_buffer_platform(size, pf, mga::BufferUsage::use_hardware);
     }
 
     void create_surface(google::protobuf::RpcController* /*controller*/,
@@ -326,15 +181,17 @@
                  mir::protobuf::Surface* response,
                  google::protobuf::Closure* done)
     {
-        response->mutable_id()->set_value(13); // TODO distinct numbers & tracking
+        response->mutable_id()->set_value(13);
         response->set_width(test_width);
         response->set_height(test_height);
         response->set_pixel_format(request->pixel_format());
-        response->mutable_buffer()->set_buffer_id(handle_id);
-        response->mutable_buffer()->set_stride(handle->anwb()->stride);
+        response->mutable_buffer()->set_buffer_id(client_buffer->id().as_uint32_t());
+
+        auto buf = client_buffer->native_buffer_handle();
+        response->mutable_buffer()->set_stride(buf->anwb()->stride);
 
         response->mutable_buffer()->set_fds_on_side_channel(1);
-        native_handle_t const* native_handle = handle->handle();
+        native_handle_t const* native_handle = buf->handle();
         for(auto i=0; i<native_handle->numFds; i++)
             response->mutable_buffer()->add_fd(native_handle->data[i]);
         for(auto i=0; i<native_handle->numInts; i++)
@@ -353,56 +210,43 @@
         ::mir::protobuf::Buffer* response,
         ::google::protobuf::Closure* done)
     {
-        {
-            std::unique_lock<std::mutex> lk(next_guard);
-            next_received = true;
-            next_cv.notify_all();
-
-            while (!next_allowed) {
-                allow_cv.wait(lk);
-            }
-            next_allowed = false;
-        }
-
-        response->set_buffer_id(handle_id);
+        std::unique_lock<std::mutex> lk(buffer_mutex);
+        std::swap(last_posted, client_buffer);
+
+        response->set_buffer_id(client_buffer->id().as_uint32_t());
+
+        auto buf = client_buffer->native_buffer_handle();
 
         response->set_fds_on_side_channel(1);
-        native_handle_t const* native_handle = handle->handle();
-        response->set_stride(handle->anwb()->stride);
+        native_handle_t const* native_handle = buf->handle();
+        response->set_stride(buf->anwb()->stride);
         for(auto i=0; i<native_handle->numFds; i++)
             response->add_fd(native_handle->data[i]);
         for(auto i=0; i<native_handle->numInts; i++)
             response->add_data(native_handle->data[native_handle->numFds+i]);
 
+
         done->Run();
     }
 
-    void wait_on_next_buffer()
-    {
-        std::unique_lock<std::mutex> lk(next_guard);
-        while (!next_received)
-            next_cv.wait(lk);
-        next_received = false;
-    }
-
-    void allow_next_continue()
-    {
-        std::unique_lock<std::mutex> lk(next_guard);
-        next_allowed = true;
-        allow_cv.notify_all();
-        lk.unlock();
-    }
-
-    void set_handle(std::shared_ptr<mg::NativeBuffer> const& pack, int id)
-    {
-        handle = pack;
-        handle_id = id;
+    std::shared_ptr<mga::Buffer> second_to_last_posted_buffer()
+    {
+        std::unique_lock<std::mutex> lk(buffer_mutex);
+        return client_buffer;
+    }
+
+    std::shared_ptr<mga::Buffer> last_posted_buffer()
+    {
+        std::unique_lock<std::mutex> lk(buffer_mutex);
+        return last_posted;
     }
 
 private:
-    std::shared_ptr<mg::NativeBuffer> handle;
+    std::shared_ptr<mga::AndroidGraphicBufferAllocator> allocator;
+    std::shared_ptr<mga::Buffer> client_buffer;
+    std::shared_ptr<mga::Buffer> last_posted;
 
-    std::mutex next_guard;
+    std::mutex buffer_mutex;
     std::condition_variable next_cv;
     std::condition_variable allow_cv;
     bool next_received;
@@ -411,195 +255,120 @@
     int handle_id;
 };
 
-
-}
 }
 
 struct TestClientIPCRender : public testing::Test
 {
-    /* kdub -- some of the (less thoroughly tested) android blob drivers annoyingly keep
-       static state about what process they are in. Once you fork, this info is invalid,
-       yet the driver uses the info and bad things happen.
-       Fork all needed processes before touching the blob! */
+    /* note: we fork here so that the loaded driver code does not fork */
     static void SetUpTestCase() {
+
         render_single_client_process = mtf::fork_and_run_in_a_different_process(
-            mt::TestClient::render_single,
-            mt::TestClient::exit_function);
-
+            std::bind(TestClient::render_cpu_pattern, std::ref(sync1), 1),
+            TestClient::exit_function);
         render_double_client_process = mtf::fork_and_run_in_a_different_process(
-            mt::TestClient::render_double,
-            mt::TestClient::exit_function);
-
-        second_render_with_same_buffer_client_process
-             = mtf::fork_and_run_in_a_different_process(
-                            mt::TestClient::render_double,
-                            mt::TestClient::exit_function);
-
-        render_accelerated_process
-             = mtf::fork_and_run_in_a_different_process(
-                            mt::TestClient::render_accelerated,
-                            mt::TestClient::exit_function);
-
-        render_accelerated_process_double
-             = mtf::fork_and_run_in_a_different_process(
-                            mt::TestClient::render_accelerated_double,
-                            mt::TestClient::exit_function);
+            std::bind(TestClient::render_cpu_pattern, std::ref(sync2), 2),
+            TestClient::exit_function);
+        render_accelerated_process = mtf::fork_and_run_in_a_different_process(
+            std::bind(TestClient::render_rgb_with_gl, std::ref(sync3), 1),
+            TestClient::exit_function);
+        render_accelerated_process_double = mtf::fork_and_run_in_a_different_process(
+            std::bind(TestClient::render_rgb_with_gl, std::ref(sync4), 2),
+            TestClient::exit_function);
     }
 
     void SetUp() {
-        ASSERT_FALSE(mtd::is_surface_flinger_running());
-
-        size = geom::Size{test_width, test_height};
-        pf = geom::PixelFormat::abgr_8888;
-
-        auto initializer = std::make_shared<mg::NullBufferInitializer>();
-        allocator = std::make_shared<mga::AndroidGraphicBufferAllocator> (initializer);
-        mg::BufferProperties properties(size, pf, mg::BufferUsage::hardware);
-        android_buffer = allocator->alloc_buffer(properties);
-        second_android_buffer = allocator->alloc_buffer(properties);
-
         buffer_converter = std::make_shared<mtd::TestGrallocMapper>();
 
-        handle = android_buffer->native_buffer_handle();
-        second_handle = second_android_buffer->native_buffer_handle();
-
         /* start a server */
-        mock_server = std::make_shared<mt::StubServerGenerator>(handle, 14);
-        test_server = std::make_shared<mt::TestProtobufServer>("./test_socket_surface", mock_server);
+        mock_server = std::make_shared<StubServerGenerator>();
+        test_server = std::make_shared<mt::TestProtobufServer>(socket_file, mock_server);
         test_server->comm->start();
     }
 
     void TearDown()
     {
         test_server.reset();
+        std::remove(socket_file);
     }
 
-    mir::protobuf::Connection response;
-
     std::shared_ptr<mt::TestProtobufServer> test_server;
-    std::shared_ptr<mt::StubServerGenerator> mock_server;
+    std::shared_ptr<StubServerGenerator> mock_server;
 
-    geom::Size size;
-    geom::PixelFormat pf;
-    mg::BufferID id1;
-    mg::BufferID id2;
     std::shared_ptr<mtd::TestGrallocMapper> buffer_converter;
-    std::shared_ptr<mtf::Process> client_process;
-
-    std::shared_ptr<mg::NativeBuffer> handle;
-    std::shared_ptr<mg::NativeBuffer> second_handle;
-
-    std::shared_ptr<mg::Buffer> android_buffer;
-    std::shared_ptr<mg::Buffer> second_android_buffer;
-    std::shared_ptr<mga::AndroidGraphicBufferAllocator>  allocator;
 
     static std::shared_ptr<mtf::Process> render_single_client_process;
     static std::shared_ptr<mtf::Process> render_double_client_process;
-    static std::shared_ptr<mtf::Process> second_render_with_same_buffer_client_process;
     static std::shared_ptr<mtf::Process> render_accelerated_process;
     static std::shared_ptr<mtf::Process> render_accelerated_process_double;
+    static mtf::CrossProcessSync sync1, sync2, sync3, sync4;
 };
+
+mtf::CrossProcessSync TestClientIPCRender::sync1;
 std::shared_ptr<mtf::Process> TestClientIPCRender::render_single_client_process;
+mtf::CrossProcessSync TestClientIPCRender::sync2;
 std::shared_ptr<mtf::Process> TestClientIPCRender::render_double_client_process;
-std::shared_ptr<mtf::Process> TestClientIPCRender::second_render_with_same_buffer_client_process;
+mtf::CrossProcessSync TestClientIPCRender::sync3;
 std::shared_ptr<mtf::Process> TestClientIPCRender::render_accelerated_process;
+mtf::CrossProcessSync TestClientIPCRender::sync4;
 std::shared_ptr<mtf::Process> TestClientIPCRender::render_accelerated_process_double;
 
 TEST_F(TestClientIPCRender, test_render_single)
 {
-    mtd::DrawPatternCheckered<2,2> rendered_pattern(mt::pattern0);
-    /* activate client */
-    render_single_client_process->cont();
+    sync1.try_signal_ready_for();
 
     /* wait for client to finish */
     EXPECT_TRUE(render_single_client_process->wait_for_termination().succeeded());
 
     /* check content */
-    auto region = buffer_converter->graphic_region_from_handle(handle->anwb());
-    EXPECT_TRUE(rendered_pattern.check(region));
+    auto buf = mock_server->last_posted_buffer();
+    auto region = buffer_converter->graphic_region_from_handle(buf->native_buffer_handle()->anwb());
+    EXPECT_TRUE(draw_pattern0.check(*region));
 }
 
 TEST_F(TestClientIPCRender, test_render_double)
 {
-    mtd::DrawPatternCheckered<2,2> rendered_pattern0(mt::pattern0);
-    mtd::DrawPatternCheckered<2,2> rendered_pattern1(mt::pattern1);
-    /* activate client */
-    render_double_client_process->cont();
-
-    /* wait for next buffer */
-    mock_server->wait_on_next_buffer();
-    auto region = buffer_converter->graphic_region_from_handle(handle->anwb());
-    EXPECT_TRUE(rendered_pattern0.check(region));
-
-    mock_server->set_handle(second_handle, 15);
-    mock_server->allow_next_continue();
+    sync2.try_signal_ready_for();
 
     /* wait for client to finish */
     EXPECT_TRUE(render_double_client_process->wait_for_termination().succeeded());
 
-    auto second_region = buffer_converter->graphic_region_from_handle(second_handle->anwb());
-    EXPECT_TRUE(rendered_pattern1.check(second_region));
-}
-
-TEST_F(TestClientIPCRender, test_second_render_with_same_buffer)
-{
-    mtd::DrawPatternCheckered<2,2> rendered_pattern(mt::pattern1);
-    /* activate client */
-    second_render_with_same_buffer_client_process->cont();
-
-    /* wait for next buffer */
-    mock_server->wait_on_next_buffer();
-    mock_server->allow_next_continue();
-
-    /* wait for client to finish */
-    EXPECT_TRUE(second_render_with_same_buffer_client_process->wait_for_termination().succeeded());
-
-    /* check content */
-    auto region = buffer_converter->graphic_region_from_handle(handle->anwb());
-    EXPECT_TRUE(rendered_pattern.check(region));
+    auto buf0 = mock_server->second_to_last_posted_buffer();
+    auto region0 = buffer_converter->graphic_region_from_handle(buf0->native_buffer_handle()->anwb());
+    EXPECT_TRUE(draw_pattern0.check(*region0));
+
+    auto buf1 = mock_server->last_posted_buffer();
+    auto region1 = buffer_converter->graphic_region_from_handle(buf1->native_buffer_handle()->anwb());
+    EXPECT_TRUE(draw_pattern1.check(*region1));
 }
 
 TEST_F(TestClientIPCRender, test_accelerated_render)
 {
     mtd::DrawPatternSolid red_pattern(0xFF0000FF);
 
-    /* activate client */
-    render_accelerated_process->cont();
-
-    /* wait for next buffer */
-    mock_server->wait_on_next_buffer();
-    mock_server->allow_next_continue();
-
+    sync3.try_signal_ready_for();
     /* wait for client to finish */
     EXPECT_TRUE(render_accelerated_process->wait_for_termination().succeeded());
 
     /* check content */
-    auto region = buffer_converter->graphic_region_from_handle(handle->anwb());
-    EXPECT_TRUE(red_pattern.check(region));
+    auto buf0 = mock_server->last_posted_buffer();
+    auto region0 = buffer_converter->graphic_region_from_handle(buf0->native_buffer_handle()->anwb());
+    EXPECT_TRUE(red_pattern.check(*region0));
 }
 
 TEST_F(TestClientIPCRender, test_accelerated_render_double)
 {
     mtd::DrawPatternSolid red_pattern(0xFF0000FF);
     mtd::DrawPatternSolid green_pattern(0xFF00FF00);
-    /* activate client */
-    render_accelerated_process_double->cont();
-
-    /* wait for next buffer */
-    mock_server->wait_on_next_buffer();
-    mock_server->set_handle(second_handle, 15);
-    mock_server->allow_next_continue();
-
-    mock_server->wait_on_next_buffer();
-    mock_server->allow_next_continue();
-
+
+    sync4.try_signal_ready_for();
     /* wait for client to finish */
     EXPECT_TRUE(render_accelerated_process_double->wait_for_termination().succeeded());
 
-    /* check content */
-    auto region = buffer_converter->graphic_region_from_handle(handle->anwb());
-    EXPECT_TRUE(red_pattern.check(region));
+    auto buf0 = mock_server->second_to_last_posted_buffer();
+    auto region0 = buffer_converter->graphic_region_from_handle(buf0->native_buffer_handle()->anwb());
+    EXPECT_TRUE(red_pattern.check(*region0));
 
-    auto second_region = buffer_converter->graphic_region_from_handle(second_handle->anwb());
-    EXPECT_TRUE(green_pattern.check(second_region));
+    auto buf1 = mock_server->last_posted_buffer();
+    auto region1 = buffer_converter->graphic_region_from_handle(buf1->native_buffer_handle()->anwb());
+    EXPECT_TRUE(green_pattern.check(*region1));
 }

=== modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
--- tests/integration-tests/graphics/android/test_buffer_integration.cpp	2013-10-15 02:59:59 +0000
+++ tests/integration-tests/graphics/android/test_buffer_integration.cpp	2013-10-16 23:18:17 +0000
@@ -68,8 +68,8 @@
 
     auto region = sw_renderer.graphic_region_from_handle(test_buffer->native_buffer_handle()->anwb());
     mtd::DrawPatternSolid red_pattern(0xFF0000FF);
-    red_pattern.draw(region);
-    EXPECT_TRUE(red_pattern.check(region));
+    red_pattern.draw(*region);
+    EXPECT_TRUE(red_pattern.check(*region));
 }
 
 TEST_F(AndroidBufferIntegration, allocator_can_create_hw_buffer)

=== modified file 'tests/integration-tests/graphics/android/test_display_integration.cpp'
--- tests/integration-tests/graphics/android/test_display_integration.cpp	2013-10-15 02:59:59 +0000
+++ tests/integration-tests/graphics/android/test_display_integration.cpp	2013-10-16 23:18:17 +0000
@@ -52,9 +52,6 @@
 protected:
     static void SetUpTestCase()
     {
-        /* this is an important precondition for acquiring the display! */
-        ASSERT_FALSE(mir::test::draw::is_surface_flinger_running());
-
         /* note about fb_device: OMAP4 drivers seem to only be able to open fb once
            per process (repeated framebuffer_{open,close}() doesn't seem to work). once we
            figure out why, we can remove fb_device in the test fixture */

=== modified file 'tests/unit-tests/draw/test_draw_patterns.cpp'
--- tests/unit-tests/draw/test_draw_patterns.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/draw/test_draw_patterns.cpp	2013-10-16 23:18:17 +0000
@@ -33,14 +33,17 @@
 
     virtual void SetUp()
     {
-        test_region = std::make_shared<MirGraphicsRegion>();
-        test_region->pixel_format = mir_pixel_format_abgr_8888;
+
+        test_region.pixel_format = mir_pixel_format_abgr_8888;
         int bytes_pp = 4;
-        test_region->width  = 100;
+        test_region.width  = 100;
         /* misaligned stride to tease out stride problems */
-        test_region->stride = 102;
-        test_region->height = 50;
-        test_region->vaddr = (char*) malloc(sizeof(char) * bytes_pp * test_region->height * test_region->stride);
+        test_region.stride = 102;
+        test_region.height = 50;
+        auto region_size =
+            sizeof(char) * bytes_pp * test_region.height * test_region.stride;
+        region = std::shared_ptr<char>(static_cast<char*>(::operator new(region_size)));
+        test_region.vaddr = region.get();
 
         uint32_t colors[2][2] = {{0x12345678, 0x23456789},
                                  {0x34567890, 0x45678901}};
@@ -51,34 +54,33 @@
     virtual void TearDown()
     {
         EXPECT_TRUE(check_stride_region_unaltered());
-        free(test_region->vaddr);
     }
 
-    std::shared_ptr<MirGraphicsRegion> test_region;
+    MirGraphicsRegion test_region;
     uint32_t pattern_colors [2][2];
-
+    std::shared_ptr<char> region;
 private:
     /* check that no pattern writes or checks the stride region */
     void write_stride_region()
     {
-        uint32_t* region = (uint32_t*) test_region->vaddr;
-        for(auto i=0; i < test_region->height; i++)
+        uint32_t* region = (uint32_t*) test_region.vaddr;
+        for(auto i=0; i < test_region.height; i++)
         {
-            for(auto j=test_region->width; j<test_region->stride; j++)
+            for(auto j=test_region.width; j<test_region.stride; j++)
             {
-                region[ (i*test_region->stride) + j ] = stride_color;
+                region[ (i*test_region.stride) + j ] = stride_color;
             }
         }
     }
 
     bool check_stride_region_unaltered()
     {
-        uint32_t* region = (uint32_t*) test_region->vaddr;
-        for(auto i=0; i < test_region->height; i++)
+        uint32_t* region = (uint32_t*) test_region.vaddr;
+        for(auto i=0; i < test_region.height; i++)
         {
-            for(auto j=test_region->width; j<test_region->stride; j++)
+            for(auto j=test_region.width; j<test_region.stride; j++)
             {
-                if(region[ ((i*test_region->stride) + j) ] != stride_color)
+                if(region[ ((i*test_region.stride) + j) ] != stride_color)
                 {
                     return false;
                 }
@@ -103,13 +105,13 @@
     mtd::DrawPatternSolid pattern(0x43214321);
 
     pattern.draw(test_region);
-    test_region->vaddr[0]++;
+    test_region.vaddr[0]++;
     EXPECT_FALSE(pattern.check(test_region));
 }
 
 TEST_F(DrawPatternsTest, solid_bad_pixel_formats)
 {
-    test_region->pixel_format = mir_pixel_format_xbgr_8888;
+    test_region.pixel_format = mir_pixel_format_xbgr_8888;
 
     mtd::DrawPatternSolid pattern(0x43214321);
 
@@ -135,13 +137,13 @@
     mtd::DrawPatternCheckered<2,2> pattern(pattern_colors);
 
     pattern.draw(test_region);
-    test_region->vaddr[0]++;
+    test_region.vaddr[0]++;
     EXPECT_FALSE(pattern.check(test_region));
 }
 
 TEST_F(DrawPatternsTest, checkered_bad_pixel_formats)
 {
-    test_region->pixel_format = mir_pixel_format_xbgr_8888;
+    test_region.pixel_format = mir_pixel_format_xbgr_8888;
 
     mtd::DrawPatternCheckered<2,2> pattern(pattern_colors);