Merge lp:~robert-ancell/mir/pause-status into lp:~mir-team/mir/trunk

[Robert Ancell]

Report when paused and resumed via configuration

[Robert Ancell (robert-ancell)]

Required for unity-system-compositor to disable XMir input on VT/session switch: https://code.launchpad.net/~robert-ancell/unity-system-compositor/app-lifecycle/+merge/186135

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1062
http://jenkins.qa.ubuntu.com/job/mir-ci/1497/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2020
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1905
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/735/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/11/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/11/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-i386-ci/11/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1064
http://jenkins.qa.ubuntu.com/job/mir-ci/1499/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2022
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1907
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/737
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/737/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/13/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-i386-ci/13/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1064
http://jenkins.qa.ubuntu.com/job/mir-ci/1502/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2027
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1912
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/740
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/740/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/16
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/16/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I'm not convinced by the naming: The ServerStatus class doesn't represent a status - it is a listener interface monitoring status transitions. Something like StatusChangeTracker.

It looks as though exceptions have been ignored. What is the correct behavior if, for example "241 + server_status->resumed();" throws. The client code expects a true or false return - not an exception.

What is the connection with the linked bug?

review: Needs Fixing

[Robert Ancell (robert-ancell)]

Tracker doesn't seem to be used anywhere else in Mir so I think that's a worse name. The "Status" part of the name is weak, since we don't have a concept of what pause() / resume() means. This is the only interface that relates to the server state, I don't see it being transition specific - it could easily be extended to contain get_state().

If the application using libmirserver throws an exception in paused() or removed() I see no reason why we should ignore that - it should stop the server. Also, we're only notifying that the pause/resume has occurred, not controlling it.

The connection to the linked bug is USC needs to know when it has been VT switched away from (i.e. paused) so that it can tell its children to suspend. In the case of XMir this allows it to know when to stop using the input devices

[Alan Griffiths (alan-griffiths)]

> Tracker doesn't seem to be used anywhere else in Mir so I think that's a worse
> name. The "Status" part of the name is weak, since we don't have a concept of
> what pause() / resume() means. This is the only interface that relates to the
> server state, I don't see it being transition specific - it could easily be
> extended to contain get_state().

This interface is used by the display server to notify state transitions - the name ServerState doesn't reflect this role. Does PauseResumeListener sound better to you?

Why would the server want to call get_state()?

> If the application using libmirserver throws an exception in paused() or
> removed() I see no reason why we should ignore that - it should stop the
> server.

It is likely to stop the server in an untidy manner: e.g. it has just restarted all the subsystems and then an exception is thrown that will probably be caught in main().

> Also, we're only notifying that the pause/resume has occurred, not
> controlling it.

So exceptions are, perhaps, not good behaviour?

> The connection to the linked bug is USC needs to know when it has been VT
> switched away from (i.e. paused) so that it can tell its children to suspend.
> In the case of XMir this allows it to know when to stop using the input
> devices

OK

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1066
http://jenkins.qa.ubuntu.com/job/mir-ci/1521/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2053
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1938
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/759
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/759/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/35/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/687/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/generic-cleanup-mbs/2338
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2064
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1949
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/343/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-autolanding/18
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-autolanding/18/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Francis Ginther (fginther)]

Amd64 build was hung, re-approving.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/688/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/generic-cleanup-mbs/2342
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2066
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1951
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/344/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-autolanding/19
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-autolanding/19/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== added file 'include/server/mir/default_pause_resume_listener.h'
--- include/server/mir/default_pause_resume_listener.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/default_pause_resume_listener.h	2013-09-18 21:09:49 +0000
@@ -0,0 +1,40 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Ancell <[e-mail]>
+ */
+
+#ifndef MIR_DEFAULT_PAUSE_RESUME_LISTENER_H_
+#define MIR_DEFAULT_PAUSE_RESUME_LISTENER_H_
+
+#include "mir/pause_resume_listener.h"
+
+namespace mir
+{
+class DefaultPauseResumeListener : public virtual PauseResumeListener
+{
+public:
+    virtual void paused()
+    {
+    }
+
+    virtual void resumed()
+    {
+    }
+};
+  
+}
+
+#endif /* MIR_DEFAULT_PAUSE_RESUME_LISTENER_H_ */
 
=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2013-09-18 16:24:32 +0000
+++ include/server/mir/default_server_configuration.h	2013-09-18 21:09:49 +0000
@@ -123,6 +123,7 @@
     virtual std::shared_ptr<compositor::Compositor> the_compositor();
     virtual std::shared_ptr<input::InputManager>    the_input_manager();
     virtual std::shared_ptr<MainLoop>               the_main_loop();
+    virtual std::shared_ptr<PauseResumeListener>    the_pause_resume_listener();
     virtual std::shared_ptr<DisplayChanger>         the_display_changer();
     virtual std::shared_ptr<graphics::Platform>     the_graphics_platform();
     virtual std::shared_ptr<input::InputConfiguration> the_input_configuration();
@@ -286,6 +287,7 @@
     CachedPtr<surfaces::SurfaceController> surface_controller;
     CachedPtr<time::TimeSource> time_source;
     CachedPtr<MainLoop> main_loop;
+    CachedPtr<PauseResumeListener> pause_resume_listener;
     CachedPtr<graphics::DisplayConfigurationPolicy> display_configuration_policy;
     CachedPtr<graphics::nested::HostConnection> host_connection;
     CachedPtr<input::NestedInputRelay> nested_input_relay;
 
=== added file 'include/server/mir/pause_resume_listener.h'
--- include/server/mir/pause_resume_listener.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/pause_resume_listener.h	2013-09-18 21:09:49 +0000
@@ -0,0 +1,40 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Ancell <[e-mail]>
+ */
+
+#ifndef MIR_PAUSE_RESUME_LISTENER_H_
+#define MIR_PAUSE_RESUME_LISTENER_H_
+
+namespace mir
+{
+
+class PauseResumeListener
+{
+public:
+    virtual void paused() = 0;
+    virtual void resumed() = 0;
+
+protected:
+    PauseResumeListener() = default;
+    virtual ~PauseResumeListener() = default;
+    PauseResumeListener(PauseResumeListener const&) = delete;
+    PauseResumeListener& operator=(PauseResumeListener const&) = delete;
+};
+
+}
+
+#endif /* MIR_PAUSE_RESUME_LISTENER_H_ */
 
=== modified file 'include/server/mir/server_configuration.h'
--- include/server/mir/server_configuration.h	2013-08-28 03:41:48 +0000
+++ include/server/mir/server_configuration.h	2013-09-18 21:09:49 +0000
@@ -49,6 +49,7 @@
 }
 
 class MainLoop;
+class PauseResumeListener;
 class DisplayChanger;
 
 class ServerConfiguration
@@ -61,6 +62,7 @@
     virtual std::shared_ptr<compositor::Compositor> the_compositor() = 0;
     virtual std::shared_ptr<input::InputManager> the_input_manager() = 0;
     virtual std::shared_ptr<MainLoop> the_main_loop() = 0;
+    virtual std::shared_ptr<PauseResumeListener> the_pause_resume_listener() = 0;
     virtual std::shared_ptr<DisplayChanger> the_display_changer() = 0;
     virtual std::shared_ptr<graphics::Platform>  the_graphics_platform() = 0;
     virtual std::shared_ptr<input::InputConfiguration> the_input_configuration() = 0;
 
=== added file 'include/test/mir_test_doubles/mock_pause_resume_listener.h'
--- include/test/mir_test_doubles/mock_pause_resume_listener.h	1970-01-01 00:00:00 +0000
+++ include/test/mir_test_doubles/mock_pause_resume_listener.h	2013-09-18 21:09:49 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Ancell <[e-mail]>
+ */
+
+#ifndef MIR_TEST_DOUBLES_MOCK_PAUSE_RESUME_LISTENER_H_
+#define MIR_TEST_DOUBLES_MOCK_PAUSE_RESUME_LISTENER_H_
+
+#include "mir/pause_resume_listener.h"
+
+#include <gmock/gmock.h>
+
+namespace mir
+{
+namespace test
+{
+namespace doubles
+{
+
+class MockPauseResumeListener : public mir::PauseResumeListener
+{
+public:
+    MOCK_METHOD0(paused, void());
+    MOCK_METHOD0(resumed, void());
+};
+
+}
+}
+}
+
+#endif /* MIR_TEST_DOUBLES_MOCK_PAUSE_RESUME_LISTENER_H_ */
 
=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-09-18 16:24:32 +0000
+++ src/server/default_server_configuration.cpp	2013-09-18 21:09:49 +0000
@@ -19,6 +19,7 @@
 #include "mir/default_server_configuration.h"
 #include "mir/abnormal_exit.h"
 #include "mir/asio_main_loop.h"
+#include "mir/default_pause_resume_listener.h"
 #include "mir/shared_library.h"
 
 #include "mir/options/program_option.h"
@@ -936,6 +937,16 @@
         });
 }
 
+
+std::shared_ptr<mir::PauseResumeListener> mir::DefaultServerConfiguration::the_pause_resume_listener()
+{
+    return pause_resume_listener(
+        []()
+        {
+            return std::make_shared<mir::DefaultPauseResumeListener>();
+        });
+}
+
 std::shared_ptr<mg::DisplayConfigurationPolicy>
 mir::DefaultServerConfiguration::the_display_configuration_policy()
 {
 
=== modified file 'src/server/display_server.cpp'
--- src/server/display_server.cpp	2013-08-28 03:41:48 +0000
+++ src/server/display_server.cpp	2013-09-18 21:09:49 +0000
@@ -21,6 +21,7 @@
 #include "mir/display_server.h"
 #include "mir/server_configuration.h"
 #include "mir/main_loop.h"
+#include "mir/pause_resume_listener.h"
 #include "mir/display_changer.h"
 
 #include "mir/compositor/compositor.h"
@@ -74,6 +75,7 @@
           communicator{config.the_communicator()},
           input_manager{config.the_input_manager()},
           main_loop{config.the_main_loop()},
+          pause_resume_listener{config.the_pause_resume_listener()},
           display_changer{config.the_display_changer()},
           paused{false},
           configure_display_on_resume{false}
@@ -113,6 +115,8 @@
             return false;
         }
 
+        pause_resume_listener->paused();
+
         return true;
     }
 
@@ -149,6 +153,8 @@
             return false;
         }
 
+        pause_resume_listener->resumed();
+
         return true;
     }
 
@@ -173,6 +179,7 @@
     std::shared_ptr<mf::Communicator> const communicator;
     std::shared_ptr<mi::InputManager> const input_manager;
     std::shared_ptr<mir::MainLoop> const main_loop;
+    std::shared_ptr<mir::PauseResumeListener> const pause_resume_listener;
     std::shared_ptr<mir::DisplayChanger> const display_changer;
     bool paused;
     bool configure_display_on_resume;
 
=== modified file 'tests/integration-tests/test_display_server_main_loop_events.cpp'
--- tests/integration-tests/test_display_server_main_loop_events.cpp	2013-08-28 03:41:48 +0000
+++ tests/integration-tests/test_display_server_main_loop_events.cpp	2013-09-18 21:09:49 +0000
@@ -22,12 +22,14 @@
 #include "mir/graphics/display_configuration_policy.h"
 #include "mir/main_loop.h"
 #include "mir/display_changer.h"
+#include "mir/pause_resume_listener.h"
 
 #include "mir_test/pipe.h"
 #include "mir_test_framework/testing_server_configuration.h"
 #include "mir_test_doubles/mock_input_manager.h"
 #include "mir_test_doubles/mock_compositor.h"
 #include "mir_test_doubles/null_display.h"
+#include "mir_test_doubles/mock_pause_resume_listener.h"
 #include "mir/run_mir.h"
 
 #include <gtest/gtest.h>
@@ -311,6 +313,71 @@
     int const resume_signal;
 };
 
+class TestPauseResumeListenerConfig : public mtf::TestingServerConfiguration
+{
+public:
+    TestPauseResumeListenerConfig()
+        : pause_signal{SIGUSR1}, resume_signal{SIGUSR2}
+    {
+    }
+
+    std::shared_ptr<mg::Display> the_display() override
+    {
+        if (!mock_display)
+        {
+            auto display = mtf::TestingServerConfiguration::the_display();
+            mock_display = std::make_shared<MockDisplay>(display,
+                                                         pause_signal,
+                                                         resume_signal,
+                                                         p.read_fd());
+        }
+
+        return mock_display;
+    }
+
+    std::shared_ptr<mir::PauseResumeListener> the_pause_resume_listener() override
+    {
+        if (!mock_pause_resume_listener)
+            mock_pause_resume_listener = std::make_shared<mtd::MockPauseResumeListener>();
+
+        return mock_pause_resume_listener;
+    }
+
+    std::shared_ptr<MockDisplay> the_mock_display()
+    {
+        the_display();
+        return mock_display;
+    }
+
+    std::shared_ptr<mtd::MockPauseResumeListener> the_mock_pause_resume_listener()
+    {
+        the_pause_resume_listener();
+        return mock_pause_resume_listener;
+    }
+
+    void emit_pause_event_and_wait_for_handler()
+    {
+        kill(getpid(), pause_signal);
+        while (!mock_display->pause_handler_invoked())
+            std::this_thread::sleep_for(std::chrono::microseconds{500});
+    }
+
+    void emit_resume_event_and_wait_for_handler()
+    {
+        kill(getpid(), resume_signal);
+        while (!mock_display->resume_handler_invoked())
+            std::this_thread::sleep_for(std::chrono::microseconds{500});
+    }
+
+private:
+    std::shared_ptr<MockDisplay> mock_display;
+    std::shared_ptr<mtd::MockPauseResumeListener> mock_pause_resume_listener;
+
+    mt::Pipe p;
+    int const pause_signal;
+    int const resume_signal;
+};
+  
 }
 
 TEST(DisplayServerMainLoopEvents, display_server_shuts_down_properly_on_sigint)
@@ -582,3 +649,35 @@
                     t.detach();
                  });
 }
+
+TEST(DisplayServerMainLoopEvents, pause_resume_listener)
+{
+    using namespace testing;
+
+    TestPauseResumeListenerConfig server_config;
+
+    auto mock_display = server_config.the_mock_display();
+    auto mock_pause_resume_listener = server_config.the_mock_pause_resume_listener();
+
+    {
+        InSequence s;
+
+        EXPECT_CALL(*mock_display, pause()).Times(1);
+        EXPECT_CALL(*mock_pause_resume_listener, paused()).Times(1);
+        EXPECT_CALL(*mock_display, resume()).Times(1);
+        EXPECT_CALL(*mock_pause_resume_listener, resumed()).Times(1);
+    }
+
+    mir::run_mir(server_config,
+                 [&server_config](mir::DisplayServer&)
+                 {
+                    std::thread t{
+                        [&]
+                        {
+                            server_config.emit_pause_event_and_wait_for_handler();
+                            server_config.emit_resume_event_and_wait_for_handler();
+                            kill(getpid(), SIGTERM);
+                        }};
+                    t.detach();
+                 });
+}