Merge lp:~mterry/mir/session-for-surface into lp:~mir-team/mir/trunk
- session-for-surface
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Alan Griffiths | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 951 | ||||
Proposed branch: | lp:~mterry/mir/session-for-surface | ||||
Merge into: | lp:~mir-team/mir/trunk | ||||
Diff against target: |
691 lines (+87/-42) 23 files modified
examples/render_surfaces.cpp (+1/-0) include/server/mir/shell/organising_surface_factory.h (+2/-0) include/server/mir/shell/surface.h (+2/-0) include/server/mir/shell/surface_builder.h (+2/-1) include/server/mir/shell/surface_factory.h (+2/-0) include/server/mir/shell/surface_source.h (+2/-0) include/server/mir/surfaces/surface_controller.h (+6/-1) include/test/mir_test_doubles/mock_surface.h (+2/-2) include/test/mir_test_doubles/mock_surface_factory.h (+2/-1) include/test/mir_test_doubles/stub_surface_builder.h (+1/-1) src/server/shell/application_session.cpp (+1/-1) src/server/shell/organising_surface_factory.cpp (+2/-1) src/server/shell/surface.cpp (+2/-1) src/server/shell/surface_source.cpp (+2/-1) src/server/surfaces/surface_controller.cpp (+2/-1) tests/acceptance-tests/test_client_input.cpp (+3/-2) tests/integration-tests/graphics/android/test_internal_client.cpp (+1/-1) tests/unit-tests/shell/test_application_session.cpp (+10/-10) tests/unit-tests/shell/test_default_focus_mechanism.cpp (+2/-2) tests/unit-tests/shell/test_organising_surface_factory.cpp (+5/-5) tests/unit-tests/shell/test_session_manager.cpp (+6/-4) tests/unit-tests/shell/test_surface.cpp (+28/-6) tests/unit-tests/surfaces/test_surface_controller.cpp (+1/-1) |
||||
To merge this branch: | bzr merge lp:~mterry/mir/session-for-surface | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alan Griffiths | Abstain | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Kevin DuBois (community) | Abstain | ||
Robert Carr (community) | Approve | ||
Robert Ancell | Approve | ||
Review via email: mp+174406@code.launchpad.net |
Commit message
Pass on the owning Session when creating Surfaces.
Description of the change
This branch exposes the owning Session when creating Surfaces.
I'm not super familiar with writing real C++11 and my use of shared_ptr may be really whack. In particular, my use of std::enable_
PS Jenkins bot (ps-jenkins) wrote : | # |
Alan Griffiths (alan-griffiths) wrote : | # |
Hi Michael, I'm afraid your use of shared pointers is off.
shared_ptr denotes shared ownership and, in this case the session owns the surface. It doesn't make sense for the surface to also own the session - at the very least that can create an ownership cycle (and cause a memory leak).
Having said that, as the session is guaranteed to outlive the surface I imagine an ordinary raw pointer will serve your purpose.
Michael Terry (mterry) wrote : | # |
Ah thanks, Alan. That makes sense. I've updated the branch to use raw pointers.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:834
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:835
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
We should be doing something different than passing the session ptr into the surface. This establishes a weird dependency circle, I think.
from 'bug report' "But I can't seem to get access to the session that a surface is being created for."
This is somewhat by design, at the moment, a design that probably needs rethinking to suit the requirements of the shell. I think we should meet on monday to chat about what exactly is needed.
(I think that we're exposing cracks in 1) the "depth id" stacking system that ms::SurfaceStack has that might need changing)
Alan Griffiths (alan-griffiths) wrote : | # |
The "scenegraph" should probably be the way to navigate from a surface to its parent session, so this might not be the best approach. OTOH it is simple and may be enough.
Michael Terry (mterry) wrote : | # |
Did you Mir folk discuss how you wanted to proceed on this?
Alan Griffiths (alan-griffiths) wrote : | # |
> Did you Mir folk discuss how you wanted to proceed on this?
/1/ fix the CI failures?
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:836
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Michael Terry (mterry) wrote : | # |
> /1/ fix the CI failures?
I think I fixed them now, please re-examine.
That aside, I had been more looking for guidance on the direction of this merge. Like, whether I should abandon this method and go a different route.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:837
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Robert Carr (robertcarr) wrote : | # |
I'm still trying to think of a better route that satisfies this case!
I think this route isn't the way to go as these kind of relationships tend to make testing/refactoring difficult. I am also a little worried it may be unsafe:
//
{
std::shared_
std::shared_
session.reset();
// Now while we do some other stuff or yield, a thread from frontend closes session causing it to be deleted (we didn't keep a reference)
LOG(surface-
}
Alan Griffiths (alan-griffiths) wrote : | # |
> I'm still trying to think of a better route that satisfies this case!
Maybe we need to clarify "this case" - this MP simply adds a parameter without a driving test case.
Consequentially we're probably all guessing at the problem it solves.
> I think this route isn't the way to go as these kind of relationships tend to
> make testing/refactoring difficult. I am also a little worried it may be
> unsafe:
I think the point is to make the session known at the point of building the surfaces::Surface - not long term storage. Vis:
72 + virtual std::weak_
Although, it isn't used in SurfaceControll
Michael Terry (mterry) wrote : | # |
Here's my use case:
I want to change u-s-c to be able to place a specified user session under the greeter session, so the user session can bleed through (e.g. imagine the pin unlock screen that shows a blurred user session underneath).
Here's my current thinking on how to do so:
A) Change u-s-c to override the_surface_
B) Then, have lightdm tell u-s-c which session is the greeter (by naming it something like "Greeter 1") and when to raise the appropriate user session inside of its layer.
Here's the missing piece:
u-s-c can't seem to tell me that a given surface belongs to a session called "Greeter 1", so I can't create it with the right depthId. Hence this proposal.
I'm open to other ways of achieving the use case. Including some special case API like "Keep this session above these others" or "Put this session below this other one". But this MP seemed more generally useful.
And yeah, I didn't need the session pointer to be stored long term. Just needed to inspect it momentarily. So for my specific use case, eventual segfaults from storing it is not as much a problem.
Robert Ancell (robert-ancell) wrote : | # |
Robert's concerns are valid in that we don't want the chance of the session being invalid but the surface object still existing. Can we just pass the session into create_surface() as context and u-s-c can store its own mapping and take responsibility for keeping track of invalid references?
Alan Griffiths (alan-griffiths) wrote : | # |
> Robert's concerns are valid in that we don't want the chance of the session
> being invalid but the surface object still existing.
Robert's concerns relate to an existing problem to be invalid - this MP doesn't make anything worse.
> Can we just pass the
> session into create_surface() as context and u-s-c can store its own mapping
> and take responsibility for keeping track of invalid references?
+1
However, there's a related design issue here: SurfaceBuilder has a destroy() member function. This makes management of the surfaces::Surface painful - forcing shell::Surface to take a SurfaceBuilder dependency and requiring the constructor to take the Session to pass on as a parameter to pass to the corresponding create function.
Without SurfaceBuilder:
PS It might also be thought that SurfaceBuilder:
PPS There are other MPs around that touch on this problem - basically we currently have no way to present a consistent view of our data model. (See for example, https:/
Michael Terry (mterry) wrote : | # |
>> Can we just pass the
>> session into create_surface() as context and u-s-c can store its own mapping
>> and take responsibility for keeping track of invalid references?
>
> +1
To be clear, that's what this branch does, eh? I don't store the pointer anywhere, that I can see. I just pass it down through the layers, and u-s-c can do what it wants with it.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:838
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Robert Ancell (robert-ancell) wrote : | # |
> >> Can we just pass the
> >> session into create_surface() as context and u-s-c can store its own
> mapping
> >> and take responsibility for keeping track of invalid references?
> >
> > +1
>
> To be clear, that's what this branch does, eh? I don't store the pointer
> anywhere, that I can see. I just pass it down through the layers, and u-s-c
> can do what it wants with it.
Oh duh. I must have misread the MP. Looks fine to me then
Robert Carr (robertcarr) wrote : | # |
What's wrong with SessionListener
Michael Terry (mterry) wrote : | # |
> What's wrong with SessionListener
Well, when I wrote this branch, that didn't exist. :) But seriously, I don't think that would help a bunch. My brief understanding is that surfaces are assigned a DepthId at creation time. And I was wanting u-s-c to be able to set DepthId based on the Session object. If we get a signal after the fact, that seems too late for DepthId purposes.
Robert Carr (robertcarr) wrote : | # |
Oh! I see. Think-o.
Ok. I still don't think this is the way to go architecturally for Mir, but I dont think it makes things worse and I don't want to block you any more! LGTM
Alan Griffiths (alan-griffiths) wrote : | # |
I too think this is a step away from where we would like to be, but we don't have a better solution proposed and I won't block if it works around a problem.
Kevin DuBois (kdub) wrote : | # |
I'll switch to abstain, I think this change will need to be rethought eventually in subsequent refactorings, it points out a problem with the class structures (or at least the classes meeting the requirements of the shell) in this part of the code.
Alan Griffiths (alan-griffiths) wrote : | # |
We've taken so long to reach "approval" that there are now merge conflicts to address.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:839
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
for_each_
Preview Diff
1 | === modified file 'examples/render_surfaces.cpp' |
2 | --- examples/render_surfaces.cpp 2013-08-02 03:01:12 +0000 |
3 | +++ examples/render_surfaces.cpp 2013-08-09 03:39:57 +0000 |
4 | @@ -407,6 +407,7 @@ |
5 | for (auto& m : moveables) |
6 | { |
7 | std::shared_ptr<ms::Surface> s = surface_builder->create_surface( |
8 | + nullptr, |
9 | msh::a_surface().of_size(surface_size) |
10 | .of_pixel_format(surface_pf) |
11 | .of_buffer_usage(mg::BufferUsage::hardware) |
12 | |
13 | === modified file 'include/server/mir/shell/organising_surface_factory.h' |
14 | --- include/server/mir/shell/organising_surface_factory.h 2013-08-01 07:44:17 +0000 |
15 | +++ include/server/mir/shell/organising_surface_factory.h 2013-08-09 03:39:57 +0000 |
16 | @@ -28,6 +28,7 @@ |
17 | namespace shell |
18 | { |
19 | class PlacementStrategy; |
20 | +class Session; |
21 | |
22 | class OrganisingSurfaceFactory : public SurfaceFactory |
23 | { |
24 | @@ -37,6 +38,7 @@ |
25 | virtual ~OrganisingSurfaceFactory(); |
26 | |
27 | std::shared_ptr<Surface> create_surface( |
28 | + Session* session, |
29 | shell::SurfaceCreationParameters const& params, |
30 | frontend::SurfaceId id, |
31 | std::shared_ptr<frontend::EventSink> const& sink) override; |
32 | |
33 | === modified file 'include/server/mir/shell/surface.h' |
34 | --- include/server/mir/shell/surface.h 2013-08-08 02:34:31 +0000 |
35 | +++ include/server/mir/shell/surface.h 2013-08-09 03:39:57 +0000 |
36 | @@ -38,6 +38,7 @@ |
37 | namespace shell |
38 | { |
39 | class InputTargeter; |
40 | +class Session; |
41 | class SurfaceBuilder; |
42 | class SurfaceConfigurator; |
43 | class SurfaceController; |
44 | @@ -47,6 +48,7 @@ |
45 | { |
46 | public: |
47 | Surface( |
48 | + Session* session, |
49 | std::shared_ptr<SurfaceBuilder> const& builder, |
50 | std::shared_ptr<SurfaceConfigurator> const& configurator, |
51 | SurfaceCreationParameters const& params, |
52 | |
53 | === modified file 'include/server/mir/shell/surface_builder.h' |
54 | --- include/server/mir/shell/surface_builder.h 2013-05-21 17:16:43 +0000 |
55 | +++ include/server/mir/shell/surface_builder.h 2013-08-09 03:39:57 +0000 |
56 | @@ -28,12 +28,13 @@ |
57 | |
58 | namespace shell |
59 | { |
60 | +class Session; |
61 | struct SurfaceCreationParameters; |
62 | |
63 | class SurfaceBuilder |
64 | { |
65 | public: |
66 | - virtual std::weak_ptr<surfaces::Surface> create_surface(SurfaceCreationParameters const& params) = 0; |
67 | + virtual std::weak_ptr<surfaces::Surface> create_surface(Session* session, SurfaceCreationParameters const& params) = 0; |
68 | |
69 | virtual void destroy_surface(std::weak_ptr<surfaces::Surface> const& surface) = 0; |
70 | protected: |
71 | |
72 | === modified file 'include/server/mir/shell/surface_factory.h' |
73 | --- include/server/mir/shell/surface_factory.h 2013-08-01 07:44:17 +0000 |
74 | +++ include/server/mir/shell/surface_factory.h 2013-08-09 03:39:57 +0000 |
75 | @@ -30,6 +30,7 @@ |
76 | } |
77 | namespace shell |
78 | { |
79 | +class Session; |
80 | class Surface; |
81 | struct SurfaceCreationParameters; |
82 | |
83 | @@ -37,6 +38,7 @@ |
84 | { |
85 | public: |
86 | virtual std::shared_ptr<Surface> create_surface( |
87 | + Session* session, |
88 | SurfaceCreationParameters const& params, |
89 | frontend::SurfaceId id, |
90 | std::shared_ptr<frontend::EventSink> const& sink) = 0; |
91 | |
92 | === modified file 'include/server/mir/shell/surface_source.h' |
93 | --- include/server/mir/shell/surface_source.h 2013-08-08 02:34:31 +0000 |
94 | +++ include/server/mir/shell/surface_source.h 2013-08-09 03:39:57 +0000 |
95 | @@ -28,6 +28,7 @@ |
96 | |
97 | namespace shell |
98 | { |
99 | +class Session; |
100 | class SurfaceBuilder; |
101 | class SurfaceConfigurator; |
102 | |
103 | @@ -39,6 +40,7 @@ |
104 | virtual ~SurfaceSource() {} |
105 | |
106 | std::shared_ptr<Surface> create_surface( |
107 | + Session* session, |
108 | shell::SurfaceCreationParameters const& params, |
109 | frontend::SurfaceId id, |
110 | std::shared_ptr<frontend::EventSink> const& sink); |
111 | |
112 | === modified file 'include/server/mir/surfaces/surface_controller.h' |
113 | --- include/server/mir/surfaces/surface_controller.h 2013-08-02 04:07:15 +0000 |
114 | +++ include/server/mir/surfaces/surface_controller.h 2013-08-09 03:39:57 +0000 |
115 | @@ -25,6 +25,11 @@ |
116 | |
117 | namespace mir |
118 | { |
119 | +namespace shell |
120 | +{ |
121 | +class Session; |
122 | +} |
123 | + |
124 | namespace surfaces |
125 | { |
126 | class SurfaceStackModel; |
127 | @@ -35,7 +40,7 @@ |
128 | public: |
129 | explicit SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack); |
130 | |
131 | - virtual std::weak_ptr<Surface> create_surface(shell::SurfaceCreationParameters const& params); |
132 | + virtual std::weak_ptr<Surface> create_surface(shell::Session* session, shell::SurfaceCreationParameters const& params); |
133 | virtual void destroy_surface(std::weak_ptr<Surface> const& surface); |
134 | |
135 | virtual void raise(std::weak_ptr<Surface> const& surface); |
136 | |
137 | === modified file 'include/test/mir_test_doubles/mock_surface.h' |
138 | --- include/test/mir_test_doubles/mock_surface.h 2013-08-08 02:34:31 +0000 |
139 | +++ include/test/mir_test_doubles/mock_surface.h 2013-08-09 03:39:57 +0000 |
140 | @@ -38,8 +38,8 @@ |
141 | |
142 | struct MockSurface : public shell::Surface |
143 | { |
144 | - MockSurface(std::shared_ptr<shell::SurfaceBuilder> const& builder) : |
145 | - shell::Surface(builder, std::make_shared<NullSurfaceConfigurator>(), shell::a_surface(), |
146 | + MockSurface(shell::Session* session, std::shared_ptr<shell::SurfaceBuilder> const& builder) : |
147 | + shell::Surface(session, builder, std::make_shared<NullSurfaceConfigurator>(), shell::a_surface(), |
148 | frontend::SurfaceId{}, std::make_shared<NullEventSink>()) |
149 | { |
150 | } |
151 | |
152 | === modified file 'include/test/mir_test_doubles/mock_surface_factory.h' |
153 | --- include/test/mir_test_doubles/mock_surface_factory.h 2013-08-01 07:44:17 +0000 |
154 | +++ include/test/mir_test_doubles/mock_surface_factory.h 2013-08-09 03:39:57 +0000 |
155 | @@ -33,7 +33,8 @@ |
156 | |
157 | struct MockSurfaceFactory : public shell::SurfaceFactory |
158 | { |
159 | - MOCK_METHOD3(create_surface, std::shared_ptr<shell::Surface>( |
160 | + MOCK_METHOD4(create_surface, std::shared_ptr<shell::Surface>( |
161 | + shell::Session*, |
162 | const shell::SurfaceCreationParameters&, |
163 | frontend::SurfaceId, |
164 | std::shared_ptr<frontend::EventSink> const&)); |
165 | |
166 | === modified file 'include/test/mir_test_doubles/stub_surface_builder.h' |
167 | --- include/test/mir_test_doubles/stub_surface_builder.h 2013-07-12 20:33:47 +0000 |
168 | +++ include/test/mir_test_doubles/stub_surface_builder.h 2013-08-09 03:39:57 +0000 |
169 | @@ -43,7 +43,7 @@ |
170 | { |
171 | } |
172 | |
173 | - std::weak_ptr<surfaces::Surface> create_surface(shell::SurfaceCreationParameters const&) |
174 | + std::weak_ptr<surfaces::Surface> create_surface(shell::Session*, shell::SurfaceCreationParameters const&) |
175 | { |
176 | auto state = std::make_shared<MockSurfaceState>(); |
177 | dummy_surface = std::make_shared<surfaces::Surface>( |
178 | |
179 | === modified file 'src/server/shell/application_session.cpp' |
180 | --- src/server/shell/application_session.cpp 2013-08-08 02:34:31 +0000 |
181 | +++ src/server/shell/application_session.cpp 2013-08-09 03:39:57 +0000 |
182 | @@ -65,7 +65,7 @@ |
183 | mf::SurfaceId msh::ApplicationSession::create_surface(const msh::SurfaceCreationParameters& params) |
184 | { |
185 | auto const id = next_id(); |
186 | - auto surf = surface_factory->create_surface(params, id, event_sink); |
187 | + auto surf = surface_factory->create_surface(this, params, id, event_sink); |
188 | |
189 | std::unique_lock<std::mutex> lock(surfaces_mutex); |
190 | surfaces[id] = surf; |
191 | |
192 | === modified file 'src/server/shell/organising_surface_factory.cpp' |
193 | --- src/server/shell/organising_surface_factory.cpp 2013-08-01 07:44:17 +0000 |
194 | +++ src/server/shell/organising_surface_factory.cpp 2013-08-09 03:39:57 +0000 |
195 | @@ -35,12 +35,13 @@ |
196 | } |
197 | |
198 | std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface( |
199 | + Session* session, |
200 | shell::SurfaceCreationParameters const& params, |
201 | frontend::SurfaceId id, |
202 | std::shared_ptr<mf::EventSink> const& sender) |
203 | { |
204 | auto placed_params = placement_strategy->place(params); |
205 | |
206 | - return underlying_factory->create_surface(placed_params, id, sender); |
207 | + return underlying_factory->create_surface(session, placed_params, id, sender); |
208 | } |
209 | |
210 | |
211 | === modified file 'src/server/shell/surface.cpp' |
212 | --- src/server/shell/surface.cpp 2013-08-08 02:34:31 +0000 |
213 | +++ src/server/shell/surface.cpp 2013-08-09 03:39:57 +0000 |
214 | @@ -40,6 +40,7 @@ |
215 | namespace mf = mir::frontend; |
216 | |
217 | msh::Surface::Surface( |
218 | + Session* session, |
219 | std::shared_ptr<SurfaceBuilder> const& builder, |
220 | std::shared_ptr<SurfaceConfigurator> const& configurator, |
221 | shell::SurfaceCreationParameters const& params, |
222 | @@ -47,7 +48,7 @@ |
223 | std::shared_ptr<mf::EventSink> const& event_sink) |
224 | : builder(builder), |
225 | configurator(configurator), |
226 | - surface(builder->create_surface(params)), |
227 | + surface(builder->create_surface(session, params)), |
228 | id(id), |
229 | event_sink(event_sink), |
230 | type_value(mir_surface_type_normal), |
231 | |
232 | === modified file 'src/server/shell/surface_source.cpp' |
233 | --- src/server/shell/surface_source.cpp 2013-08-08 02:34:31 +0000 |
234 | +++ src/server/shell/surface_source.cpp 2013-08-09 03:39:57 +0000 |
235 | @@ -40,10 +40,11 @@ |
236 | } |
237 | |
238 | std::shared_ptr<msh::Surface> msh::SurfaceSource::create_surface( |
239 | + msh::Session* session, |
240 | shell::SurfaceCreationParameters const& params, |
241 | frontend::SurfaceId id, |
242 | std::shared_ptr<mf::EventSink> const& sender) |
243 | { |
244 | - return std::make_shared<Surface>(surface_builder, surface_configurator, params, id, sender); |
245 | + return std::make_shared<Surface>(session, surface_builder, surface_configurator, params, id, sender); |
246 | } |
247 | |
248 | |
249 | === modified file 'src/server/surfaces/surface_controller.cpp' |
250 | --- src/server/surfaces/surface_controller.cpp 2013-07-19 02:44:21 +0000 |
251 | +++ src/server/surfaces/surface_controller.cpp 2013-08-09 03:39:57 +0000 |
252 | @@ -20,13 +20,14 @@ |
253 | #include "mir/surfaces/surface_stack_model.h" |
254 | |
255 | namespace ms = mir::surfaces; |
256 | +namespace msh = mir::shell; |
257 | |
258 | ms::SurfaceController::SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack) : |
259 | surface_stack(surface_stack) |
260 | { |
261 | } |
262 | |
263 | -std::weak_ptr<ms::Surface> ms::SurfaceController::create_surface(shell::SurfaceCreationParameters const& params) |
264 | +std::weak_ptr<ms::Surface> ms::SurfaceController::create_surface(msh::Session*, shell::SurfaceCreationParameters const& params) |
265 | { |
266 | return surface_stack->create_surface(params); |
267 | } |
268 | |
269 | === modified file 'tests/acceptance-tests/test_client_input.cpp' |
270 | --- tests/acceptance-tests/test_client_input.cpp 2013-08-02 09:05:39 +0000 |
271 | +++ tests/acceptance-tests/test_client_input.cpp 2013-08-09 03:39:57 +0000 |
272 | @@ -620,11 +620,12 @@ |
273 | { |
274 | } |
275 | |
276 | - std::shared_ptr<msh::Surface> create_surface(msh::SurfaceCreationParameters const& params, |
277 | + std::shared_ptr<msh::Surface> create_surface(msh::Session* session, |
278 | + msh::SurfaceCreationParameters const& params, |
279 | mf::SurfaceId id, |
280 | std::shared_ptr<mf::EventSink> const& sink) |
281 | { |
282 | - auto surface = underlying_factory->create_surface(params, id, sink); |
283 | + auto surface = underlying_factory->create_surface(session, params, id, sink); |
284 | |
285 | surface->set_input_region(input_rectangles); |
286 | |
287 | |
288 | === modified file 'tests/integration-tests/graphics/android/test_internal_client.cpp' |
289 | --- tests/integration-tests/graphics/android/test_internal_client.cpp 2013-08-08 02:34:31 +0000 |
290 | +++ tests/integration-tests/graphics/android/test_internal_client.cpp 2013-08-09 03:39:57 +0000 |
291 | @@ -110,7 +110,7 @@ |
292 | auto surface_controller = std::make_shared<ms::SurfaceController>(ss); |
293 | auto surface_source = std::make_shared<msh::SurfaceSource>(surface_controller, std::make_shared<mtd::NullSurfaceConfigurator>()); |
294 | auto mir_surface = std::make_shared<ForwardingInternalSurface>( |
295 | - surface_source->create_surface(params, id, std::shared_ptr<mf::EventSink>())); |
296 | + surface_source->create_surface(nullptr, params, id, std::shared_ptr<mf::EventSink>())); |
297 | |
298 | auto options = std::shared_ptr<mo::ProgramOption>(); |
299 | auto report = std::shared_ptr<mg::NullDisplayReport>(); |
300 | |
301 | === modified file 'tests/unit-tests/shell/test_application_session.cpp' |
302 | --- tests/unit-tests/shell/test_application_session.cpp 2013-08-08 02:34:31 +0000 |
303 | +++ tests/unit-tests/shell/test_application_session.cpp 2013-08-09 03:39:57 +0000 |
304 | @@ -49,7 +49,7 @@ |
305 | { |
306 | mtd::StubSurfaceBuilder surface_builder; |
307 | |
308 | - return std::make_shared<mtd::MockSurface>(std::make_shared<mtd::StubSurfaceBuilder>()); |
309 | + return std::make_shared<mtd::MockSurface>(nullptr, std::make_shared<mtd::StubSurfaceBuilder>()); |
310 | } |
311 | } |
312 | |
313 | @@ -61,9 +61,9 @@ |
314 | |
315 | mtd::NullEventSink sender; |
316 | mtd::MockSurfaceFactory surface_factory; |
317 | - ON_CALL(surface_factory, create_surface(_,_,_)).WillByDefault(Return(mock_surface)); |
318 | + ON_CALL(surface_factory, create_surface(_,_,_,_)).WillByDefault(Return(mock_surface)); |
319 | |
320 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)); |
321 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)); |
322 | EXPECT_CALL(*mock_surface, destroy()); |
323 | |
324 | mtd::MockSessionListener listener; |
325 | @@ -90,11 +90,11 @@ |
326 | |
327 | { |
328 | InSequence seq; |
329 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1) |
330 | - .WillOnce(Return(make_mock_surface())); |
331 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1) |
332 | - .WillOnce(Return(make_mock_surface())); |
333 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1) |
334 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1) |
335 | + .WillOnce(Return(make_mock_surface())); |
336 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1) |
337 | + .WillOnce(Return(make_mock_surface())); |
338 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1) |
339 | .WillOnce(Return(make_mock_surface())); |
340 | } |
341 | |
342 | @@ -129,13 +129,13 @@ |
343 | auto mock_surface = make_mock_surface(); |
344 | |
345 | mtd::MockSurfaceFactory surface_factory; |
346 | - ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(Return(mock_surface)); |
347 | + ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(mock_surface)); |
348 | |
349 | msh::ApplicationSession app_session(mt::fake_shared(surface_factory), "Foo", |
350 | std::make_shared<mtd::NullSnapshotStrategy>(), |
351 | std::make_shared<msh::NullSessionListener>(), mt::fake_shared(sender)); |
352 | |
353 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)); |
354 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)); |
355 | |
356 | { |
357 | InSequence seq; |
358 | |
359 | === modified file 'tests/unit-tests/shell/test_default_focus_mechanism.cpp' |
360 | --- tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-08-02 04:07:15 +0000 |
361 | +++ tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-08-09 03:39:57 +0000 |
362 | @@ -68,7 +68,7 @@ |
363 | using namespace ::testing; |
364 | |
365 | NiceMock<MockShellSession> app1; |
366 | - mtd::MockSurface mock_surface(std::make_shared<mtd::StubSurfaceBuilder>()); |
367 | + mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>()); |
368 | { |
369 | InSequence seq; |
370 | EXPECT_CALL(app1, default_surface()).Times(1) |
371 | @@ -88,7 +88,7 @@ |
372 | using namespace ::testing; |
373 | |
374 | NiceMock<MockShellSession> app1; |
375 | - mtd::MockSurface mock_surface(std::make_shared<mtd::StubSurfaceBuilder>()); |
376 | + mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>()); |
377 | { |
378 | InSequence seq; |
379 | EXPECT_CALL(app1, default_surface()).Times(1) |
380 | |
381 | === modified file 'tests/unit-tests/shell/test_organising_surface_factory.cpp' |
382 | --- tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-01 07:44:17 +0000 |
383 | +++ tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-09 03:39:57 +0000 |
384 | @@ -46,7 +46,7 @@ |
385 | using namespace ::testing; |
386 | |
387 | underlying_surface_factory = std::make_shared<mtd::MockSurfaceFactory>(); |
388 | - ON_CALL(*underlying_surface_factory, create_surface(_, _, _)).WillByDefault(Return(null_surface)); |
389 | + ON_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(null_surface)); |
390 | |
391 | placement_strategy = std::make_shared<MockPlacementStrategy>(); |
392 | } |
393 | @@ -63,13 +63,13 @@ |
394 | |
395 | msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy); |
396 | |
397 | - EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _)).Times(1); |
398 | + EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).Times(1); |
399 | |
400 | auto params = msh::a_surface(); |
401 | EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1) |
402 | .WillOnce(Return(msh::a_surface())); |
403 | |
404 | - factory.create_surface(params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>()); |
405 | + factory.create_surface(nullptr, params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>()); |
406 | } |
407 | |
408 | TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory) |
409 | @@ -85,7 +85,7 @@ |
410 | |
411 | EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1) |
412 | .WillOnce(Return(placed_params)); |
413 | - EXPECT_CALL(*underlying_surface_factory, create_surface(placed_params, mf::SurfaceId(), sink)); |
414 | + EXPECT_CALL(*underlying_surface_factory, create_surface(nullptr, placed_params, mf::SurfaceId(), sink)); |
415 | |
416 | - factory.create_surface(params, mf::SurfaceId(), sink); |
417 | + factory.create_surface(nullptr, params, mf::SurfaceId(), sink); |
418 | } |
419 | |
420 | === modified file 'tests/unit-tests/shell/test_session_manager.cpp' |
421 | --- tests/unit-tests/shell/test_session_manager.cpp 2013-08-08 02:34:31 +0000 |
422 | +++ tests/unit-tests/shell/test_session_manager.cpp 2013-08-09 03:39:57 +0000 |
423 | @@ -110,10 +110,11 @@ |
424 | { |
425 | using namespace ::testing; |
426 | |
427 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1); |
428 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1); |
429 | |
430 | - ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault( |
431 | + ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault( |
432 | Return(std::make_shared<msh::Surface>( |
433 | + nullptr, |
434 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
435 | msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>()))); |
436 | |
437 | @@ -147,8 +148,9 @@ |
438 | TEST_F(SessionManagerSetup, create_surface_for_session_forwards_and_then_focuses_session) |
439 | { |
440 | using namespace ::testing; |
441 | - ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault( |
442 | + ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault( |
443 | Return(std::make_shared<msh::Surface>( |
444 | + nullptr, |
445 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
446 | msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>()))); |
447 | |
448 | @@ -157,7 +159,7 @@ |
449 | InSequence seq; |
450 | |
451 | EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation |
452 | - EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1); |
453 | + EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1); |
454 | EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Post Surface creation |
455 | } |
456 | |
457 | |
458 | === modified file 'tests/unit-tests/shell/test_surface.cpp' |
459 | --- tests/unit-tests/shell/test_surface.cpp 2013-08-08 02:34:31 +0000 |
460 | +++ tests/unit-tests/shell/test_surface.cpp 2013-08-09 03:39:57 +0000 |
461 | @@ -61,7 +61,7 @@ |
462 | { |
463 | } |
464 | |
465 | - std::weak_ptr<ms::Surface> create_surface(msh::SurfaceCreationParameters const& ) |
466 | + std::weak_ptr<ms::Surface> create_surface(msh::Session*, msh::SurfaceCreationParameters const& ) |
467 | { |
468 | auto state = std::make_shared<mtd::MockSurfaceState>(); |
469 | dummy_surface = std::make_shared<ms::Surface>(state, stub_buffer_stream_, std::shared_ptr<mi::InputChannel>()); |
470 | @@ -94,14 +94,14 @@ |
471 | MockSurfaceBuilder() |
472 | { |
473 | using namespace testing; |
474 | - ON_CALL(*this, create_surface(_)). |
475 | + ON_CALL(*this, create_surface(_, _)). |
476 | WillByDefault(Invoke(&self, &StubSurfaceBuilder::create_surface)); |
477 | |
478 | ON_CALL(*this, destroy_surface(_)). |
479 | WillByDefault(Invoke(&self, &StubSurfaceBuilder::destroy_surface)); |
480 | } |
481 | |
482 | - MOCK_METHOD1(create_surface, std::weak_ptr<ms::Surface> (const msh::SurfaceCreationParameters&)); |
483 | + MOCK_METHOD2(create_surface, std::weak_ptr<ms::Surface> (msh::Session*, const msh::SurfaceCreationParameters&)); |
484 | |
485 | MOCK_METHOD1(destroy_surface, void (std::weak_ptr<ms::Surface> const&)); |
486 | |
487 | @@ -141,10 +141,11 @@ |
488 | MockSurfaceBuilder surface_builder; |
489 | |
490 | InSequence sequence; |
491 | - EXPECT_CALL(surface_builder, create_surface(params)).Times(1); |
492 | + EXPECT_CALL(surface_builder, create_surface(nullptr, params)).Times(1); |
493 | EXPECT_CALL(surface_builder, destroy_surface(_)).Times(1); |
494 | |
495 | msh::Surface test( |
496 | + nullptr, |
497 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
498 | params, stub_id, stub_sender); |
499 | } |
500 | @@ -157,12 +158,13 @@ |
501 | MockSurfaceBuilder surface_builder; |
502 | |
503 | InSequence sequence; |
504 | - EXPECT_CALL(surface_builder, create_surface(params)).Times(1) |
505 | + EXPECT_CALL(surface_builder, create_surface(nullptr, params)).Times(1) |
506 | .WillOnce(Throw(std::runtime_error(__PRETTY_FUNCTION__))); |
507 | EXPECT_CALL(surface_builder, destroy_surface(_)).Times(Exactly(0)); |
508 | |
509 | EXPECT_THROW({ |
510 | msh::Surface test( |
511 | + nullptr, |
512 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
513 | params, stub_id, stub_sender); |
514 | }, std::runtime_error); |
515 | @@ -174,10 +176,11 @@ |
516 | MockSurfaceBuilder surface_builder; |
517 | |
518 | InSequence sequence; |
519 | - EXPECT_CALL(surface_builder, create_surface(_)).Times(AnyNumber()); |
520 | + EXPECT_CALL(surface_builder, create_surface(nullptr, _)).Times(AnyNumber()); |
521 | EXPECT_CALL(surface_builder, destroy_surface(_)).Times(0); |
522 | |
523 | msh::Surface test( |
524 | + nullptr, |
525 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
526 | msh::a_surface(), stub_id, stub_sender); |
527 | |
528 | @@ -194,6 +197,7 @@ |
529 | TEST_F(ShellSurface, size_throw_behavior) |
530 | { |
531 | msh::Surface test( |
532 | + nullptr, |
533 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
534 | msh::a_surface(), stub_id, stub_sender); |
535 | |
536 | @@ -211,6 +215,7 @@ |
537 | TEST_F(ShellSurface, top_left_throw_behavior) |
538 | { |
539 | msh::Surface test( |
540 | + nullptr, |
541 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
542 | msh::a_surface(), stub_id, stub_sender); |
543 | |
544 | @@ -228,6 +233,7 @@ |
545 | TEST_F(ShellSurface, name_throw_behavior) |
546 | { |
547 | msh::Surface test( |
548 | + nullptr, |
549 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
550 | msh::a_surface(), stub_id, stub_sender); |
551 | |
552 | @@ -245,6 +251,7 @@ |
553 | TEST_F(ShellSurface, pixel_format_throw_behavior) |
554 | { |
555 | msh::Surface test( |
556 | + nullptr, |
557 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
558 | msh::a_surface(), stub_id, stub_sender); |
559 | |
560 | @@ -262,6 +269,7 @@ |
561 | TEST_F(ShellSurface, hide_throw_behavior) |
562 | { |
563 | msh::Surface test( |
564 | + nullptr, |
565 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
566 | msh::a_surface(), stub_id, stub_sender); |
567 | |
568 | @@ -279,6 +287,7 @@ |
569 | TEST_F(ShellSurface, show_throw_behavior) |
570 | { |
571 | msh::Surface test( |
572 | + nullptr, |
573 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
574 | msh::a_surface(), stub_id, stub_sender); |
575 | |
576 | @@ -296,6 +305,7 @@ |
577 | TEST_F(ShellSurface, destroy_throw_behavior) |
578 | { |
579 | msh::Surface test( |
580 | + nullptr, |
581 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
582 | msh::a_surface(), stub_id, stub_sender); |
583 | |
584 | @@ -313,6 +323,7 @@ |
585 | TEST_F(ShellSurface, force_request_to_complete_throw_behavior) |
586 | { |
587 | msh::Surface test( |
588 | + nullptr, |
589 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
590 | msh::a_surface(), stub_id, stub_sender); |
591 | |
592 | @@ -330,6 +341,7 @@ |
593 | TEST_F(ShellSurface, advance_client_buffer_throw_behavior) |
594 | { |
595 | msh::Surface test( |
596 | + nullptr, |
597 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
598 | msh::a_surface(), stub_id, stub_sender); |
599 | |
600 | @@ -347,6 +359,7 @@ |
601 | TEST_F(ShellSurface, input_fds_throw_behavior) |
602 | { |
603 | msh::Surface test( |
604 | + nullptr, |
605 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
606 | msh::a_surface(), stub_id, stub_sender); |
607 | |
608 | @@ -362,6 +375,7 @@ |
609 | using namespace testing; |
610 | |
611 | msh::Surface surf( |
612 | + nullptr, |
613 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
614 | msh::a_surface(), stub_id, stub_sender); |
615 | |
616 | @@ -375,6 +389,7 @@ |
617 | using namespace testing; |
618 | |
619 | msh::Surface surf( |
620 | + nullptr, |
621 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
622 | msh::a_surface(), stub_id, stub_sender); |
623 | |
624 | @@ -409,6 +424,7 @@ |
625 | using namespace testing; |
626 | |
627 | msh::Surface surf( |
628 | + nullptr, |
629 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
630 | msh::a_surface(), stub_id, stub_sender); |
631 | |
632 | @@ -449,6 +465,7 @@ |
633 | EXPECT_CALL(configurator, attribute_set(_, mir_surface_attrib_state, mir_surface_state_minimized)).Times(1); |
634 | |
635 | msh::Surface surf( |
636 | + nullptr, |
637 | mt::fake_shared(surface_builder), mt::fake_shared(configurator), |
638 | msh::a_surface(), stub_id, stub_sender); |
639 | |
640 | @@ -460,6 +477,7 @@ |
641 | using namespace ::testing; |
642 | |
643 | msh::Surface test( |
644 | + nullptr, |
645 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
646 | msh::a_surface(), stub_id, stub_sender); |
647 | |
648 | @@ -474,6 +492,7 @@ |
649 | using namespace ::testing; |
650 | |
651 | msh::Surface test( |
652 | + nullptr, |
653 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
654 | msh::a_surface(), stub_id, stub_sender); |
655 | surface_builder.reset_surface(); |
656 | @@ -490,6 +509,7 @@ |
657 | using namespace ::testing; |
658 | |
659 | msh::Surface test( |
660 | + nullptr, |
661 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
662 | msh::a_surface(), stub_id, stub_sender); |
663 | |
664 | @@ -507,6 +527,7 @@ |
665 | TEST_F(ShellSurface, with_most_recent_buffer_do_uses_compositor_buffer) |
666 | { |
667 | msh::Surface test( |
668 | + nullptr, |
669 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
670 | msh::a_surface(), stub_id, stub_sender); |
671 | |
672 | @@ -528,6 +549,7 @@ |
673 | |
674 | mtd::MockSurfaceController surface_controller; |
675 | msh::Surface test( |
676 | + nullptr, |
677 | mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(), |
678 | msh::a_surface(), stub_id, stub_sender); |
679 | |
680 | |
681 | === modified file 'tests/unit-tests/surfaces/test_surface_controller.cpp' |
682 | --- tests/unit-tests/surfaces/test_surface_controller.cpp 2013-07-19 02:44:21 +0000 |
683 | +++ tests/unit-tests/surfaces/test_surface_controller.cpp 2013-08-09 03:39:57 +0000 |
684 | @@ -52,7 +52,7 @@ |
685 | EXPECT_CALL(model, create_surface(_)).Times(1).WillOnce(Return(null_surface)); |
686 | EXPECT_CALL(model, destroy_surface(_)).Times(1); |
687 | |
688 | - auto surface = controller.create_surface(msh::a_surface()); |
689 | + auto surface = controller.create_surface(nullptr, msh::a_surface()); |
690 | controller.destroy_surface(surface); |
691 | } |
692 |
FAILED: Continuous integration, rev:833 jenkins. qa.ubuntu. com/job/ mir-ci/ 997/ jenkins. qa.ubuntu. com/job/ mir-android- saucy-i386- build/1295/ console jenkins. qa.ubuntu. com/job/ mir-clang- saucy-amd64- build/1180/ console jenkins. qa.ubuntu. com/job/ mir-saucy- amd64-ci/ 235/console jenkins. qa.ubuntu. com/job/ mir-vm- ci-build/ ./distribution= quantal, flavor= amd64/633
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins: 8080/job/ mir-ci/ 997/rebuild
http://