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
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
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_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.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:833
http://jenkins.qa.ubuntu.com/job/mir-ci/997/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1180/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/235/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/633

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

review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
Michael Terry (mterry) wrote :

Ah thanks, Alan. That makes sense. I've updated the branch to use raw pointers.

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

FAILED: Continuous integration, rev:834
http://jenkins.qa.ubuntu.com/job/mir-ci/998/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1297/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/236/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/634

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

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

FAILED: Continuous integration, rev:835
http://jenkins.qa.ubuntu.com/job/mir-ci/999/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1298/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1183/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/237
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/237/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/635

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

review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
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
Revision history for this message
Michael Terry (mterry) wrote :

Did you Mir folk discuss how you wanted to proceed on this?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Did you Mir folk discuss how you wanted to proceed on this?

/1/ fix the CI failures?

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

FAILED: Continuous integration, rev:836
http://jenkins.qa.ubuntu.com/job/mir-ci/1053/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1370/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1255/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/291
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/291/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/677

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

review: Needs Fixing (continuous-integration)
Revision history for this message
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.

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

PASSED: Continuous integration, rev:837
http://jenkins.qa.ubuntu.com/job/mir-ci/1055/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1372
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1257
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/293
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/293/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/679

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

review: Approve (continuous-integration)
Revision history for this message
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!

}

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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?

Revision history for this message
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)

Revision history for this message
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.

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

PASSED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/mir-ci/1118/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1464
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1349
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/356
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/356/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
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
Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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
Revision history for this message
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
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

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

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

PASSED: Continuous integration, rev:839
http://jenkins.qa.ubuntu.com/job/mir-ci/1265/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1676
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1561
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/503
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/503/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
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
=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp 2013-08-02 03:01:12 +0000
+++ examples/render_surfaces.cpp 2013-08-09 03:39:57 +0000
@@ -407,6 +407,7 @@
407 for (auto& m : moveables)407 for (auto& m : moveables)
408 {408 {
409 std::shared_ptr<ms::Surface> s = surface_builder->create_surface(409 std::shared_ptr<ms::Surface> s = surface_builder->create_surface(
410 nullptr,
410 msh::a_surface().of_size(surface_size)411 msh::a_surface().of_size(surface_size)
411 .of_pixel_format(surface_pf)412 .of_pixel_format(surface_pf)
412 .of_buffer_usage(mg::BufferUsage::hardware)413 .of_buffer_usage(mg::BufferUsage::hardware)
413414
=== modified file 'include/server/mir/shell/organising_surface_factory.h'
--- include/server/mir/shell/organising_surface_factory.h 2013-08-01 07:44:17 +0000
+++ include/server/mir/shell/organising_surface_factory.h 2013-08-09 03:39:57 +0000
@@ -28,6 +28,7 @@
28namespace shell28namespace shell
29{29{
30class PlacementStrategy;30class PlacementStrategy;
31class Session;
3132
32class OrganisingSurfaceFactory : public SurfaceFactory33class OrganisingSurfaceFactory : public SurfaceFactory
33{34{
@@ -37,6 +38,7 @@
37 virtual ~OrganisingSurfaceFactory();38 virtual ~OrganisingSurfaceFactory();
3839
39 std::shared_ptr<Surface> create_surface(40 std::shared_ptr<Surface> create_surface(
41 Session* session,
40 shell::SurfaceCreationParameters const& params,42 shell::SurfaceCreationParameters const& params,
41 frontend::SurfaceId id,43 frontend::SurfaceId id,
42 std::shared_ptr<frontend::EventSink> const& sink) override;44 std::shared_ptr<frontend::EventSink> const& sink) override;
4345
=== modified file 'include/server/mir/shell/surface.h'
--- include/server/mir/shell/surface.h 2013-08-08 02:34:31 +0000
+++ include/server/mir/shell/surface.h 2013-08-09 03:39:57 +0000
@@ -38,6 +38,7 @@
38namespace shell38namespace shell
39{39{
40class InputTargeter;40class InputTargeter;
41class Session;
41class SurfaceBuilder;42class SurfaceBuilder;
42class SurfaceConfigurator;43class SurfaceConfigurator;
43class SurfaceController;44class SurfaceController;
@@ -47,6 +48,7 @@
47{48{
48public:49public:
49 Surface(50 Surface(
51 Session* session,
50 std::shared_ptr<SurfaceBuilder> const& builder,52 std::shared_ptr<SurfaceBuilder> const& builder,
51 std::shared_ptr<SurfaceConfigurator> const& configurator,53 std::shared_ptr<SurfaceConfigurator> const& configurator,
52 SurfaceCreationParameters const& params,54 SurfaceCreationParameters const& params,
5355
=== modified file 'include/server/mir/shell/surface_builder.h'
--- include/server/mir/shell/surface_builder.h 2013-05-21 17:16:43 +0000
+++ include/server/mir/shell/surface_builder.h 2013-08-09 03:39:57 +0000
@@ -28,12 +28,13 @@
2828
29namespace shell29namespace shell
30{30{
31class Session;
31struct SurfaceCreationParameters;32struct SurfaceCreationParameters;
3233
33class SurfaceBuilder34class SurfaceBuilder
34{35{
35public:36public:
36 virtual std::weak_ptr<surfaces::Surface> create_surface(SurfaceCreationParameters const& params) = 0;37 virtual std::weak_ptr<surfaces::Surface> create_surface(Session* session, SurfaceCreationParameters const& params) = 0;
3738
38 virtual void destroy_surface(std::weak_ptr<surfaces::Surface> const& surface) = 0;39 virtual void destroy_surface(std::weak_ptr<surfaces::Surface> const& surface) = 0;
39protected:40protected:
4041
=== modified file 'include/server/mir/shell/surface_factory.h'
--- include/server/mir/shell/surface_factory.h 2013-08-01 07:44:17 +0000
+++ include/server/mir/shell/surface_factory.h 2013-08-09 03:39:57 +0000
@@ -30,6 +30,7 @@
30}30}
31namespace shell31namespace shell
32{32{
33class Session;
33class Surface;34class Surface;
34struct SurfaceCreationParameters;35struct SurfaceCreationParameters;
3536
@@ -37,6 +38,7 @@
37{38{
38public:39public:
39 virtual std::shared_ptr<Surface> create_surface(40 virtual std::shared_ptr<Surface> create_surface(
41 Session* session,
40 SurfaceCreationParameters const& params,42 SurfaceCreationParameters const& params,
41 frontend::SurfaceId id,43 frontend::SurfaceId id,
42 std::shared_ptr<frontend::EventSink> const& sink) = 0;44 std::shared_ptr<frontend::EventSink> const& sink) = 0;
4345
=== modified file 'include/server/mir/shell/surface_source.h'
--- include/server/mir/shell/surface_source.h 2013-08-08 02:34:31 +0000
+++ include/server/mir/shell/surface_source.h 2013-08-09 03:39:57 +0000
@@ -28,6 +28,7 @@
2828
29namespace shell29namespace shell
30{30{
31class Session;
31class SurfaceBuilder;32class SurfaceBuilder;
32class SurfaceConfigurator;33class SurfaceConfigurator;
3334
@@ -39,6 +40,7 @@
39 virtual ~SurfaceSource() {}40 virtual ~SurfaceSource() {}
4041
41 std::shared_ptr<Surface> create_surface(42 std::shared_ptr<Surface> create_surface(
43 Session* session,
42 shell::SurfaceCreationParameters const& params,44 shell::SurfaceCreationParameters const& params,
43 frontend::SurfaceId id,45 frontend::SurfaceId id,
44 std::shared_ptr<frontend::EventSink> const& sink);46 std::shared_ptr<frontend::EventSink> const& sink);
4547
=== modified file 'include/server/mir/surfaces/surface_controller.h'
--- include/server/mir/surfaces/surface_controller.h 2013-08-02 04:07:15 +0000
+++ include/server/mir/surfaces/surface_controller.h 2013-08-09 03:39:57 +0000
@@ -25,6 +25,11 @@
2525
26namespace mir26namespace mir
27{27{
28namespace shell
29{
30class Session;
31}
32
28namespace surfaces33namespace surfaces
29{34{
30class SurfaceStackModel;35class SurfaceStackModel;
@@ -35,7 +40,7 @@
35public:40public:
36 explicit SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack);41 explicit SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack);
3742
38 virtual std::weak_ptr<Surface> create_surface(shell::SurfaceCreationParameters const& params);43 virtual std::weak_ptr<Surface> create_surface(shell::Session* session, shell::SurfaceCreationParameters const& params);
39 virtual void destroy_surface(std::weak_ptr<Surface> const& surface);44 virtual void destroy_surface(std::weak_ptr<Surface> const& surface);
4045
41 virtual void raise(std::weak_ptr<Surface> const& surface);46 virtual void raise(std::weak_ptr<Surface> const& surface);
4247
=== modified file 'include/test/mir_test_doubles/mock_surface.h'
--- include/test/mir_test_doubles/mock_surface.h 2013-08-08 02:34:31 +0000
+++ include/test/mir_test_doubles/mock_surface.h 2013-08-09 03:39:57 +0000
@@ -38,8 +38,8 @@
3838
39struct MockSurface : public shell::Surface39struct MockSurface : public shell::Surface
40{40{
41 MockSurface(std::shared_ptr<shell::SurfaceBuilder> const& builder) :41 MockSurface(shell::Session* session, std::shared_ptr<shell::SurfaceBuilder> const& builder) :
42 shell::Surface(builder, std::make_shared<NullSurfaceConfigurator>(), shell::a_surface(), 42 shell::Surface(session, builder, std::make_shared<NullSurfaceConfigurator>(), shell::a_surface(),
43 frontend::SurfaceId{}, std::make_shared<NullEventSink>())43 frontend::SurfaceId{}, std::make_shared<NullEventSink>())
44 {44 {
45 }45 }
4646
=== modified file 'include/test/mir_test_doubles/mock_surface_factory.h'
--- include/test/mir_test_doubles/mock_surface_factory.h 2013-08-01 07:44:17 +0000
+++ include/test/mir_test_doubles/mock_surface_factory.h 2013-08-09 03:39:57 +0000
@@ -33,7 +33,8 @@
3333
34struct MockSurfaceFactory : public shell::SurfaceFactory34struct MockSurfaceFactory : public shell::SurfaceFactory
35{35{
36 MOCK_METHOD3(create_surface, std::shared_ptr<shell::Surface>(36 MOCK_METHOD4(create_surface, std::shared_ptr<shell::Surface>(
37 shell::Session*,
37 const shell::SurfaceCreationParameters&,38 const shell::SurfaceCreationParameters&,
38 frontend::SurfaceId,39 frontend::SurfaceId,
39 std::shared_ptr<frontend::EventSink> const&));40 std::shared_ptr<frontend::EventSink> const&));
4041
=== modified file 'include/test/mir_test_doubles/stub_surface_builder.h'
--- include/test/mir_test_doubles/stub_surface_builder.h 2013-07-12 20:33:47 +0000
+++ include/test/mir_test_doubles/stub_surface_builder.h 2013-08-09 03:39:57 +0000
@@ -43,7 +43,7 @@
43 {43 {
44 }44 }
4545
46 std::weak_ptr<surfaces::Surface> create_surface(shell::SurfaceCreationParameters const&)46 std::weak_ptr<surfaces::Surface> create_surface(shell::Session*, shell::SurfaceCreationParameters const&)
47 {47 {
48 auto state = std::make_shared<MockSurfaceState>();48 auto state = std::make_shared<MockSurfaceState>();
49 dummy_surface = std::make_shared<surfaces::Surface>(49 dummy_surface = std::make_shared<surfaces::Surface>(
5050
=== modified file 'src/server/shell/application_session.cpp'
--- src/server/shell/application_session.cpp 2013-08-08 02:34:31 +0000
+++ src/server/shell/application_session.cpp 2013-08-09 03:39:57 +0000
@@ -65,7 +65,7 @@
65mf::SurfaceId msh::ApplicationSession::create_surface(const msh::SurfaceCreationParameters& params)65mf::SurfaceId msh::ApplicationSession::create_surface(const msh::SurfaceCreationParameters& params)
66{66{
67 auto const id = next_id();67 auto const id = next_id();
68 auto surf = surface_factory->create_surface(params, id, event_sink);68 auto surf = surface_factory->create_surface(this, params, id, event_sink);
6969
70 std::unique_lock<std::mutex> lock(surfaces_mutex);70 std::unique_lock<std::mutex> lock(surfaces_mutex);
71 surfaces[id] = surf;71 surfaces[id] = surf;
7272
=== modified file 'src/server/shell/organising_surface_factory.cpp'
--- src/server/shell/organising_surface_factory.cpp 2013-08-01 07:44:17 +0000
+++ src/server/shell/organising_surface_factory.cpp 2013-08-09 03:39:57 +0000
@@ -35,12 +35,13 @@
35}35}
3636
37std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface(37std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface(
38 Session* session,
38 shell::SurfaceCreationParameters const& params,39 shell::SurfaceCreationParameters const& params,
39 frontend::SurfaceId id,40 frontend::SurfaceId id,
40 std::shared_ptr<mf::EventSink> const& sender)41 std::shared_ptr<mf::EventSink> const& sender)
41{42{
42 auto placed_params = placement_strategy->place(params);43 auto placed_params = placement_strategy->place(params);
4344
44 return underlying_factory->create_surface(placed_params, id, sender);45 return underlying_factory->create_surface(session, placed_params, id, sender);
45}46}
4647
4748
=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp 2013-08-08 02:34:31 +0000
+++ src/server/shell/surface.cpp 2013-08-09 03:39:57 +0000
@@ -40,6 +40,7 @@
40namespace mf = mir::frontend;40namespace mf = mir::frontend;
4141
42msh::Surface::Surface(42msh::Surface::Surface(
43 Session* session,
43 std::shared_ptr<SurfaceBuilder> const& builder,44 std::shared_ptr<SurfaceBuilder> const& builder,
44 std::shared_ptr<SurfaceConfigurator> const& configurator,45 std::shared_ptr<SurfaceConfigurator> const& configurator,
45 shell::SurfaceCreationParameters const& params,46 shell::SurfaceCreationParameters const& params,
@@ -47,7 +48,7 @@
47 std::shared_ptr<mf::EventSink> const& event_sink)48 std::shared_ptr<mf::EventSink> const& event_sink)
48 : builder(builder),49 : builder(builder),
49 configurator(configurator),50 configurator(configurator),
50 surface(builder->create_surface(params)),51 surface(builder->create_surface(session, params)),
51 id(id),52 id(id),
52 event_sink(event_sink),53 event_sink(event_sink),
53 type_value(mir_surface_type_normal),54 type_value(mir_surface_type_normal),
5455
=== modified file 'src/server/shell/surface_source.cpp'
--- src/server/shell/surface_source.cpp 2013-08-08 02:34:31 +0000
+++ src/server/shell/surface_source.cpp 2013-08-09 03:39:57 +0000
@@ -40,10 +40,11 @@
40}40}
4141
42std::shared_ptr<msh::Surface> msh::SurfaceSource::create_surface(42std::shared_ptr<msh::Surface> msh::SurfaceSource::create_surface(
43 msh::Session* session,
43 shell::SurfaceCreationParameters const& params,44 shell::SurfaceCreationParameters const& params,
44 frontend::SurfaceId id,45 frontend::SurfaceId id,
45 std::shared_ptr<mf::EventSink> const& sender)46 std::shared_ptr<mf::EventSink> const& sender)
46{47{
47 return std::make_shared<Surface>(surface_builder, surface_configurator, params, id, sender);48 return std::make_shared<Surface>(session, surface_builder, surface_configurator, params, id, sender);
48}49}
4950
5051
=== modified file 'src/server/surfaces/surface_controller.cpp'
--- src/server/surfaces/surface_controller.cpp 2013-07-19 02:44:21 +0000
+++ src/server/surfaces/surface_controller.cpp 2013-08-09 03:39:57 +0000
@@ -20,13 +20,14 @@
20#include "mir/surfaces/surface_stack_model.h"20#include "mir/surfaces/surface_stack_model.h"
2121
22namespace ms = mir::surfaces;22namespace ms = mir::surfaces;
23namespace msh = mir::shell;
2324
24ms::SurfaceController::SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack) :25ms::SurfaceController::SurfaceController(std::shared_ptr<SurfaceStackModel> const& surface_stack) :
25 surface_stack(surface_stack)26 surface_stack(surface_stack)
26{27{
27}28}
2829
29std::weak_ptr<ms::Surface> ms::SurfaceController::create_surface(shell::SurfaceCreationParameters const& params)30std::weak_ptr<ms::Surface> ms::SurfaceController::create_surface(msh::Session*, shell::SurfaceCreationParameters const& params)
30{31{
31 return surface_stack->create_surface(params);32 return surface_stack->create_surface(params);
32}33}
3334
=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp 2013-08-02 09:05:39 +0000
+++ tests/acceptance-tests/test_client_input.cpp 2013-08-09 03:39:57 +0000
@@ -620,11 +620,12 @@
620 {620 {
621 }621 }
622 622
623 std::shared_ptr<msh::Surface> create_surface(msh::SurfaceCreationParameters const& params,623 std::shared_ptr<msh::Surface> create_surface(msh::Session* session,
624 msh::SurfaceCreationParameters const& params,
624 mf::SurfaceId id,625 mf::SurfaceId id,
625 std::shared_ptr<mf::EventSink> const& sink)626 std::shared_ptr<mf::EventSink> const& sink)
626 {627 {
627 auto surface = underlying_factory->create_surface(params, id, sink);628 auto surface = underlying_factory->create_surface(session, params, id, sink);
628629
629 surface->set_input_region(input_rectangles);630 surface->set_input_region(input_rectangles);
630631
631632
=== modified file 'tests/integration-tests/graphics/android/test_internal_client.cpp'
--- tests/integration-tests/graphics/android/test_internal_client.cpp 2013-08-08 02:34:31 +0000
+++ tests/integration-tests/graphics/android/test_internal_client.cpp 2013-08-09 03:39:57 +0000
@@ -110,7 +110,7 @@
110 auto surface_controller = std::make_shared<ms::SurfaceController>(ss);110 auto surface_controller = std::make_shared<ms::SurfaceController>(ss);
111 auto surface_source = std::make_shared<msh::SurfaceSource>(surface_controller, std::make_shared<mtd::NullSurfaceConfigurator>());111 auto surface_source = std::make_shared<msh::SurfaceSource>(surface_controller, std::make_shared<mtd::NullSurfaceConfigurator>());
112 auto mir_surface = std::make_shared<ForwardingInternalSurface>(112 auto mir_surface = std::make_shared<ForwardingInternalSurface>(
113 surface_source->create_surface(params, id, std::shared_ptr<mf::EventSink>()));113 surface_source->create_surface(nullptr, params, id, std::shared_ptr<mf::EventSink>()));
114114
115 auto options = std::shared_ptr<mo::ProgramOption>(); 115 auto options = std::shared_ptr<mo::ProgramOption>();
116 auto report = std::shared_ptr<mg::NullDisplayReport>(); 116 auto report = std::shared_ptr<mg::NullDisplayReport>();
117117
=== modified file 'tests/unit-tests/shell/test_application_session.cpp'
--- tests/unit-tests/shell/test_application_session.cpp 2013-08-08 02:34:31 +0000
+++ tests/unit-tests/shell/test_application_session.cpp 2013-08-09 03:39:57 +0000
@@ -49,7 +49,7 @@
49{49{
50 mtd::StubSurfaceBuilder surface_builder;50 mtd::StubSurfaceBuilder surface_builder;
5151
52 return std::make_shared<mtd::MockSurface>(std::make_shared<mtd::StubSurfaceBuilder>());52 return std::make_shared<mtd::MockSurface>(nullptr, std::make_shared<mtd::StubSurfaceBuilder>());
53}53}
54}54}
5555
@@ -61,9 +61,9 @@
6161
62 mtd::NullEventSink sender;62 mtd::NullEventSink sender;
63 mtd::MockSurfaceFactory surface_factory;63 mtd::MockSurfaceFactory surface_factory;
64 ON_CALL(surface_factory, create_surface(_,_,_)).WillByDefault(Return(mock_surface));64 ON_CALL(surface_factory, create_surface(_,_,_,_)).WillByDefault(Return(mock_surface));
6565
66 EXPECT_CALL(surface_factory, create_surface(_, _, _));66 EXPECT_CALL(surface_factory, create_surface(_, _, _, _));
67 EXPECT_CALL(*mock_surface, destroy());67 EXPECT_CALL(*mock_surface, destroy());
68 68
69 mtd::MockSessionListener listener;69 mtd::MockSessionListener listener;
@@ -90,11 +90,11 @@
9090
91 {91 {
92 InSequence seq;92 InSequence seq;
93 EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)93 EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
94 .WillOnce(Return(make_mock_surface()));94 .WillOnce(Return(make_mock_surface()));
95 EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)95 EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
96 .WillOnce(Return(make_mock_surface()));96 .WillOnce(Return(make_mock_surface()));
97 EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)97 EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
98 .WillOnce(Return(make_mock_surface()));98 .WillOnce(Return(make_mock_surface()));
99 }99 }
100100
@@ -129,13 +129,13 @@
129 auto mock_surface = make_mock_surface();129 auto mock_surface = make_mock_surface();
130130
131 mtd::MockSurfaceFactory surface_factory;131 mtd::MockSurfaceFactory surface_factory;
132 ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(Return(mock_surface));132 ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(mock_surface));
133133
134 msh::ApplicationSession app_session(mt::fake_shared(surface_factory), "Foo",134 msh::ApplicationSession app_session(mt::fake_shared(surface_factory), "Foo",
135 std::make_shared<mtd::NullSnapshotStrategy>(),135 std::make_shared<mtd::NullSnapshotStrategy>(),
136 std::make_shared<msh::NullSessionListener>(), mt::fake_shared(sender));136 std::make_shared<msh::NullSessionListener>(), mt::fake_shared(sender));
137137
138 EXPECT_CALL(surface_factory, create_surface(_, _, _));138 EXPECT_CALL(surface_factory, create_surface(_, _, _, _));
139139
140 {140 {
141 InSequence seq;141 InSequence seq;
142142
=== modified file 'tests/unit-tests/shell/test_default_focus_mechanism.cpp'
--- tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-08-02 04:07:15 +0000
+++ tests/unit-tests/shell/test_default_focus_mechanism.cpp 2013-08-09 03:39:57 +0000
@@ -68,7 +68,7 @@
68 using namespace ::testing;68 using namespace ::testing;
69 69
70 NiceMock<MockShellSession> app1;70 NiceMock<MockShellSession> app1;
71 mtd::MockSurface mock_surface(std::make_shared<mtd::StubSurfaceBuilder>());71 mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
72 {72 {
73 InSequence seq;73 InSequence seq;
74 EXPECT_CALL(app1, default_surface()).Times(1)74 EXPECT_CALL(app1, default_surface()).Times(1)
@@ -88,7 +88,7 @@
88 using namespace ::testing;88 using namespace ::testing;
89 89
90 NiceMock<MockShellSession> app1;90 NiceMock<MockShellSession> app1;
91 mtd::MockSurface mock_surface(std::make_shared<mtd::StubSurfaceBuilder>());91 mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
92 {92 {
93 InSequence seq;93 InSequence seq;
94 EXPECT_CALL(app1, default_surface()).Times(1)94 EXPECT_CALL(app1, default_surface()).Times(1)
9595
=== modified file 'tests/unit-tests/shell/test_organising_surface_factory.cpp'
--- tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-01 07:44:17 +0000
+++ tests/unit-tests/shell/test_organising_surface_factory.cpp 2013-08-09 03:39:57 +0000
@@ -46,7 +46,7 @@
46 using namespace ::testing;46 using namespace ::testing;
4747
48 underlying_surface_factory = std::make_shared<mtd::MockSurfaceFactory>();48 underlying_surface_factory = std::make_shared<mtd::MockSurfaceFactory>();
49 ON_CALL(*underlying_surface_factory, create_surface(_, _, _)).WillByDefault(Return(null_surface));49 ON_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(null_surface));
5050
51 placement_strategy = std::make_shared<MockPlacementStrategy>();51 placement_strategy = std::make_shared<MockPlacementStrategy>();
52 }52 }
@@ -63,13 +63,13 @@
6363
64 msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy);64 msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy);
6565
66 EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _)).Times(1);66 EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).Times(1);
6767
68 auto params = msh::a_surface();68 auto params = msh::a_surface();
69 EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)69 EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
70 .WillOnce(Return(msh::a_surface()));70 .WillOnce(Return(msh::a_surface()));
7171
72 factory.create_surface(params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());72 factory.create_surface(nullptr, params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());
73}73}
7474
75TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory)75TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory)
@@ -85,7 +85,7 @@
8585
86 EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)86 EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
87 .WillOnce(Return(placed_params));87 .WillOnce(Return(placed_params));
88 EXPECT_CALL(*underlying_surface_factory, create_surface(placed_params, mf::SurfaceId(), sink));88 EXPECT_CALL(*underlying_surface_factory, create_surface(nullptr, placed_params, mf::SurfaceId(), sink));
8989
90 factory.create_surface(params, mf::SurfaceId(), sink);90 factory.create_surface(nullptr, params, mf::SurfaceId(), sink);
91}91}
9292
=== modified file 'tests/unit-tests/shell/test_session_manager.cpp'
--- tests/unit-tests/shell/test_session_manager.cpp 2013-08-08 02:34:31 +0000
+++ tests/unit-tests/shell/test_session_manager.cpp 2013-08-09 03:39:57 +0000
@@ -110,10 +110,11 @@
110{110{
111 using namespace ::testing;111 using namespace ::testing;
112112
113 EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);113 EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1);
114114
115 ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(115 ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
116 Return(std::make_shared<msh::Surface>(116 Return(std::make_shared<msh::Surface>(
117 nullptr,
117 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),118 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
118 msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));119 msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
119120
@@ -147,8 +148,9 @@
147TEST_F(SessionManagerSetup, create_surface_for_session_forwards_and_then_focuses_session)148TEST_F(SessionManagerSetup, create_surface_for_session_forwards_and_then_focuses_session)
148{149{
149 using namespace ::testing;150 using namespace ::testing;
150 ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(151 ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
151 Return(std::make_shared<msh::Surface>(152 Return(std::make_shared<msh::Surface>(
153 nullptr,
152 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),154 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
153 msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));155 msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
154156
@@ -157,7 +159,7 @@
157 InSequence seq;159 InSequence seq;
158160
159 EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation161 EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation
160 EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);162 EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1);
161 EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Post Surface creation163 EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Post Surface creation
162 }164 }
163165
164166
=== modified file 'tests/unit-tests/shell/test_surface.cpp'
--- tests/unit-tests/shell/test_surface.cpp 2013-08-08 02:34:31 +0000
+++ tests/unit-tests/shell/test_surface.cpp 2013-08-09 03:39:57 +0000
@@ -61,7 +61,7 @@
61 {61 {
62 }62 }
6363
64 std::weak_ptr<ms::Surface> create_surface(msh::SurfaceCreationParameters const& )64 std::weak_ptr<ms::Surface> create_surface(msh::Session*, msh::SurfaceCreationParameters const& )
65 {65 {
66 auto state = std::make_shared<mtd::MockSurfaceState>();66 auto state = std::make_shared<mtd::MockSurfaceState>();
67 dummy_surface = std::make_shared<ms::Surface>(state, stub_buffer_stream_, std::shared_ptr<mi::InputChannel>());67 dummy_surface = std::make_shared<ms::Surface>(state, stub_buffer_stream_, std::shared_ptr<mi::InputChannel>());
@@ -94,14 +94,14 @@
94 MockSurfaceBuilder()94 MockSurfaceBuilder()
95 {95 {
96 using namespace testing;96 using namespace testing;
97 ON_CALL(*this, create_surface(_)).97 ON_CALL(*this, create_surface(_, _)).
98 WillByDefault(Invoke(&self, &StubSurfaceBuilder::create_surface));98 WillByDefault(Invoke(&self, &StubSurfaceBuilder::create_surface));
9999
100 ON_CALL(*this, destroy_surface(_)).100 ON_CALL(*this, destroy_surface(_)).
101 WillByDefault(Invoke(&self, &StubSurfaceBuilder::destroy_surface));101 WillByDefault(Invoke(&self, &StubSurfaceBuilder::destroy_surface));
102 }102 }
103103
104 MOCK_METHOD1(create_surface, std::weak_ptr<ms::Surface> (const msh::SurfaceCreationParameters&));104 MOCK_METHOD2(create_surface, std::weak_ptr<ms::Surface> (msh::Session*, const msh::SurfaceCreationParameters&));
105105
106 MOCK_METHOD1(destroy_surface, void (std::weak_ptr<ms::Surface> const&));106 MOCK_METHOD1(destroy_surface, void (std::weak_ptr<ms::Surface> const&));
107107
@@ -141,10 +141,11 @@
141 MockSurfaceBuilder surface_builder;141 MockSurfaceBuilder surface_builder;
142142
143 InSequence sequence;143 InSequence sequence;
144 EXPECT_CALL(surface_builder, create_surface(params)).Times(1);144 EXPECT_CALL(surface_builder, create_surface(nullptr, params)).Times(1);
145 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(1);145 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(1);
146146
147 msh::Surface test(147 msh::Surface test(
148 nullptr,
148 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),149 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
149 params, stub_id, stub_sender);150 params, stub_id, stub_sender);
150}151}
@@ -157,12 +158,13 @@
157 MockSurfaceBuilder surface_builder;158 MockSurfaceBuilder surface_builder;
158159
159 InSequence sequence;160 InSequence sequence;
160 EXPECT_CALL(surface_builder, create_surface(params)).Times(1)161 EXPECT_CALL(surface_builder, create_surface(nullptr, params)).Times(1)
161 .WillOnce(Throw(std::runtime_error(__PRETTY_FUNCTION__)));162 .WillOnce(Throw(std::runtime_error(__PRETTY_FUNCTION__)));
162 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(Exactly(0));163 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(Exactly(0));
163164
164 EXPECT_THROW({165 EXPECT_THROW({
165 msh::Surface test(166 msh::Surface test(
167 nullptr,
166 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),168 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
167 params, stub_id, stub_sender);169 params, stub_id, stub_sender);
168 }, std::runtime_error);170 }, std::runtime_error);
@@ -174,10 +176,11 @@
174 MockSurfaceBuilder surface_builder;176 MockSurfaceBuilder surface_builder;
175177
176 InSequence sequence;178 InSequence sequence;
177 EXPECT_CALL(surface_builder, create_surface(_)).Times(AnyNumber());179 EXPECT_CALL(surface_builder, create_surface(nullptr, _)).Times(AnyNumber());
178 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(0);180 EXPECT_CALL(surface_builder, destroy_surface(_)).Times(0);
179181
180 msh::Surface test(182 msh::Surface test(
183 nullptr,
181 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),184 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
182 msh::a_surface(), stub_id, stub_sender);185 msh::a_surface(), stub_id, stub_sender);
183186
@@ -194,6 +197,7 @@
194TEST_F(ShellSurface, size_throw_behavior)197TEST_F(ShellSurface, size_throw_behavior)
195{198{
196 msh::Surface test(199 msh::Surface test(
200 nullptr,
197 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),201 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
198 msh::a_surface(), stub_id, stub_sender);202 msh::a_surface(), stub_id, stub_sender);
199203
@@ -211,6 +215,7 @@
211TEST_F(ShellSurface, top_left_throw_behavior)215TEST_F(ShellSurface, top_left_throw_behavior)
212{216{
213 msh::Surface test(217 msh::Surface test(
218 nullptr,
214 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),219 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
215 msh::a_surface(), stub_id, stub_sender);220 msh::a_surface(), stub_id, stub_sender);
216221
@@ -228,6 +233,7 @@
228TEST_F(ShellSurface, name_throw_behavior)233TEST_F(ShellSurface, name_throw_behavior)
229{234{
230 msh::Surface test(235 msh::Surface test(
236 nullptr,
231 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),237 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
232 msh::a_surface(), stub_id, stub_sender);238 msh::a_surface(), stub_id, stub_sender);
233239
@@ -245,6 +251,7 @@
245TEST_F(ShellSurface, pixel_format_throw_behavior)251TEST_F(ShellSurface, pixel_format_throw_behavior)
246{252{
247 msh::Surface test(253 msh::Surface test(
254 nullptr,
248 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),255 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
249 msh::a_surface(), stub_id, stub_sender);256 msh::a_surface(), stub_id, stub_sender);
250257
@@ -262,6 +269,7 @@
262TEST_F(ShellSurface, hide_throw_behavior)269TEST_F(ShellSurface, hide_throw_behavior)
263{270{
264 msh::Surface test(271 msh::Surface test(
272 nullptr,
265 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),273 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
266 msh::a_surface(), stub_id, stub_sender);274 msh::a_surface(), stub_id, stub_sender);
267275
@@ -279,6 +287,7 @@
279TEST_F(ShellSurface, show_throw_behavior)287TEST_F(ShellSurface, show_throw_behavior)
280{288{
281 msh::Surface test(289 msh::Surface test(
290 nullptr,
282 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),291 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
283 msh::a_surface(), stub_id, stub_sender);292 msh::a_surface(), stub_id, stub_sender);
284293
@@ -296,6 +305,7 @@
296TEST_F(ShellSurface, destroy_throw_behavior)305TEST_F(ShellSurface, destroy_throw_behavior)
297{306{
298 msh::Surface test(307 msh::Surface test(
308 nullptr,
299 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),309 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
300 msh::a_surface(), stub_id, stub_sender);310 msh::a_surface(), stub_id, stub_sender);
301311
@@ -313,6 +323,7 @@
313TEST_F(ShellSurface, force_request_to_complete_throw_behavior)323TEST_F(ShellSurface, force_request_to_complete_throw_behavior)
314{324{
315 msh::Surface test(325 msh::Surface test(
326 nullptr,
316 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),327 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
317 msh::a_surface(), stub_id, stub_sender);328 msh::a_surface(), stub_id, stub_sender);
318329
@@ -330,6 +341,7 @@
330TEST_F(ShellSurface, advance_client_buffer_throw_behavior)341TEST_F(ShellSurface, advance_client_buffer_throw_behavior)
331{342{
332 msh::Surface test(343 msh::Surface test(
344 nullptr,
333 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),345 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
334 msh::a_surface(), stub_id, stub_sender);346 msh::a_surface(), stub_id, stub_sender);
335347
@@ -347,6 +359,7 @@
347TEST_F(ShellSurface, input_fds_throw_behavior)359TEST_F(ShellSurface, input_fds_throw_behavior)
348{360{
349 msh::Surface test(361 msh::Surface test(
362 nullptr,
350 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),363 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
351 msh::a_surface(), stub_id, stub_sender);364 msh::a_surface(), stub_id, stub_sender);
352365
@@ -362,6 +375,7 @@
362 using namespace testing;375 using namespace testing;
363376
364 msh::Surface surf(377 msh::Surface surf(
378 nullptr,
365 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),379 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
366 msh::a_surface(), stub_id, stub_sender);380 msh::a_surface(), stub_id, stub_sender);
367381
@@ -375,6 +389,7 @@
375 using namespace testing;389 using namespace testing;
376390
377 msh::Surface surf(391 msh::Surface surf(
392 nullptr,
378 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),393 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
379 msh::a_surface(), stub_id, stub_sender);394 msh::a_surface(), stub_id, stub_sender);
380395
@@ -409,6 +424,7 @@
409 using namespace testing;424 using namespace testing;
410425
411 msh::Surface surf(426 msh::Surface surf(
427 nullptr,
412 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),428 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
413 msh::a_surface(), stub_id, stub_sender);429 msh::a_surface(), stub_id, stub_sender);
414430
@@ -449,6 +465,7 @@
449 EXPECT_CALL(configurator, attribute_set(_, mir_surface_attrib_state, mir_surface_state_minimized)).Times(1);465 EXPECT_CALL(configurator, attribute_set(_, mir_surface_attrib_state, mir_surface_state_minimized)).Times(1);
450466
451 msh::Surface surf(467 msh::Surface surf(
468 nullptr,
452 mt::fake_shared(surface_builder), mt::fake_shared(configurator),469 mt::fake_shared(surface_builder), mt::fake_shared(configurator),
453 msh::a_surface(), stub_id, stub_sender);470 msh::a_surface(), stub_id, stub_sender);
454471
@@ -460,6 +477,7 @@
460 using namespace ::testing;477 using namespace ::testing;
461478
462 msh::Surface test(479 msh::Surface test(
480 nullptr,
463 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),481 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
464 msh::a_surface(), stub_id, stub_sender);482 msh::a_surface(), stub_id, stub_sender);
465 483
@@ -474,6 +492,7 @@
474 using namespace ::testing;492 using namespace ::testing;
475493
476 msh::Surface test(494 msh::Surface test(
495 nullptr,
477 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),496 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
478 msh::a_surface(), stub_id, stub_sender);497 msh::a_surface(), stub_id, stub_sender);
479 surface_builder.reset_surface();498 surface_builder.reset_surface();
@@ -490,6 +509,7 @@
490 using namespace ::testing;509 using namespace ::testing;
491 510
492 msh::Surface test(511 msh::Surface test(
512 nullptr,
493 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),513 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
494 msh::a_surface(), stub_id, stub_sender);514 msh::a_surface(), stub_id, stub_sender);
495 515
@@ -507,6 +527,7 @@
507TEST_F(ShellSurface, with_most_recent_buffer_do_uses_compositor_buffer)527TEST_F(ShellSurface, with_most_recent_buffer_do_uses_compositor_buffer)
508{528{
509 msh::Surface test(529 msh::Surface test(
530 nullptr,
510 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),531 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
511 msh::a_surface(), stub_id, stub_sender);532 msh::a_surface(), stub_id, stub_sender);
512533
@@ -528,6 +549,7 @@
528549
529 mtd::MockSurfaceController surface_controller;550 mtd::MockSurfaceController surface_controller;
530 msh::Surface test(551 msh::Surface test(
552 nullptr,
531 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),553 mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
532 msh::a_surface(), stub_id, stub_sender);554 msh::a_surface(), stub_id, stub_sender);
533 555
534556
=== modified file 'tests/unit-tests/surfaces/test_surface_controller.cpp'
--- tests/unit-tests/surfaces/test_surface_controller.cpp 2013-07-19 02:44:21 +0000
+++ tests/unit-tests/surfaces/test_surface_controller.cpp 2013-08-09 03:39:57 +0000
@@ -52,7 +52,7 @@
52 EXPECT_CALL(model, create_surface(_)).Times(1).WillOnce(Return(null_surface));52 EXPECT_CALL(model, create_surface(_)).Times(1).WillOnce(Return(null_surface));
53 EXPECT_CALL(model, destroy_surface(_)).Times(1);53 EXPECT_CALL(model, destroy_surface(_)).Times(1);
5454
55 auto surface = controller.create_surface(msh::a_surface());55 auto surface = controller.create_surface(nullptr, msh::a_surface());
56 controller.destroy_surface(surface);56 controller.destroy_surface(surface);
57}57}
5858

Subscribers

People subscribed via source and target branches