Merge lp:~alan-griffiths/mir/rework-BroadcastingSessionEventSink-for-greyback into lp:mir

[Alan Griffiths]

Reimplement BroadcastingSessionEventSink using a similar "observer" idiom to that we use elsewhere.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:4234
https://mir-jenkins.ubuntu.com/job/mir-ci/3562/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4881
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5096
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4918/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4918
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4918/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3562/rebuild

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

So, I'd quite like to gradually migrate all our observer-pattern code to use mir::ObserverRegistrar<>.

Both for consistency's sake, but also because being able to associate the Observer with an Executor makes MediatingDisplayChanger's callbacks somewhat nicer.

review: Needs Information

[Chris Halse Rogers (raof)]

Oh. I marked that as “needs information”. I actually meant “let me just finish the branch that migrates to ObserverRegistrar, and see how that looks.

[Alan Griffiths (alan-griffiths)]

> Oh. I marked that as “needs information”. I actually meant “let me just finish
> the branch that migrates to ObserverRegistrar, and see how that looks.

Sure. I was just offering a viable solution that didn't lead to intermittent test failures.

[Chris Halse Rogers (raof)]

Yeah, let's just land this.

I can migrate stuff to ObserverRegistrar later.

review: Approve

=== modified file 'src/include/common/mir/thread_safe_list.h'
--- src/include/common/mir/thread_safe_list.h	2017-07-28 17:00:43 +0000
+++ src/include/common/mir/thread_safe_list.h	2017-08-18 15:06:41 +0000
@@ -34,7 +34,7 @@
  *    - copy-assignable
  *  - add():
  *    - copy-assignable
- *    - operator bool: returns whether this is a valid element
+ *    - conversion to bool: indicates whether this is a valid element
  *  - remove(), remove_all():
  *    - copy-assignable
  *    - Element{}: default construction should create an invalid element
@@ -42,7 +42,7 @@
  *    - bool operator!=: inequality of elements
  *  - clear():
  *    - copy-assignable
- *    - Element{}: default construction should create an invalid element
+ *    - Element{}: value initialization should create an invalid element
  */
 
 template<class Element>
@@ -60,7 +60,7 @@
     {
         ListItem() {}
         RecursiveReadWriteMutex mutex;
-        Element element;
+        Element element{};
         std::atomic<ListItem*> next{nullptr};
 
         ~ListItem() { delete next.load(); }

=== modified file 'src/include/server/mir/scene/session_event_handler_register.h'
--- src/include/server/mir/scene/session_event_handler_register.h	2017-07-28 17:00:43 +0000
+++ src/include/server/mir/scene/session_event_handler_register.h	2017-08-18 15:06:41 +0000
@@ -19,26 +19,21 @@
 #ifndef MIR_SCENE_SESSION_EVENT_HANDLER_REGISTER_H_
 #define MIR_SCENE_SESSION_EVENT_HANDLER_REGISTER_H_
 
-#include <functional>
 #include <memory>
 
 namespace mir
 {
 namespace scene
 {
-class Session;
+class SessionEventSink;
 
 class SessionEventHandlerRegister
 {
 public:
     virtual ~SessionEventHandlerRegister() = default;
 
-    virtual void register_focus_change_handler(
-        std::function<void(std::shared_ptr<Session> const& session)> const& handler) = 0;
-    virtual void register_no_focus_handler(
-        std::function<void()> const& handler) = 0;
-    virtual void register_session_stopping_handler(
-        std::function<void(std::shared_ptr<Session> const& session)> const& handler) = 0;
+    virtual void add(SessionEventSink* handler) = 0;
+    virtual void remove(SessionEventSink* handler) = 0;
 
 protected:
     SessionEventHandlerRegister() = default;

=== renamed file 'src/server/scene/session_event_sink.h' => 'src/include/server/mir/scene/session_event_sink.h'
=== modified file 'src/server/input/config_changer.cpp'
--- src/server/input/config_changer.cpp	2017-07-28 17:00:43 +0000
+++ src/server/input/config_changer.cpp	2017-08-18 15:06:41 +0000
@@ -19,6 +19,7 @@
 #include "config_changer.h"
 
 #include "mir/scene/session_event_handler_register.h"
+#include "mir/scene/session_event_sink.h"
 #include "mir/scene/session_container.h"
 #include "mir/scene/session.h"
 #include "mir/input/device.h"
@@ -137,9 +138,30 @@
     std::vector<std::shared_ptr<mi::Device>> added;
     std::vector<MirInputDeviceId> removed;
 };
-
 }
 
+struct mi::ConfigChanger::SessionObserver : ms::SessionEventSink
+{
+    SessionObserver(mi::ConfigChanger& self) : self{self} {}
+
+    void handle_focus_change(std::shared_ptr<mir::scene::Session> const& session) override
+    {
+        self.focus_change_handler(session);
+    }
+
+    void handle_no_focus() override
+    {
+        self.no_focus_handler();
+    }
+
+    void handle_session_stopping(std::shared_ptr<mir::scene::Session> const& session) override
+    {
+        self.session_stopping_handler(session);
+    }
+
+    mi::ConfigChanger& self;
+};
+
 mi::ConfigChanger::ConfigChanger(
     std::shared_ptr<InputManager> const& manager,
     std::shared_ptr<InputDeviceHub> const& devices,
@@ -152,31 +174,16 @@
       session_event_handler_register{session_event_handler_register},
       devices_wrapper_DO_NOT_USE{devices_wrapper},
       device_observer(std::make_shared<DeviceChangeTracker>(*this)),
+      session_observer{std::make_unique<SessionObserver>(*this)},
       base_configuration_applied(true)
 {
     devices->add_observer(device_observer);
-
-    session_event_handler_register->register_focus_change_handler(
-        [this](std::shared_ptr<ms::Session> const& session)
-        {
-            focus_change_handler(session);
-        });
-
-    session_event_handler_register->register_no_focus_handler(
-        [this]
-        {
-            no_focus_handler();
-        });
-
-    session_event_handler_register->register_session_stopping_handler(
-        [this](std::shared_ptr<ms::Session> const& session)
-        {
-            session_stopping_handler(session);
-        });
+    session_event_handler_register->add(session_observer.get());
 }
 
 mi::ConfigChanger::~ConfigChanger()
 {
+    session_event_handler_register->remove(session_observer.get());
     devices->remove_observer(device_observer);
 }
 

=== modified file 'src/server/input/config_changer.h'
--- src/server/input/config_changer.h	2017-07-28 17:00:43 +0000
+++ src/server/input/config_changer.h	2017-08-18 15:06:41 +0000
@@ -70,6 +70,8 @@
     // needs to be owned (but not used) in Mir, where better?
     std::shared_ptr<InputDeviceHub> const devices_wrapper_DO_NOT_USE;
     std::shared_ptr<InputDeviceObserver> const device_observer;
+    struct SessionObserver;
+    std::unique_ptr<SessionObserver> const session_observer;
     bool base_configuration_applied;
 
     std::weak_ptr<frontend::Session> focused_session;

=== modified file 'src/server/scene/broadcasting_session_event_sink.cpp'
--- src/server/scene/broadcasting_session_event_sink.cpp	2017-07-28 17:00:43 +0000
+++ src/server/scene/broadcasting_session_event_sink.cpp	2017-08-18 15:06:41 +0000
@@ -20,75 +20,32 @@
 
 namespace ms = mir::scene;
 
-/*
- * TODO: Use Boost.Signals2 for this when we default to Boost 1.54, see
- * https://svn.boost.org/trac/boost/ticket/8102 . For now, just use a custom
- * mechanism with coarse locking. When emitting events, we copy the handlers
- * to a temporary vector to avoid calling handlers while locked, while still
- * being thread-safe.
- */
-
 void ms::BroadcastingSessionEventSink::handle_focus_change(
     std::shared_ptr<Session> const& session)
 {
-    std::vector<std::function<void(std::shared_ptr<Session> const&)>> handlers;
-
-    {
-        std::lock_guard<std::mutex> lg{handler_mutex};
-        handlers = focus_change_handlers;
-    }
-
-    for (auto& handler : handlers)
-        handler(session);
+    for_each([&](SessionEventSink* observer)
+        { observer->handle_focus_change(session); });
 }
 
 void ms::BroadcastingSessionEventSink::handle_no_focus()
 {
-    std::vector<std::function<void()>> handlers;
-
-    {
-        std::lock_guard<std::mutex> lg{handler_mutex};
-        handlers = no_focus_handlers;
-    }
-
-    for (auto& handler : handlers)
-        handler();
+    for_each([&](SessionEventSink* observer)
+        { observer->handle_no_focus(); });
 }
 
 void ms::BroadcastingSessionEventSink::handle_session_stopping(
     std::shared_ptr<Session> const& session)
 {
-    std::vector<std::function<void(std::shared_ptr<Session> const&)>> handlers;
-
-    {
-        std::lock_guard<std::mutex> lg{handler_mutex};
-        handlers = session_stopping_handlers;
-    }
-
-    for (auto& handler : handlers)
-        handler(session);
-}
-
-void ms::BroadcastingSessionEventSink::register_focus_change_handler(
-    std::function<void(std::shared_ptr<Session> const& session)> const& handler)
-{
-    std::lock_guard<std::mutex> lg{handler_mutex};
-
-    focus_change_handlers.push_back(handler);
-}
-
-void ms::BroadcastingSessionEventSink::register_no_focus_handler(
-    std::function<void()> const& handler)
-{
-    std::lock_guard<std::mutex> lg{handler_mutex};
-
-    no_focus_handlers.push_back(handler);
-}
-
-void ms::BroadcastingSessionEventSink::register_session_stopping_handler(
-    std::function<void(std::shared_ptr<Session> const& session)> const& handler)
-{
-    std::lock_guard<std::mutex> lg{handler_mutex};
-
-    session_stopping_handlers.push_back(handler);
+    for_each([&](SessionEventSink* observer)
+        { observer->handle_session_stopping(session); });
+}
+
+void ms::BroadcastingSessionEventSink::add(SessionEventSink* handler)
+{
+    ThreadSafeList<SessionEventSink*>::add(handler);
+}
+
+void ms::BroadcastingSessionEventSink::remove(SessionEventSink* handler)
+{
+    ThreadSafeList<SessionEventSink*>::remove(handler);
 }

=== modified file 'src/server/scene/broadcasting_session_event_sink.h'
--- src/server/scene/broadcasting_session_event_sink.h	2017-07-28 17:00:43 +0000
+++ src/server/scene/broadcasting_session_event_sink.h	2017-08-18 15:06:41 +0000
@@ -19,36 +19,26 @@
 #ifndef MIR_SCENE_BROADCASTING_SESSION_EVENT_SINK_H_
 #define MIR_SCENE_BROADCASTING_SESSION_EVENT_SINK_H_
 
-#include "session_event_sink.h"
+#include "mir/scene/session_event_sink.h"
 #include "mir/scene/session_event_handler_register.h"
 
-#include <vector>
-#include <mutex>
+#include "mir/thread_safe_list.h"
 
 namespace mir
 {
 namespace scene
 {
 class BroadcastingSessionEventSink : public SessionEventSink,
-                                     public SessionEventHandlerRegister
+                                     public SessionEventHandlerRegister,
+                                     private ThreadSafeList<SessionEventSink*>
 {
 public:
-    void handle_focus_change(std::shared_ptr<Session> const& session);
-    void handle_no_focus();
-    void handle_session_stopping(std::shared_ptr<Session> const& session);
-
-    void register_focus_change_handler(
-        std::function<void(std::shared_ptr<Session> const& session)> const& handler);
-    void register_no_focus_handler(
-        std::function<void()> const& handler);
-    void register_session_stopping_handler(
-        std::function<void(std::shared_ptr<Session> const& session)> const& handler);
-
-private:
-    std::mutex handler_mutex;
-    std::vector<std::function<void(std::shared_ptr<Session> const&)>> focus_change_handlers;
-    std::vector<std::function<void()>> no_focus_handlers;
-    std::vector<std::function<void(std::shared_ptr<Session> const&)>> session_stopping_handlers;
+    void handle_focus_change(std::shared_ptr<Session> const& session) override;
+    void handle_no_focus() override;
+    void handle_session_stopping(std::shared_ptr<Session> const& session) override;
+
+    void add(SessionEventSink* handler) override;
+    void remove(SessionEventSink* handler) override;
 };
 
 }

=== modified file 'src/server/scene/mediating_display_changer.cpp'
--- src/server/scene/mediating_display_changer.cpp	2017-08-09 15:03:31 +0000
+++ src/server/scene/mediating_display_changer.cpp	2017-08-18 15:06:41 +0000
@@ -23,6 +23,7 @@
 #include "mir/scene/session_container.h"
 #include "mir/scene/session.h"
 #include "mir/scene/session_event_handler_register.h"
+#include "mir/scene/session_event_sink.h"
 #include "mir/graphics/display.h"
 #include "mir/compositor/compositor.h"
 #include "mir/geometry/rectangles.h"
@@ -124,6 +125,45 @@
 };
 }
 
+struct ms::MediatingDisplayChanger::SessionObserver : ms::SessionEventSink
+{
+    SessionObserver(ms::MediatingDisplayChanger* self) : self{self} {}
+
+    void handle_focus_change(std::shared_ptr<mir::scene::Session> const& session) override
+    {
+        auto const weak_session = std::weak_ptr<ms::Session>(session);
+        self->server_action_queue->enqueue(
+            self,
+            [self=self,weak_session]
+                {
+                    if (auto const session = weak_session.lock())
+                        self->focus_change_handler(session);
+                });
+    }
+
+    void handle_no_focus() override
+    {
+        self->server_action_queue->enqueue(
+            self,
+            [self=self] { self->no_focus_handler(); });
+    }
+
+    void handle_session_stopping(std::shared_ptr<mir::scene::Session> const& session) override
+    {
+        auto const weak_session = std::weak_ptr<ms::Session>(session);
+        self->server_action_queue->enqueue(
+            self,
+            [self=self,weak_session]
+            {
+                if (auto const session = weak_session.lock())
+                    self->session_stopping_handler(session);
+            });
+    }
+
+    ms::MediatingDisplayChanger* const self;
+};
+
+
 ms::MediatingDisplayChanger::MediatingDisplayChanger(
     std::shared_ptr<mg::Display> const& display,
     std::shared_ptr<mc::Compositor> const& compositor,
@@ -142,45 +182,18 @@
       observer{observer},
       base_configuration_{display->configuration()},
       base_configuration_applied{true},
-      alarm_factory{alarm_factory}
+      alarm_factory{alarm_factory},
+      session_observer{std::make_unique<SessionObserver>(this)}
 {
-    session_event_handler_register->register_focus_change_handler(
-        [this](std::shared_ptr<ms::Session> const& 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())
-                        focus_change_handler(session);
-                });
-        });
-
-    session_event_handler_register->register_no_focus_handler(
-        [this]
-        {
-            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)
-        {
-            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);
-                });
-        });
-
+    session_event_handler_register->add(session_observer.get());
     observer->initial_configuration(base_configuration_);
 }
 
+ms::MediatingDisplayChanger::~MediatingDisplayChanger()
+{
+    session_event_handler_register->remove(session_observer.get());
+}
+
 void ms::MediatingDisplayChanger::configure(
     std::shared_ptr<mf::Session> const& session,
     std::shared_ptr<mg::DisplayConfiguration> const& conf)

=== modified file 'src/server/scene/mediating_display_changer.h'
--- src/server/scene/mediating_display_changer.h	2017-07-28 17:00:43 +0000
+++ src/server/scene/mediating_display_changer.h	2017-08-18 15:06:41 +0000
@@ -65,7 +65,9 @@
         std::shared_ptr<graphics::DisplayConfigurationObserver> const& observer,
         std::shared_ptr<time::AlarmFactory> const& alarm_factory);
 
-    /* From mir::frontend::DisplayChanger */
+    ~MediatingDisplayChanger();
+
+        /* From mir::frontend::DisplayChanger */
     std::shared_ptr<graphics::DisplayConfiguration> base_configuration() override;
     void configure(std::shared_ptr<frontend::Session> const& session,
                    std::shared_ptr<graphics::DisplayConfiguration> const& conf) override;
@@ -118,6 +120,8 @@
     std::shared_ptr<time::AlarmFactory> const alarm_factory;
     std::unique_ptr<time::Alarm> preview_configuration_timeout;
     std::weak_ptr<frontend::Session> currently_previewing_session;
+    struct SessionObserver;
+    std::unique_ptr<SessionObserver> const session_observer;
 };
 
 }

=== modified file 'src/server/scene/session_manager.cpp'
--- src/server/scene/session_manager.cpp	2017-07-28 17:00:43 +0000
+++ src/server/scene/session_manager.cpp	2017-08-18 15:06:41 +0000
@@ -25,7 +25,7 @@
 #include "mir/scene/prompt_session.h"
 #include "mir/scene/application_not_responding_detector.h"
 #include "mir/shell/surface_stack.h"
-#include "session_event_sink.h"
+#include "mir/scene/session_event_sink.h"
 #include "mir/frontend/event_sink.h"
 #include "mir/graphics/display.h"
 #include "mir/graphics/display_configuration.h"

=== modified file 'tests/include/mir/test/doubles/null_session_event_sink.h'
--- tests/include/mir/test/doubles/null_session_event_sink.h	2017-07-28 17:00:43 +0000
+++ tests/include/mir/test/doubles/null_session_event_sink.h	2017-08-18 15:06:41 +0000
@@ -19,7 +19,7 @@
 #ifndef MIR_TEST_DOUBLES_NULL_SESSION_EVENT_SINK_H_
 #define MIR_TEST_DOUBLES_NULL_SESSION_EVENT_SINK_H_
 
-#include "src/server/scene/session_event_sink.h"
+#include "src/include/server/mir/scene/session_event_sink.h"
 
 namespace mir
 {

=== modified file 'tests/unit-tests/scene/test_abstract_shell.cpp'
--- tests/unit-tests/scene/test_abstract_shell.cpp	2017-07-28 17:00:43 +0000
+++ tests/unit-tests/scene/test_abstract_shell.cpp	2017-08-18 15:06:41 +0000
@@ -26,7 +26,7 @@
 
 #include "src/server/report/null/shell_report.h"
 #include "src/server/scene/default_session_container.h"
-#include "src/server/scene/session_event_sink.h"
+#include "src/include/server/mir/scene/session_event_sink.h"
 #include "src/server/scene/session_manager.h"
 
 #include "mir/test/doubles/mock_window_manager.h"

=== modified file 'tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp'
--- tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp	2017-07-28 17:00:43 +0000
+++ tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp	2017-08-18 15:06:41 +0000
@@ -21,79 +21,74 @@
 #include "mir/test/fake_shared.h"
 
 #include <gtest/gtest.h>
+#include <gmock/gmock.h>
 
 namespace ms = mir::scene;
 namespace mtd = mir::test::doubles;
 namespace mt = mir::test;
+using namespace testing;
+
+namespace
+{
+struct MockEventSink : ms::SessionEventSink
+{
+    MOCK_METHOD1(handle_focus_change, void (std::shared_ptr<ms::Session> const& session));
+    MOCK_METHOD1(handle_session_stopping, void (std::shared_ptr<ms::Session> const& session));
+    MOCK_METHOD0(handle_no_focus, void ());
+};
+}
 
 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_focus_change)
 {
     mtd::StubSession session1;
-    std::vector<ms::Session*> handler_called(3, nullptr);
+    MockEventSink handler_called[3];
 
     ms::BroadcastingSessionEventSink events;
 
+    std::shared_ptr<ms::Session> session1ptr{mt::fake_shared(session1)};
+
     for (auto& h : handler_called)
     {
-        events.register_focus_change_handler(
-            [&h](std::shared_ptr<ms::Session> const& session)
-            {
-                h = session.get();
-            });
-    }
-
-    events.handle_focus_change(mt::fake_shared(session1));
-
-    for (unsigned int i = 0; i < handler_called.size(); i++)
-    {
-        EXPECT_EQ(&session1, handler_called[i]) << " i = " << i;
-    }
+        events.add(&h);
+
+        EXPECT_CALL(h, handle_focus_change(session1ptr)).Times(1);
+    }
+
+    events.handle_focus_change(session1ptr);
 }
 
 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_no_focus)
 {
     mtd::StubSession session1;
-    std::vector<int> handler_called(3, 0);
+    MockEventSink handler_called[3];
 
     ms::BroadcastingSessionEventSink events;
 
     for (auto& h : handler_called)
     {
-        events.register_no_focus_handler(
-            [&h]
-            {
-                h = 1;
-            });
+        events.add(&h);
+
+        EXPECT_CALL(h, handle_no_focus()).Times(1);
     }
 
     events.handle_no_focus();
-
-    for (unsigned int i = 0; i < handler_called.size(); i++)
-    {
-        EXPECT_EQ(1, handler_called[i]) << " i = " << i;
-    }
 }
 
 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_session_stopping)
 {
     mtd::StubSession session1;
-    std::vector<ms::Session*> handler_called(3, nullptr);
+    MockEventSink handler_called[3];
 
     ms::BroadcastingSessionEventSink events;
 
+    std::shared_ptr<ms::Session> session1ptr{mt::fake_shared(session1)};
+
     for (auto& h : handler_called)
     {
-        events.register_session_stopping_handler(
-            [&h](std::shared_ptr<ms::Session> const& session)
-            {
-                h = session.get();
-            });
-    }
-
-    events.handle_session_stopping(mt::fake_shared(session1));
-
-    for (unsigned int i = 0; i < handler_called.size(); i++)
-    {
-        EXPECT_EQ(&session1, handler_called[i]) << " i = " << i;
-    }
+        events.add(&h);
+
+        EXPECT_CALL(h, handle_session_stopping(session1ptr)).Times(1);
+    }
+
+    events.handle_session_stopping(session1ptr);
 }

=== modified file 'tests/unit-tests/scene/test_session_manager.cpp'
--- tests/unit-tests/scene/test_session_manager.cpp	2017-07-28 17:00:43 +0000
+++ tests/unit-tests/scene/test_session_manager.cpp	2017-08-18 15:06:41 +0000
@@ -24,7 +24,7 @@
 
 #include "src/server/scene/basic_surface.h"
 #include "src/server/scene/default_session_container.h"
-#include "src/server/scene/session_event_sink.h"
+#include "src/include/server/mir/scene/session_event_sink.h"
 #include "src/server/report/null_report_factory.h"
 
 #include "mir/test/doubles/mock_surface_stack.h"