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

[Alexandros Frantzis]

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

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)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1620
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1593/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/150
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/150
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/112
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/112/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/110
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/110/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/350
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/350/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1409
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7066

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1593/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

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

[Alexandros Frantzis (afrantzis)]

> 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.

[Alan Griffiths (alan-griffiths)]

No obvious problems

review: Approve

=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2014-05-09 09:56:45 +0000
+++ include/server/mir/default_server_configuration.h	2014-05-12 15:56:28 +0000
@@ -26,6 +26,8 @@
 
 namespace mir
 {
+class ServerActionQueue;
+
 namespace compositor
 {
 class Renderer;
@@ -255,6 +257,7 @@
     /** @} */
 
     virtual std::shared_ptr<time::Clock> the_clock();
+    virtual std::shared_ptr<ServerActionQueue> the_server_action_queue();
 
 protected:
     std::shared_ptr<options::Option> the_options() const;

=== modified file 'include/server/mir/display_changer.h'
--- include/server/mir/display_changer.h	2013-08-13 13:54:24 +0000
+++ include/server/mir/display_changer.h	2014-05-12 15:56:28 +0000
@@ -39,6 +39,9 @@
         std::shared_ptr<graphics::DisplayConfiguration> const& conf,
         SystemStateHandling pause_resume_system) = 0;
 
+    virtual void pause_display_config_processing() = 0;
+    virtual void resume_display_config_processing() = 0;
+
 protected:
     DisplayChanger() = default;
     DisplayChanger(DisplayChanger const&) = delete;

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2014-05-05 03:36:45 +0000
+++ src/server/default_server_configuration.cpp	2014-05-12 15:56:28 +0000
@@ -175,6 +175,10 @@
         });
 }
 
+std::shared_ptr<mir::ServerActionQueue> mir::DefaultServerConfiguration::the_server_action_queue()
+{
+    return the_main_loop();
+}
 
 std::shared_ptr<mir::ServerStatusListener> mir::DefaultServerConfiguration::the_server_status_listener()
 {

=== modified file 'src/server/display_server.cpp'
--- src/server/display_server.cpp	2014-05-09 09:17:44 +0000
+++ src/server/display_server.cpp	2014-05-12 15:56:28 +0000
@@ -105,8 +105,8 @@
                 [this] { input_manager->start(); }};
 
             TryButRevertIfUnwinding display_config_processing{
-                [this] { main_loop->pause_processing_for(this); },
-                [this] { main_loop->resume_processing_for(this); }};
+                [this] { display_changer->pause_display_config_processing(); },
+                [this] { display_changer->resume_display_config_processing(); }};
 
             TryButRevertIfUnwinding comp{
                 [this] { compositor->stop(); },
@@ -141,8 +141,8 @@
                 [this] { connector->stop(); }};
 
             TryButRevertIfUnwinding display_config_processing{
-                [this] { main_loop->resume_processing_for(this); },
-                [this] { main_loop->pause_processing_for(this); }};
+                [this] { display_changer->resume_display_config_processing(); },
+                [this] { display_changer->pause_display_config_processing(); }};
 
             TryButRevertIfUnwinding input{
                 [this] { input_manager->start(); },
@@ -166,17 +166,11 @@
 
     void configure_display()
     {
-        /* This is temporary, we will eventually use DisplayChanger directly */
-        main_loop->enqueue(
-            this,
-            [this]
-            {
-                std::shared_ptr<graphics::DisplayConfiguration> conf =
-                    display->configuration();
+        std::shared_ptr<graphics::DisplayConfiguration> conf =
+            display->configuration();
 
-                display_changer->configure_for_hardware_change(
-                    conf, DisplayChanger::PauseResumeSystem);
-            });
+        display_changer->configure_for_hardware_change(
+            conf, DisplayChanger::PauseResumeSystem);
     }
 
     std::shared_ptr<mg::Platform> const graphics_platform; // Hold this so the platform is loaded once

=== modified file 'src/server/scene/default_configuration.cpp'
--- src/server/scene/default_configuration.cpp	2014-05-09 09:56:45 +0000
+++ src/server/scene/default_configuration.cpp	2014-05-12 15:56:28 +0000
@@ -129,7 +129,8 @@
                 the_compositor(),
                 the_display_configuration_policy(),
                 the_session_container(),
-                the_session_event_handler_register());
+                the_session_event_handler_register(),
+                the_server_action_queue());
         });
 
 }

=== modified file 'src/server/scene/mediating_display_changer.cpp'
--- src/server/scene/mediating_display_changer.cpp	2014-04-15 05:31:19 +0000
+++ src/server/scene/mediating_display_changer.cpp	2014-05-12 15:56:28 +0000
@@ -24,6 +24,7 @@
 #include "mir/compositor/compositor.h"
 #include "mir/graphics/display_configuration_policy.h"
 #include "mir/graphics/display_configuration.h"
+#include "mir/server_action_queue.h"
 
 namespace mf = mir::frontend;
 namespace ms = mir::scene;
@@ -62,73 +63,70 @@
     std::shared_ptr<mc::Compositor> const& compositor,
     std::shared_ptr<mg::DisplayConfigurationPolicy> const& display_configuration_policy,
     std::shared_ptr<SessionContainer> const& session_container,
-    std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register)
+    std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register,
+    std::shared_ptr<ServerActionQueue> const& server_action_queue)
     : display{display},
       compositor{compositor},
       display_configuration_policy{display_configuration_policy},
       session_container{session_container},
       session_event_handler_register{session_event_handler_register},
+      server_action_queue{server_action_queue},
       base_configuration{display->configuration()},
       base_configuration_applied{true}
 {
     session_event_handler_register->register_focus_change_handler(
         [this](std::shared_ptr<ms::Session> const& session)
         {
-            std::lock_guard<std::mutex> lg{configuration_mutex};
-
-            focused_session = session;
-
-            /*
-             * If the newly focused session has a display configuration, apply it.
-             * Otherwise if we aren't currently using the base configuration,
-             * apply that.
-             */
-            auto it = config_map.find(session);
-            if (it != config_map.end())
-            {
-                apply_config(it->second, PauseResumeSystem);
-            }
-            else if (!base_configuration_applied)
-            {
-                apply_base_config(PauseResumeSystem);
-            }
+            auto const weak_session = std::weak_ptr<ms::Session>(session);
+            this->server_action_queue->enqueue(
+                this,
+                [this,weak_session]
+                {
+                    if (auto const session = weak_session.lock())
+                        focus_change_handler(session);
+                });
         });
 
     session_event_handler_register->register_no_focus_handler(
         [this]
         {
-            std::lock_guard<std::mutex> lg{configuration_mutex};
-
-            focused_session.reset();
-            if (!base_configuration_applied)
-            {
-                apply_base_config(PauseResumeSystem);
-            }
+            this->server_action_queue->enqueue(
+                this,
+                [this] { no_focus_handler(); });
         });
 
     session_event_handler_register->register_session_stopping_handler(
         [this](std::shared_ptr<ms::Session> const& session)
         {
-            std::lock_guard<std::mutex> lg{configuration_mutex};
-
-            config_map.erase(session);
+            auto const weak_session = std::weak_ptr<ms::Session>(session);
+            this->server_action_queue->enqueue(
+                this,
+                [this,weak_session]
+                {
+                    if (auto const session = weak_session.lock())
+                        session_stopping_handler(session);
+                });
         });
-
 }
 
 void ms::MediatingDisplayChanger::configure(
     std::shared_ptr<mf::Session> const& session,
     std::shared_ptr<mg::DisplayConfiguration> const& conf)
 {
-    std::lock_guard<std::mutex> lg{configuration_mutex};
-
-    config_map[session] = conf;
-
-    /* If the session is focused, apply the configuration */
-    if (focused_session.lock() == session)
-    {
-        apply_config(conf, PauseResumeSystem);
-    }
+    server_action_queue->enqueue(
+        this,
+        [this, session, conf]
+        {
+            std::lock_guard<std::mutex> lg{configuration_mutex};
+
+            config_map[session] = conf;
+
+            /* If the session is focused, apply the configuration */
+            if (focused_session.lock() == session)
+            {
+                apply_config(conf, PauseResumeSystem);
+            }
+        });
 }
 
 std::shared_ptr<mg::DisplayConfiguration>
@@ -143,21 +141,36 @@
     std::shared_ptr<graphics::DisplayConfiguration> const& conf,
     SystemStateHandling pause_resume_system)
 {
-    std::lock_guard<std::mutex> lg{configuration_mutex};
-
-    display_configuration_policy->apply_to(*conf);
-    base_configuration = conf;
-    if (base_configuration_applied)
-        apply_base_config(pause_resume_system);
-
-    /*
-     * Clear all the per-session configurations, since they may have become
-     * invalid due to the hardware change.
-     */
-    config_map.clear();
-
-    /* Send the new configuration to all the sessions */
-    send_config_to_all_sessions(conf);
+    server_action_queue->enqueue(
+        this,
+        [this, conf, pause_resume_system]
+        {
+            std::lock_guard<std::mutex> lg{configuration_mutex};
+
+            display_configuration_policy->apply_to(*conf);
+            base_configuration = conf;
+            if (base_configuration_applied)
+                apply_base_config(pause_resume_system);
+
+            /*
+             * Clear all the per-session configurations, since they may have become
+             * invalid due to the hardware change.
+             */
+            config_map.clear();
+
+            /* Send the new configuration to all the sessions */
+            send_config_to_all_sessions(conf);
+        });
+}
+
+void ms::MediatingDisplayChanger::pause_display_config_processing()
+{
+    server_action_queue->pause_processing_for(this);
+}
+
+void ms::MediatingDisplayChanger::resume_display_config_processing()
+{
+    server_action_queue->resume_processing_for(this);
 }
 
 void ms::MediatingDisplayChanger::apply_config(
@@ -196,3 +209,45 @@
             session->send_display_config(*conf);
         });
 }
+
+void ms::MediatingDisplayChanger::focus_change_handler(
+    std::shared_ptr<ms::Session> const& session)
+{
+    std::lock_guard<std::mutex> lg{configuration_mutex};
+
+    focused_session = session;
+
+    /*
+     * If the newly focused session has a display configuration, apply it.
+     * Otherwise if we aren't currently using the base configuration,
+     * apply that.
+     */
+    auto const it = config_map.find(session);
+    if (it != config_map.end())
+    {
+        apply_config(it->second, PauseResumeSystem);
+    }
+    else if (!base_configuration_applied)
+    {
+        apply_base_config(PauseResumeSystem);
+    }
+}
+
+void ms::MediatingDisplayChanger::no_focus_handler()
+{
+    std::lock_guard<std::mutex> lg{configuration_mutex};
+
+    focused_session.reset();
+    if (!base_configuration_applied)
+    {
+        apply_base_config(PauseResumeSystem);
+    }
+}
+
+void ms::MediatingDisplayChanger::session_stopping_handler(
+    std::shared_ptr<ms::Session> const& session)
+{
+    std::lock_guard<std::mutex> lg{configuration_mutex};
+
+    config_map.erase(session);
+}

=== modified file 'src/server/scene/mediating_display_changer.h'
--- src/server/scene/mediating_display_changer.h	2014-03-26 05:48:59 +0000
+++ src/server/scene/mediating_display_changer.h	2014-05-12 15:56:28 +0000
@@ -27,6 +27,7 @@
 
 namespace mir
 {
+class ServerActionQueue;
 
 namespace graphics
 {
@@ -38,6 +39,7 @@
 {
 class SessionEventHandlerRegister;
 class SessionContainer;
+class Session;
 
 class MediatingDisplayChanger : public frontend::DisplayChanger,
                                 public mir::DisplayChanger
@@ -48,7 +50,8 @@
         std::shared_ptr<compositor::Compositor> const& compositor,
         std::shared_ptr<graphics::DisplayConfigurationPolicy> const& display_configuration_policy,
         std::shared_ptr<SessionContainer> const& session_container,
-        std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register);
+        std::shared_ptr<SessionEventHandlerRegister> const& session_event_handler_register,
+        std::shared_ptr<ServerActionQueue> const& server_action_queue);
 
     /* From mir::frontend::DisplayChanger */
     std::shared_ptr<graphics::DisplayConfiguration> active_configuration();
@@ -60,7 +63,14 @@
         std::shared_ptr<graphics::DisplayConfiguration> const& conf,
         SystemStateHandling pause_resume_system);
 
+    void pause_display_config_processing() override;
+    void resume_display_config_processing() override;
+
 private:
+    void focus_change_handler(std::shared_ptr<Session> const& session);
+    void no_focus_handler();
+    void session_stopping_handler(std::shared_ptr<Session> const& session);
+
     void apply_config(std::shared_ptr<graphics::DisplayConfiguration> const& conf,
                       SystemStateHandling pause_resume_system);
     void apply_base_config(SystemStateHandling pause_resume_system);
@@ -72,6 +82,7 @@
     std::shared_ptr<graphics::DisplayConfigurationPolicy> const display_configuration_policy;
     std::shared_ptr<SessionContainer> const session_container;
     std::shared_ptr<SessionEventHandlerRegister> const session_event_handler_register;
+    std::shared_ptr<ServerActionQueue> const server_action_queue;
     std::mutex configuration_mutex;
     std::map<std::weak_ptr<frontend::Session>,
              std::shared_ptr<graphics::DisplayConfiguration>,

=== modified file 'tests/acceptance-tests/test_display_configuration.cpp'
--- tests/acceptance-tests/test_display_configuration.cpp	2014-05-05 03:36:45 +0000
+++ tests/acceptance-tests/test_display_configuration.cpp	2014-05-12 15:56:28 +0000
@@ -17,6 +17,7 @@
  *              Kevin DuBois <[e-mail]>
  */
 
+#include "mir/server_action_queue.h"
 #include "mir/frontend/session_authorizer.h"
 #include "mir/graphics/event_handler_register.h"
 #include "src/server/scene/global_event_sender.h"
@@ -35,6 +36,7 @@
 #include "mir_test/fake_shared.h"
 #include "mir_test/pipe.h"
 #include "mir_test/cross_process_action.h"
+#include "mir_test/wait_condition.h"
 
 #include "mir_toolkit/mir_client_library.h"
 
@@ -154,6 +156,16 @@
     testing::NiceMock<MockDisplay> mock_display;
 };
 
+void wait_for_server_actions_to_finish(mir::ServerActionQueue& server_action_queue)
+{
+    mt::WaitCondition last_action_done;
+    server_action_queue.enqueue(
+        &last_action_done,
+        [&] { last_action_done.wake_up_everyone(); });
+
+    last_action_done.wait_for_at_most_seconds(5);
+}
+
 }
 
 using DisplayConfigurationTest = BespokeDisplayServerTestFixture;
@@ -481,12 +493,14 @@
             t = std::thread([this](){
                 verify_connection_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                     EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
                 });
 
                 verify_apply_config_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                 });
             });
@@ -561,12 +575,14 @@
             t = std::thread([this](){
                 verify_apply_config_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                     EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
                 });
 
                 verify_focus_change_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                 });
             });
@@ -655,12 +671,14 @@
             t = std::thread([this](){
                 verify_apply_config_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                     EXPECT_CALL(platform->mock_display, configure(testing::_)).Times(1);
                 });
 
                 verify_focus_change_expectations.exec([&]
                 {
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     testing::Mock::VerifyAndClearExpectations(&platform->mock_display);
                 });
             });
@@ -741,6 +759,7 @@
                 {
                     using namespace testing;
 
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     Mock::VerifyAndClearExpectations(&platform->mock_display);
                     /*
                      * A client with a per-session config is active, the base configuration
@@ -750,6 +769,7 @@
                     platform->mock_display.emit_configuration_change_event(
                         mt::fake_shared(changed_stub_display_config));
                     platform->mock_display.wait_for_configuration_change_handler();
+                    wait_for_server_actions_to_finish(*the_server_action_queue());
                     Mock::VerifyAndClearExpectations(&platform->mock_display);
                 });
             });

=== modified file 'tests/integration-tests/test_display_server_main_loop_events.cpp'
--- tests/integration-tests/test_display_server_main_loop_events.cpp	2014-05-12 15:23:20 +0000
+++ tests/integration-tests/test_display_server_main_loop_events.cpp	2014-05-12 15:56:28 +0000
@@ -20,11 +20,12 @@
 #include "mir/frontend/connector.h"
 #include "mir/graphics/display_configuration.h"
 #include "mir/graphics/display_configuration_policy.h"
-#include "mir/main_loop.h"
-#include "mir/display_changer.h"
+#include "mir/server_action_queue.h"
+#include "mir/graphics/event_handler_register.h"
 #include "mir/server_status_listener.h"
 
 #include "mir_test/pipe.h"
+#include "mir_test/wait_condition.h"
 #include "mir_test_framework/testing_server_configuration.h"
 #include "mir_test_doubles/mock_input_manager.h"
 #include "mir_test_doubles/mock_input_dispatcher.h"
@@ -68,14 +69,6 @@
     void remove_endpoint() const {}
 };
 
-class MockDisplayChanger : public mir::DisplayChanger
-{
-public:
-    MOCK_METHOD2(configure_for_hardware_change,
-                 void(std::shared_ptr<mg::DisplayConfiguration> const& conf,
-                      SystemStateHandling pause_resume_system));
-};
-
 class MockDisplay : public mtd::NullDisplay
 {
 public:
@@ -260,14 +253,6 @@
         return mock_input_dispatcher;
     }
 
-    std::shared_ptr<mir::DisplayChanger> the_display_changer() override
-    {
-        if (!mock_display_changer)
-            mock_display_changer = std::make_shared<MockDisplayChanger>();
-
-        return mock_display_changer;
-    }
-
     std::shared_ptr<MockDisplay> the_mock_display()
     {
         the_display();
@@ -298,12 +283,6 @@
         return mock_input_dispatcher;
     }
 
-    std::shared_ptr<MockDisplayChanger> the_mock_display_changer()
-    {
-        the_display_changer();
-        return mock_display_changer;
-    }
-
     void emit_pause_event_and_wait_for_handler()
     {
         kill(getpid(), pause_signal);
@@ -325,13 +304,21 @@
             std::this_thread::sleep_for(std::chrono::microseconds{500});
     }
 
+    void wait_for_server_actions_to_finish()
+    {
+        mt::WaitCondition last_action_done;
+        the_server_action_queue()->enqueue(&last_action_done,
+            [&] { last_action_done.wake_up_everyone(); });
+
+        last_action_done.wait_for_at_most_seconds(5);
+    }
+
 private:
     std::shared_ptr<mtd::MockCompositor> mock_compositor;
     std::shared_ptr<MockDisplay> mock_display;
     std::shared_ptr<MockConnector> mock_connector;
     std::shared_ptr<mtd::MockInputManager> mock_input_manager;
     std::shared_ptr<mtd::MockInputDispatcher> mock_input_dispatcher;
-    std::shared_ptr<MockDisplayChanger> mock_display_changer;
 
     mt::Pipe p;
     int const pause_signal;
@@ -553,7 +540,6 @@
     auto mock_connector = server_config.the_mock_connector();
     auto mock_input_manager = server_config.the_mock_input_manager();
     auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
-    auto mock_display_changer = server_config.the_mock_display_changer();
 
     {
         InSequence s;
@@ -564,10 +550,10 @@
         EXPECT_CALL(*mock_input_manager, start()).Times(1);
         EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
 
-        /* Configuration change event */
-        EXPECT_CALL(*mock_display_changer,
-                    configure_for_hardware_change(_, mir::DisplayChanger::PauseResumeSystem))
-            .Times(1);
+        /* Change configuration */
+        EXPECT_CALL(*mock_compositor, stop()).Times(1);
+        EXPECT_CALL(*mock_display, configure(_)).Times(1);
+        EXPECT_CALL(*mock_compositor, start()).Times(1);
 
         /* Stop */
         EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
@@ -583,6 +569,7 @@
                         [&]
                         {
                             server_config.emit_configuration_change_event_and_wait_for_handler();
+                            server_config.wait_for_server_actions_to_finish();
                             kill(getpid(), SIGTERM);
                         }};
                     t.detach();
@@ -600,7 +587,6 @@
     auto mock_connector = server_config.the_mock_connector();
     auto mock_input_manager = server_config.the_mock_input_manager();
     auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
-    auto mock_display_changer = server_config.the_mock_display_changer();
 
     {
         InSequence s;
@@ -618,16 +604,17 @@
         EXPECT_CALL(*mock_connector, stop()).Times(1);
         EXPECT_CALL(*mock_display, pause()) .Times(1);
 
-        /* Resume and reconfigure event */
+        /* Resume event */
         EXPECT_CALL(*mock_display, resume()).Times(1);
         EXPECT_CALL(*mock_connector, start()).Times(1);
         EXPECT_CALL(*mock_input_manager, start()).Times(1);
         EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
         EXPECT_CALL(*mock_compositor, start()).Times(1);
 
-        EXPECT_CALL(*mock_display_changer,
-                    configure_for_hardware_change(_, mir::DisplayChanger::PauseResumeSystem))
-            .Times(1);
+        /* Change configuration (after resuming) */
+        EXPECT_CALL(*mock_compositor, stop()).Times(1);
+        EXPECT_CALL(*mock_display, configure(_)).Times(1);
+        EXPECT_CALL(*mock_compositor, start()).Times(1);
 
         /* Stop */
         EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
@@ -645,6 +632,7 @@
                             server_config.emit_pause_event_and_wait_for_handler();
                             server_config.emit_configuration_change_event_and_wait_for_handler();
                             server_config.emit_resume_event_and_wait_for_handler();
+                            server_config.wait_for_server_actions_to_finish();
 
                             kill(getpid(), SIGTERM);
                         }};

=== modified file 'tests/unit-tests/scene/test_mediating_display_changer.cpp'
--- tests/unit-tests/scene/test_mediating_display_changer.cpp	2014-05-05 12:36:41 +0000
+++ tests/unit-tests/scene/test_mediating_display_changer.cpp	2014-05-12 15:56:28 +0000
@@ -20,6 +20,7 @@
 #include "src/server/scene/session_container.h"
 #include "mir/graphics/display_configuration_policy.h"
 #include "src/server/scene/broadcasting_session_event_sink.h"
+#include "mir/server_action_queue.h"
 
 #include "mir_test_doubles/mock_display.h"
 #include "mir_test_doubles/mock_compositor.h"
@@ -30,6 +31,8 @@
 #include "mir_test/fake_shared.h"
 #include "mir_test/display_config_matchers.h"
 
+#include <mutex>
+
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
@@ -39,6 +42,9 @@
 namespace ms = mir::scene;
 namespace mg = mir::graphics;
 
+namespace
+{
+
 class MockDisplayConfigurationPolicy : public mg::DisplayConfigurationPolicy
 {
 public:
@@ -84,6 +90,36 @@
     mutable mg::DisplayConfiguration* conf_ptr;
 };
 
+struct StubServerActionQueue : mir::ServerActionQueue
+{
+    void flush()
+    {
+        std::lock_guard<std::mutex> lock{mutex};
+        for (auto const& action : actions)
+            action();
+        actions.clear();
+    }
+
+    void enqueue(void const* /*owner*/, mir::ServerAction const& action) override
+    {
+        std::lock_guard<std::mutex> lock{mutex};
+        actions.push_back(action);
+    }
+
+    void pause_processing_for(void const* /*owner*/) override {}
+    void resume_processing_for(void const* /*owner*/) override {}
+
+    std::vector<mir::ServerAction> actions;
+    std::mutex mutex;
+};
+
+struct MockServerActionQueue : mir::ServerActionQueue
+{
+    MOCK_METHOD2(enqueue, void(void const*, mir::ServerAction const&));
+    MOCK_METHOD1(pause_processing_for, void(void const*));
+    MOCK_METHOD1(resume_processing_for, void(void const*));
+};
+
 struct MediatingDisplayChangerTest : public ::testing::Test
 {
     MediatingDisplayChangerTest()
@@ -95,7 +131,8 @@
                       mt::fake_shared(mock_compositor),
                       mt::fake_shared(mock_conf_policy),
                       mt::fake_shared(stub_session_container),
-                      mt::fake_shared(session_event_sink));
+                      mt::fake_shared(session_event_sink),
+                      mt::fake_shared(server_action_queue));
     }
 
     testing::NiceMock<MockDisplay> mock_display;
@@ -104,9 +141,12 @@
     StubSessionContainer stub_session_container;
     ms::BroadcastingSessionEventSink session_event_sink;
     mtd::StubDisplayConfig base_config;
+    StubServerActionQueue server_action_queue;
     std::shared_ptr<ms::MediatingDisplayChanger> changer;
 };
 
+}
+
 TEST_F(MediatingDisplayChangerTest, returns_active_configuration_from_display)
 {
     using namespace testing;
@@ -131,6 +171,8 @@
     session_event_sink.handle_focus_change(session);
     changer->configure(session,
                        mt::fake_shared(conf));
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, doesnt_apply_config_for_unfocused_session)
@@ -144,6 +186,8 @@
 
     changer->configure(std::make_shared<mtd::StubSceneSession>(),
                        mt::fake_shared(conf));
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, handles_hardware_change_properly_when_pausing_system)
@@ -160,6 +204,8 @@
 
     changer->configure_for_hardware_change(mt::fake_shared(conf),
                                            mir::DisplayChanger::PauseResumeSystem);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, handles_hardware_change_properly_when_retaining_system_state)
@@ -176,6 +222,8 @@
 
     changer->configure_for_hardware_change(mt::fake_shared(conf),
                                            mir::DisplayChanger::RetainSystemState);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, hardware_change_doesnt_apply_base_config_if_per_session_config_is_active)
@@ -190,6 +238,7 @@
 
     session_event_sink.handle_focus_change(session1);
 
+    server_action_queue.flush();
     Mock::VerifyAndClearExpectations(&mock_compositor);
     Mock::VerifyAndClearExpectations(&mock_display);
 
@@ -200,6 +249,8 @@
 
     changer->configure_for_hardware_change(conf,
                                            mir::DisplayChanger::PauseResumeSystem);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, notifies_all_sessions_on_hardware_config_change)
@@ -217,6 +268,8 @@
 
     changer->configure_for_hardware_change(mt::fake_shared(conf),
                                            mir::DisplayChanger::PauseResumeSystem);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, focusing_a_session_with_attached_config_applies_config)
@@ -234,6 +287,8 @@
     EXPECT_CALL(mock_compositor, start());
 
     session_event_sink.handle_focus_change(session1);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, focusing_a_session_without_attached_config_applies_base_config)
@@ -248,6 +303,7 @@
 
     session_event_sink.handle_focus_change(session1);
 
+    server_action_queue.flush();
     Mock::VerifyAndClearExpectations(&mock_compositor);
     Mock::VerifyAndClearExpectations(&mock_display);
 
@@ -257,6 +313,8 @@
     EXPECT_CALL(mock_compositor, start());
 
     session_event_sink.handle_focus_change(session2);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, losing_focus_applies_base_config)
@@ -270,6 +328,7 @@
 
     session_event_sink.handle_focus_change(session1);
 
+    server_action_queue.flush();
     Mock::VerifyAndClearExpectations(&mock_compositor);
     Mock::VerifyAndClearExpectations(&mock_display);
 
@@ -279,6 +338,8 @@
     EXPECT_CALL(mock_compositor, start());
 
     session_event_sink.handle_no_focus();
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, base_config_is_not_applied_if_already_active)
@@ -298,6 +359,8 @@
     session_event_sink.handle_focus_change(session1);
     session_event_sink.handle_focus_change(session2);
     session_event_sink.handle_no_focus();
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, hardware_change_invalidates_session_configs)
@@ -312,6 +375,7 @@
     changer->configure_for_hardware_change(conf,
                                            mir::DisplayChanger::PauseResumeSystem);
 
+    server_action_queue.flush();
     Mock::VerifyAndClearExpectations(&mock_compositor);
     Mock::VerifyAndClearExpectations(&mock_display);
 
@@ -324,6 +388,8 @@
     EXPECT_CALL(mock_compositor, start()).Times(0);
 
     session_event_sink.handle_focus_change(session1);
+
+    server_action_queue.flush();
 }
 
 TEST_F(MediatingDisplayChangerTest, session_stopping_invalidates_session_config)
@@ -337,6 +403,7 @@
 
     session_event_sink.handle_session_stopping(session1);
 
+    server_action_queue.flush();
     Mock::VerifyAndClearExpectations(&mock_compositor);
     Mock::VerifyAndClearExpectations(&mock_display);
 
@@ -349,4 +416,60 @@
     EXPECT_CALL(mock_compositor, start()).Times(0);
 
     session_event_sink.handle_focus_change(session1);
+
+    server_action_queue.flush();
+}
+
+TEST_F(MediatingDisplayChangerTest, uses_server_action_queue_for_configuration_actions)
+{
+    using namespace testing;
+
+    auto const conf = std::make_shared<mtd::NullDisplayConfiguration>();
+    auto const session1 = std::make_shared<mtd::StubSceneSession>();
+    auto const session2 = std::make_shared<mtd::StubSceneSession>();
+    MockServerActionQueue mock_server_action_queue;
+
+    stub_session_container.insert_session(session1);
+    stub_session_container.insert_session(session2);
+
+    ms::MediatingDisplayChanger display_changer(
+      mt::fake_shared(mock_display),
+      mt::fake_shared(mock_compositor),
+      mt::fake_shared(mock_conf_policy),
+      mt::fake_shared(stub_session_container),
+      mt::fake_shared(session_event_sink),
+      mt::fake_shared(mock_server_action_queue));
+
+    void const* owner{nullptr};
+
+    EXPECT_CALL(mock_server_action_queue, enqueue(_, _))
+        .WillOnce(SaveArg<0>(&owner));
+    display_changer.configure(session1, conf);
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
+    display_changer.configure_for_hardware_change(
+        conf,
+        mir::DisplayChanger::PauseResumeSystem);
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
+    session_event_sink.handle_focus_change(session1);
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
+    session_event_sink.handle_focus_change(session2);
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, enqueue(owner, _));
+    session_event_sink.handle_no_focus();
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, pause_processing_for(owner));
+    display_changer.pause_display_config_processing();
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
+
+    EXPECT_CALL(mock_server_action_queue, resume_processing_for(owner));
+    display_changer.resume_display_config_processing();
+    Mock::VerifyAndClearExpectations(&mock_server_action_queue);
 }