Mir

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

  1. optional-callbacks
  2. Merge into trunk
Proposed by Daniel van Vugt
Status: Superseded
Proposed branch: lp:~vanvugt/mir/optional-callbacks
Merge into: lp:~mir-team/mir/trunk
Diff against target: 998 lines (+258/-193)
20 files modified
examples/demo_client.c (+1/-6)
examples/demo_client_accelerated.cpp (+1/-7)
examples/demo_client_unaccelerated.c (+1/-7)
include/mir_toolkit/mir_client_library.h (+20/-13)
include/mir_toolkit/mir_client_library_lightdm.h (+1/-2)
src/client/mir_client_library.cpp (+20/-7)
src/client/mir_connection.cpp (+10/-8)
src/client/mir_connection.h (+3/-5)
src/client/mir_wait_handle.cpp (+32/-0)
src/client/mir_wait_handle.h (+8/-0)
tests/acceptance-tests/test_client_library.cpp (+44/-10)
tests/acceptance-tests/test_focus_management_api.cpp (+2/-10)
tests/acceptance-tests/test_surfaceloop.cpp (+6/-31)
tests/integration-tests/test_display_info.cpp (+1/-7)
tests/integration-tests/test_drm_auth_magic.cpp (+2/-8)
tests/integration-tests/test_error_reporting.cpp (+2/-25)
tests/integration-tests/test_surfaceloop.cpp (+8/-34)
tests/unit-tests/client/test_client_mir_surface.cpp (+1/-5)
tests/unit-tests/client/test_mir_connection.cpp (+4/-8)
tests/unit-tests/client/test_wait_handle.cpp (+91/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/optional-callbacks
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Needs Information
Review via email: mp+149994@code.launchpad.net

This proposal has been superseded by a proposal from 2013-03-12.

Commit message

Demonstrate how to make callbacks optional to the client API. Starting with
mir_connect() only. Callbacks are optionally attached to a MirWaitHandle.
This simplifies most client use-cases so you will see a lot of logic is
removed.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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

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

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

my initial reaction is that I like the idea of optional callbacks. Seeing as this is a change to the client api though, I'll mark as 'needs info' until we get more opinions on the matter in

review: Needs Information
lp:~vanvugt/mir/optional-callbacks updated
416. By Daniel van Vugt

Merge latest lp:mir

417. By Daniel van Vugt

Fix conflicts and merge problems with upstream changes.

418. By Daniel van Vugt

Merge test_wait.cpp into test_wait_handle.cpp

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:418
http://jenkins.qa.ubuntu.com/job/mir-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/20//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/20//rebuild/?

review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

I think register_callback is a bit misleading - as you can have only one callback set_callback seems more appropriate.

Is both context and callback_owner required? This seems redundant.

I don't like the use of void pointers in the callbacks, that is a step backwards from having a typed callback in the client functions.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Yeah I agree about set_callback. Not sure what I was thinking at the time. register_... sounds clumsy.

Yes context and owner are required to support the existing callbacks (client demos) without modification. But we could modify the clients to support single parameter callbacks.

Yeah I know there's a type safety issue with void* and have two proposals for fixing it:
  (1) Remove "owner" as mentioned above. Then "context" is opaque to Mir and is never cast; or
  (2) https://code.launchpad.net/~vanvugt/mir/simple-connect/+merge/147305

lp:~vanvugt/mir/optional-callbacks updated
419. By Daniel van Vugt

Merge latest lp:mir

420. By Daniel van Vugt

Clean up: "register_callback" --> "set_callback"

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Ok, "register" has been changed to "set".

I am willing to remove "owner" to improve type strength, but would want confirmation that people agree, before I invest the effort to modify all the clients and tests' callbacks.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:420
http://jenkins.qa.ubuntu.com/job/mir-ci/50/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/51//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/50//rebuild/?

review: Approve (continuous-integration)
lp:~vanvugt/mir/optional-callbacks updated
421. By Daniel van Vugt

Merge latest lp:mir

422. By Daniel van Vugt

Fix conflict (build failure) introduced by upstream changes.

423. By Daniel van Vugt

Simplified, and removed the type safety problem.

Unmerged revisions

423. By Daniel van Vugt

Simplified, and removed the type safety problem.

422. By Daniel van Vugt

Fix conflict (build failure) introduced by upstream changes.

421. By Daniel van Vugt

Merge latest lp:mir

420. By Daniel van Vugt

Clean up: "register_callback" --> "set_callback"

419. By Daniel van Vugt

Merge latest lp:mir

418. By Daniel van Vugt

Merge test_wait.cpp into test_wait_handle.cpp

417. By Daniel van Vugt

Fix conflicts and merge problems with upstream changes.

416. By Daniel van Vugt

Merge latest lp:mir

415. By Daniel van Vugt

Shrink the diff a little.

414. By Daniel van Vugt

Merge latest lp:mir and fix numerous conflicts with mir_toolkit changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/demo_client.c'
2--- examples/demo_client.c 2013-02-26 16:48:46 +0000
3+++ examples/demo_client.c 2013-03-12 03:44:21 +0000
4@@ -48,11 +48,6 @@
5 /// needs to be accessible both to callbacks and to the control function.
6 /// \snippet demo_client.c Callback_tag
7 ///\internal [Callback_tag]
8-// Callback to update MirDemoState on connection
9-static void connection_callback(MirConnection *new_connection, void *context)
10-{
11- ((MirDemoState*)context)->connection = new_connection;
12-}
13
14 // Callback to update MirDemoState on surface_create
15 static void surface_create_callback(MirSurface *new_surface, void *context)
16@@ -93,7 +88,7 @@
17 /// \snippet demo_client.c connect_tag
18 ///\internal [connect_tag]
19 // Call mir_connect and wait for callback to complete.
20- mir_wait_for(mir_connect(server, __PRETTY_FUNCTION__, connection_callback, &mcd));
21+ mir_wait_for(mir_connect(server, __PRETTY_FUNCTION__, &mcd.connection));
22 puts("Connected");
23 ///\internal [connect_tag]
24
25
26=== modified file 'examples/demo_client_accelerated.cpp'
27--- examples/demo_client_accelerated.cpp 2013-02-26 16:48:46 +0000
28+++ examples/demo_client_accelerated.cpp 2013-03-12 03:44:21 +0000
29@@ -32,12 +32,6 @@
30 static MirConnection *connection = 0;
31 static MirSurface *surface = 0;
32
33-static void set_connection(MirConnection *new_connection, void * context)
34-{
35- (void)context;
36- connection = new_connection;
37-}
38-
39 static void surface_create_callback(MirSurface *new_surface, void *context)
40 {
41 (void)context;
42@@ -76,7 +70,7 @@
43
44 puts("Starting");
45
46- mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, set_connection, 0));
47+ mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, &connection));
48 puts("Connected");
49
50 assert(connection != NULL);
51
52=== modified file 'examples/demo_client_unaccelerated.c'
53--- examples/demo_client_unaccelerated.c 2013-02-26 16:48:46 +0000
54+++ examples/demo_client_unaccelerated.c 2013-03-12 03:44:21 +0000
55@@ -30,12 +30,6 @@
56 static MirConnection *connection = 0;
57 static MirSurface *surface = 0;
58
59-static void set_connection(MirConnection *new_connection, void * context)
60-{
61- (void)context;
62- connection = new_connection;
63-}
64-
65 static void surface_create_callback(MirSurface *new_surface, void *context)
66 {
67 (void)context;
68@@ -136,7 +130,7 @@
69
70 puts("Starting");
71
72- mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, set_connection, 0));
73+ mir_wait_for(mir_connect(socket_file, __PRETTY_FUNCTION__, &connection));
74 puts("Connected");
75
76 assert(connection != NULL);
77
78=== modified file 'include/mir_toolkit/mir_client_library.h'
79--- include/mir_toolkit/mir_client_library.h 2013-03-04 16:02:02 +0000
80+++ include/mir_toolkit/mir_client_library.h 2013-03-12 03:44:21 +0000
81@@ -41,12 +41,11 @@
82 typedef struct MirWaitHandle MirWaitHandle;
83
84 /**
85- * Callback to be passed when issuing a mir_connect request.
86- * \param [in] connection the new connection
87- * \param [in,out] client_context context provided by client in calling
88- * mir_connect
89+ * A generic callback that can be used with mir_callback_on().
90+ * \param [in] self The object being operated on.
91+ * \param [in] context Context data for this callback.
92 */
93-typedef void (*mir_connected_callback)(MirConnection *connection, void *client_context);
94+typedef void (*mir_generic_callback)(void *self, void *context);
95
96 /**
97 * Callback to be passed when calling:
98@@ -147,18 +146,15 @@
99 * Request a connection to the MIR server. The supplied callback is
100 * called when the connection is established, or fails. The returned
101 * wait handle remains valid until the connection has been released.
102- * \param [in] server a name identifying the server
103- * \param [in] app_name a name referring to the application
104- * \param [in] callback callback function to be invoked when request
105- * completes
106- * \param [in,out] client_context passed to the callback function
107- * \return a handle that can be passed to mir_wait_for
108+ * \param [in] server a name identifying the server
109+ * \param [in] app_name a name referring to the application
110+ * \param [out] result where to store the connection handle on completion
111+ * \return a handle that can be passed to mir_wait_for
112 */
113 MirWaitHandle *mir_connect(
114 char const *server,
115 char const *app_name,
116- mir_connected_callback callback,
117- void *client_context);
118+ MirConnection **result);
119
120 /**
121 * Test for a valid connection
122@@ -316,6 +312,17 @@
123 void mir_wait_for(MirWaitHandle *wait_handle);
124
125 /**
126+ * Register a callback to be called when the specified wait handle is
127+ * completed.
128+ * \param [in] wait The wait handle to callback on.
129+ * \param [in] cb The callback to call when 'wait' is signaled.
130+ * \param [in] context Extra data to pass to the callback.
131+ */
132+void mir_callback_on(MirWaitHandle *wait,
133+ mir_generic_callback cb,
134+ void *context);
135+
136+/**
137 * Return the id of the surface. (Only useful for debug output.)
138 * \param [in] surface the surface
139 * \return an internal id that identifies the surfaceS
140
141=== modified file 'include/mir_toolkit/mir_client_library_lightdm.h'
142--- include/mir_toolkit/mir_client_library_lightdm.h 2013-03-04 16:02:02 +0000
143+++ include/mir_toolkit/mir_client_library_lightdm.h 2013-03-12 03:44:21 +0000
144@@ -41,8 +41,7 @@
145 char const *server,
146 int lightdm_id,
147 char const *app_name,
148- mir_connected_callback callback,
149- void *client_context);
150+ MirConnection **result);
151
152
153 /**
154
155=== modified file 'src/client/mir_client_library.cpp'
156--- src/client/mir_client_library.cpp 2013-03-04 16:02:02 +0000
157+++ src/client/mir_client_library.cpp 2013-03-12 03:44:21 +0000
158@@ -44,7 +44,7 @@
159 mir_toolkit::MirConnection error_connection;
160 }
161
162-mir_toolkit::MirWaitHandle* mir_toolkit::mir_connect(char const* socket_file, char const* name, mir_connected_callback callback, void * context)
163+mir_toolkit::MirWaitHandle* mir_toolkit::mir_connect(char const* socket_file, char const* name, MirConnection **result)
164 {
165
166 try
167@@ -57,12 +57,17 @@
168 log,
169 client_platform_factory);
170
171- return connection->connect(name, callback, context);
172+ return connection->connect(name, result);
173 }
174 catch (std::exception const& x)
175 {
176 error_connection.set_error_message(x.what());
177- callback(&error_connection, context);
178+
179+ // Test cases expect a non-null error connection but why not null
180+ // for simpler error handling?
181+ if (result)
182+ *result = &error_connection;
183+
184 return 0;
185 }
186 }
187@@ -180,6 +185,14 @@
188 wait_handle->wait_for_result();
189 }
190
191+void mir_toolkit::mir_callback_on(MirWaitHandle *wait,
192+ mir_generic_callback cb,
193+ void *context)
194+{
195+ if (wait)
196+ wait->set_callback(cb, context);
197+}
198+
199 mir_toolkit::MirEGLNativeWindowType mir_toolkit::mir_surface_get_egl_native_window(MirSurface *surface)
200 {
201 return surface->generate_native_window();
202@@ -197,8 +210,7 @@
203 char const *server,
204 int lightdm_id,
205 char const *app_name,
206- mir_connected_callback callback,
207- void *client_context)
208+ MirConnection **result)
209 try
210 {
211 auto log = std::make_shared<mcl::ConsoleLogger>();
212@@ -209,12 +221,13 @@
213 log,
214 client_platform_factory);
215
216- return connection->connect(lightdm_id, app_name, callback, client_context);
217+ return connection->connect(lightdm_id, app_name, result);
218 }
219 catch (std::exception const& x)
220 {
221 error_connection.set_error_message(x.what());
222- callback(&error_connection, client_context);
223+ if (result)
224+ *result = 0;
225 return 0;
226 }
227
228
229=== modified file 'src/client/mir_connection.cpp'
230--- src/client/mir_connection.cpp 2013-03-04 16:02:02 +0000
231+++ src/client/mir_connection.cpp 2013-03-12 03:44:21 +0000
232@@ -37,6 +37,7 @@
233 server(0),
234 error_message("ERROR")
235 {
236+ connect_wait_handle.set_callback_owner(this);
237 }
238
239 mir_toolkit::MirConnection::MirConnection(
240@@ -53,6 +54,7 @@
241 valid_connections.insert(this);
242 }
243 connect_result.set_error("connect not called");
244+ connect_wait_handle.set_callback_owner(this);
245 }
246
247 mir_toolkit::MirConnection::~MirConnection()
248@@ -126,7 +128,7 @@
249 return new_wait_handle;
250 }
251
252-void mir_toolkit::MirConnection::connected(mir_connected_callback callback, void * context)
253+void mir_toolkit::MirConnection::connected(MirConnection **result)
254 {
255 /*
256 * We need to create the client platform after the connection has been
257@@ -136,14 +138,15 @@
258 platform = client_platform_factory->create_client_platform(this);
259 native_display = platform->create_egl_native_display();
260
261- callback(this, context);
262+ if (result)
263+ *result = this;
264+
265 connect_wait_handle.result_received();
266 }
267
268 mir_toolkit::MirWaitHandle* mir_toolkit::MirConnection::connect(
269 const char* app_name,
270- mir_connected_callback callback,
271- void * context)
272+ MirConnection **result)
273 {
274 connect_parameters.set_application_name(app_name);
275 server.connect(
276@@ -151,15 +154,14 @@
277 &connect_parameters,
278 &connect_result,
279 google::protobuf::NewCallback(
280- this, &MirConnection::connected, callback, context));
281+ this, &MirConnection::connected, result));
282 return &connect_wait_handle;
283 }
284
285 mir_toolkit::MirWaitHandle* mir_toolkit::MirConnection::connect(
286 int lightdm_id,
287 const char* app_name,
288- mir_connected_callback callback,
289- void * context)
290+ MirConnection **result)
291 {
292 connect_parameters.set_application_name(app_name);
293 connect_parameters.set_lightdm_id(lightdm_id);
294@@ -168,7 +170,7 @@
295 &connect_parameters,
296 &connect_result,
297 google::protobuf::NewCallback(
298- this, &MirConnection::connected, callback, context));
299+ this, &MirConnection::connected, result));
300 return &connect_wait_handle;
301 }
302
303
304=== modified file 'src/client/mir_connection.h'
305--- src/client/mir_connection.h 2013-03-04 16:02:02 +0000
306+++ src/client/mir_connection.h 2013-03-12 03:44:21 +0000
307@@ -71,14 +71,12 @@
308
309 MirWaitHandle* connect(
310 const char* app_name,
311- mir_connected_callback callback,
312- void * context);
313+ MirConnection **result);
314
315 MirWaitHandle* connect(
316 int lightdm_id,
317 const char* app_name,
318- mir_connected_callback callback,
319- void * context);
320+ MirConnection **result);
321
322 void select_focus_by_lightdm_id(int lightdm_id);
323
324@@ -128,7 +126,7 @@
325 struct SurfaceRelease;
326
327 void done_disconnect();
328- void connected(mir_connected_callback callback, void * context);
329+ void connected(MirConnection **result);
330 void released(SurfaceRelease );
331 void done_drm_auth_magic(mir_drm_auth_magic_callback callback, void* context);
332 };
333
334=== modified file 'src/client/mir_wait_handle.cpp'
335--- src/client/mir_wait_handle.cpp 2013-03-07 07:19:39 +0000
336+++ src/client/mir_wait_handle.cpp 2013-03-12 03:44:21 +0000
337@@ -22,6 +22,10 @@
338 mir_toolkit::MirWaitHandle::MirWaitHandle() :
339 guard(),
340 wait_condition(),
341+ callback(nullptr),
342+ callback_owner(nullptr),
343+ callback_arg(nullptr),
344+ called_back(false),
345 expecting(0),
346 received(0)
347 {
348@@ -42,6 +46,7 @@
349 {
350 std::unique_lock<std::mutex> lock(guard);
351
352+ called_back = false;
353 received++;
354 wait_condition.notify_all();
355 }
356@@ -53,7 +58,34 @@
357 while ((!expecting && !received) || (received < expecting))
358 wait_condition.wait(lock);
359
360+ if (!called_back && callback)
361+ {
362+ callback(callback_owner, callback_arg);
363+ called_back = true;
364+ }
365+
366 received = 0;
367 expecting = 0;
368 }
369
370+void mir_toolkit::MirWaitHandle::set_callback(Callback cb, void *context)
371+{
372+ std::unique_lock<std::mutex> lock(guard);
373+
374+ callback = cb;
375+ callback_arg = context;
376+ called_back = false;
377+ if (received && callback)
378+ {
379+ callback(callback_owner, callback_arg);
380+ called_back = true;
381+ }
382+}
383+
384+void mir_toolkit::MirWaitHandle::set_callback_owner(void *owner)
385+{
386+ std::unique_lock<std::mutex> lock(guard);
387+
388+ callback_owner = owner;
389+}
390+
391
392=== modified file 'src/client/mir_wait_handle.h'
393--- src/client/mir_wait_handle.h 2013-03-07 07:19:39 +0000
394+++ src/client/mir_wait_handle.h 2013-03-12 03:44:21 +0000
395@@ -31,14 +31,22 @@
396 MirWaitHandle();
397 ~MirWaitHandle();
398
399+ typedef void (*Callback)(void *owner, void *context);
400+
401 void expect_result();
402 void result_received();
403 void wait_for_result();
404+ void set_callback(Callback cb, void *context);
405+ void set_callback_owner(void *owner);
406
407 private:
408 std::mutex guard;
409 std::condition_variable wait_condition;
410
411+ Callback callback;
412+ void *callback_owner;
413+ void *callback_arg;
414+ bool called_back;
415 int expecting;
416 int received;
417 };
418
419=== modified file 'tests/acceptance-tests/test_client_library.cpp'
420--- tests/acceptance-tests/test_client_library.cpp 2013-02-26 16:48:46 +0000
421+++ tests/acceptance-tests/test_client_library.cpp 2013-03-12 03:44:21 +0000
422@@ -109,9 +109,36 @@
423 {
424 void exec()
425 {
426- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
427-
428- ASSERT_TRUE(connection != NULL);
429+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
430+ &connection));
431+
432+ ASSERT_TRUE(connection != NULL);
433+ EXPECT_TRUE(mir_connection_is_valid(connection));
434+ EXPECT_STREQ(mir_connection_get_error_message(connection), "");
435+
436+ mir_connection_release(connection);
437+ }
438+ } client_config;
439+
440+ launch_client_process(client_config);
441+}
442+
443+TEST_F(DefaultDisplayServerTestFixture, connect_calls_back)
444+{
445+ struct ClientConfig : ClientConfigCommon
446+ {
447+ void exec()
448+ {
449+ MirConnection *conn = nullptr;
450+ MirWaitHandle *wait = mir_connect(mir_test_socket,
451+ __PRETTY_FUNCTION__,
452+ &conn);
453+ mir_callback_on(wait, (mir_generic_callback)connection_callback,
454+ this);
455+ mir_wait_for(wait);
456+
457+ ASSERT_TRUE(connection != NULL);
458+ EXPECT_EQ(connection, conn);
459 EXPECT_TRUE(mir_connection_is_valid(connection));
460 EXPECT_STREQ(mir_connection_get_error_message(connection), "");
461
462@@ -129,7 +156,8 @@
463 void exec()
464 {
465
466- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
467+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
468+ &connection));
469
470 ASSERT_TRUE(connection != NULL);
471 EXPECT_TRUE(mir_connection_is_valid(connection));
472@@ -200,7 +228,8 @@
473
474 void exec()
475 {
476- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
477+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
478+ &connection));
479
480 ASSERT_TRUE(connection != NULL);
481 EXPECT_TRUE(mir_connection_is_valid(connection));
482@@ -252,7 +281,8 @@
483 void exec()
484 {
485
486- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
487+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
488+ &connection));
489
490 ASSERT_TRUE(connection != NULL);
491 EXPECT_TRUE(mir_connection_is_valid(connection));
492@@ -288,7 +318,8 @@
493 {
494 void exec()
495 {
496- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
497+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
498+ &connection));
499 ASSERT_TRUE(connection != NULL);
500
501 MirPlatformPackage platform_package;
502@@ -314,7 +345,8 @@
503 {
504 void exec()
505 {
506- mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this));
507+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
508+ &connection));
509 ASSERT_TRUE(connection != NULL);
510
511 MirDisplayInfo display_info;
512@@ -338,7 +370,8 @@
513 {
514 void exec()
515 {
516- mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
517+ mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__,
518+ &connection));
519 ASSERT_TRUE(connection != NULL);
520
521 char const* error = mir_connection_get_error_message(connection);
522@@ -360,7 +393,8 @@
523 {
524 void exec()
525 {
526- mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__, connection_callback, this));
527+ mir_wait_for(mir_connect("garbage", __PRETTY_FUNCTION__,
528+ &connection));
529
530 MirSurfaceParameters const request_params =
531 {
532
533=== modified file 'tests/acceptance-tests/test_focus_management_api.cpp'
534--- tests/acceptance-tests/test_focus_management_api.cpp 2013-03-06 12:25:34 +0000
535+++ tests/acceptance-tests/test_focus_management_api.cpp 2013-03-12 03:44:21 +0000
536@@ -56,12 +56,6 @@
537 {
538 }
539
540- static void connection_callback(MirConnection* connection, void* context)
541- {
542- ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
543- config->connection = connection;
544- }
545-
546 static void create_surface_callback(MirSurface* surface, void* context)
547 {
548 ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
549@@ -200,8 +194,7 @@
550 mir_test_socket,
551 lightdm_id,
552 __PRETTY_FUNCTION__,
553- connection_callback,
554- this));
555+ &connection));
556
557 ASSERT_TRUE(connection != NULL);
558
559@@ -229,8 +222,7 @@
560 mir_wait_for(mir_connect(
561 mir_test_socket,
562 __PRETTY_FUNCTION__,
563- connection_callback,
564- this));
565+ &connection));
566
567 wait_for(focus_ready);
568 mir_select_focus_by_lightdm_id(connection, lightdm_id);
569
570=== modified file 'tests/acceptance-tests/test_surfaceloop.cpp'
571--- tests/acceptance-tests/test_surfaceloop.cpp 2013-02-26 16:48:46 +0000
572+++ tests/acceptance-tests/test_surfaceloop.cpp 2013-03-12 03:44:21 +0000
573@@ -96,28 +96,6 @@
574 {
575 }
576
577- static void connection_callback(MirConnection * connection, void * context)
578- {
579- ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
580- config->connected(connection);
581- }
582-
583- void connected(MirConnection * new_connection)
584- {
585- std::unique_lock<std::mutex> lock(guard);
586- connection = new_connection;
587- wait_condition.notify_all();
588- }
589-
590- void wait_for_connect()
591- {
592- std::unique_lock<std::mutex> lock(guard);
593- while (!connection)
594- wait_condition.wait(lock);
595- }
596-
597- std::mutex guard;
598- std::condition_variable wait_condition;
599 MirConnection * connection;
600 static const int max_surface_count = 5;
601 SurfaceSync ssync[max_surface_count];
602@@ -160,9 +138,8 @@
603 {
604 void exec()
605 {
606- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
607-
608- wait_for_connect();
609+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
610+ &connection));
611
612 MirSurfaceParameters request_params =
613 {
614@@ -213,9 +190,8 @@
615 {
616 void exec()
617 {
618- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
619-
620- wait_for_connect();
621+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
622+ &connection));
623
624 MirSurfaceParameters request_params =
625 {
626@@ -257,9 +233,8 @@
627 {
628 void exec()
629 {
630- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
631-
632- wait_for_connect();
633+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
634+ &connection));
635
636 MirSurfaceParameters request_params =
637 {
638
639=== modified file 'tests/integration-tests/test_display_info.cpp'
640--- tests/integration-tests/test_display_info.cpp 2013-02-26 16:48:46 +0000
641+++ tests/integration-tests/test_display_info.cpp 2013-03-12 03:44:21 +0000
642@@ -106,12 +106,6 @@
643
644 };
645
646-void connection_callback(MirConnection* connection, void* context)
647-{
648- auto connection_ptr = static_cast<MirConnection**>(context);
649- *connection_ptr = connection;
650-}
651-
652 }
653
654 TEST_F(BespokeDisplayServerTestFixture, display_info_reaches_client)
655@@ -139,7 +133,7 @@
656 {
657 MirConnection* connection{nullptr};
658 mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
659- connection_callback, &connection));
660+ &connection));
661
662 MirDisplayInfo info;
663
664
665=== modified file 'tests/integration-tests/test_drm_auth_magic.cpp'
666--- tests/integration-tests/test_drm_auth_magic.cpp 2013-02-26 16:48:46 +0000
667+++ tests/integration-tests/test_drm_auth_magic.cpp 2013-03-12 03:44:21 +0000
668@@ -83,12 +83,6 @@
669 MOCK_METHOD1(drm_auth_magic, void(unsigned int));
670 };
671
672-void connection_callback(MirConnection* connection, void* context)
673-{
674- auto connection_ptr = static_cast<MirConnection**>(context);
675- *connection_ptr = connection;
676-}
677-
678 void drm_auth_magic_callback(int status, void* client_context)
679 {
680 auto status_ptr = static_cast<int*>(client_context);
681@@ -127,7 +121,7 @@
682 {
683 MirConnection* connection{nullptr};
684 mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
685- connection_callback, &connection));
686+ &connection));
687
688 int const no_error{0};
689 int status{67};
690@@ -176,7 +170,7 @@
691 {
692 MirConnection* connection{nullptr};
693 mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
694- connection_callback, &connection));
695+ &connection));
696
697 int status{67};
698
699
700=== modified file 'tests/integration-tests/test_error_reporting.cpp'
701--- tests/integration-tests/test_error_reporting.cpp 2013-03-06 12:25:34 +0000
702+++ tests/integration-tests/test_error_reporting.cpp 2013-03-12 03:44:21 +0000
703@@ -151,28 +151,6 @@
704 {
705 }
706
707- static void connection_callback(MirConnection * connection, void * context)
708- {
709- ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
710- config->connected(connection);
711- }
712-
713- void connected(MirConnection * new_connection)
714- {
715- std::unique_lock<std::mutex> lock(guard);
716- connection = new_connection;
717- wait_condition.notify_all();
718- }
719-
720- void wait_for_connect()
721- {
722- std::unique_lock<std::mutex> lock(guard);
723- while (!connection)
724- wait_condition.wait(lock);
725- }
726-
727- std::mutex guard;
728- std::condition_variable wait_condition;
729 MirConnection * connection;
730 static const int max_surface_count = 5;
731 SurfaceSync ssync[max_surface_count];
732@@ -255,9 +233,8 @@
733 {
734 void exec()
735 {
736- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
737-
738- wait_for_connect();
739+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
740+ &connection));
741
742 ASSERT_TRUE(connection != NULL);
743 EXPECT_FALSE(mir_connection_is_valid(connection));
744
745=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
746--- tests/integration-tests/test_surfaceloop.cpp 2013-02-26 16:48:46 +0000
747+++ tests/integration-tests/test_surfaceloop.cpp 2013-03-12 03:44:21 +0000
748@@ -183,28 +183,6 @@
749 {
750 }
751
752- static void connection_callback(MirConnection * connection, void * context)
753- {
754- ClientConfigCommon * config = reinterpret_cast<ClientConfigCommon *>(context);
755- config->connected(connection);
756- }
757-
758- void connected(MirConnection * new_connection)
759- {
760- std::unique_lock<std::mutex> lock(guard);
761- connection = new_connection;
762- wait_condition.notify_all();
763- }
764-
765- void wait_for_connect()
766- {
767- std::unique_lock<std::mutex> lock(guard);
768- while (!connection)
769- wait_condition.wait(lock);
770- }
771-
772- std::mutex guard;
773- std::condition_variable wait_condition;
774 MirConnection * connection;
775 static const int max_surface_count = 5;
776 SurfaceSync ssync[max_surface_count];
777@@ -270,9 +248,8 @@
778 {
779 void exec()
780 {
781- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
782-
783- wait_for_connect();
784+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
785+ &connection));
786
787 ASSERT_TRUE(connection != NULL);
788 EXPECT_TRUE(mir_connection_is_valid(connection));
789@@ -373,9 +350,8 @@
790 {
791 void exec()
792 {
793- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
794-
795- wait_for_connect();
796+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
797+ &connection));
798
799 ASSERT_TRUE(connection != NULL);
800 EXPECT_TRUE(mir_connection_is_valid(connection));
801@@ -512,9 +488,8 @@
802 {
803 void exec()
804 {
805- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
806-
807- wait_for_connect();
808+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
809+ &connection));
810
811 MirSurfaceParameters const request_params =
812 {
813@@ -561,9 +536,8 @@
814 {
815 void exec()
816 {
817- mir_connect(mir_test_socket, __PRETTY_FUNCTION__, connection_callback, this);
818-
819- wait_for_connect();
820+ mir_wait_for(mir_connect(mir_test_socket, __PRETTY_FUNCTION__,
821+ &connection));
822
823 MirSurfaceParameters const request_params =
824 {
825
826=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
827--- tests/unit-tests/client/test_client_mir_surface.cpp 2013-02-26 16:48:46 +0000
828+++ tests/unit-tests/client/test_client_mir_surface.cpp 2013-03-12 03:44:21 +0000
829@@ -201,10 +201,6 @@
830
831 namespace mt = mir::test;
832
833-void connected_callback(MirConnection* /*connection*/, void * /*client_context*/)
834-{
835-}
836-
837 struct CallBack
838 {
839 void msg() {}
840@@ -233,7 +229,7 @@
841 channel = mcl::make_rpc_channel("./test_socket_surface", logger);
842 connection = std::make_shared<MirConnection>(channel, logger, platform_factory);
843 MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
844- connected_callback, 0);
845+ nullptr);
846 wait_handle->wait_for_result();
847 client_comm_channel = std::make_shared<mir::protobuf::DisplayServer::Stub>(channel.get());
848 }
849
850=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
851--- tests/unit-tests/client/test_mir_connection.cpp 2013-01-02 15:57:13 +0000
852+++ tests/unit-tests/client/test_mir_connection.cpp 2013-03-12 03:44:21 +0000
853@@ -98,10 +98,6 @@
854 std::shared_ptr<mcl::ClientPlatform> platform;
855 };
856
857-void connected_callback(MirConnection* /*connection*/, void * /*client_context*/)
858-{
859-}
860-
861 void drm_auth_magic_callback(int status, void* client_context)
862 {
863 auto status_ptr = static_cast<int*>(client_context);
864@@ -144,7 +140,7 @@
865 .WillOnce(Return(native_display));
866
867 MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
868- connected_callback, 0);
869+ nullptr);
870 wait_handle->wait_for_result();
871
872 EGLNativeDisplayType connection_native_display = connection->egl_native_display();
873@@ -167,7 +163,7 @@
874 .Times(1);
875
876 MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
877- connected_callback, 0);
878+ nullptr);
879 wait_handle->wait_for_result();
880
881 int const no_error{0};
882@@ -211,7 +207,7 @@
883 .WillOnce(Invoke(fill_display_info));
884
885 MirWaitHandle* wait_handle = connection->connect("MirClientSurfaceTest",
886- connected_callback, 0);
887+ nullptr);
888 wait_handle->wait_for_result();
889
890 MirDisplayInfo info;
891@@ -236,7 +232,7 @@
892 .WillOnce(Invoke(fill_display_info_100));
893
894 MirWaitHandle* wait_handle = connection->connect("MirConnectionTest",
895- connected_callback, 0);
896+ nullptr);
897 wait_handle->wait_for_result();
898
899 MirDisplayInfo info;
900
901=== modified file 'tests/unit-tests/client/test_wait_handle.cpp'
902--- tests/unit-tests/client/test_wait_handle.cpp 2013-03-07 07:19:39 +0000
903+++ tests/unit-tests/client/test_wait_handle.cpp 2013-03-12 03:44:21 +0000
904@@ -112,3 +112,94 @@
905 }
906 t.join();
907 }
908+
909+TEST(WaitHandle, no_callback)
910+{
911+ MirWaitHandle w;
912+
913+ w.result_received();
914+ EXPECT_NO_FATAL_FAILURE(w.wait_for_result());
915+}
916+
917+namespace
918+{
919+ void *cb_owner = nullptr;
920+ void *cb_context = nullptr;
921+
922+ void callback(void *owner, void *context)
923+ {
924+ cb_owner = owner;
925+ cb_context = context;
926+ }
927+}
928+
929+TEST(WaitHandle, callback_before_signal)
930+{
931+ MirWaitHandle w;
932+ int alpha, beta, gamma, delta;
933+
934+ cb_owner = 0;
935+ cb_context = 0;
936+ w.set_callback_owner(&alpha);
937+ w.set_callback(callback, &beta);
938+ w.result_received();
939+ w.wait_for_result();
940+ EXPECT_EQ(cb_owner, &alpha);
941+ EXPECT_EQ(cb_context, &beta);
942+
943+ w.result_received();
944+ w.wait_for_result();
945+ EXPECT_EQ(cb_owner, &alpha);
946+ EXPECT_EQ(cb_context, &beta);
947+
948+ cb_owner = 0;
949+ cb_context = 0;
950+ w.set_callback_owner(&gamma);
951+ w.set_callback(callback, &delta);
952+ w.result_received();
953+ w.wait_for_result();
954+ EXPECT_EQ(cb_owner, &gamma);
955+ EXPECT_EQ(cb_context, &delta);
956+
957+ w.set_callback(nullptr, nullptr);
958+ EXPECT_NO_FATAL_FAILURE({
959+ w.result_received();
960+ w.wait_for_result();
961+ });
962+}
963+
964+TEST(WaitHandle, callback_after_signal)
965+{
966+ MirWaitHandle w;
967+ int alpha, beta, gamma, delta;
968+
969+ cb_owner = 0;
970+ cb_context = 0;
971+ w.result_received();
972+ w.set_callback_owner(&alpha);
973+ w.set_callback(callback, &beta);
974+ w.wait_for_result();
975+ EXPECT_EQ(cb_owner, &alpha);
976+ EXPECT_EQ(cb_context, &beta);
977+
978+ w.result_received();
979+ w.wait_for_result();
980+ EXPECT_EQ(cb_owner, &alpha);
981+ EXPECT_EQ(cb_context, &beta);
982+
983+ cb_owner = 0;
984+ cb_context = 0;
985+
986+ w.result_received();
987+ w.set_callback_owner(&gamma);
988+ w.set_callback(callback, &delta);
989+ w.wait_for_result();
990+ EXPECT_EQ(cb_owner, &gamma);
991+ EXPECT_EQ(cb_context, &delta);
992+
993+ w.set_callback(nullptr, nullptr);
994+ EXPECT_NO_FATAL_FAILURE({
995+ w.result_received();
996+ w.wait_for_result();
997+ });
998+}

Subscribers

People subscribed via source and target branches