Merge lp:~vanvugt/mir/remove-callbacks-mir_connect into lp:~mir-team/mir/trunk

[Daniel van Vugt]

Remove mir_connect callback from the public API.

First pass at simplifying the client API by removing callbacks. This is part
of a greater effort to resolve LP: #1112195, with more proposals to come.

[Alan Griffiths (alan-griffiths)]

We were asked to provided a callback based API.

Ignoring the stakeholders (I'm sure Thomas can provide a list) by removing the callbacks they asked is a bad idea.

review: Disapprove

[Thomas Voß (thomas-voss)]

In our conversations, both our toolkit experts (loicm and kaleo) expressed a requirement for an asynchronous API based on callbacks and added support for that requirement. I will disapprove until we have clarified with the stakeholders.

review: Disapprove

[Daniel van Vugt (vanvugt)]

The API is still asynchronous. I'm not sure the stakeholders even conceived that it could be so without callbacks. And if callbacks are still required, they should be attached to a MirWaitHandle in the spirit of:
  mir_callback_on_completion(waithandle, callback, data);

I hope we can resolve this. The API is presently so awful that I refuse to use it. And I won't be the only one.

[Daniel van Vugt (vanvugt)]

An alternate solution, as discussed in bug 1112195 is to simply add the synchronous functions that most people will want to use:
  connection = mir_connect(...);
Though that suggests the existing functions like "mir_connect" change to something like "mir_async_connect" or "mir_background_connect".

The approach proposed here however is a nice middle-ground, not requiring duplicate functions to elegantly support both asynchronous and synchronous operation.

[Daniel van Vugt (vanvugt)]

Design issues should be discussed in bug 1112195...

But I have many possible compromise solutions in mine. Will propose later.

[Daniel van Vugt (vanvugt)]

*in mind

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:408
http://s-jenkins:8080/job/mir-ci/1036/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1036/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/1036//rebuild/?

review: Approve (continuous-integration)

Unmerged revisions

408. By Daniel van Vugt on 2013-02-07

Remove mir_connect callback from the public API.

First pass at simplifying the client API by removing callbacks. This is part
of a greater effort to resolve LP: #1112195, with more proposals to come.

407. By Alan Griffiths on 2013-02-06

3rd_party/android-input: remove reliance on hardware_legacy/power.(c|h).

Approved by Alexandros Frantzis, PS Jenkins bot.

406. By Alan Griffiths on 2013-02-06

3rd_party/android-input: remove reliance on cutils/atomic.(c|h).

Approved by Alexandros Frantzis, PS Jenkins bot.

405. By Alan Griffiths on 2013-02-06

3rd_party/android-input: move the files we want to keep to "input".

Approved by Thomas Voß, PS Jenkins bot.

404. By Kevin DuBois on 2013-02-06

remove the wrappers for <thread> and <chrono> that were used to work around poor stdlib support on android, now that we have proper support of these libraries in the chroot.

Approved by Alan Griffiths, PS Jenkins bot.

403. By Alan Griffiths on 2013-02-05

graphics: Silence "narrowing conversion" with g++ 4.7.

Approved by PS Jenkins bot, Kevin DuBois.

402. By Kevin DuBois on 2013-02-05

stride was not being put into the native window type correctly. This caused render glitches when client was rendering. Add test that catches this condition

fixes: bug #1111612. Fixes: https://bugs.launchpad.net/bugs/1111612.

Approved by Alexandros Frantzis, Robert Carr, PS Jenkins bot, Alan Griffiths.

401. By Alan Griffiths on 2013-02-05

various: An assortment of nitpicky tidy-up.

Approved by Alexandros Frantzis, PS Jenkins bot.

400. By Alan Griffiths on 2013-02-05

3rd_party/android-input: stub our small use of TextOutput.

Approved by Kevin DuBois, PS Jenkins bot.

399. By Daniel van Vugt on 2013-02-05

mir_client_library.h: Documentation cleanup

1. Make the comments properly C-compatible so that clients written in pure C
   can build. (LP: #1116104)
2. Fix invalid Doxygen syntax which was generating bad docs (LP: #1116120). Fixes: https://bugs.launchpad.net/bugs/1116104, https://bugs.launchpad.net/bugs/1116120.

Approved by PS Jenkins bot, Alan Griffiths.

=== modified file 'examples/demo_client.c'
--- examples/demo_client.c	2012-12-27 14:44:59 +0000
+++ examples/demo_client.c	2013-02-07 10:24:19 +0000
@@ -48,11 +48,6 @@
 /// needs to be accessible both to callbacks and to the control function.
 /// \snippet demo_client.c Callback_tag
 ///\internal [Callback_tag]
-// Callback to update MirDemoState on connection
-static void connection_callback(MirConnection *new_connection, void *context)
-{
-    ((MirDemoState*)context)->connection = new_connection;
-}
 
 // Callback to update MirDemoState on surface_create
 static void surface_create_callback(MirSurface *new_surface, void *context)
@@ -93,7 +88,7 @@
     /// \snippet demo_client.c connect_tag
     ///\internal [connect_tag]
     // Call mir_connect and wait for callback to complete.
-    mir_wait_for(mir_connect(server, __PRETTY_FUNCTION__, connection_callback, &mcd));
+    mir_wait_for(mir_connect(server, __PRETTY_FUNCTION__, &mcd.connection));
     puts("Connected");
     ///\internal [connect_tag]
 

=== modified file 'examples/demo_client_accelerated.cpp'
--- examples/demo_client_accelerated.cpp	2013-01-28 21:35:03 +0000
+++ examples/demo_client_accelerated.cpp	2013-02-07 10:24:19 +0000
@@ -32,12 +32,6 @@
 static MirConnection *connection = 0;
 static MirSurface *surface = 0;
 
-static void set_connection(MirConnection *new_connection, void * context)
-{
-    (void)context;
-    connection = new_connection;
-}
-
 static void surface_create_callback(MirSurface *new_surface, void *context)
 {
     (void)context;
@@ -76,7 +70,7 @@
 
     puts("Starting");
 
-    mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, set_connection, 0));
+    mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, &connection));
     puts("Connected");
 
     assert(connection != NULL);

=== modified file 'examples/demo_client_unaccelerated.c'
--- examples/demo_client_unaccelerated.c	2012-12-19 16:01:00 +0000
+++ examples/demo_client_unaccelerated.c	2013-02-07 10:24:19 +0000
@@ -30,12 +30,6 @@
 static MirConnection *connection = 0;
 static MirSurface *surface = 0;
 
-static void set_connection(MirConnection *new_connection, void * context)
-{
-    (void)context;
-    connection = new_connection;
-}
-
 static void surface_create_callback(MirSurface *new_surface, void *context)
 {
     (void)context;
@@ -136,7 +130,7 @@
 
     puts("Starting");
 
-    mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, set_connection, 0));
+    mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, &connection));
     puts("Connected");
 
     assert(connection != NULL);

=== modified file 'include/mir_client/mir_client_library.h'
--- include/mir_client/mir_client_library.h	2013-02-05 10:28:36 +0000
+++ include/mir_client/mir_client_library.h	2013-02-07 10:24:19 +0000
@@ -38,14 +38,6 @@
 typedef struct MirWaitHandle MirWaitHandle;
 
 /**
- * Callback to be passed when issuing a mir_connect request.
- *   \param [in] connection          the new connection
- *   \param [in,out] client_context  context provided by client in calling
- *                                   mir_connect
- */
-typedef void (*mir_connected_callback)(MirConnection *connection, void *client_context);
-
-/**
  * Callback to be passed when calling:
  *  - mir_surface_create
  *  - mir_surface_next_buffer
@@ -146,16 +138,13 @@
  * wait handle remains valid until the connection has been released.
  *   \param [in] server       a name identifying the server
  *   \param [in] app_name     a name referring to the application
- *   \param [in] callback     callback function to be invoked when request
- *                            completes
- *   \param [in,out] context  passed to the callback function
- *   \return                  a handle that can be passed to mir_wait_for
+ *   \param [out] result      where to store the connection handle on
+ *                            completion.
  */
 MirWaitHandle *mir_connect(
     char const *server,
     char const *app_name,
-    mir_connected_callback callback,
-    void *client_context);
+    MirConnection **result);
 
 /**
  * Test for a valid connection

=== modified file 'include/mir_client/mir_client_library_lightdm.h'
--- include/mir_client/mir_client_library_lightdm.h	2013-02-05 14:50:25 +0000
+++ include/mir_client/mir_client_library_lightdm.h	2013-02-07 10:24:19 +0000
@@ -40,8 +40,7 @@
     char const *server,
     int lightdm_id,
     char const *app_name,
-    mir_connected_callback callback,
-    void *client_context);
+    MirConnection **result);
 
 
 /**

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-02-05 18:12:46 +0000
+++ src/client/mir_client_library.cpp	2013-02-07 10:24:19 +0000
@@ -44,7 +44,8 @@
 MirConnection error_connection;
 }
 
-MirWaitHandle* mir_connect(char const* socket_file, char const* name, mir_connected_callback callback, void * context)
+MirWaitHandle* mir_connect(char const* socket_file, char const* name,
+                           MirConnection **result)
 {
 
     try
@@ -57,12 +58,17 @@
             log,
             client_platform_factory);
 
-        return connection->connect(name, callback, context);
+        return connection->connect(name, result);
     }
     catch (std::exception const& x)
     {
         error_connection.set_error_message(x.what());
-        callback(&error_connection, context);
+
+        // Test cases expect a non-null error connection but why not null
+        // for simpler error handling?
+        if (result)
+            *result = &error_connection;
+
         return 0;
     }
 }
@@ -195,8 +201,7 @@
     char const *server,
     int lightdm_id,
     char const *app_name,
-    mir_connected_callback callback,
-    void *client_context)
+    MirConnection **result)
 try
 {
     auto log = std::make_shared<mcl::ConsoleLogger>();
@@ -207,12 +212,13 @@
         log,
         client_platform_factory);
 
-    return connection->connect(lightdm_id, app_name, callback, client_context);
+    return connection->connect(lightdm_id, app_name, result);
 }
 catch (std::exception const& x)
 {
     error_connection.set_error_message(x.what());
-    callback(&error_connection, client_context);
+    if (result)
+        *result = 0;
     return 0;
 }
 

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-02-05 18:12:46 +0000
+++ src/client/mir_connection.cpp	2013-02-07 10:24:19 +0000
@@ -126,7 +126,7 @@
     return new_wait_handle;
 }
 
-void MirConnection::connected(mir_connected_callback callback, void * context)
+void MirConnection::connected(MirConnection **result)
 {
     /*
      * We need to create the client platform after the connection has been
@@ -136,14 +136,15 @@
     platform = client_platform_factory->create_client_platform(this);
     native_display = platform->create_egl_native_display();
 
-    callback(this, context);
+    if (result)
+        *result = this;
+
     connect_wait_handle.result_received();
 }
 
 MirWaitHandle* MirConnection::connect(
     const char* app_name,
-    mir_connected_callback callback,
-    void * context)
+    MirConnection **result)
 {
     connect_parameters.set_application_name(app_name);
     server.connect(
@@ -151,15 +152,14 @@
         &connect_parameters,
         &connect_result,
         google::protobuf::NewCallback(
-            this, &MirConnection::connected, callback, context));
+            this, &MirConnection::connected, result));
     return &connect_wait_handle;
 }
 
 MirWaitHandle* MirConnection::connect(
     int lightdm_id,
     const char* app_name,
-    mir_connected_callback callback,
-    void * context)
+    MirConnection **result)
 {
     connect_parameters.set_application_name(app_name);
     connect_parameters.set_lightdm_id(lightdm_id);
@@ -168,7 +168,7 @@
         &connect_parameters,
         &connect_result,
         google::protobuf::NewCallback(
-            this, &MirConnection::connected, callback, context));
+            this, &MirConnection::connected, result));
     return &connect_wait_handle;
 }
 

=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2013-02-05 18:12:46 +0000
+++ src/client/mir_connection.h	2013-02-07 10:24:19 +0000
@@ -71,14 +71,12 @@
 
     MirWaitHandle* connect(
         const char* app_name,
-        mir_connected_callback callback,
-        void * context);
+        MirConnection **result);
 
     MirWaitHandle* connect(
         int lightdm_id,
         const char* app_name,
-        mir_connected_callback callback,
-        void * context);
+        MirConnection **result);
 
     void select_focus_by_lightdm_id(int lightdm_id);
 
@@ -128,7 +126,7 @@
     struct SurfaceRelease;
 
     void done_disconnect();
-    void connected(mir_connected_callback callback, void * context);
+    void connected(MirConnection **result);
     void released(SurfaceRelease );
     void done_drm_auth_magic(mir_drm_auth_magic_callback callback, void* context);
 };

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2013-02-05 18:12:46 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2013-02-07 10:24:19 +0000
@@ -53,12 +53,6 @@
     {
     }
 
-    static void connection_callback(MirConnection * connection, void * context)
-    {
-        ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
-        config->connection = connection;
-    }
-
     static void create_surface_callback(MirSurface * surface, void * context)
     {
         ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
@@ -109,7 +103,8 @@
     {
         void exec()
         {
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -129,7 +124,8 @@
         void exec()
         {
 
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -200,7 +196,8 @@
 
         void exec()
         {
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -252,7 +249,8 @@
         void exec()
         {
 
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -288,7 +286,8 @@
     {
         void exec()
         {
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
             ASSERT_TRUE(connection != NULL);
 
             MirPlatformPackage platform_package;
@@ -314,7 +313,8 @@
     {
         void exec()
         {
-            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
             ASSERT_TRUE(connection != NULL);
 
             MirDisplayInfo display_info;
@@ -338,7 +338,8 @@
     {
         void exec()
         {
-            mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__,
+                                     &connection));
             ASSERT_TRUE(connection != NULL);
 
             char const* error = mir_connection_get_error_message(connection);
@@ -360,7 +361,8 @@
     {
         void exec()
         {
-            mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
+            mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters const request_params =
             {

=== modified file 'tests/acceptance-tests/test_focus_management_api.cpp'
--- tests/acceptance-tests/test_focus_management_api.cpp	2013-02-05 18:12:46 +0000
+++ tests/acceptance-tests/test_focus_management_api.cpp	2013-02-07 10:24:19 +0000
@@ -56,12 +56,6 @@
     {
     }
 
-    static void connection_callback(MirConnection* connection, void* context)
-    {
-        ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
-        config->connection = connection;
-    }
-
     static void create_surface_callback(MirSurface* surface, void* context)
     {
         ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
@@ -200,8 +194,7 @@
                 mir_test_socket,
                 lightdm_id,
                 __PRETTY_FUNCTION__,
-                connection_callback,
-                this));
+                &connection));
 
             ASSERT_TRUE(connection != NULL);
 
@@ -229,8 +222,7 @@
             mir_wait_for(mir_connect(
                 mir_test_socket,
                 __PRETTY_FUNCTION__,
-                connection_callback,
-                this));
+                &connection));
 
             wait_for(focus_ready);
             mir_select_focus_by_lightdm_id(connection, lightdm_id);

=== modified file 'tests/acceptance-tests/test_surfaceloop.cpp'
--- tests/acceptance-tests/test_surfaceloop.cpp	2013-01-11 23:47:40 +0000
+++ tests/acceptance-tests/test_surfaceloop.cpp	2013-02-07 10:24:19 +0000
@@ -96,28 +96,6 @@
     {
     }
 
-    static void connection_callback(MirConnection * connection, void * context)
-    {
-        ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
-        config->connected(connection);
-    }
-
-    void connected(MirConnection * new_connection)
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        connection = new_connection;
-        wait_condition.notify_all();
-    }
-
-    void wait_for_connect()
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        while (!connection)
-            wait_condition.wait(lock);
-    }
-
-    std::mutex guard;
-    std::condition_variable wait_condition;
     MirConnection * connection;
     static const int max_surface_count = 5;
     SurfaceSync ssync[max_surface_count];
@@ -160,9 +138,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters request_params =
             {
@@ -213,9 +190,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters request_params =
             {
@@ -257,9 +233,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters request_params =
             {

=== modified file 'tests/integration-tests/test_display_info.cpp'
--- tests/integration-tests/test_display_info.cpp	2013-01-11 23:47:40 +0000
+++ tests/integration-tests/test_display_info.cpp	2013-02-07 10:24:19 +0000
@@ -106,12 +106,6 @@
 
 };
 
-void connection_callback(MirConnection* connection, void* context)
-{
-    auto connection_ptr = static_cast<MirConnection**>(context);
-    *connection_ptr = connection;
-}
-
 }
 
 TEST_F(BespokeDisplayServerTestFixture, display_info_reaches_client)
@@ -139,7 +133,7 @@
         {
             MirConnection* connection{nullptr};
             mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
-                                     connection_callback, &connection));
+                                     &connection));
 
             MirDisplayInfo info;
 

=== modified file 'tests/integration-tests/test_drm_auth_magic.cpp'
--- tests/integration-tests/test_drm_auth_magic.cpp	2013-02-05 18:12:46 +0000
+++ tests/integration-tests/test_drm_auth_magic.cpp	2013-02-07 10:24:19 +0000
@@ -83,12 +83,6 @@
     MOCK_METHOD1(drm_auth_magic, void(unsigned int));
 };
 
-void connection_callback(MirConnection* connection, void* context)
-{
-    auto connection_ptr = static_cast<MirConnection**>(context);
-    *connection_ptr = connection;
-}
-
 void drm_auth_magic_callback(int status, void* client_context)
 {
     auto status_ptr = static_cast<int*>(client_context);
@@ -127,7 +121,7 @@
         {
             MirConnection* connection{nullptr};
             mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
-                                     connection_callback, &connection));
+                                     &connection));
 
             int const no_error{0};
             int status{67};
@@ -176,7 +170,7 @@
         {
             MirConnection* connection{nullptr};
             mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
-                                     connection_callback, &connection));
+                                     &connection));
 
             int status{67};
 

=== modified file 'tests/integration-tests/test_error_reporting.cpp'
--- tests/integration-tests/test_error_reporting.cpp	2013-02-05 18:12:46 +0000
+++ tests/integration-tests/test_error_reporting.cpp	2013-02-07 10:24:19 +0000
@@ -151,28 +151,6 @@
     {
     }
 
-    static void connection_callback(MirConnection * connection, void * context)
-    {
-        ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
-        config->connected(connection);
-    }
-
-    void connected(MirConnection * new_connection)
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        connection = new_connection;
-        wait_condition.notify_all();
-    }
-
-    void wait_for_connect()
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        while (!connection)
-            wait_condition.wait(lock);
-    }
-
-    std::mutex guard;
-    std::condition_variable wait_condition;
     MirConnection * connection;
     static const int max_surface_count = 5;
     SurfaceSync ssync[max_surface_count];
@@ -255,9 +233,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_FALSE(mir_connection_is_valid(connection));

=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
--- tests/integration-tests/test_surfaceloop.cpp	2013-02-05 18:12:46 +0000
+++ tests/integration-tests/test_surfaceloop.cpp	2013-02-07 10:24:19 +0000
@@ -183,28 +183,6 @@
     {
     }
 
-    static void connection_callback(MirConnection * connection, void * context)
-    {
-        ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
-        config->connected(connection);
-    }
-
-    void connected(MirConnection * new_connection)
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        connection = new_connection;
-        wait_condition.notify_all();
-    }
-
-    void wait_for_connect()
-    {
-        std::unique_lock<std::mutex> lock(guard);
-        while (!connection)
-            wait_condition.wait(lock);
-    }
-
-    std::mutex guard;
-    std::condition_variable wait_condition;
     MirConnection * connection;
     static const int max_surface_count = 5;
     SurfaceSync ssync[max_surface_count];
@@ -270,9 +248,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -373,9 +350,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             ASSERT_TRUE(connection != NULL);
             EXPECT_TRUE(mir_connection_is_valid(connection));
@@ -512,9 +488,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters const request_params =
             {
@@ -561,9 +536,8 @@
     {
         void exec()
         {
-            mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
-
-            wait_for_connect();
+            mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
+                                     &connection));
 
             MirSurfaceParameters const request_params =
             {

=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2013-01-02 15:57:13 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2013-02-07 10:24:19 +0000
@@ -201,10 +201,6 @@
 
 namespace mt = mir::test;
 
-void connected_callback(MirConnection* /*connection*/, void * /*client_context*/)
-{
-}
-
 struct CallBack
 {
     void msg() {}
@@ -233,7 +229,7 @@
         channel = mcl::make_rpc_channel("./test_socket_surface", logger);
         connection = std::make_shared<MirConnection>(channel, logger, platform_factory);
         MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
-                                                         connected_callback, 0);
+                                                         nullptr);
         wait_handle->wait_for_result();
         client_comm_channel = std::make_shared<mir::protobuf::DisplayServer::Stub>(channel.get());
     }

=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2013-01-02 15:57:13 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2013-02-07 10:24:19 +0000
@@ -98,10 +98,6 @@
     std::shared_ptr<mcl::ClientPlatform> platform;
 };
 
-void connected_callback(MirConnection* /*connection*/, void * /*client_context*/)
-{
-}
-
 void drm_auth_magic_callback(int status, void* client_context)
 {
     auto status_ptr = static_cast<int*>(client_context);
@@ -144,7 +140,7 @@
         .WillOnce(Return(native_display));
 
     MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
-                                                     connected_callback, 0);
+                                                     nullptr);
     wait_handle->wait_for_result();
 
     EGLNativeDisplayType connection_native_display = connection->egl_native_display();
@@ -167,7 +163,7 @@
         .Times(1);
 
     MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
-                                                     connected_callback, 0);
+                                                     nullptr);
     wait_handle->wait_for_result();
 
     int const no_error{0};
@@ -211,7 +207,7 @@
         .WillOnce(Invoke(fill_display_info));
 
     MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
-                                                     connected_callback, 0);
+                                                     nullptr);
     wait_handle->wait_for_result();
 
     MirDisplayInfo info;
@@ -236,7 +232,7 @@
         .WillOnce(Invoke(fill_display_info_100));
 
     MirWaitHandle* wait_handle = connection->connect("MirConnectionTest",
-                                                     connected_callback, 0);
+                                                     nullptr);
     wait_handle->wait_for_result();
 
     MirDisplayInfo info;