Mir

Merge lp:~afrantzis/mir/mediating-display-changer-enqueue-actions into lp:mir

Proposed by Alexandros Frantzis
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 1630
Proposed branch: lp:~afrantzis/mir/mediating-display-changer-enqueue-actions
Merge into: lp:mir
Diff against target: 927 lines (+308/-106)
10 files modified
include/server/mir/default_server_configuration.h (+3/-0)
include/server/mir/display_changer.h (+3/-0)
src/server/default_server_configuration.cpp (+4/-0)
src/server/display_server.cpp (+8/-14)
src/server/scene/default_configuration.cpp (+2/-1)
src/server/scene/mediating_display_changer.cpp (+109/-54)
src/server/scene/mediating_display_changer.h (+12/-1)
tests/acceptance-tests/test_display_configuration.cpp (+20/-0)
tests/integration-tests/test_display_server_main_loop_events.cpp (+23/-35)
tests/unit-tests/scene/test_mediating_display_changer.cpp (+124/-1)
To merge this branch: bzr merge lp:~afrantzis/mir/mediating-display-changer-enqueue-actions
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Kevin DuBois (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+219224@code.launchpad.net

Commit message

scene: Use the ServerActionQueue for all display config actions from MediatingDisplayChanger

Description of the change

scene: Use the ServerActionQueue for all display config actions from MediatingDisplayChanger

This MP serializes all display configuration actions from MediatingDisplayChanger by using the ServerActionQueue interface. This ensures that all display configuration actions are performed only when it is safe to do so (i.e. not when the display server is paused).

This change also fixes https://bugs.launchpad.net/mir/+bug/1294510 which is caused by trying to change back to the default configuration during shutdown when both of the following are true:

 1. The server is paused (e.g. changing to a different terminal and shutting down the mir server from there)
 2. If the "focused" mir connection before the pause had a custom display configuration (e.g. Unity8)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

I don't see a reason to block, esp given the 'critical' bug. I wasn't able to assure myself that the 'this' captures in MediatingDisplayChanger have sane lifetimes. +1, might revisit some of the locking and lifetimes tomorrow on a fresh brain.

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> I wasn't able to assure myself that the 'this' captures in MediatingDisplayChanger have sane lifetimes.

All the enqueued actions run in the context of the ServerActionQueue, which is the main loop. All server objects, including the MediatingDisplayChanger ("this"), remain alive at least as long as the main loop is running, so there shouldn't be any problem with lifetimes.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

No obvious problems

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/default_server_configuration.h'
2--- include/server/mir/default_server_configuration.h 2014-05-09 09:56:45 +0000
3+++ include/server/mir/default_server_configuration.h 2014-05-12 15:56:28 +0000
4@@ -26,6 +26,8 @@
5
6 namespace mir
7 {
8+class ServerActionQueue;
9+
10 namespace compositor
11 {
12 class Renderer;
13@@ -255,6 +257,7 @@
14 /** @} */
15
16 virtual std::shared_ptr<time::Clock> the_clock();
17+ virtual std::shared_ptr<ServerActionQueue> the_server_action_queue();
18
19 protected:
20 std::shared_ptr<options::Option> the_options() const;
21
22=== modified file 'include/server/mir/display_changer.h'
23--- include/server/mir/display_changer.h 2013-08-13 13:54:24 +0000
24+++ include/server/mir/display_changer.h 2014-05-12 15:56:28 +0000
25@@ -39,6 +39,9 @@
26 std::shared_ptr<graphics::DisplayConfiguration> const& conf,
27 SystemStateHandling pause_resume_system) = 0;
28
29+ virtual void pause_display_config_processing() = 0;
30+ virtual void resume_display_config_processing() = 0;
31+
32 protected:
33 DisplayChanger() = default;
34 DisplayChanger(DisplayChanger const&) = delete;
35
36=== modified file 'src/server/default_server_configuration.cpp'
37--- src/server/default_server_configuration.cpp 2014-05-05 03:36:45 +0000
38+++ src/server/default_server_configuration.cpp 2014-05-12 15:56:28 +0000
39@@ -175,6 +175,10 @@
40 });
41 }
42
43+std::shared_ptr<mir::ServerActionQueue> mir::DefaultServerConfiguration::the_server_action_queue()
44+{
45+ return the_main_loop();
46+}
47
48 std::shared_ptr<mir::ServerStatusListener> mir::DefaultServerConfiguration::the_server_status_listener()
49 {
50
51=== modified file 'src/server/display_server.cpp'
52--- src/server/display_server.cpp 2014-05-09 09:17:44 +0000
53+++ src/server/display_server.cpp 2014-05-12 15:56:28 +0000
54@@ -105,8 +105,8 @@
55 [this] { input_manager->start(); }};
56
57 TryButRevertIfUnwinding display_config_processing{
58- [this] { main_loop->pause_processing_for(this); },
59- [this] { main_loop->resume_processing_for(this); }};
60+ [this] { display_changer->pause_display_config_processing(); },
61+ [this] { display_changer->resume_display_config_processing(); }};
62
63 TryButRevertIfUnwinding comp{
64 [this] { compositor->stop(); },
65@@ -141,8 +141,8 @@
66 [this] { connector->stop(); }};
67
68 TryButRevertIfUnwinding display_config_processing{
69- [this] { main_loop->resume_processing_for(this); },
70- [this] { main_loop->pause_processing_for(this); }};
71+ [this] { display_changer->resume_display_config_processing(); },
72+ [this] { display_changer->pause_display_config_processing(); }};
73
74 TryButRevertIfUnwinding input{
75 [this] { input_manager->start(); },
76@@ -166,17 +166,11 @@
77
78 void configure_display()
79 {
80- /* This is temporary, we will eventually use DisplayChanger directly */
81- main_loop->enqueue(
82- this,
83- [this]
84- {
85- std::shared_ptr<graphics::DisplayConfiguration> conf =
86- display->configuration();
87+ std::shared_ptr<graphics::DisplayConfiguration> conf =
88+ display->configuration();
89
90- display_changer->configure_for_hardware_change(
91- conf, DisplayChanger::PauseResumeSystem);
92- });
93+ display_changer->configure_for_hardware_change(
94+ conf, DisplayChanger::PauseResumeSystem);
95 }
96
97 std::shared_ptr<mg::Platform> const graphics_platform; // Hold this so the platform is loaded once
98
99=== modified file 'src/server/scene/default_configuration.cpp'
100--- src/server/scene/default_configuration.cpp 2014-05-09 09:56:45 +0000
101+++ src/server/scene/default_configuration.cpp 2014-05-12 15:56:28 +0000
102@@ -129,7 +129,8 @@
103 the_compositor(),
104 the_display_configuration_policy(),
105 the_session_container(),
106- the_session_event_handler_register());
107+ the_session_event_handler_register(),
108+ the_server_action_queue());
109 });
110
111 }
112
113=== modified file 'src/server/scene/mediating_display_changer.cpp'
114--- src/server/scene/mediating_display_changer.cpp 2014-04-15 05:31:19 +0000
115+++ src/server/scene/mediating_display_changer.cpp 2014-05-12 15:56:28 +0000
116@@ -24,6 +24,7 @@
117 #include "mir/compositor/compositor.h"
118 #include "mir/graphics/display_configuration_policy.h"
119 #include "mir/graphics/display_configuration.h"
120+#include "mir/server_action_queue.h"
121
122 namespace mf = mir::frontend;
123 namespace ms = mir::scene;
124@@ -62,73 +63,70 @@
125 std::shared_ptr<mc::Compositor> const& compositor,
126 std::shared_ptr<mg::DisplayConfigurationPolicy> const& display_configuration_policy,
127 std::shared_ptr<SessionContainer> const& session_container,
128- std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register)
129+ std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register,
130+ std::shared_ptr<ServerActionQueue> const& server_action_queue)
131 : display{display},
132 compositor{compositor},
133 display_configuration_policy{display_configuration_policy},
134 session_container{session_container},
135 session_event_handler_register{session_event_handler_register},
136+ server_action_queue{server_action_queue},
137 base_configuration{display->configuration()},
138 base_configuration_applied{true}
139 {
140 session_event_handler_register->register_focus_change_handler(
141 [this](std::shared_ptr<ms::Session> const& session)
142 {
143- std::lock_guard<std::mutex> lg{configuration_mutex};
144-
145- focused_session = session;
146-
147- /*
148- * If the newly focused session has a display configuration, apply it.
149- * Otherwise if we aren't currently using the base configuration,
150- * apply that.
151- */
152- auto it = config_map.find(session);
153- if (it != config_map.end())
154- {
155- apply_config(it->second, PauseResumeSystem);
156- }
157- else if (!base_configuration_applied)
158- {
159- apply_base_config(PauseResumeSystem);
160- }
161+ auto const weak_session = std::weak_ptr<ms::Session>(session);
162+ this->server_action_queue->enqueue(
163+ this,
164+ [this,weak_session]
165+ {
166+ if (auto const session = weak_session.lock())
167+ focus_change_handler(session);
168+ });
169 });
170
171 session_event_handler_register->register_no_focus_handler(
172 [this]
173 {
174- std::lock_guard<std::mutex> lg{configuration_mutex};
175-
176- focused_session.reset();
177- if (!base_configuration_applied)
178- {
179- apply_base_config(PauseResumeSystem);
180- }
181+ this->server_action_queue->enqueue(
182+ this,
183+ [this] { no_focus_handler(); });
184 });
185
186 session_event_handler_register->register_session_stopping_handler(
187 [this](std::shared_ptr<ms::Session> const& session)
188 {
189- std::lock_guard<std::mutex> lg{configuration_mutex};
190-
191- config_map.erase(session);
192+ auto const weak_session = std::weak_ptr<ms::Session>(session);
193+ this->server_action_queue->enqueue(
194+ this,
195+ [this,weak_session]
196+ {
197+ if (auto const session = weak_session.lock())
198+ session_stopping_handler(session);
199+ });
200 });
201-
202 }
203
204 void ms::MediatingDisplayChanger::configure(
205 std::shared_ptr<mf::Session> const& session,
206 std::shared_ptr<mg::DisplayConfiguration> const& conf)
207 {
208- std::lock_guard<std::mutex> lg{configuration_mutex};
209-
210- config_map[session] = conf;
211-
212- /* If the session is focused, apply the configuration */
213- if (focused_session.lock() == session)
214- {
215- apply_config(conf, PauseResumeSystem);
216- }
217+ server_action_queue->enqueue(
218+ this,
219+ [this, session, conf]
220+ {
221+ std::lock_guard<std::mutex> lg{configuration_mutex};
222+
223+ config_map[session] = conf;
224+
225+ /* If the session is focused, apply the configuration */
226+ if (focused_session.lock() == session)
227+ {
228+ apply_config(conf, PauseResumeSystem);
229+ }
230+ });
231 }
232
233 std::shared_ptr<mg::DisplayConfiguration>
234@@ -143,21 +141,36 @@
235 std::shared_ptr<graphics::DisplayConfiguration> const& conf,
236 SystemStateHandling pause_resume_system)
237 {
238- std::lock_guard<std::mutex> lg{configuration_mutex};
239-
240- display_configuration_policy->apply_to(*conf);
241- base_configuration = conf;
242- if (base_configuration_applied)
243- apply_base_config(pause_resume_system);
244-
245- /*
246- * Clear all the per-session configurations, since they may have become
247- * invalid due to the hardware change.
248- */
249- config_map.clear();
250-
251- /* Send the new configuration to all the sessions */
252- send_config_to_all_sessions(conf);
253+ server_action_queue->enqueue(
254+ this,
255+ [this, conf, pause_resume_system]
256+ {
257+ std::lock_guard<std::mutex> lg{configuration_mutex};
258+
259+ display_configuration_policy->apply_to(*conf);
260+ base_configuration = conf;
261+ if (base_configuration_applied)
262+ apply_base_config(pause_resume_system);
263+
264+ /*
265+ * Clear all the per-session configurations, since they may have become
266+ * invalid due to the hardware change.
267+ */
268+ config_map.clear();
269+
270+ /* Send the new configuration to all the sessions */
271+ send_config_to_all_sessions(conf);
272+ });
273+}
274+
275+void ms::MediatingDisplayChanger::pause_display_config_processing()
276+{
277+ server_action_queue->pause_processing_for(this);
278+}
279+
280+void ms::MediatingDisplayChanger::resume_display_config_processing()
281+{
282+ server_action_queue->resume_processing_for(this);
283 }
284
285 void ms::MediatingDisplayChanger::apply_config(
286@@ -196,3 +209,45 @@
287 session->send_display_config(*conf);
288 });
289 }
290+
291+void ms::MediatingDisplayChanger::focus_change_handler(
292+ std::shared_ptr<ms::Session> const& session)
293+{
294+ std::lock_guard<std::mutex> lg{configuration_mutex};
295+
296+ focused_session = session;
297+
298+ /*
299+ * If the newly focused session has a display configuration, apply it.
300+ * Otherwise if we aren't currently using the base configuration,
301+ * apply that.
302+ */
303+ auto const it = config_map.find(session);
304+ if (it != config_map.end())
305+ {
306+ apply_config(it->second, PauseResumeSystem);
307+ }
308+ else if (!base_configuration_applied)
309+ {
310+ apply_base_config(PauseResumeSystem);
311+ }
312+}
313+
314+void ms::MediatingDisplayChanger::no_focus_handler()
315+{
316+ std::lock_guard<std::mutex> lg{configuration_mutex};
317+
318+ focused_session.reset();
319+ if (!base_configuration_applied)
320+ {
321+ apply_base_config(PauseResumeSystem);
322+ }
323+}
324+
325+void ms::MediatingDisplayChanger::session_stopping_handler(
326+ std::shared_ptr<ms::Session> const& session)
327+{
328+ std::lock_guard<std::mutex> lg{configuration_mutex};
329+
330+ config_map.erase(session);
331+}
332
333=== modified file 'src/server/scene/mediating_display_changer.h'
334--- src/server/scene/mediating_display_changer.h 2014-03-26 05:48:59 +0000
335+++ src/server/scene/mediating_display_changer.h 2014-05-12 15:56:28 +0000
336@@ -27,6 +27,7 @@
337
338 namespace mir
339 {
340+class ServerActionQueue;
341
342 namespace graphics
343 {
344@@ -38,6 +39,7 @@
345 {
346 class SessionEventHandlerRegister;
347 class SessionContainer;
348+class Session;
349
350 class MediatingDisplayChanger : public frontend::DisplayChanger,
351 public mir::DisplayChanger
352@@ -48,7 +50,8 @@
353 std::shared_ptr<compositor::Compositor> const& compositor,
354 std::shared_ptr<graphics::DisplayConfigurationPolicy> const& display_configuration_policy,
355 std::shared_ptr<SessionContainer> const& session_container,
356- std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register);
357+ std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register,
358+ std::shared_ptr<ServerActionQueue> const& server_action_queue);
359
360 /* From mir::frontend::DisplayChanger */
361 std::shared_ptr<graphics::DisplayConfiguration> active_configuration();
362@@ -60,7 +63,14 @@
363 std::shared_ptr<graphics::DisplayConfiguration> const& conf,
364 SystemStateHandling pause_resume_system);
365
366+ void pause_display_config_processing() override;
367+ void resume_display_config_processing() override;
368+
369 private:
370+ void focus_change_handler(std::shared_ptr<Session> const& session);
371+ void no_focus_handler();
372+ void session_stopping_handler(std::shared_ptr<Session> const& session);
373+
374 void apply_config(std::shared_ptr<graphics::DisplayConfiguration> const& conf,
375 SystemStateHandling pause_resume_system);
376 void apply_base_config(SystemStateHandling pause_resume_system);
377@@ -72,6 +82,7 @@
378 std::shared_ptr<graphics::DisplayConfigurationPolicy> const display_configuration_policy;
379 std::shared_ptr<SessionContainer> const session_container;
380 std::shared_ptr<SessionEventHandlerRegister> const session_event_handler_register;
381+ std::shared_ptr<ServerActionQueue> const server_action_queue;
382 std::mutex configuration_mutex;
383 std::map<std::weak_ptr<frontend::Session>,
384 std::shared_ptr<graphics::DisplayConfiguration>,
385
386=== modified file 'tests/acceptance-tests/test_display_configuration.cpp'
387--- tests/acceptance-tests/test_display_configuration.cpp 2014-05-05 03:36:45 +0000
388+++ tests/acceptance-tests/test_display_configuration.cpp 2014-05-12 15:56:28 +0000
389@@ -17,6 +17,7 @@
390 * Kevin DuBois <kevin.dubois@canonical.com>
391 */
392
393+#include "mir/server_action_queue.h"
394 #include "mir/frontend/session_authorizer.h"
395 #include "mir/graphics/event_handler_register.h"
396 #include "src/server/scene/global_event_sender.h"
397@@ -35,6 +36,7 @@
398 #include "mir_test/fake_shared.h"
399 #include "mir_test/pipe.h"
400 #include "mir_test/cross_process_action.h"
401+#include "mir_test/wait_condition.h"
402
403 #include "mir_toolkit/mir_client_library.h"
404
405@@ -154,6 +156,16 @@
406 testing::NiceMock<MockDisplay> mock_display;
407 };
408
409+void wait_for_server_actions_to_finish(mir::ServerActionQueue& server_action_queue)
410+{
411+ mt::WaitCondition last_action_done;
412+ server_action_queue.enqueue(
413+ &last_action_done,
414+ [&] { last_action_done.wake_up_everyone(); });
415+
416+ last_action_done.wait_for_at_most_seconds(5);
417+}
418+
419 }
420
421 using DisplayConfigurationTest = BespokeDisplayServerTestFixture;
422@@ -481,12 +493,14 @@
423 t = std::thread([this](){
424 verify_connection_expectations.exec([&]
425 {
426+ wait_for_server_actions_to_finish(*the_server_action_queue());
427 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
428 EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
429 });
430
431 verify_apply_config_expectations.exec([&]
432 {
433+ wait_for_server_actions_to_finish(*the_server_action_queue());
434 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
435 });
436 });
437@@ -561,12 +575,14 @@
438 t = std::thread([this](){
439 verify_apply_config_expectations.exec([&]
440 {
441+ wait_for_server_actions_to_finish(*the_server_action_queue());
442 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
443 EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
444 });
445
446 verify_focus_change_expectations.exec([&]
447 {
448+ wait_for_server_actions_to_finish(*the_server_action_queue());
449 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
450 });
451 });
452@@ -655,12 +671,14 @@
453 t = std::thread([this](){
454 verify_apply_config_expectations.exec([&]
455 {
456+ wait_for_server_actions_to_finish(*the_server_action_queue());
457 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
458 EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
459 });
460
461 verify_focus_change_expectations.exec([&]
462 {
463+ wait_for_server_actions_to_finish(*the_server_action_queue());
464 testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
465 });
466 });
467@@ -741,6 +759,7 @@
468 {
469 using namespace testing;
470
471+ wait_for_server_actions_to_finish(*the_server_action_queue());
472 Mock::VerifyAndClearExpectations(&platform->mock_display);
473 /*
474 * A client with a per-session config is active, the base configuration
475@@ -750,6 +769,7 @@
476 platform->mock_display.emit_configuration_change_event(
477 mt::fake_shared(changed_stub_display_config));
478 platform->mock_display.wait_for_configuration_change_handler();
479+ wait_for_server_actions_to_finish(*the_server_action_queue());
480 Mock::VerifyAndClearExpectations(&platform->mock_display);
481 });
482 });
483
484=== modified file 'tests/integration-tests/test_display_server_main_loop_events.cpp'
485--- tests/integration-tests/test_display_server_main_loop_events.cpp 2014-05-12 15:23:20 +0000
486+++ tests/integration-tests/test_display_server_main_loop_events.cpp 2014-05-12 15:56:28 +0000
487@@ -20,11 +20,12 @@
488 #include "mir/frontend/connector.h"
489 #include "mir/graphics/display_configuration.h"
490 #include "mir/graphics/display_configuration_policy.h"
491-#include "mir/main_loop.h"
492-#include "mir/display_changer.h"
493+#include "mir/server_action_queue.h"
494+#include "mir/graphics/event_handler_register.h"
495 #include "mir/server_status_listener.h"
496
497 #include "mir_test/pipe.h"
498+#include "mir_test/wait_condition.h"
499 #include "mir_test_framework/testing_server_configuration.h"
500 #include "mir_test_doubles/mock_input_manager.h"
501 #include "mir_test_doubles/mock_input_dispatcher.h"
502@@ -68,14 +69,6 @@
503 void remove_endpoint() const {}
504 };
505
506-class MockDisplayChanger : public mir::DisplayChanger
507-{
508-public:
509- MOCK_METHOD2(configure_for_hardware_change,
510- void(std::shared_ptr<mg::DisplayConfiguration> const& conf,
511- SystemStateHandling pause_resume_system));
512-};
513-
514 class MockDisplay : public mtd::NullDisplay
515 {
516 public:
517@@ -260,14 +253,6 @@
518 return mock_input_dispatcher;
519 }
520
521- std::shared_ptr<mir::DisplayChanger> the_display_changer() override
522- {
523- if (!mock_display_changer)
524- mock_display_changer = std::make_shared<MockDisplayChanger>();
525-
526- return mock_display_changer;
527- }
528-
529 std::shared_ptr<MockDisplay> the_mock_display()
530 {
531 the_display();
532@@ -298,12 +283,6 @@
533 return mock_input_dispatcher;
534 }
535
536- std::shared_ptr<MockDisplayChanger> the_mock_display_changer()
537- {
538- the_display_changer();
539- return mock_display_changer;
540- }
541-
542 void emit_pause_event_and_wait_for_handler()
543 {
544 kill(getpid(), pause_signal);
545@@ -325,13 +304,21 @@
546 std::this_thread::sleep_for(std::chrono::microseconds{500});
547 }
548
549+ void wait_for_server_actions_to_finish()
550+ {
551+ mt::WaitCondition last_action_done;
552+ the_server_action_queue()->enqueue(&last_action_done,
553+ [&] { last_action_done.wake_up_everyone(); });
554+
555+ last_action_done.wait_for_at_most_seconds(5);
556+ }
557+
558 private:
559 std::shared_ptr<mtd::MockCompositor> mock_compositor;
560 std::shared_ptr<MockDisplay> mock_display;
561 std::shared_ptr<MockConnector> mock_connector;
562 std::shared_ptr<mtd::MockInputManager> mock_input_manager;
563 std::shared_ptr<mtd::MockInputDispatcher> mock_input_dispatcher;
564- std::shared_ptr<MockDisplayChanger> mock_display_changer;
565
566 mt::Pipe p;
567 int const pause_signal;
568@@ -553,7 +540,6 @@
569 auto mock_connector = server_config.the_mock_connector();
570 auto mock_input_manager = server_config.the_mock_input_manager();
571 auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
572- auto mock_display_changer = server_config.the_mock_display_changer();
573
574 {
575 InSequence s;
576@@ -564,10 +550,10 @@
577 EXPECT_CALL(*mock_input_manager, start()).Times(1);
578 EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
579
580- /* Configuration change event */
581- EXPECT_CALL(*mock_display_changer,
582- configure_for_hardware_change(_, mir::DisplayChanger::PauseResumeSystem))
583- .Times(1);
584+ /* Change configuration */
585+ EXPECT_CALL(*mock_compositor, stop()).Times(1);
586+ EXPECT_CALL(*mock_display, configure(_)).Times(1);
587+ EXPECT_CALL(*mock_compositor, start()).Times(1);
588
589 /* Stop */
590 EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
591@@ -583,6 +569,7 @@
592 [&]
593 {
594 server_config.emit_configuration_change_event_and_wait_for_handler();
595+ server_config.wait_for_server_actions_to_finish();
596 kill(getpid(), SIGTERM);
597 }};
598 t.detach();
599@@ -600,7 +587,6 @@
600 auto mock_connector = server_config.the_mock_connector();
601 auto mock_input_manager = server_config.the_mock_input_manager();
602 auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
603- auto mock_display_changer = server_config.the_mock_display_changer();
604
605 {
606 InSequence s;
607@@ -618,16 +604,17 @@
608 EXPECT_CALL(*mock_connector, stop()).Times(1);
609 EXPECT_CALL(*mock_display, pause()) .Times(1);
610
611- /* Resume and reconfigure event */
612+ /* Resume event */
613 EXPECT_CALL(*mock_display, resume()).Times(1);
614 EXPECT_CALL(*mock_connector, start()).Times(1);
615 EXPECT_CALL(*mock_input_manager, start()).Times(1);
616 EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
617 EXPECT_CALL(*mock_compositor, start()).Times(1);
618
619- EXPECT_CALL(*mock_display_changer,
620- configure_for_hardware_change(_, mir::DisplayChanger::PauseResumeSystem))
621- .Times(1);
622+ /* Change configuration (after resuming) */
623+ EXPECT_CALL(*mock_compositor, stop()).Times(1);
624+ EXPECT_CALL(*mock_display, configure(_)).Times(1);
625+ EXPECT_CALL(*mock_compositor, start()).Times(1);
626
627 /* Stop */
628 EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
629@@ -645,6 +632,7 @@
630 server_config.emit_pause_event_and_wait_for_handler();
631 server_config.emit_configuration_change_event_and_wait_for_handler();
632 server_config.emit_resume_event_and_wait_for_handler();
633+ server_config.wait_for_server_actions_to_finish();
634
635 kill(getpid(), SIGTERM);
636 }};
637
638=== modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
639--- tests/unit-tests/scene/test_mediating_display_changer.cpp 2014-05-05 12:36:41 +0000
640+++ tests/unit-tests/scene/test_mediating_display_changer.cpp 2014-05-12 15:56:28 +0000
641@@ -20,6 +20,7 @@
642 #include "src/server/scene/session_container.h"
643 #include "mir/graphics/display_configuration_policy.h"
644 #include "src/server/scene/broadcasting_session_event_sink.h"
645+#include "mir/server_action_queue.h"
646
647 #include "mir_test_doubles/mock_display.h"
648 #include "mir_test_doubles/mock_compositor.h"
649@@ -30,6 +31,8 @@
650 #include "mir_test/fake_shared.h"
651 #include "mir_test/display_config_matchers.h"
652
653+#include <mutex>
654+
655 #include <gmock/gmock.h>
656 #include <gtest/gtest.h>
657
658@@ -39,6 +42,9 @@
659 namespace ms = mir::scene;
660 namespace mg = mir::graphics;
661
662+namespace
663+{
664+
665 class MockDisplayConfigurationPolicy : public mg::DisplayConfigurationPolicy
666 {
667 public:
668@@ -84,6 +90,36 @@
669 mutable mg::DisplayConfiguration* conf_ptr;
670 };
671
672+struct StubServerActionQueue : mir::ServerActionQueue
673+{
674+ void flush()
675+ {
676+ std::lock_guard<std::mutex> lock{mutex};
677+ for (auto const& action : actions)
678+ action();
679+ actions.clear();
680+ }
681+
682+ void enqueue(void const* /*owner*/, mir::ServerAction const& action) override
683+ {
684+ std::lock_guard<std::mutex> lock{mutex};
685+ actions.push_back(action);
686+ }
687+
688+ void pause_processing_for(void const* /*owner*/) override {}
689+ void resume_processing_for(void const* /*owner*/) override {}
690+
691+ std::vector<mir::ServerAction> actions;
692+ std::mutex mutex;
693+};
694+
695+struct MockServerActionQueue : mir::ServerActionQueue
696+{
697+ MOCK_METHOD2(enqueue, void(void const*, mir::ServerAction const&));
698+ MOCK_METHOD1(pause_processing_for, void(void const*));
699+ MOCK_METHOD1(resume_processing_for, void(void const*));
700+};
701+
702 struct MediatingDisplayChangerTest : public ::testing::Test
703 {
704 MediatingDisplayChangerTest()
705@@ -95,7 +131,8 @@
706 mt::fake_shared(mock_compositor),
707 mt::fake_shared(mock_conf_policy),
708 mt::fake_shared(stub_session_container),
709- mt::fake_shared(session_event_sink));
710+ mt::fake_shared(session_event_sink),
711+ mt::fake_shared(server_action_queue));
712 }
713
714 testing::NiceMock<MockDisplay> mock_display;
715@@ -104,9 +141,12 @@
716 StubSessionContainer stub_session_container;
717 ms::BroadcastingSessionEventSink session_event_sink;
718 mtd::StubDisplayConfig base_config;
719+ StubServerActionQueue server_action_queue;
720 std::shared_ptr<ms::MediatingDisplayChanger> changer;
721 };
722
723+}
724+
725 TEST_F(MediatingDisplayChangerTest, returns_active_configuration_from_display)
726 {
727 using namespace testing;
728@@ -131,6 +171,8 @@
729 session_event_sink.handle_focus_change(session);
730 changer->configure(session,
731 mt::fake_shared(conf));
732+
733+ server_action_queue.flush();
734 }
735
736 TEST_F(MediatingDisplayChangerTest, doesnt_apply_config_for_unfocused_session)
737@@ -144,6 +186,8 @@
738
739 changer->configure(std::make_shared<mtd::StubSceneSession>(),
740 mt::fake_shared(conf));
741+
742+ server_action_queue.flush();
743 }
744
745 TEST_F(MediatingDisplayChangerTest, handles_hardware_change_properly_when_pausing_system)
746@@ -160,6 +204,8 @@
747
748 changer->configure_for_hardware_change(mt::fake_shared(conf),
749 mir::DisplayChanger::PauseResumeSystem);
750+
751+ server_action_queue.flush();
752 }
753
754 TEST_F(MediatingDisplayChangerTest, handles_hardware_change_properly_when_retaining_system_state)
755@@ -176,6 +222,8 @@
756
757 changer->configure_for_hardware_change(mt::fake_shared(conf),
758 mir::DisplayChanger::RetainSystemState);
759+
760+ server_action_queue.flush();
761 }
762
763 TEST_F(MediatingDisplayChangerTest, hardware_change_doesnt_apply_base_config_if_per_session_config_is_active)
764@@ -190,6 +238,7 @@
765
766 session_event_sink.handle_focus_change(session1);
767
768+ server_action_queue.flush();
769 Mock::VerifyAndClearExpectations(&mock_compositor);
770 Mock::VerifyAndClearExpectations(&mock_display);
771
772@@ -200,6 +249,8 @@
773
774 changer->configure_for_hardware_change(conf,
775 mir::DisplayChanger::PauseResumeSystem);
776+
777+ server_action_queue.flush();
778 }
779
780 TEST_F(MediatingDisplayChangerTest, notifies_all_sessions_on_hardware_config_change)
781@@ -217,6 +268,8 @@
782
783 changer->configure_for_hardware_change(mt::fake_shared(conf),
784 mir::DisplayChanger::PauseResumeSystem);
785+
786+ server_action_queue.flush();
787 }
788
789 TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_attached_config_applies_config)
790@@ -234,6 +287,8 @@
791 EXPECT_CALL(mock_compositor, start());
792
793 session_event_sink.handle_focus_change(session1);
794+
795+ server_action_queue.flush();
796 }
797
798 TEST_F(MediatingDisplayChangerTest, focusing_a_session_without_attached_config_applies_base_config)
799@@ -248,6 +303,7 @@
800
801 session_event_sink.handle_focus_change(session1);
802
803+ server_action_queue.flush();
804 Mock::VerifyAndClearExpectations(&mock_compositor);
805 Mock::VerifyAndClearExpectations(&mock_display);
806
807@@ -257,6 +313,8 @@
808 EXPECT_CALL(mock_compositor, start());
809
810 session_event_sink.handle_focus_change(session2);
811+
812+ server_action_queue.flush();
813 }
814
815 TEST_F(MediatingDisplayChangerTest, losing_focus_applies_base_config)
816@@ -270,6 +328,7 @@
817
818 session_event_sink.handle_focus_change(session1);
819
820+ server_action_queue.flush();
821 Mock::VerifyAndClearExpectations(&mock_compositor);
822 Mock::VerifyAndClearExpectations(&mock_display);
823
824@@ -279,6 +338,8 @@
825 EXPECT_CALL(mock_compositor, start());
826
827 session_event_sink.handle_no_focus();
828+
829+ server_action_queue.flush();
830 }
831
832 TEST_F(MediatingDisplayChangerTest, base_config_is_not_applied_if_already_active)
833@@ -298,6 +359,8 @@
834 session_event_sink.handle_focus_change(session1);
835 session_event_sink.handle_focus_change(session2);
836 session_event_sink.handle_no_focus();
837+
838+ server_action_queue.flush();
839 }
840
841 TEST_F(MediatingDisplayChangerTest, hardware_change_invalidates_session_configs)
842@@ -312,6 +375,7 @@
843 changer->configure_for_hardware_change(conf,
844 mir::DisplayChanger::PauseResumeSystem);
845
846+ server_action_queue.flush();
847 Mock::VerifyAndClearExpectations(&mock_compositor);
848 Mock::VerifyAndClearExpectations(&mock_display);
849
850@@ -324,6 +388,8 @@
851 EXPECT_CALL(mock_compositor, start()).Times(0);
852
853 session_event_sink.handle_focus_change(session1);
854+
855+ server_action_queue.flush();
856 }
857
858 TEST_F(MediatingDisplayChangerTest, session_stopping_invalidates_session_config)
859@@ -337,6 +403,7 @@
860
861 session_event_sink.handle_session_stopping(session1);
862
863+ server_action_queue.flush();
864 Mock::VerifyAndClearExpectations(&mock_compositor);
865 Mock::VerifyAndClearExpectations(&mock_display);
866
867@@ -349,4 +416,60 @@
868 EXPECT_CALL(mock_compositor, start()).Times(0);
869
870 session_event_sink.handle_focus_change(session1);
871+
872+ server_action_queue.flush();
873+}
874+
875+TEST_F(MediatingDisplayChangerTest, uses_server_action_queue_for_configuration_actions)
876+{
877+ using namespace testing;
878+
879+ auto const conf = std::make_shared<mtd::NullDisplayConfiguration>();
880+ auto const session1 = std::make_shared<mtd::StubSceneSession>();
881+ auto const session2 = std::make_shared<mtd::StubSceneSession>();
882+ MockServerActionQueue mock_server_action_queue;
883+
884+ stub_session_container.insert_session(session1);
885+ stub_session_container.insert_session(session2);
886+
887+ ms::MediatingDisplayChanger display_changer(
888+ mt::fake_shared(mock_display),
889+ mt::fake_shared(mock_compositor),
890+ mt::fake_shared(mock_conf_policy),
891+ mt::fake_shared(stub_session_container),
892+ mt::fake_shared(session_event_sink),
893+ mt::fake_shared(mock_server_action_queue));
894+
895+ void const* owner{nullptr};
896+
897+ EXPECT_CALL(mock_server_action_queue, enqueue(_, _))
898+ .WillOnce(SaveArg<0>(&owner));
899+ display_changer.configure(session1, conf);
900+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
901+
902+ EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
903+ display_changer.configure_for_hardware_change(
904+ conf,
905+ mir::DisplayChanger::PauseResumeSystem);
906+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
907+
908+ EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
909+ session_event_sink.handle_focus_change(session1);
910+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
911+
912+ EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
913+ session_event_sink.handle_focus_change(session2);
914+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
915+
916+ EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
917+ session_event_sink.handle_no_focus();
918+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
919+
920+ EXPECT_CALL(mock_server_action_queue, pause_processing_for(owner));
921+ display_changer.pause_display_config_processing();
922+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
923+
924+ EXPECT_CALL(mock_server_action_queue, resume_processing_for(owner));
925+ display_changer.resume_display_config_processing();
926+ Mock::VerifyAndClearExpectations(&mock_server_action_queue);
927 }

Subscribers

People subscribed via source and target branches