Mir

Merge lp:~mterry/mir/session-for-surface into lp:~mir-team/mir/trunk

  1. session-for-surface
  2. Merge into trunk
Proposed by Michael Terry on 2013-07-12
Status: Merged
Approved by: Alan Griffiths on 2013-08-09
Approved revision: 839
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
Reviewer Review Type Date Requested Status
Alan Griffiths 2013-07-12 Abstain on 2013-08-09
PS Jenkins bot (community) continuous-integration Approve on 2013-08-09
Kevin DuBois (community) Abstain on 2013-08-01
Robert Carr (community) Approve on 2013-07-30
Robert Ancell Approve on 2013-07-29
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_shared_from_this means that all ApplicationSession objects must be created inside of a shared_ptr... I didn't know how else to have it pass on its "this" pointer as a shared_ptr.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
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.

review: Needs Fixing
lp:~mterry/mir/session-for-surface updated on 2013-07-12
834. By Michael Terry on 2013-07-12

Use raw pointers instead of fancy-pants shared_ptrs

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 :
review: Needs Fixing (continuous-integration)
lp:~mterry/mir/session-for-surface updated on 2013-07-12
835. By Michael Terry on 2013-07-12

Merge from trunk

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
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)

review: Needs Fixing
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.

review: Abstain
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?

lp:~mterry/mir/session-for-surface updated on 2013-07-18
836. By Michael Terry on 2013-07-18

Merge from trunk

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~mterry/mir/session-for-surface updated on 2013-07-18
837. By Michael Terry on 2013-07-18

Fix compile errors

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 :
review: Approve (continuous-integration)
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_ptr<msh::Session> session = some_session_from_somewhere();
std::shared_ptr<msh::Surface> surface = session->default_surface();
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->session()->name()); SEGFAULT!

}

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_ptr<surfaces::Surface> create_surface(Session* session, SurfaceCreationParameters const& params) = 0;

Although, it isn't used in SurfaceController::create_surface() - which I would imagine is the intended consumer.

review: Needs Information
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_builder(), note which surfaces are associated with the greeter, and create those surfaces with a different depth than other surfaces (so they float above in a different layer than all other sessions).
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::destroy() shell::Surface would have no such dependency and could simply be passed the constructed surfaces::Surface from a context.

PS It might also be thought that SurfaceBuilder::destroy() is the underlying reason we have both surfaces::Surface and shell::Surface implementations - which has been the subject of several cleanup attempts.

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://code.launchpad.net/~robertcarr/mir/session-transactions/+merge/175669)

lp:~mterry/mir/session-for-surface updated on 2013-07-29
838. By Michael Terry on 2013-07-29

Merge from trunk

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 :
review: Approve (continuous-integration)
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

review: Approve
Robert Carr (robertcarr) wrote :

What's wrong with SessionListener::surface_created(Session& session, std::shared_ptr<Surface> const& surface) as landed in trunk last week?

review: Needs Information
Michael Terry (mterry) wrote :

> What's wrong with SessionListener::surface_created(Session& session, std::shared_ptr<Surface> const& surface) as landed in trunk last week?

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

review: Approve
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.

review: Abstain
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.

review: Abstain
Alan Griffiths (alan-griffiths) wrote :

We've taken so long to reach "approval" that there are now merge conflicts to address.

review: Needs Fixing
lp:~mterry/mir/session-for-surface updated on 2013-08-09
839. By Michael Terry on 2013-08-09

Merge from trunk

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

for_each_display_buffer

review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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

Subscribers

People subscribed via source and target branches