Merge lp:~afrantzis/mir/mediating-display-changer-enqueue-actions into lp:mir
- mediating-display-changer-enqueue-actions
- Merge into development-branch
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 |
Related bugs: |
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 MediatingDispla
Description of the change
scene: Use the ServerActionQueue for all display config actions from MediatingDispla
This MP serializes all display configuration actions from MediatingDispla
This change also fixes https:/
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)
PS Jenkins bot (ps-jenkins) wrote : | # |
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 MediatingDispla
Alexandros Frantzis (afrantzis) wrote : | # |
> I wasn't able to assure myself that the 'this' captures in MediatingDispla
All the enqueued actions run in the context of the ServerActionQueue, which is the main loop. All server objects, including the MediatingDispla
Alan Griffiths (alan-griffiths) wrote : | # |
No obvious problems
Preview Diff
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 | } |
PASSED: Continuous integration, rev:1620 jenkins. qa.ubuntu. com/job/ mir-team- mir-development -branch- ci/1593/ jenkins. qa.ubuntu. com/job/ mir-android- utopic- i386-build/ 149 jenkins. qa.ubuntu. com/job/ mir-clang- utopic- amd64-build/ 150 jenkins. qa.ubuntu. com/job/ mir-mediumtests -utopic- touch/150 jenkins. qa.ubuntu. com/job/ mir-team- mir-development -branch- utopic- amd64-ci/ 112 jenkins. qa.ubuntu. com/job/ mir-team- mir-development -branch- utopic- amd64-ci/ 112/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ mir-team- mir-development -branch- utopic- armhf-ci/ 110 jenkins. qa.ubuntu. com/job/ mir-team- mir-development -branch- utopic- armhf-ci/ 110/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- utopic- armhf/350 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- utopic- armhf/350/ artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ mir-mediumtests -runner- mako/1409 s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 7066
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/mir- team-mir- development- branch- ci/1593/ rebuild
http://