Mir

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

Proposed by Alan Griffiths
Status: Merged
Merged at revision: 4232
Proposed branch: lp:~alan-griffiths/mir/rework-BroadcastingSessionEventSink
Merge into: lp:mir
Diff against target: 573 lines (+142/-179)
12 files modified
src/include/server/mir/scene/session_event_handler_register.h (+3/-8)
src/server/input/config_changer.cpp (+26/-19)
src/server/input/config_changer.h (+2/-0)
src/server/scene/broadcasting_session_event_sink.cpp (+16/-59)
src/server/scene/broadcasting_session_event_sink.h (+10/-20)
src/server/scene/mediating_display_changer.cpp (+48/-35)
src/server/scene/mediating_display_changer.h (+5/-1)
src/server/scene/session_manager.cpp (+1/-1)
tests/include/mir/test/doubles/null_session_event_sink.h (+1/-1)
tests/unit-tests/scene/test_abstract_shell.cpp (+1/-1)
tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp (+28/-33)
tests/unit-tests/scene/test_session_manager.cpp (+1/-1)
To merge this branch: bzr merge lp:~alan-griffiths/mir/rework-BroadcastingSessionEventSink
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Gerry Boland (community) Approve
Review via email: mp+329255@code.launchpad.net

Commit message

Reimplement BroadcastingSessionEventSink using the same "observer" idiom we use elsewhere.

To post a comment you must log in.
Revision history for this message
Gerry Boland (gerboland) wrote :

+ std::shared_ptr<SessionObserver> const session_observer;
does it really need to be shared? Couldn't the ConfigChanger keep a unique_ptr, and pass the raw pointer to the event register/sink?

Admittedly that probably causes more a lot more code churn.

Rest looks fine to me

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4233
https://mir-jenkins.ubuntu.com/job/mir-ci/3561/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4880
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5095
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5084
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5084
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5084
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4917/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4917/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4917/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4917/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4917/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/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4917/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/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4917/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4917
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4917/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> + std::shared_ptr<SessionObserver> const session_observer;
> does it really need to be shared? Couldn't the ConfigChanger keep a
> unique_ptr, and pass the raw pointer to the event register/sink?

Probably, but then it will look different to other "observers" in the codebase.

> Admittedly that probably causes more a lot more code churn.

I imagine not. - just using "ThreadSafeList<SessionEventSink*>" instead of BasicObservers<SessionEventSink>.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I imagine not. - just using "ThreadSafeList<SessionEventSink*>" instead of
> BasicObservers<SessionEventSink>.

I needed to tweak ThreadSafeList to make this work (which might be worthwhile in its own right):

lp:~alan-griffiths/mir/rework-BroadcastingSessionEventSink-for-greyback/+merge/329262

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/include/server/mir/scene/session_event_handler_register.h'
2--- src/include/server/mir/scene/session_event_handler_register.h 2017-07-28 17:00:43 +0000
3+++ src/include/server/mir/scene/session_event_handler_register.h 2017-08-18 12:54:44 +0000
4@@ -19,26 +19,21 @@
5 #ifndef MIR_SCENE_SESSION_EVENT_HANDLER_REGISTER_H_
6 #define MIR_SCENE_SESSION_EVENT_HANDLER_REGISTER_H_
7
8-#include <functional>
9 #include <memory>
10
11 namespace mir
12 {
13 namespace scene
14 {
15-class Session;
16+class SessionEventSink;
17
18 class SessionEventHandlerRegister
19 {
20 public:
21 virtual ~SessionEventHandlerRegister() = default;
22
23- virtual void register_focus_change_handler(
24- std::function<void(std::shared_ptr<Session> const& session)> const& handler) = 0;
25- virtual void register_no_focus_handler(
26- std::function<void()> const& handler) = 0;
27- virtual void register_session_stopping_handler(
28- std::function<void(std::shared_ptr<Session> const& session)> const& handler) = 0;
29+ virtual void add(std::shared_ptr<SessionEventSink> const& handler) = 0;
30+ virtual void remove(std::shared_ptr<SessionEventSink> const& handler) = 0;
31
32 protected:
33 SessionEventHandlerRegister() = default;
34
35=== renamed file 'src/server/scene/session_event_sink.h' => 'src/include/server/mir/scene/session_event_sink.h'
36=== modified file 'src/server/input/config_changer.cpp'
37--- src/server/input/config_changer.cpp 2017-07-28 17:00:43 +0000
38+++ src/server/input/config_changer.cpp 2017-08-18 12:54:44 +0000
39@@ -19,6 +19,7 @@
40 #include "config_changer.h"
41
42 #include "mir/scene/session_event_handler_register.h"
43+#include "mir/scene/session_event_sink.h"
44 #include "mir/scene/session_container.h"
45 #include "mir/scene/session.h"
46 #include "mir/input/device.h"
47@@ -137,9 +138,30 @@
48 std::vector<std::shared_ptr<mi::Device>> added;
49 std::vector<MirInputDeviceId> removed;
50 };
51-
52 }
53
54+struct mi::ConfigChanger::SessionObserver : ms::SessionEventSink
55+{
56+ SessionObserver(mi::ConfigChanger& self) : self{self} {}
57+
58+ void handle_focus_change(std::shared_ptr<mir::scene::Session> const& session) override
59+ {
60+ self.focus_change_handler(session);
61+ }
62+
63+ void handle_no_focus() override
64+ {
65+ self.no_focus_handler();
66+ }
67+
68+ void handle_session_stopping(std::shared_ptr<mir::scene::Session> const& session) override
69+ {
70+ self.session_stopping_handler(session);
71+ }
72+
73+ mi::ConfigChanger& self;
74+};
75+
76 mi::ConfigChanger::ConfigChanger(
77 std::shared_ptr<InputManager> const& manager,
78 std::shared_ptr<InputDeviceHub> const& devices,
79@@ -152,31 +174,16 @@
80 session_event_handler_register{session_event_handler_register},
81 devices_wrapper_DO_NOT_USE{devices_wrapper},
82 device_observer(std::make_shared<DeviceChangeTracker>(*this)),
83+ session_observer{std::make_shared<SessionObserver>(*this)},
84 base_configuration_applied(true)
85 {
86 devices->add_observer(device_observer);
87-
88- session_event_handler_register->register_focus_change_handler(
89- [this](std::shared_ptr<ms::Session> const& session)
90- {
91- focus_change_handler(session);
92- });
93-
94- session_event_handler_register->register_no_focus_handler(
95- [this]
96- {
97- no_focus_handler();
98- });
99-
100- session_event_handler_register->register_session_stopping_handler(
101- [this](std::shared_ptr<ms::Session> const& session)
102- {
103- session_stopping_handler(session);
104- });
105+ session_event_handler_register->add(session_observer);
106 }
107
108 mi::ConfigChanger::~ConfigChanger()
109 {
110+ session_event_handler_register->remove(session_observer);
111 devices->remove_observer(device_observer);
112 }
113
114
115=== modified file 'src/server/input/config_changer.h'
116--- src/server/input/config_changer.h 2017-07-28 17:00:43 +0000
117+++ src/server/input/config_changer.h 2017-08-18 12:54:44 +0000
118@@ -70,6 +70,8 @@
119 // needs to be owned (but not used) in Mir, where better?
120 std::shared_ptr<InputDeviceHub> const devices_wrapper_DO_NOT_USE;
121 std::shared_ptr<InputDeviceObserver> const device_observer;
122+ struct SessionObserver;
123+ std::shared_ptr<SessionObserver> const session_observer;
124 bool base_configuration_applied;
125
126 std::weak_ptr<frontend::Session> focused_session;
127
128=== modified file 'src/server/scene/broadcasting_session_event_sink.cpp'
129--- src/server/scene/broadcasting_session_event_sink.cpp 2017-07-28 17:00:43 +0000
130+++ src/server/scene/broadcasting_session_event_sink.cpp 2017-08-18 12:54:44 +0000
131@@ -20,75 +20,32 @@
132
133 namespace ms = mir::scene;
134
135-/*
136- * TODO: Use Boost.Signals2 for this when we default to Boost 1.54, see
137- * https://svn.boost.org/trac/boost/ticket/8102 . For now, just use a custom
138- * mechanism with coarse locking. When emitting events, we copy the handlers
139- * to a temporary vector to avoid calling handlers while locked, while still
140- * being thread-safe.
141- */
142-
143 void ms::BroadcastingSessionEventSink::handle_focus_change(
144 std::shared_ptr<Session> const& session)
145 {
146- std::vector<std::function<void(std::shared_ptr<Session> const&)>> handlers;
147-
148- {
149- std::lock_guard<std::mutex> lg{handler_mutex};
150- handlers = focus_change_handlers;
151- }
152-
153- for (auto& handler : handlers)
154- handler(session);
155+ for_each([&](std::shared_ptr<SessionEventSink> const& observer)
156+ { observer->handle_focus_change(session); });
157 }
158
159 void ms::BroadcastingSessionEventSink::handle_no_focus()
160 {
161- std::vector<std::function<void()>> handlers;
162-
163- {
164- std::lock_guard<std::mutex> lg{handler_mutex};
165- handlers = no_focus_handlers;
166- }
167-
168- for (auto& handler : handlers)
169- handler();
170+ for_each([&](std::shared_ptr<SessionEventSink> const& observer)
171+ { observer->handle_no_focus(); });
172 }
173
174 void ms::BroadcastingSessionEventSink::handle_session_stopping(
175 std::shared_ptr<Session> const& session)
176 {
177- std::vector<std::function<void(std::shared_ptr<Session> const&)>> handlers;
178-
179- {
180- std::lock_guard<std::mutex> lg{handler_mutex};
181- handlers = session_stopping_handlers;
182- }
183-
184- for (auto& handler : handlers)
185- handler(session);
186-}
187-
188-void ms::BroadcastingSessionEventSink::register_focus_change_handler(
189- std::function<void(std::shared_ptr<Session> const& session)> const& handler)
190-{
191- std::lock_guard<std::mutex> lg{handler_mutex};
192-
193- focus_change_handlers.push_back(handler);
194-}
195-
196-void ms::BroadcastingSessionEventSink::register_no_focus_handler(
197- std::function<void()> const& handler)
198-{
199- std::lock_guard<std::mutex> lg{handler_mutex};
200-
201- no_focus_handlers.push_back(handler);
202-}
203-
204-void ms::BroadcastingSessionEventSink::register_session_stopping_handler(
205- std::function<void(std::shared_ptr<Session> const& session)> const& handler)
206-{
207- std::lock_guard<std::mutex> lg{handler_mutex};
208-
209- session_stopping_handlers.push_back(handler);
210+ for_each([&](std::shared_ptr<SessionEventSink> const& observer)
211+ { observer->handle_session_stopping(session); });
212+}
213+
214+void ms::BroadcastingSessionEventSink::add(std::shared_ptr<SessionEventSink> const& handler)
215+{
216+ BasicObservers<SessionEventSink>::add(handler);
217+}
218+
219+void ms::BroadcastingSessionEventSink::remove(std::shared_ptr<SessionEventSink> const& handler)
220+{
221+ BasicObservers<SessionEventSink>::remove(handler);
222 }
223
224=== modified file 'src/server/scene/broadcasting_session_event_sink.h'
225--- src/server/scene/broadcasting_session_event_sink.h 2017-07-28 17:00:43 +0000
226+++ src/server/scene/broadcasting_session_event_sink.h 2017-08-18 12:54:44 +0000
227@@ -19,36 +19,26 @@
228 #ifndef MIR_SCENE_BROADCASTING_SESSION_EVENT_SINK_H_
229 #define MIR_SCENE_BROADCASTING_SESSION_EVENT_SINK_H_
230
231-#include "session_event_sink.h"
232+#include "mir/scene/session_event_sink.h"
233 #include "mir/scene/session_event_handler_register.h"
234
235-#include <vector>
236-#include <mutex>
237+#include "mir/basic_observers.h"
238
239 namespace mir
240 {
241 namespace scene
242 {
243 class BroadcastingSessionEventSink : public SessionEventSink,
244- public SessionEventHandlerRegister
245+ public SessionEventHandlerRegister,
246+ private BasicObservers<SessionEventSink>
247 {
248 public:
249- void handle_focus_change(std::shared_ptr<Session> const& session);
250- void handle_no_focus();
251- void handle_session_stopping(std::shared_ptr<Session> const& session);
252-
253- void register_focus_change_handler(
254- std::function<void(std::shared_ptr<Session> const& session)> const& handler);
255- void register_no_focus_handler(
256- std::function<void()> const& handler);
257- void register_session_stopping_handler(
258- std::function<void(std::shared_ptr<Session> const& session)> const& handler);
259-
260-private:
261- std::mutex handler_mutex;
262- std::vector<std::function<void(std::shared_ptr<Session> const&)>> focus_change_handlers;
263- std::vector<std::function<void()>> no_focus_handlers;
264- std::vector<std::function<void(std::shared_ptr<Session> const&)>> session_stopping_handlers;
265+ void handle_focus_change(std::shared_ptr<Session> const& session) override;
266+ void handle_no_focus() override;
267+ void handle_session_stopping(std::shared_ptr<Session> const& session) override;
268+
269+ void add(std::shared_ptr<SessionEventSink> const& handler) override;
270+ void remove(std::shared_ptr<SessionEventSink> const& handler) override;
271 };
272
273 }
274
275=== modified file 'src/server/scene/mediating_display_changer.cpp'
276--- src/server/scene/mediating_display_changer.cpp 2017-08-09 15:03:31 +0000
277+++ src/server/scene/mediating_display_changer.cpp 2017-08-18 12:54:44 +0000
278@@ -23,6 +23,7 @@
279 #include "mir/scene/session_container.h"
280 #include "mir/scene/session.h"
281 #include "mir/scene/session_event_handler_register.h"
282+#include "mir/scene/session_event_sink.h"
283 #include "mir/graphics/display.h"
284 #include "mir/compositor/compositor.h"
285 #include "mir/geometry/rectangles.h"
286@@ -124,6 +125,45 @@
287 };
288 }
289
290+struct ms::MediatingDisplayChanger::SessionObserver : ms::SessionEventSink
291+{
292+ SessionObserver(ms::MediatingDisplayChanger* self) : self{self} {}
293+
294+ void handle_focus_change(std::shared_ptr<mir::scene::Session> const& session) override
295+ {
296+ auto const weak_session = std::weak_ptr<ms::Session>(session);
297+ self->server_action_queue->enqueue(
298+ self,
299+ [self=self,weak_session]
300+ {
301+ if (auto const session = weak_session.lock())
302+ self->focus_change_handler(session);
303+ });
304+ }
305+
306+ void handle_no_focus() override
307+ {
308+ self->server_action_queue->enqueue(
309+ self,
310+ [self=self] { self->no_focus_handler(); });
311+ }
312+
313+ void handle_session_stopping(std::shared_ptr<mir::scene::Session> const& session) override
314+ {
315+ auto const weak_session = std::weak_ptr<ms::Session>(session);
316+ self->server_action_queue->enqueue(
317+ self,
318+ [self=self,weak_session]
319+ {
320+ if (auto const session = weak_session.lock())
321+ self->session_stopping_handler(session);
322+ });
323+ }
324+
325+ ms::MediatingDisplayChanger* const self;
326+};
327+
328+
329 ms::MediatingDisplayChanger::MediatingDisplayChanger(
330 std::shared_ptr<mg::Display> const& display,
331 std::shared_ptr<mc::Compositor> const& compositor,
332@@ -142,45 +182,18 @@
333 observer{observer},
334 base_configuration_{display->configuration()},
335 base_configuration_applied{true},
336- alarm_factory{alarm_factory}
337+ alarm_factory{alarm_factory},
338+ session_observer{std::make_shared<SessionObserver>(this)}
339 {
340- session_event_handler_register->register_focus_change_handler(
341- [this](std::shared_ptr<ms::Session> const& session)
342- {
343- auto const weak_session = std::weak_ptr<ms::Session>(session);
344- this->server_action_queue->enqueue(
345- this,
346- [this,weak_session]
347- {
348- if (auto const session = weak_session.lock())
349- focus_change_handler(session);
350- });
351- });
352-
353- session_event_handler_register->register_no_focus_handler(
354- [this]
355- {
356- this->server_action_queue->enqueue(
357- this,
358- [this] { no_focus_handler(); });
359- });
360-
361- session_event_handler_register->register_session_stopping_handler(
362- [this](std::shared_ptr<ms::Session> const& session)
363- {
364- auto const weak_session = std::weak_ptr<ms::Session>(session);
365- this->server_action_queue->enqueue(
366- this,
367- [this,weak_session]
368- {
369- if (auto const session = weak_session.lock())
370- session_stopping_handler(session);
371- });
372- });
373-
374+ session_event_handler_register->add(session_observer);
375 observer->initial_configuration(base_configuration_);
376 }
377
378+ms::MediatingDisplayChanger::~MediatingDisplayChanger()
379+{
380+ session_event_handler_register->remove(session_observer);
381+}
382+
383 void ms::MediatingDisplayChanger::configure(
384 std::shared_ptr<mf::Session> const& session,
385 std::shared_ptr<mg::DisplayConfiguration> const& conf)
386
387=== modified file 'src/server/scene/mediating_display_changer.h'
388--- src/server/scene/mediating_display_changer.h 2017-07-28 17:00:43 +0000
389+++ src/server/scene/mediating_display_changer.h 2017-08-18 12:54:44 +0000
390@@ -65,7 +65,9 @@
391 std::shared_ptr<graphics::DisplayConfigurationObserver> const& observer,
392 std::shared_ptr<time::AlarmFactory> const& alarm_factory);
393
394- /* From mir::frontend::DisplayChanger */
395+ ~MediatingDisplayChanger();
396+
397+ /* From mir::frontend::DisplayChanger */
398 std::shared_ptr<graphics::DisplayConfiguration> base_configuration() override;
399 void configure(std::shared_ptr<frontend::Session> const& session,
400 std::shared_ptr<graphics::DisplayConfiguration> const& conf) override;
401@@ -118,6 +120,8 @@
402 std::shared_ptr<time::AlarmFactory> const alarm_factory;
403 std::unique_ptr<time::Alarm> preview_configuration_timeout;
404 std::weak_ptr<frontend::Session> currently_previewing_session;
405+ struct SessionObserver;
406+ std::shared_ptr<SessionObserver> const session_observer;
407 };
408
409 }
410
411=== modified file 'src/server/scene/session_manager.cpp'
412--- src/server/scene/session_manager.cpp 2017-07-28 17:00:43 +0000
413+++ src/server/scene/session_manager.cpp 2017-08-18 12:54:44 +0000
414@@ -25,7 +25,7 @@
415 #include "mir/scene/prompt_session.h"
416 #include "mir/scene/application_not_responding_detector.h"
417 #include "mir/shell/surface_stack.h"
418-#include "session_event_sink.h"
419+#include "mir/scene/session_event_sink.h"
420 #include "mir/frontend/event_sink.h"
421 #include "mir/graphics/display.h"
422 #include "mir/graphics/display_configuration.h"
423
424=== modified file 'tests/include/mir/test/doubles/null_session_event_sink.h'
425--- tests/include/mir/test/doubles/null_session_event_sink.h 2017-07-28 17:00:43 +0000
426+++ tests/include/mir/test/doubles/null_session_event_sink.h 2017-08-18 12:54:44 +0000
427@@ -19,7 +19,7 @@
428 #ifndef MIR_TEST_DOUBLES_NULL_SESSION_EVENT_SINK_H_
429 #define MIR_TEST_DOUBLES_NULL_SESSION_EVENT_SINK_H_
430
431-#include "src/server/scene/session_event_sink.h"
432+#include "src/include/server/mir/scene/session_event_sink.h"
433
434 namespace mir
435 {
436
437=== modified file 'tests/unit-tests/scene/test_abstract_shell.cpp'
438--- tests/unit-tests/scene/test_abstract_shell.cpp 2017-07-28 17:00:43 +0000
439+++ tests/unit-tests/scene/test_abstract_shell.cpp 2017-08-18 12:54:44 +0000
440@@ -26,7 +26,7 @@
441
442 #include "src/server/report/null/shell_report.h"
443 #include "src/server/scene/default_session_container.h"
444-#include "src/server/scene/session_event_sink.h"
445+#include "src/include/server/mir/scene/session_event_sink.h"
446 #include "src/server/scene/session_manager.h"
447
448 #include "mir/test/doubles/mock_window_manager.h"
449
450=== modified file 'tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp'
451--- tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp 2017-07-28 17:00:43 +0000
452+++ tests/unit-tests/scene/test_broadcasting_session_event_sink.cpp 2017-08-18 12:54:44 +0000
453@@ -21,79 +21,74 @@
454 #include "mir/test/fake_shared.h"
455
456 #include <gtest/gtest.h>
457+#include <gmock/gmock.h>
458
459 namespace ms = mir::scene;
460 namespace mtd = mir::test::doubles;
461 namespace mt = mir::test;
462+using namespace testing;
463+
464+namespace
465+{
466+struct MockEventSink : ms::SessionEventSink
467+{
468+ MOCK_METHOD1(handle_focus_change, void (std::shared_ptr<ms::Session> const& session));
469+ MOCK_METHOD1(handle_session_stopping, void (std::shared_ptr<ms::Session> const& session));
470+ MOCK_METHOD0(handle_no_focus, void ());
471+};
472+}
473
474 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_focus_change)
475 {
476 mtd::StubSession session1;
477- std::vector<ms::Session*> handler_called(3, nullptr);
478+ MockEventSink handler_called[3];
479
480 ms::BroadcastingSessionEventSink events;
481
482+ std::shared_ptr<ms::Session> session1ptr{mt::fake_shared(session1)};
483+
484 for (auto& h : handler_called)
485 {
486- events.register_focus_change_handler(
487- [&h](std::shared_ptr<ms::Session> const& session)
488- {
489- h = session.get();
490- });
491+ events.add(mt::fake_shared(h));
492+
493+ EXPECT_CALL(h, handle_focus_change(session1ptr)).Times(1);
494 }
495
496 events.handle_focus_change(mt::fake_shared(session1));
497-
498- for (unsigned int i = 0; i < handler_called.size(); i++)
499- {
500- EXPECT_EQ(&session1, handler_called[i]) << " i = " << i;
501- }
502 }
503
504 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_no_focus)
505 {
506 mtd::StubSession session1;
507- std::vector<int> handler_called(3, 0);
508+ MockEventSink handler_called[3];
509
510 ms::BroadcastingSessionEventSink events;
511
512 for (auto& h : handler_called)
513 {
514- events.register_no_focus_handler(
515- [&h]
516- {
517- h = 1;
518- });
519+ events.add(mt::fake_shared(h));
520+
521+ EXPECT_CALL(h, handle_no_focus()).Times(1);
522 }
523
524 events.handle_no_focus();
525-
526- for (unsigned int i = 0; i < handler_called.size(); i++)
527- {
528- EXPECT_EQ(1, handler_called[i]) << " i = " << i;
529- }
530 }
531
532 TEST(BroadcastingSessionEventSinkTest, emits_and_handles_session_stopping)
533 {
534 mtd::StubSession session1;
535- std::vector<ms::Session*> handler_called(3, nullptr);
536+ MockEventSink handler_called[3];
537
538 ms::BroadcastingSessionEventSink events;
539
540+ std::shared_ptr<ms::Session> session1ptr{mt::fake_shared(session1)};
541+
542 for (auto& h : handler_called)
543 {
544- events.register_session_stopping_handler(
545- [&h](std::shared_ptr<ms::Session> const& session)
546- {
547- h = session.get();
548- });
549+ events.add(mt::fake_shared(h));
550+
551+ EXPECT_CALL(h, handle_session_stopping(session1ptr)).Times(1);
552 }
553
554 events.handle_session_stopping(mt::fake_shared(session1));
555-
556- for (unsigned int i = 0; i < handler_called.size(); i++)
557- {
558- EXPECT_EQ(&session1, handler_called[i]) << " i = " << i;
559- }
560 }
561
562=== modified file 'tests/unit-tests/scene/test_session_manager.cpp'
563--- tests/unit-tests/scene/test_session_manager.cpp 2017-07-28 17:00:43 +0000
564+++ tests/unit-tests/scene/test_session_manager.cpp 2017-08-18 12:54:44 +0000
565@@ -24,7 +24,7 @@
566
567 #include "src/server/scene/basic_surface.h"
568 #include "src/server/scene/default_session_container.h"
569-#include "src/server/scene/session_event_sink.h"
570+#include "src/include/server/mir/scene/session_event_sink.h"
571 #include "src/server/report/null_report_factory.h"
572
573 #include "mir/test/doubles/mock_surface_stack.h"

Subscribers

People subscribed via source and target branches