Mir

Merge lp:~andreas-pokorny/mir/observable-session-container into lp:mir

Proposed by Andreas Pokorny on 2017-04-28
Status: Merged
Approved by: Alan Griffiths on 2017-06-07
Approved revision: 4135
Merged at revision: 4190
Proposed branch: lp:~andreas-pokorny/mir/observable-session-container
Merge into: lp:mir
Diff against target: 233 lines (+108/-6)
4 files modified
include/server/mir/scene/session_coordinator.h (+3/-0)
src/server/scene/session_manager.cpp (+56/-6)
src/server/scene/session_manager.h (+6/-0)
tests/unit-tests/scene/test_session_manager.cpp (+43/-0)
To merge this branch: bzr merge lp:~andreas-pokorny/mir/observable-session-container
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve on 2017-06-07
Alan Griffiths 2017-04-28 Approve on 2017-06-07
Review via email: mp+323412@code.launchpad.net

Commit message

Make the SessionManager observerable via the SessionListener interface

This adds the capability to register interest in the information previously handed out only to the session listener and turns "the session listener" into one of many.

Description of the change

This allows a component in mirserver to track sessions and their surfaces.

There is some code in SessionManager that could be turned into session listeners but thats something for another cleanup.

To post a comment you must log in.
Alan Griffiths (alan-griffiths) wrote :

Nit:

+struct ms::SessionManager::SessionObservers : mir::Executor, mir::ObserverMultiplexer<ms::SessionListener>
+{
+ SessionObservers()
+ : ObserverMultiplexer<ms::SessionListener>(static_cast<Executor&>(*this))
+ {
+ }
+ void spawn(std::function<void()>&& work) override
+ {
+ work();
+ }

SRP would suggest separating out SyncExecutor, but as this is all in an implementation file I won't be that strict.

Alan Griffiths (alan-griffiths) wrote :

Why is this needed? Does it make something possible for a server that wasn't before?

review: Needs Information
Andreas Pokorny (andreas-pokorny) wrote :

> Why is this needed? Does it make something possible for a server that wasn't
> before?

It allows to add another component to track sessions and which windows belong to which session. If you loo at the current session_manager.cpp you will see that it is essentially already an unrolled registry of session listeners. I did this change while I was working on supporting the client controlled hardware cursors - which requires tracking those two in relation.

This would not be necessary if we would store that information in our scene and if we would define explicit update and evaluate-update... In fact this would simplify a bunch of code paths that we currently have as scattered observer tuples..

So in the use case I was working on I coulde figure out which session / window would be in charge of the cursor .. or wether there is currently no surface on top that wants to do that.

Alan Griffiths (alan-griffiths) wrote :

OK

review: Approve
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/scene/session_coordinator.h'
2--- include/server/mir/scene/session_coordinator.h 2017-01-18 09:06:28 +0000
3+++ include/server/mir/scene/session_coordinator.h 2017-04-28 19:54:05 +0000
4@@ -35,6 +35,7 @@
5 namespace scene
6 {
7 class Session;
8+class SessionListener;
9 class Surface;
10 struct SurfaceCreationParameters;
11
12@@ -53,6 +54,8 @@
13
14 virtual std::shared_ptr<Session> successor_of(std::shared_ptr<Session> const&) const = 0;
15
16+ virtual void add_listener(std::shared_ptr<SessionListener> const&) = 0;
17+ virtual void remove_listener(std::shared_ptr<SessionListener> const&) = 0;
18 protected:
19 SessionCoordinator() = default;
20 virtual ~SessionCoordinator() = default;
21
22=== modified file 'src/server/scene/session_manager.cpp'
23--- src/server/scene/session_manager.cpp 2017-02-28 08:53:57 +0000
24+++ src/server/scene/session_manager.cpp 2017-04-28 19:54:05 +0000
25@@ -29,6 +29,7 @@
26 #include "mir/frontend/event_sink.h"
27 #include "mir/graphics/display.h"
28 #include "mir/graphics/display_configuration.h"
29+#include "mir/observer_multiplexer.h"
30
31 #include <boost/throw_exception.hpp>
32
33@@ -42,6 +43,43 @@
34 namespace mg = mir::graphics;
35 namespace msh = mir::shell;
36
37+struct ms::SessionManager::SessionObservers : mir::Executor, mir::ObserverMultiplexer<ms::SessionListener>
38+{
39+ SessionObservers()
40+ : ObserverMultiplexer<ms::SessionListener>(static_cast<Executor&>(*this))
41+ {
42+ }
43+ void spawn(std::function<void()>&& work) override
44+ {
45+ work();
46+ }
47+
48+ void starting(std::shared_ptr<Session> const& session) override
49+ {
50+ for_each_observer(&SessionListener::starting, session);
51+ }
52+ void stopping(std::shared_ptr<Session> const& session) override
53+ {
54+ for_each_observer(&SessionListener::stopping, session);
55+ }
56+ void focused(std::shared_ptr<Session> const& session) override
57+ {
58+ for_each_observer(&SessionListener::focused, session);
59+ }
60+ void unfocused() override
61+ {
62+ for_each_observer(&SessionListener::unfocused);
63+ }
64+ void surface_created(Session& session, std::shared_ptr<Surface> const& surface) override
65+ {
66+ for_each_observer(&SessionListener::surface_created, std::ref(session), surface);
67+ }
68+ void destroying_surface(Session& session, std::shared_ptr<Surface> const& surface) override
69+ {
70+ for_each_observer(&SessionListener::destroying_surface, std::ref(session), surface);
71+ }
72+};
73+
74 ms::SessionManager::SessionManager(
75 std::shared_ptr<shell::SurfaceStack> const& surface_stack,
76 std::shared_ptr<SurfaceFactory> const& surface_factory,
77@@ -52,7 +90,8 @@
78 std::shared_ptr<SessionListener> const& session_listener,
79 std::shared_ptr<graphics::Display const> const& display,
80 std::shared_ptr<ApplicationNotRespondingDetector> const& anr_detector,
81- std::shared_ptr<graphics::GraphicBufferAllocator> const& allocator) :
82+ std::shared_ptr<graphics::GraphicBufferAllocator> const& allocator) :
83+ observers(std::make_shared<SessionObservers>()),
84 surface_stack(surface_stack),
85 surface_factory(surface_factory),
86 buffer_stream_factory(buffer_stream_factory),
87@@ -64,6 +103,7 @@
88 anr_detector{anr_detector},
89 allocator(allocator)
90 {
91+ observers->register_interest(session_listener);
92 }
93
94 ms::SessionManager::~SessionManager() noexcept
95@@ -98,14 +138,14 @@
96 client_pid,
97 name,
98 snapshot_strategy,
99- session_listener,
100+ observers,
101 *display->configuration(),
102 sender,
103 allocator);
104
105 app_container->insert_session(new_session);
106
107- session_listener->starting(new_session);
108+ observers->starting(new_session);
109
110 anr_detector->register_session(new_session.get(), [sender]()
111 {
112@@ -118,13 +158,13 @@
113 void ms::SessionManager::set_focus_to(std::shared_ptr<Session> const& session)
114 {
115 session_event_sink->handle_focus_change(session);
116- session_listener->focused(session);
117+ observers->focused(session);
118 }
119
120 void ms::SessionManager::unset_focus()
121 {
122 session_event_sink->handle_no_focus();
123- session_listener->unfocused();
124+ observers->unfocused();
125 }
126
127 void ms::SessionManager::close_session(std::shared_ptr<Session> const& session)
128@@ -135,7 +175,7 @@
129
130 session_event_sink->handle_session_stopping(scene_session);
131
132- session_listener->stopping(scene_session);
133+ observers->stopping(scene_session);
134
135 app_container->remove_session(scene_session);
136 }
137@@ -146,3 +186,13 @@
138 {
139 return app_container->successor_of(session);
140 }
141+
142+void ms::SessionManager::add_listener(std::shared_ptr<SessionListener> const& listener)
143+{
144+ observers->register_interest(listener);
145+}
146+
147+void ms::SessionManager::remove_listener(std::shared_ptr<SessionListener> const& listener)
148+{
149+ observers->unregister_interest(*listener);
150+}
151
152=== modified file 'src/server/scene/session_manager.h'
153--- src/server/scene/session_manager.h 2017-01-18 02:29:37 +0000
154+++ src/server/scene/session_manager.h 2017-04-28 19:54:05 +0000
155@@ -20,6 +20,7 @@
156 #define MIR_SCENE_APPLICATION_MANAGER_H_
157
158 #include "mir/scene/session_coordinator.h"
159+#include "mir/scene/session_listener.h"
160
161 #include <memory>
162 #include <vector>
163@@ -76,11 +77,16 @@
164 void set_focus_to(std::shared_ptr<Session> const& focus) override;
165 void unset_focus() override;
166
167+ void add_listener(std::shared_ptr<SessionListener> const& listener) override;
168+ void remove_listener(std::shared_ptr<SessionListener> const& listener) override;
169+
170 protected:
171 SessionManager(const SessionManager&) = delete;
172 SessionManager& operator=(const SessionManager&) = delete;
173
174 private:
175+ struct SessionObservers;
176+ std::shared_ptr<SessionObservers> const observers;
177 std::shared_ptr<shell::SurfaceStack> const surface_stack;
178 std::shared_ptr<SurfaceFactory> const surface_factory;
179 std::shared_ptr<BufferStreamFactory> const buffer_stream_factory;
180
181=== modified file 'tests/unit-tests/scene/test_session_manager.cpp'
182--- tests/unit-tests/scene/test_session_manager.cpp 2017-02-15 14:45:41 +0000
183+++ tests/unit-tests/scene/test_session_manager.cpp 2017-04-28 19:54:05 +0000
184@@ -184,6 +184,49 @@
185 session_manager.close_session(session);
186 }
187
188+TEST_F(SessionManagerSessionListenerSetup, additional_listeners_receive_session_callbacks)
189+{
190+ using namespace ::testing;
191+
192+ auto additional_listener = std::make_shared<testing::NiceMock<mtd::MockSessionListener>>();
193+ EXPECT_CALL(*additional_listener, starting(_)).Times(1);
194+ EXPECT_CALL(*additional_listener, stopping(_)).Times(1);
195+
196+ session_manager.add_listener(additional_listener);
197+ auto session = session_manager.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
198+ session_manager.close_session(session);
199+}
200+
201+TEST_F(SessionManagerSessionListenerSetup, additional_listeners_receive_focus_changes)
202+{
203+ using namespace ::testing;
204+
205+ auto additional_listener = std::make_shared<testing::NiceMock<mtd::MockSessionListener>>();
206+ EXPECT_CALL(*additional_listener, starting(_)).Times(1);
207+ EXPECT_CALL(*additional_listener, focused(_)).Times(1);
208+ EXPECT_CALL(*additional_listener, unfocused()).Times(1);
209+
210+ session_manager.add_listener(additional_listener);
211+ auto session = session_manager.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
212+ session_manager.set_focus_to(session);
213+ session_manager.unset_focus();
214+}
215+
216+TEST_F(SessionManagerSessionListenerSetup, additional_listeners_receive_surface_creation)
217+{
218+ using namespace ::testing;
219+ mtd::NullEventSink event_sink;
220+ auto additional_listener = std::make_shared<testing::NiceMock<mtd::MockSessionListener>>();
221+ EXPECT_CALL(*additional_listener, starting(_)).Times(1);
222+ EXPECT_CALL(*additional_listener, surface_created(_,_)).Times(1);
223+
224+ session_manager.add_listener(additional_listener);
225+ auto session = session_manager.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
226+ auto bs = session->create_buffer_stream(
227+ mg::BufferProperties{{640, 480}, mir_pixel_format_abgr_8888, mg::BufferUsage::hardware});
228+ session->create_surface(ms::SurfaceCreationParameters().with_buffer_stream(bs), mt::fake_shared(event_sink));
229+}
230+
231 namespace
232 {
233 struct SessionManagerSessionEventsSetup : public testing::Test

Subscribers

People subscribed via source and target branches