Mir

Merge lp:~alan-griffiths/mir/fix-1511798 into lp:mir

  1. fix-1511798
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Rejected
Rejected by: Alan Griffiths
Proposed branch: lp:~alan-griffiths/mir/fix-1511798
Merge into: lp:mir
Prerequisite: lp:~afrantzis/mir/display-configuration-clone
Diff against target: 225 lines (+121/-5)
9 files modified
src/include/server/mir/frontend/display_changer.h (+1/-0)
src/server/frontend/session_mediator.cpp (+2/-0)
src/server/frontend/unauthorized_display_changer.cpp (+4/-0)
src/server/frontend/unauthorized_display_changer.h (+4/-3)
src/server/graphics/nested/display.cpp (+1/-1)
src/server/scene/mediating_display_changer.cpp (+23/-1)
src/server/scene/mediating_display_changer.h (+1/-0)
tests/acceptance-tests/test_nested_mir.cpp (+82/-0)
tests/include/mir/test/doubles/null_display_changer.h (+3/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1511798
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Disapprove
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+276422@code.launchpad.net

Commit message

scene, graphics: fix updating of display configuration when applications and nested servers exit

Description of the change

scene, graphics: fix updating of display configuration when applications and nested servers exit

Note: after proposing this I found the exact scenario described in lp:1511798 needs some additional fixes. (But passing these tests remains necessary.)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3074
http://jenkins.qa.ubuntu.com/job/mir-ci/5476/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4741
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3648/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4683/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1630/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4684/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/642/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3075
http://jenkins.qa.ubuntu.com/job/mir-ci/5509/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4791
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3698/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4733/console
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/691/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1663/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/691/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4734/console
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/692/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3078
http://jenkins.qa.ubuntu.com/job/mir-ci/5511/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4798
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3705
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4740/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/698/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1665
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1665/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/698
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/698/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4741
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4741/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7345/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24915
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/699/console

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

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

"socket.gaierror: [Errno -3] Temporary failure in name resolution"

CI failing again

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Investigation indicates that the problem lies with our shell/WM code. In some cases when removing a focused session, neither a handle_focus_change nor a handle_no_focus event is emitted.

This MP is a workaround that covers one effect of the problem, but I think we should solve the problem at its core.

review: Disapprove
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

An alternative fix: https://code.launchpad.net/~afrantzis/mir/fix-1511798-alternative/+merge/276852

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/include/server/mir/frontend/display_changer.h'
2--- src/include/server/mir/frontend/display_changer.h 2015-11-04 07:43:28 +0000
3+++ src/include/server/mir/frontend/display_changer.h 2015-11-05 10:19:41 +0000
4@@ -40,6 +40,7 @@
5 virtual std::shared_ptr<graphics::DisplayConfiguration> base_configuration() = 0;
6 virtual void configure(std::shared_ptr<Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&) = 0;
7 virtual std::future<void> set_base_configuration(std::shared_ptr<graphics::DisplayConfiguration> const&) = 0;
8+ virtual void remove(std::shared_ptr<Session> const& session) = 0;
9
10 protected:
11 DisplayChanger() = default;
12
13=== modified file 'src/server/frontend/session_mediator.cpp'
14--- src/server/frontend/session_mediator.cpp 2015-11-05 07:45:43 +0000
15+++ src/server/frontend/session_mediator.cpp 2015-11-05 10:19:41 +0000
16@@ -115,6 +115,7 @@
17 if (auto session = weak_session.lock())
18 {
19 report->session_error(session->name(), __PRETTY_FUNCTION__, "connection dropped without disconnect");
20+ display_changer->remove(session);
21 shell->close_session(session);
22 }
23 }
24@@ -484,6 +485,7 @@
25
26 report->session_disconnect_called(session->name());
27
28+ display_changer->remove(session);
29 shell->close_session(session);
30 weak_session.reset();
31
32
33=== modified file 'src/server/frontend/unauthorized_display_changer.cpp'
34--- src/server/frontend/unauthorized_display_changer.cpp 2015-11-04 07:43:28 +0000
35+++ src/server/frontend/unauthorized_display_changer.cpp 2015-11-05 10:19:41 +0000
36@@ -42,3 +42,7 @@
37 {
38 BOOST_THROW_EXCEPTION(std::runtime_error("not authorized to set base display configurations"));
39 }
40+
41+void mf::UnauthorizedDisplayChanger::remove(std::shared_ptr<Session> const& /*session*/)
42+{
43+}
44
45=== modified file 'src/server/frontend/unauthorized_display_changer.h'
46--- src/server/frontend/unauthorized_display_changer.h 2015-11-04 07:43:28 +0000
47+++ src/server/frontend/unauthorized_display_changer.h 2015-11-05 10:19:41 +0000
48@@ -31,9 +31,10 @@
49 public:
50 explicit UnauthorizedDisplayChanger(std::shared_ptr<frontend::DisplayChanger> const& changer);
51
52- std::shared_ptr<graphics::DisplayConfiguration> base_configuration();
53- void configure(std::shared_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&);
54- std::future<void> set_base_configuration(std::shared_ptr<graphics::DisplayConfiguration> const&);
55+ std::shared_ptr<graphics::DisplayConfiguration> base_configuration() override;
56+ void configure(std::shared_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&) override;
57+ std::future<void> set_base_configuration(std::shared_ptr<graphics::DisplayConfiguration> const&) override;
58+ void remove(std::shared_ptr<Session> const& session) override;
59
60 private:
61 std::shared_ptr<frontend::DisplayChanger> const changer;
62
63=== modified file 'src/server/graphics/nested/display.cpp'
64--- src/server/graphics/nested/display.cpp 2015-11-04 17:59:20 +0000
65+++ src/server/graphics/nested/display.cpp 2015-11-05 10:19:41 +0000
66@@ -256,7 +256,7 @@
67 mir_buffer_usage_hardware,
68 static_cast<uint32_t>(output.id.as_value()));
69
70- result[output.id] = std::make_shared<mgn::detail::DisplaySyncGroup>(
71+ result[output.id] = std::make_shared<mgn::detail::DisplaySyncGroup>(
72 std::make_shared<mgn::detail::DisplayBuffer>(
73 egl_display,
74 host_surface,
75
76=== modified file 'src/server/scene/mediating_display_changer.cpp'
77--- src/server/scene/mediating_display_changer.cpp 2015-10-26 03:33:22 +0000
78+++ src/server/scene/mediating_display_changer.cpp 2015-11-05 10:19:41 +0000
79@@ -158,7 +158,29 @@
80 {
81 std::lock_guard<std::mutex> lg{configuration_mutex};
82
83- return base_configuration_;
84+ return base_configuration_->clone();
85+}
86+
87+void ms::MediatingDisplayChanger::remove(std::shared_ptr<frontend::Session> const& session)
88+{
89+ {
90+ std::lock_guard<std::mutex> lg{configuration_mutex};
91+
92+ if (!config_map.erase(session) || base_configuration_applied || session != focused_session.lock())
93+ return;
94+ }
95+
96+ server_action_queue->enqueue(
97+ this, [this]
98+ {
99+ std::lock_guard<std::mutex> lg{configuration_mutex};
100+
101+ if (!base_configuration_applied && end(config_map) == config_map.find(focused_session))
102+ {
103+ apply_base_config(PauseResumeSystem);
104+ focused_session.reset();
105+ }
106+ });
107 }
108
109 void ms::MediatingDisplayChanger::configure_for_hardware_change(
110
111=== modified file 'src/server/scene/mediating_display_changer.h'
112--- src/server/scene/mediating_display_changer.h 2015-10-26 03:33:22 +0000
113+++ src/server/scene/mediating_display_changer.h 2015-11-05 10:19:41 +0000
114@@ -61,6 +61,7 @@
115 std::shared_ptr<graphics::DisplayConfiguration> base_configuration() override;
116 void configure(std::shared_ptr<frontend::Session> const& session,
117 std::shared_ptr<graphics::DisplayConfiguration> const& conf) override;
118+ void remove(std::shared_ptr<frontend::Session> const& session) override;
119
120 /* From mir::DisplayChanger */
121 void configure_for_hardware_change(
122
123=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
124--- tests/acceptance-tests/test_nested_mir.cpp 2015-11-04 17:59:20 +0000
125+++ tests/acceptance-tests/test_nested_mir.cpp 2015-11-05 10:19:41 +0000
126@@ -714,3 +714,85 @@
127 mir_surface_release_sync(surface);
128 mir_connection_release(connection);
129 }
130+
131+// lp:1511798
132+TEST_F(NestedServer, display_configuration_reset_when_application_exits)
133+{
134+ NestedMirRunner nested_mir{new_connection()};
135+ ignore_rebuild_of_egl_context();
136+
137+ auto const connection = mir_connect_sync(nested_mir.new_connection().c_str(), __PRETTY_FUNCTION__);
138+
139+ // Need a painted surface to have focus
140+ auto const painted_surface = make_and_paint_surface(connection);
141+
142+ auto const configuration = mir_connection_create_display_config(connection);
143+
144+ configuration->outputs->used = false;
145+
146+ mt::WaitCondition initial_condition;
147+
148+ EXPECT_CALL(*the_mock_display_configuration_report(), new_configuration(_))
149+ .WillRepeatedly(InvokeWithoutArgs([&] { initial_condition.wake_up_everyone(); }));
150+
151+ mir_wait_for(mir_connection_apply_display_config(connection, configuration));
152+
153+ mir_display_config_destroy(configuration);
154+ mir_surface_release_sync(painted_surface);
155+
156+ // Wait for initial config to be applied
157+ initial_condition.wait_for_at_most_seconds(1);
158+ Mock::VerifyAndClearExpectations(the_mock_display_configuration_report().get());
159+ ASSERT_TRUE(initial_condition.woken());
160+
161+ mt::WaitCondition condition;
162+ EXPECT_CALL(*the_mock_display_configuration_report(), new_configuration(_))
163+ .WillRepeatedly(InvokeWithoutArgs([&] { condition.wake_up_everyone(); }));
164+
165+ mir_connection_release(connection);
166+
167+ condition.wait_for_at_most_seconds(1);
168+ EXPECT_TRUE(condition.woken());
169+ Mock::VerifyAndClearExpectations(the_mock_display_configuration_report().get());
170+}
171+
172+// lp:1511798
173+TEST_F(NestedServer, display_configuration_reset_when_nested_server_exits)
174+{
175+ mt::WaitCondition condition;
176+
177+ {
178+ NestedMirRunner nested_mir{new_connection()};
179+ ignore_rebuild_of_egl_context();
180+
181+ auto const connection = mir_connect_sync(nested_mir.new_connection().c_str(), __PRETTY_FUNCTION__);
182+
183+ // Need a painted surface to have focus
184+ auto const painted_surface = make_and_paint_surface(connection);
185+
186+ std::shared_ptr<mg::DisplayConfiguration> const new_config{nested_mir.server.the_display()->configuration()};
187+ new_config->for_each_output([](mg::UserDisplayConfigurationOutput& output)
188+ { output.orientation = mir_orientation_inverted;});
189+
190+ mt::WaitCondition initial_condition;
191+
192+ EXPECT_CALL(*the_mock_display_configuration_report(), new_configuration(_))
193+ .WillRepeatedly(InvokeWithoutArgs([&] { initial_condition.wake_up_everyone(); }));
194+
195+ nested_mir.server.the_display_configuration_controller()->set_base_configuration(new_config);
196+
197+ // Wait for initial config to be applied
198+ initial_condition.wait_for_at_most_seconds(1);
199+ Mock::VerifyAndClearExpectations(the_mock_display_configuration_report().get());
200+ ASSERT_TRUE(initial_condition.woken());
201+
202+ EXPECT_CALL(*the_mock_display_configuration_report(), new_configuration(_))
203+ .WillRepeatedly(InvokeWithoutArgs([&] { condition.wake_up_everyone(); }));
204+
205+ mir_surface_release_sync(painted_surface);
206+ mir_connection_release(connection);
207+ }
208+
209+ condition.wait_for_at_most_seconds(1);
210+ EXPECT_TRUE(condition.woken());
211+}
212
213=== modified file 'tests/include/mir/test/doubles/null_display_changer.h'
214--- tests/include/mir/test/doubles/null_display_changer.h 2015-11-04 07:43:28 +0000
215+++ tests/include/mir/test/doubles/null_display_changer.h 2015-11-05 10:19:41 +0000
216@@ -46,6 +46,9 @@
217 task();
218 return std::move(fut);
219 }
220+ void remove(std::shared_ptr<frontend::Session> const&) override
221+ {
222+ }
223 };
224 }
225 }

Subscribers

People subscribed via source and target branches