Mir

Merge lp:~albaguirre/mir/fix-1573572 into lp:mir

  1. fix-1573572
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Merged at revision: 3474
Proposed branch: lp:~albaguirre/mir/fix-1573572
Merge into: lp:mir
Diff against target: 170 lines (+90/-0)
5 files modified
src/server/frontend/screencast_buffer_tracker.cpp (+8/-0)
src/server/frontend/screencast_buffer_tracker.h (+5/-0)
src/server/frontend/session_mediator.cpp (+15/-0)
src/server/frontend/session_mediator.h (+2/-0)
tests/unit-tests/frontend/test_session_mediator.cpp (+60/-0)
To merge this branch: bzr merge lp:~albaguirre/mir/fix-1573572
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Needs Fixing
Cemil Azizoglu (community) Approve
Review via email: mp+292653@code.launchpad.net

Commit message

Remove orphaned screencast sessions on disconnection

When a client disconnects either normally or due to a crash, remove any screencasts associated with the session.

To post a comment you must log in.
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Looks good

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

PASSED: Continuous integration, rev:3473
https://mir-jenkins.ubuntu.com/job/mir-ci/892/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/926
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/965
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/956
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/956
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/936/artifact/output/*zip*/output.zip

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

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

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/221/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/927/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/237/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/966
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/957
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/957
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/937/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/937/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/937/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/937/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/937/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"[FAILED] ClientLatency.triple_buffered_client_has_less_than_two_frames_latency"

Unrelated.

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

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/222/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/929/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/238/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/968
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/939/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/939/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/939/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"21:49:08 + echo 'SETUP FAILED - CAN'\''T CONTINUE'
21:49:08 SETUP FAILED - CAN'T CONTINUE"

Looks like a device setup issue.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/frontend/screencast_buffer_tracker.cpp'
2--- src/server/frontend/screencast_buffer_tracker.cpp 2016-04-21 00:30:17 +0000
3+++ src/server/frontend/screencast_buffer_tracker.cpp 2016-04-22 15:21:32 +0000
4@@ -31,3 +31,11 @@
5 {
6 tracker.erase(id);
7 }
8+
9+void mf::ScreencastBufferTracker::for_each_session(std::function<void(ScreencastSessionId)> f) const
10+{
11+ for (auto const& entry : tracker)
12+ {
13+ f(entry.first);
14+ }
15+}
16
17=== modified file 'src/server/frontend/screencast_buffer_tracker.h'
18--- src/server/frontend/screencast_buffer_tracker.h 2016-04-21 00:30:17 +0000
19+++ src/server/frontend/screencast_buffer_tracker.h 2016-04-22 15:21:32 +0000
20@@ -20,6 +20,7 @@
21
22 #include "mir/frontend/screencast.h"
23
24+#include <functional>
25 #include <unordered_set>
26 #include <unordered_map>
27
28@@ -49,6 +50,10 @@
29 /* removes all tracked buffers associated with id */
30 void remove_session(ScreencastSessionId id);
31
32+ /* Iterates over all tracked session ids */
33+ void for_each_session(std::function<void(ScreencastSessionId)> f) const;
34+
35+
36 private:
37 ScreencastBufferTracker(ScreencastBufferTracker const&) = delete;
38 ScreencastBufferTracker& operator=(ScreencastBufferTracker const&) = delete;
39
40=== modified file 'src/server/frontend/session_mediator.cpp'
41--- src/server/frontend/session_mediator.cpp 2016-04-21 00:30:17 +0000
42+++ src/server/frontend/session_mediator.cpp 2016-04-22 15:21:32 +0000
43@@ -121,6 +121,7 @@
44 report->session_error(session->name(), __PRETTY_FUNCTION__, "connection dropped without disconnect");
45 shell->close_session(session);
46 }
47+ destroy_screencast_sessions();
48 }
49
50 void mf::SessionMediator::client_pid(int pid)
51@@ -536,6 +537,7 @@
52 report->session_disconnect_called(session->name());
53
54 shell->close_session(session);
55+ destroy_screencast_sessions();
56 weak_session.reset();
57
58 done->Run();
59@@ -1173,3 +1175,16 @@
60
61 return config;
62 }
63+
64+void mf::SessionMediator::destroy_screencast_sessions()
65+{
66+ std::vector<ScreencastSessionId> ids_to_untrack;
67+ screencast_buffer_tracker.for_each_session([this, &ids_to_untrack](ScreencastSessionId id)
68+ {
69+ screencast->destroy_session(id);
70+ ids_to_untrack.push_back(id);
71+ });
72+
73+ for (auto const& id : ids_to_untrack)
74+ screencast_buffer_tracker.remove_session(id);
75+}
76
77=== modified file 'src/server/frontend/session_mediator.h'
78--- src/server/frontend/session_mediator.h 2016-04-21 00:30:17 +0000
79+++ src/server/frontend/session_mediator.h 2016-04-22 15:21:32 +0000
80@@ -249,6 +249,8 @@
81
82 virtual std::function<void(std::shared_ptr<Session> const&)> prompt_session_connect_handler() const;
83
84+ void destroy_screencast_sessions();
85+
86 pid_t client_pid_;
87 std::shared_ptr<Shell> const shell;
88 std::shared_ptr<graphics::PlatformIpcOperations> const ipc_operations;
89
90=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
91--- tests/unit-tests/frontend/test_session_mediator.cpp 2016-04-21 00:30:17 +0000
92+++ tests/unit-tests/frontend/test_session_mediator.cpp 2016-04-22 15:21:32 +0000
93@@ -44,6 +44,7 @@
94 #include "mir/test/doubles/mock_frontend_surface.h"
95 #include "mir/test/doubles/mock_event_sink.h"
96 #include "mir/test/doubles/mock_event_sink_factory.h"
97+#include "mir/test/doubles/mock_screencast.h"
98 #include "mir/test/doubles/stub_buffer.h"
99 #include "mir/test/doubles/mock_buffer.h"
100 #include "mir/test/doubles/stub_session.h"
101@@ -266,6 +267,20 @@
102 mt::fake_shared(mock_hub));
103 }
104
105+ std::shared_ptr<mf::SessionMediator> create_session_mediator_with_screencast(
106+ std::shared_ptr<mf::Screencast> const& screencast)
107+ {
108+ return std::make_shared<mf::SessionMediator>(
109+ shell, mt::fake_shared(mock_ipc_operations), graphics_changer,
110+ surface_pixel_formats, report,
111+ std::make_shared<mtd::NullEventSinkFactory>(),
112+ std::make_shared<mtd::NullMessageSender>(),
113+ resource_cache, screencast, &connector, nullptr, nullptr,
114+ std::make_shared<mtd::NullANRDetector>(),
115+ mir::cookie::Authority::create(),
116+ mt::fake_shared(mock_hub));
117+ }
118+
119 MockConnector connector;
120 testing::NiceMock<mtd::MockPlatformIpcOperations> mock_ipc_operations;
121 testing::NiceMock<mtd::MockInputDeviceHub> mock_hub;
122@@ -1321,3 +1336,48 @@
123
124 EXPECT_THAT(connection.input_devices(), mt::InputDevicesMatch(devices));
125 }
126+
127+TEST_F(SessionMediator, disconnect_removes_orphaned_screencast_sessions)
128+{
129+ using namespace ::testing;
130+ auto mock_screencast = std::make_shared<NiceMock<mtd::MockScreencast>>();
131+ auto mediator = create_session_mediator_with_screencast(mock_screencast);
132+
133+ mf::ScreencastSessionId expected_id{7};
134+ mtd::StubBuffer stub_buffer;
135+ ON_CALL(*mock_screencast, create_session(_,_,_,_,_))
136+ .WillByDefault(Return(expected_id));
137+ ON_CALL(*mock_screencast, capture(_))
138+ .WillByDefault(Return(mt::fake_shared(stub_buffer)));
139+
140+ EXPECT_CALL(*mock_screencast, destroy_session(expected_id));
141+
142+ mp::ScreencastParameters screencast_parameters;
143+ mp::Screencast screencast;
144+ mediator->connect(&connect_parameters, &connection, null_callback.get());
145+ mediator->create_screencast(&screencast_parameters, &screencast, null_callback.get());
146+ mediator->disconnect(nullptr, nullptr, null_callback.get());
147+}
148+
149+TEST_F(SessionMediator, destructor_removes_orphaned_screencast_sessions)
150+{
151+ using namespace ::testing;
152+ auto mock_screencast = std::make_shared<NiceMock<mtd::MockScreencast>>();
153+ auto mediator = create_session_mediator_with_screencast(mock_screencast);
154+
155+ mf::ScreencastSessionId expected_id{7};
156+ mtd::StubBuffer stub_buffer;
157+ ON_CALL(*mock_screencast, create_session(_,_,_,_,_))
158+ .WillByDefault(Return(expected_id));
159+ ON_CALL(*mock_screencast, capture(_))
160+ .WillByDefault(Return(mt::fake_shared(stub_buffer)));
161+
162+ EXPECT_CALL(*mock_screencast, destroy_session(expected_id));
163+
164+ mp::ScreencastParameters screencast_parameters;
165+ mp::Screencast screencast;
166+ mediator->create_screencast(&screencast_parameters, &screencast, null_callback.get());
167+
168+ mediator.reset();
169+
170+}

Subscribers

People subscribed via source and target branches