Merge lp:~robertcarr/mir/add-session-listener into lp:~mir-team/mir/trunk

[Robert Carr]

Add a session listener, intended for the shell to maintain the Launcher model.

Add a session listener, intended for the shell to maintain the Launcher model.

[Robert Ancell (robert-ancell)]

The appeared and vanished names sounds a bit dramatic but can't think off hand of something better :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:644
http://jenkins.qa.ubuntu.com/job/mir-ci/534/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/597/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/480/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/18/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/272

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

65 + virtual void application_appeared(std::shared_ptr<Session> const& session) = 0;
66 + virtual void application_vanished(std::shared_ptr<Session> const& session) = 0;

This is a *session* listener - so why is it expecting notifications about *applications*?

Maybe:

virtual void starting(std::shared_ptr<Session> const& session) = 0; and,
virtual void stopping(std::shared_ptr<Session> const& session) = 0?

review: Needs Fixing

[Robert Carr (robertcarr)]

I like these names, it says something about the fact that when the Session has been created (but before a Surface has been created) nothing has really "appeared", rather: the application is starting. Applied in r465

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:646
http://jenkins.qa.ubuntu.com/job/mir-ci/540/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/609
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/492
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/24
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/277

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2013-04-26 12:21:57 +0000
+++ include/server/mir/default_server_configuration.h	2013-05-01 20:55:28 +0000
@@ -56,6 +56,7 @@
 class FocusSetter;
 class FocusSequence;
 class PlacementStrategy;
+class SessionListener;
 class FocusController;
 class SessionManager;
 }
@@ -157,6 +158,7 @@
     virtual std::shared_ptr<shell::FocusSetter>       the_shell_focus_setter();
     virtual std::shared_ptr<shell::FocusSequence>     the_shell_focus_sequence();
     virtual std::shared_ptr<shell::PlacementStrategy> the_shell_placement_strategy();
+    virtual std::shared_ptr<shell::SessionListener>   the_shell_session_listener();
     /** @} */
 
     /** @name shell configuration - dependencies
@@ -227,6 +229,7 @@
     CachedPtr<shell::FocusSetter>       shell_focus_setter;
     CachedPtr<shell::FocusSequence>     shell_focus_sequence;
     CachedPtr<shell::PlacementStrategy> shell_placement_strategy;
+    CachedPtr<shell::SessionListener> shell_session_listener;
     CachedPtr<compositor::CompositingStrategy> compositing_strategy;
     CachedPtr<compositor::OverlayRenderer> overlay_renderer;
     CachedPtr<compositor::Compositor> compositor;

=== added file 'include/server/mir/shell/null_session_listener.h'
--- include/server/mir/shell/null_session_listener.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/shell/null_session_listener.h	2013-05-01 20:55:28 +0000
@@ -0,0 +1,48 @@
+/*
+ * Copyright © 2012 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 Carr <[e-mail]>
+ */
+
+#ifndef MIR_SHELL_NULL_SESSION_LISTENER_H_
+#define MIR_SHELL_NULL_SESSION_LISTENER_H_
+
+#include "mir/shell/session_listener.h"
+
+namespace mir
+{
+namespace shell
+{
+class Session;
+
+class NullSessionListener : public SessionListener
+{
+public:
+    NullSessionListener() = default;
+    virtual ~NullSessionListener() noexcept(true) = default;
+
+    void starting(std::shared_ptr<Session> const&) override {}
+    void stopping(std::shared_ptr<Session> const&) override {}
+
+protected:
+    NullSessionListener(const NullSessionListener&) = delete;
+    NullSessionListener& operator=(const NullSessionListener&) = delete;
+};
+
+}
+}
+
+
+#endif // MIR_SHELL_NULL_SESSION_LISTENER_H_

=== added file 'include/server/mir/shell/session_listener.h'
--- include/server/mir/shell/session_listener.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/shell/session_listener.h	2013-05-01 20:55:28 +0000
@@ -0,0 +1,48 @@
+/*
+ * Copyright © 2012 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 Carr <[e-mail]>
+ */
+
+#ifndef MIR_SHELL_SESSION_LISTENER_H_
+#define MIR_SHELL_SESSION_LISTENER_H_
+
+#include <memory>
+
+namespace mir
+{
+namespace shell
+{
+class Session;
+
+class SessionListener
+{
+public:
+    virtual void starting(std::shared_ptr<Session> const& session) = 0;
+    virtual void stopping(std::shared_ptr<Session> const& session) = 0;
+
+protected:
+    SessionListener() = default;
+    virtual ~SessionListener() = default;
+
+    SessionListener(const SessionListener&) = delete;
+    SessionListener& operator=(const SessionListener&) = delete;
+};
+
+}
+}
+
+
+#endif // MIR_SHELL_SESSION_LISTENER_H_

=== modified file 'include/server/mir/shell/session_manager.h'
--- include/server/mir/shell/session_manager.h	2013-04-29 23:21:58 +0000
+++ include/server/mir/shell/session_manager.h	2013-05-01 20:55:28 +0000
@@ -41,8 +41,9 @@
 class SessionContainer;
 class FocusSequence;
 class FocusSetter;
+class Session;
 class InputTargetListener;
-class Session;
+class SessionListener;
 
 class SessionManager : public frontend::Shell, public shell::FocusController
 {
@@ -51,7 +52,8 @@
                             std::shared_ptr<SessionContainer> const& app_container,
                             std::shared_ptr<FocusSequence> const& focus_sequence,
                             std::shared_ptr<FocusSetter> const& focus_setter,
-                            std::shared_ptr<InputTargetListener> const& input_target_listener);
+                            std::shared_ptr<InputTargetListener> const& input_target_listener,
+                            std::shared_ptr<SessionListener> const& session_listener);
     virtual ~SessionManager();
 
     virtual std::shared_ptr<frontend::Session> open_session(std::string const& name, std::shared_ptr<events::EventSink> const& sink);
@@ -72,6 +74,7 @@
     std::shared_ptr<FocusSequence> const focus_sequence;
     std::shared_ptr<FocusSetter> const focus_setter;
     std::shared_ptr<InputTargetListener> const input_target_listener;
+    std::shared_ptr<SessionListener> const session_listener;
 
     std::mutex mutex;
     std::weak_ptr<Session> focus_application;

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-04-26 12:21:57 +0000
+++ src/server/default_server_configuration.cpp	2013-05-01 20:55:28 +0000
@@ -39,6 +39,7 @@
 #include "mir/shell/default_session_container.h"
 #include "mir/shell/consuming_placement_strategy.h"
 #include "mir/shell/organising_surface_factory.h"
+#include "mir/shell/null_session_listener.h"
 #include "mir/graphics/display.h"
 #include "mir/graphics/gl_renderer.h"
 #include "mir/graphics/renderer.h"
@@ -344,6 +345,16 @@
         });
 }
 
+std::shared_ptr<msh::SessionListener>
+mir::DefaultServerConfiguration::the_shell_session_listener()
+{
+    return shell_session_listener(
+        [this]
+        {
+            return std::make_shared<msh::NullSessionListener>();
+        });
+}
+
 std::shared_ptr<msh::SessionManager>
 mir::DefaultServerConfiguration::the_session_manager()
 {
@@ -355,7 +366,8 @@
                 the_shell_session_container(),
                 the_shell_focus_sequence(),
                 the_shell_focus_setter(),
-                the_input_target_listener());
+                the_input_target_listener(),
+                the_shell_session_listener());
         });
 }
 

=== modified file 'src/server/shell/session_manager.cpp'
--- src/server/shell/session_manager.cpp	2013-04-29 23:21:58 +0000
+++ src/server/shell/session_manager.cpp	2013-05-01 20:55:28 +0000
@@ -25,6 +25,7 @@
 #include "mir/shell/session.h"
 #include "mir/shell/surface.h"
 #include "mir/shell/input_target_listener.h"
+#include "mir/shell/session_listener.h"
 
 #include <memory>
 #include <cassert>
@@ -37,18 +38,21 @@
     std::shared_ptr<msh::SessionContainer> const& container,
     std::shared_ptr<msh::FocusSequence> const& sequence,
     std::shared_ptr<msh::FocusSetter> const& focus_setter,
-    std::shared_ptr<msh::InputTargetListener> const& input_target_listener) :
+    std::shared_ptr<msh::InputTargetListener> const& input_target_listener,
+    std::shared_ptr<msh::SessionListener> const& session_listener) :
     surface_factory(surface_factory),
     app_container(container),
     focus_sequence(sequence),
     focus_setter(focus_setter),
-    input_target_listener(input_target_listener)
+    input_target_listener(input_target_listener),
+    session_listener(session_listener)
 {
     assert(surface_factory);
     assert(sequence);
     assert(container);
     assert(focus_setter);
     assert(input_target_listener);
+    assert(session_listener);
 }
 
 msh::SessionManager::~SessionManager()
@@ -59,11 +63,12 @@
     std::string const& name,
     std::shared_ptr<events::EventSink> const& sink)
 {
-    auto new_session = std::make_shared<msh::ApplicationSession>(surface_factory, input_target_listener, name, sink);
+    std::shared_ptr<msh::Session> new_session = std::make_shared<msh::ApplicationSession>(surface_factory, input_target_listener, name, sink);
 
     app_container->insert_session(new_session);
     
     input_target_listener->input_application_opened(new_session);
+    session_listener->starting(new_session);
 
     set_focus_to_locked(std::unique_lock<std::mutex>(mutex), new_session);
 
@@ -88,6 +93,7 @@
     auto shell_session = std::dynamic_pointer_cast<Session>(session);
 
     input_target_listener->input_application_closed(shell_session);
+    session_listener->stopping(shell_session);
 
     app_container->remove_session(shell_session);
 

=== modified file 'tests/death-tests/test_application_manager_death.cpp'
--- tests/death-tests/test_application_manager_death.cpp	2013-04-24 05:22:20 +0000
+++ tests/death-tests/test_application_manager_death.cpp	2013-05-01 20:55:28 +0000
@@ -35,7 +35,8 @@
                                                std::shared_ptr<msh::SessionContainer>(),
                                                std::shared_ptr<msh::FocusSequence>(),
                                                std::shared_ptr<msh::FocusSetter>(),
-                                               std::shared_ptr<msh::InputTargetListener>()),
+                                               std::shared_ptr<msh::InputTargetListener>(),
+                                               std::shared_ptr<msh::SessionListener>()),
                 ::testing::KilledBySignal(SIGABRT),
                 ".*");
 }

=== modified file 'tests/integration-tests/shell/test_session_manager.cpp'
--- tests/integration-tests/shell/test_session_manager.cpp	2013-04-25 09:48:54 +0000
+++ tests/integration-tests/shell/test_session_manager.cpp	2013-05-01 20:55:28 +0000
@@ -22,6 +22,7 @@
 #include "mir/shell/focus_setter.h"
 #include "mir/shell/registration_order_focus_sequence.h"
 #include "mir/shell/default_session_container.h"
+#include "mir/shell/null_session_listener.h"
 #include "mir/surfaces/buffer_bundle.h"
 #include "mir/surfaces/surface.h"
 #include "mir/compositor/buffer_swapper.h"
@@ -53,13 +54,15 @@
     mtd::MockFocusSetter focus_setter;
     std::shared_ptr<mf::Session> new_session;
     mtd::StubInputTargetListener input_target_listener;
+    msh::NullSessionListener session_listener;
 
     msh::SessionManager session_manager(
             mt::fake_shared(surface_factory),
             container,
             mt::fake_shared(sequence),
             mt::fake_shared(focus_setter),
-            mt::fake_shared(input_target_listener));
+            mt::fake_shared(input_target_listener),
+            mt::fake_shared(session_listener));
 
     EXPECT_CALL(focus_setter, set_focus_to(_)).Times(3);
 
@@ -89,13 +92,15 @@
     mtd::MockFocusSetter focus_setter;
     std::shared_ptr<mf::Session> new_session;
     mtd::StubInputTargetListener input_target_listener;
+    msh::NullSessionListener session_listener;
 
     msh::SessionManager session_manager(
             mt::fake_shared(surface_factory),
             container,
             mt::fake_shared(sequence),
             mt::fake_shared(focus_setter),
-            mt::fake_shared(input_target_listener));
+            mt::fake_shared(input_target_listener),
+            mt::fake_shared(session_listener));
 
     EXPECT_CALL(focus_setter, set_focus_to(_)).Times(3);
 

=== modified file 'tests/unit-tests/shell/test_session_manager.cpp'
--- tests/unit-tests/shell/test_session_manager.cpp	2013-04-29 23:21:58 +0000
+++ tests/unit-tests/shell/test_session_manager.cpp	2013-05-01 20:55:28 +0000
@@ -22,6 +22,9 @@
 #include "mir/shell/default_session_container.h"
 #include "mir/shell/session.h"
 #include "mir/shell/input_target_listener.h"
+#include "mir/shell/surface.h"
+#include "mir/shell/session_listener.h"
+#include "mir/shell/null_session_listener.h"
 #include "mir/frontend/surface_creation_parameters.h"
 #include "mir/surfaces/surface.h"
 #include "mir/input/input_channel.h"
@@ -35,8 +38,6 @@
 #include "mir_test_doubles/stub_input_target_listener.h"
 #include "mir_test_doubles/mock_input_target_listener.h"
 
-#include "mir/shell/surface.h"
-
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
@@ -75,7 +76,8 @@
                         mt::fake_shared(container),
                         mt::fake_shared(focus_sequence),
                         mt::fake_shared(focus_setter),
-                        mt::fake_shared(input_target_listener))
+                        mt::fake_shared(input_target_listener),
+                        mt::fake_shared(session_listener))
     {
     }
 
@@ -85,6 +87,7 @@
     MockFocusSequence focus_sequence;
     testing::NiceMock<mtd::MockFocusSetter> focus_setter; // Inelegant but some tests need a stub
     mtd::StubInputTargetListener input_target_listener;
+    msh::NullSessionListener session_listener;
 
     msh::SessionManager session_manager;
 };
@@ -178,7 +181,8 @@
                         mt::fake_shared(container),
                         mt::fake_shared(focus_sequence),
                         mt::fake_shared(focus_setter),
-                        mt::fake_shared(input_target_listener))
+                        mt::fake_shared(input_target_listener),
+                        mt::fake_shared(session_listener))
     {
     }
 
@@ -188,13 +192,14 @@
     testing::NiceMock<MockFocusSequence> focus_sequence;
     testing::NiceMock<mtd::MockFocusSetter> focus_setter; // Inelegant but some tests need a stub
     mtd::MockInputTargetListener input_target_listener;
+    msh::NullSessionListener session_listener;
 
     msh::SessionManager session_manager;
 };
 
 }
 
-TEST_F(SessionManagerInputTargetListenerSetup, listener_is_notified_of_session_and_surfacelifecycle)
+TEST_F(SessionManagerInputTargetListenerSetup, input_listener_is_notified_of_session_and_surfacelifecycle)
 {
     using namespace ::testing;
 
@@ -227,3 +232,50 @@
         session_manager.close_session(session);
     }
 }
+
+namespace 
+{
+
+struct MockSessionListener : public msh::SessionListener
+{
+    virtual ~MockSessionListener() noexcept(true) {}
+    MOCK_METHOD1(starting, void(std::shared_ptr<msh::Session> const&));
+    MOCK_METHOD1(stopping, void(std::shared_ptr<msh::Session> const&));
+};
+
+struct SessionManagerSessionTargetListenerSetup : public testing::Test
+{
+    SessionManagerSessionTargetListenerSetup()
+      : session_manager(mt::fake_shared(surface_factory),
+                        mt::fake_shared(container),
+                        mt::fake_shared(focus_sequence),
+                        mt::fake_shared(focus_setter),
+                        mt::fake_shared(input_target_listener),
+                        mt::fake_shared(session_listener))
+    {
+    }
+
+    mtd::StubSurfaceBuilder surface_builder;
+    mtd::MockSurfaceFactory surface_factory;
+    testing::NiceMock<MockSessionContainer> container;    // Inelegant but some tests need a stub
+    testing::NiceMock<MockFocusSequence> focus_sequence;
+    testing::NiceMock<mtd::MockFocusSetter> focus_setter; // Inelegant but some tests need a stub
+    mtd::StubInputTargetListener input_target_listener;
+    MockSessionListener session_listener;
+
+    msh::SessionManager session_manager;
+};
+}
+
+TEST_F(SessionManagerSessionTargetListenerSetup, session_listener_is_notified_of_lifecycle)
+{
+    using namespace ::testing;
+
+    EXPECT_CALL(session_listener, starting(_)).Times(1);
+    EXPECT_CALL(session_listener, stopping(_)).Times(1);
+
+    EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
+    
+    auto session = session_manager.open_session("XPlane", std::shared_ptr<me::EventSink>());
+    session_manager.close_session(session);
+}