Mir

Merge lp:~robertcarr/mir/session-for-placement-strategy into lp:~mir-team/mir/trunk

  1. session-for-placement-strategy
  2. Merge into trunk
Proposed by Robert Carr
Status: Merged
Approved by: Robert Carr
Approved revision: no longer in the source branch.
Merged at revision: 973
Proposed branch: lp:~robertcarr/mir/session-for-placement-strategy
Merge into: lp:~mir-team/mir/trunk
Diff against target: 292 lines (+107/-19)
11 files modified
examples/demo-shell/fullscreen_placement_strategy.cpp (+1/-1)
examples/demo-shell/fullscreen_placement_strategy.h (+1/-1)
include/server/mir/shell/consuming_placement_strategy.h (+1/-1)
include/server/mir/shell/placement_strategy.h (+2/-1)
include/test/mir_test_doubles/mock_shell_session.h (+1/-0)
include/test/mir_test_doubles/stub_shell_session.h (+81/-0)
src/server/shell/consuming_placement_strategy.cpp (+1/-1)
src/server/shell/organising_surface_factory.cpp (+1/-1)
tests/acceptance-tests/test_client_input.cpp (+1/-1)
tests/unit-tests/shell/test_consuming_placement_strategy.cpp (+4/-2)
tests/unit-tests/shell/test_organising_surface_factory.cpp (+13/-10)
To merge this branch: bzr merge lp:~robertcarr/mir/session-for-placement-strategy
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Robert Ancell Approve
Review via email: mp+179811@code.launchpad.net

Commit message

Pass session through placement strategy.

Description of the change

Pass session through placement strategy.

20:13 < tvoss> racarr, can you fix that please?
 :p

Seems useful for OSK in particular and perhaps other shell sorts of things

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

PASSED: Continuous integration, rev:971
http://jenkins.qa.ubuntu.com/job/mir-ci/1287/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1717
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1602
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/525
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/525/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Passing through context is always good. Could the tests check that the correct session is passed through rather than just ignoring the value?

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

I was hesitant to write the test that way because I had to add a stub_shell_session which will be duplicated with the one in connect-display-request. I decided to just bite the bullet though :p

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:972
http://jenkins.qa.ubuntu.com/job/mir-ci/1288/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1718/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1603/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/526/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

I am not familiar with the specific use case that requires this, but seems reasonable.

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

Added missing file.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:974
http://jenkins.qa.ubuntu.com/job/mir-ci/1297/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1728
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1613
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/535
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/535/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/demo-shell/fullscreen_placement_strategy.cpp'
2--- examples/demo-shell/fullscreen_placement_strategy.cpp 2013-07-24 05:22:03 +0000
3+++ examples/demo-shell/fullscreen_placement_strategy.cpp 2013-08-13 22:20:57 +0000
4@@ -31,7 +31,7 @@
5 {
6 }
7
8-msh::SurfaceCreationParameters me::FullscreenPlacementStrategy::place(
9+msh::SurfaceCreationParameters me::FullscreenPlacementStrategy::place(msh::Session const& /* session */,
10 msh::SurfaceCreationParameters const& request_parameters)
11 {
12 auto placed_parameters = request_parameters;
13
14=== modified file 'examples/demo-shell/fullscreen_placement_strategy.h'
15--- examples/demo-shell/fullscreen_placement_strategy.h 2013-07-24 05:22:03 +0000
16+++ examples/demo-shell/fullscreen_placement_strategy.h 2013-08-13 22:20:57 +0000
17@@ -38,7 +38,7 @@
18 FullscreenPlacementStrategy(std::shared_ptr<shell::DisplayLayout> const& display_layout);
19 ~FullscreenPlacementStrategy() = default;
20
21- shell::SurfaceCreationParameters place(shell::SurfaceCreationParameters const& request_parameters);
22+ shell::SurfaceCreationParameters place(shell::Session const&, shell::SurfaceCreationParameters const& request_parameters);
23
24 protected:
25 FullscreenPlacementStrategy(FullscreenPlacementStrategy const&) = delete;
26
27=== modified file 'include/server/mir/shell/consuming_placement_strategy.h'
28--- include/server/mir/shell/consuming_placement_strategy.h 2013-07-24 05:22:03 +0000
29+++ include/server/mir/shell/consuming_placement_strategy.h 2013-08-13 22:20:57 +0000
30@@ -36,7 +36,7 @@
31 std::shared_ptr<DisplayLayout> const& display_layout);
32 virtual ~ConsumingPlacementStrategy() {}
33
34- virtual shell::SurfaceCreationParameters place(shell::SurfaceCreationParameters const& request_parameters);
35+ virtual shell::SurfaceCreationParameters place(shell::Session const& session, shell::SurfaceCreationParameters const& request_parameters);
36
37 protected:
38 ConsumingPlacementStrategy(ConsumingPlacementStrategy const&) = delete;
39
40=== modified file 'include/server/mir/shell/placement_strategy.h'
41--- include/server/mir/shell/placement_strategy.h 2013-05-21 17:16:43 +0000
42+++ include/server/mir/shell/placement_strategy.h 2013-08-13 22:20:57 +0000
43@@ -24,6 +24,7 @@
44
45 namespace shell
46 {
47+class Session;
48 struct SurfaceCreationParameters;
49
50 class PlacementStrategy
51@@ -32,7 +33,7 @@
52 virtual ~PlacementStrategy() {}
53 // TODO: It is strange to work in terms of SurfaceCreationParameters here,
54 // perhaps a new interface is needed.
55- virtual SurfaceCreationParameters place(SurfaceCreationParameters const& request_parameters) = 0;
56+ virtual SurfaceCreationParameters place(shell::Session const& session, SurfaceCreationParameters const& request_parameters) = 0;
57
58
59 protected:
60
61=== modified file 'include/test/mir_test_doubles/mock_shell_session.h'
62--- include/test/mir_test_doubles/mock_shell_session.h 2013-08-08 19:47:31 +0000
63+++ include/test/mir_test_doubles/mock_shell_session.h 2013-08-13 22:20:57 +0000
64@@ -21,6 +21,7 @@
65
66 #include "mir/shell/session.h"
67 #include "mir/shell/surface_creation_parameters.h"
68+#include "mir/graphics/display_configuration.h"
69
70 #include <gmock/gmock.h>
71
72
73=== added file 'include/test/mir_test_doubles/stub_shell_session.h'
74--- include/test/mir_test_doubles/stub_shell_session.h 1970-01-01 00:00:00 +0000
75+++ include/test/mir_test_doubles/stub_shell_session.h 2013-08-13 22:20:57 +0000
76@@ -0,0 +1,81 @@
77+/*
78+ * Copyright © 2013 Canonical Ltd.
79+ *
80+ * This program is free software: you can redistribute it and/or modify it
81+ * under the terms of the GNU General Public License version 3,
82+ * as published by the Free Software Foundation.
83+ *
84+ * This program is distributed in the hope that it will be useful,
85+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
86+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
87+ * GNU General Public License for more details.
88+ *
89+ * You should have received a copy of the GNU General Public License
90+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
91+ *
92+ * Authored by: Robert Carr <robert.carr@canonical.com>
93+ */
94+
95+#ifndef MIR_TEST_DOUBLES_STUB_SHELL_SESSION_H_
96+#define MIR_TEST_DOUBLES_STUB_SHELL_SESSION_H_
97+
98+#include "mir/shell/session.h"
99+
100+namespace mir
101+{
102+namespace test
103+{
104+namespace doubles
105+{
106+
107+struct StubShellSession : public shell::Session
108+{
109+ frontend::SurfaceId create_surface(shell::SurfaceCreationParameters const& /* params */) override
110+ {
111+ return frontend::SurfaceId{0};
112+ }
113+ void destroy_surface(frontend::SurfaceId /* surface */) override
114+ {
115+ }
116+ std::shared_ptr<frontend::Surface> get_surface(frontend::SurfaceId /* surface */) const override
117+ {
118+ return std::shared_ptr<frontend::Surface>();
119+ }
120+ std::string name() const override
121+ {
122+ return std::string();
123+ }
124+ void force_requests_to_complete() override
125+ {
126+ }
127+ void hide() override
128+ {
129+ }
130+ void show() override
131+ {
132+ }
133+ int configure_surface(frontend::SurfaceId, MirSurfaceAttrib, int) override
134+ {
135+ return 0;
136+ }
137+
138+ void send_display_config(graphics::DisplayConfiguration const&) override
139+ {
140+ }
141+
142+ void take_snapshot(shell::SnapshotCallback const&) override
143+ {
144+ }
145+
146+ std::shared_ptr<shell::Surface> default_surface() const override
147+ {
148+ return std::shared_ptr<shell::Surface>();
149+ }
150+};
151+
152+}
153+}
154+} // namespace mir
155+
156+#endif // MIR_TEST_DOUBLES_STUB_SHELL_SESSION_H_
157+
158
159=== modified file 'src/server/shell/consuming_placement_strategy.cpp'
160--- src/server/shell/consuming_placement_strategy.cpp 2013-07-24 05:22:03 +0000
161+++ src/server/shell/consuming_placement_strategy.cpp 2013-08-13 22:20:57 +0000
162@@ -32,7 +32,7 @@
163 {
164 }
165
166-msh::SurfaceCreationParameters msh::ConsumingPlacementStrategy::place(msh::SurfaceCreationParameters const& request_parameters)
167+msh::SurfaceCreationParameters msh::ConsumingPlacementStrategy::place(msh::Session const& /* session */, msh::SurfaceCreationParameters const& request_parameters)
168 {
169 auto placed_parameters = request_parameters;
170
171
172=== modified file 'src/server/shell/organising_surface_factory.cpp'
173--- src/server/shell/organising_surface_factory.cpp 2013-08-09 03:39:35 +0000
174+++ src/server/shell/organising_surface_factory.cpp 2013-08-13 22:20:57 +0000
175@@ -40,7 +40,7 @@
176 frontend::SurfaceId id,
177 std::shared_ptr<mf::EventSink> const& sender)
178 {
179- auto placed_params = placement_strategy->place(params);
180+ auto placed_params = placement_strategy->place(*session, params);
181
182 return underlying_factory->create_surface(session, placed_params, id, sender);
183 }
184
185=== modified file 'tests/acceptance-tests/test_client_input.cpp'
186--- tests/acceptance-tests/test_client_input.cpp 2013-08-09 03:39:35 +0000
187+++ tests/acceptance-tests/test_client_input.cpp 2013-08-13 22:20:57 +0000
188@@ -506,7 +506,7 @@
189 {
190 }
191
192- msh::SurfaceCreationParameters place(msh::SurfaceCreationParameters const& request_parameters)
193+ msh::SurfaceCreationParameters place(msh::Session const&, msh::SurfaceCreationParameters const& request_parameters)
194 {
195 auto placed = request_parameters;
196 auto const& name = request_parameters.name;
197
198=== modified file 'tests/unit-tests/shell/test_consuming_placement_strategy.cpp'
199--- tests/unit-tests/shell/test_consuming_placement_strategy.cpp 2013-07-24 05:22:03 +0000
200+++ tests/unit-tests/shell/test_consuming_placement_strategy.cpp 2013-08-13 22:20:57 +0000
201@@ -17,6 +17,7 @@
202 */
203
204 #include "mir_test_doubles/mock_display_layout.h"
205+#include "mir_test_doubles/stub_shell_session.h"
206
207 #include "mir/shell/consuming_placement_strategy.h"
208 #include "mir/shell/surface_creation_parameters.h"
209@@ -42,6 +43,7 @@
210 }
211
212 std::shared_ptr<mtd::MockDisplayLayout> display_layout;
213+ mtd::StubShellSession session;
214 };
215 }
216
217@@ -57,7 +59,7 @@
218
219 msh::ConsumingPlacementStrategy placement_strategy(display_layout);
220
221- placement_strategy.place(input_params);
222+ placement_strategy.place(session, input_params);
223 }
224
225 TEST_F(ConsumingPlacementStrategySetup, parameters_with_geometry_are_clipped)
226@@ -72,5 +74,5 @@
227
228 msh::ConsumingPlacementStrategy placement_strategy(display_layout);
229
230- placement_strategy.place(input_params);
231+ placement_strategy.place(session, input_params);
232 }
233
234=== modified file 'tests/unit-tests/shell/test_organising_surface_factory.cpp'
235--- tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-09 03:39:35 +0000
236+++ tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-13 22:20:57 +0000
237@@ -16,12 +16,14 @@
238 * Authored by: Robert Carr <robert.carr@canonical.com>
239 */
240
241-#include <mir_test_doubles/mock_surface_factory.h>
242-#include <mir_test_doubles/null_event_sink.h>
243+#include "mir/shell/organising_surface_factory.h"
244+#include "mir/shell/placement_strategy.h"
245+#include "mir/shell/surface_creation_parameters.h"
246+#include "mir/shell/session.h"
247
248-#include <mir/shell/organising_surface_factory.h>
249-#include <mir/shell/placement_strategy.h>
250-#include <mir/shell/surface_creation_parameters.h>
251+#include "mir_test_doubles/mock_surface_factory.h"
252+#include "mir_test_doubles/stub_shell_session.h"
253+#include "mir_test_doubles/null_event_sink.h"
254
255 #include <gtest/gtest.h>
256 #include <gmock/gmock.h>
257@@ -36,7 +38,7 @@
258
259 struct MockPlacementStrategy : public msh::PlacementStrategy
260 {
261- MOCK_METHOD1(place, msh::SurfaceCreationParameters(msh::SurfaceCreationParameters const&));
262+ MOCK_METHOD2(place, msh::SurfaceCreationParameters(msh::Session const&, msh::SurfaceCreationParameters const&));
263 };
264
265 struct OrganisingSurfaceFactorySetup : public testing::Test
266@@ -62,14 +64,15 @@
267 using namespace ::testing;
268
269 msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy);
270-
271+
272+ mtd::StubShellSession session;
273 EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).Times(1);
274
275 auto params = msh::a_surface();
276- EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
277+ EXPECT_CALL(*placement_strategy, place(Ref(session), Ref(params))).Times(1)
278 .WillOnce(Return(msh::a_surface()));
279
280- factory.create_surface(nullptr, params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());
281+ factory.create_surface(&session, params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());
282 }
283
284 TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory)
285@@ -83,7 +86,7 @@
286 auto placed_params = params;
287 placed_params.size.width = geom::Width{100};
288
289- EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
290+ EXPECT_CALL(*placement_strategy, place(_, Ref(params))).Times(1)
291 .WillOnce(Return(placed_params));
292 EXPECT_CALL(*underlying_surface_factory, create_surface(nullptr, placed_params, mf::SurfaceId(), sink));
293

Subscribers

People subscribed via source and target branches