Merge lp:~vanvugt/mir/remove-unused-session-param into lp:mir

[Daniel van Vugt]

Remove unused "session" parameters threaded throughout various classes,
but never used.

Update: This is now the first step on the road to tidying up the various SurfaceFactory interactions.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1411
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/891/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/973
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/970
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/565
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/621
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/621/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/628
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/628/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/566
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/566/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/542
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4081

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/891/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I think a reasonable PlacementStrategy implementation could make use of this parameter - it would be better to add an example that, e.g. divides the screen into areas (one for each session) - and places surfaces accordingly.

review: Disapprove

[Daniel van Vugt (vanvugt)]

I presently cannot imagine a use for for Session parameter myself. But regardless, we shouldn't keep unused logic. Parameters should only exist if they're presently used, or will be used in the immediate future.

[lp:~vanvugt/mir/remove-unused-session-param]

1412. By Daniel van Vugt on 2014-02-20

Merge latest development-branch

1413. By Daniel van Vugt on 2014-02-20

Remove lots more unused Session parameters

1414. By Daniel van Vugt on 2014-02-20

Fix Android build too

[Daniel van Vugt (vanvugt)]

Now applied the same cleanup to SurfaceFactory, which never uses its "Session*" parameter either. Probably fortunately.

[lp:~vanvugt/mir/remove-unused-session-param]

1415. By Daniel van Vugt on 2014-02-20

Remove another unused mention of Session

[Alan Griffiths (alan-griffiths)]

http://bazaar.launchpad.net/~mir-team/unity-mir/trunk/view/head:/src/unity-mir/initialsurfaceplacementstrategy.cpp#L38

[Daniel van Vugt (vanvugt)]

Yes, unity-mir will need fixing, but it doesn't appear to have a functional use for "session":

$ grep -r requestPlacementFor *
src/unity-mir/initialsurfaceplacementstrategy.cpp: Q_EMIT requestPlacementForSession(&session, x, y);
src/unity-mir/initialsurfaceplacementstrategy.h: void requestPlacementForSession(mir::shell::Session const* session, uint32_t &x, uint32_t &y);
src/modules/Unity/Application/application_manager.cpp: QObject::connect(m_mirServer->placementStrategy(), &InitialSurfacePlacementStrategy::requestPlacementForSession,

Unless it's in a different project?

[Daniel van Vugt (vanvugt)]

BTW, unity-mir will need modification for just about every server API cleanup I've attempted and envisioned of late. It's unfortunate, but will be worthwhile. We just have to check unity-mir regularly to ensure we at least have a plan for how unity-mir can and should be modified when Mir does.

[Alan Griffiths (alan-griffiths)]

> Unless it's in a different project?

We accepted an MP adding that parameter without any tests because Michael Terry submitted it. I assumed he needed it in unity8.

[Daniel van Vugt (vanvugt)]

Requested a review from mterry. Maybe he can help identify why this code exists.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1414
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/904/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/991
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/988
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/583
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/634
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/634/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/641
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/641/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/584
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/584/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/559
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4124

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/904/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1415
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/906/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/994
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/991
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/586
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/636
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/636/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/643
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/643/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/587
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/587/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/562
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4133

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/906/rebuild

review: Approve (continuous-integration)

[Michael Terry (mterry)]

I want it for in-progress branch https://code.launchpad.net/~mterry/unity-system-compositor/greeter-depth/+merge/207549

The idea is that I can inspect surface name at surface creation time, to know whether the surface belongs to a greeter or user session, to be able to properly assign depth ID.

If there's now a better way to do such filtering, let me know. But the parameter *is* being used by me right now.

[Michael Terry (mterry)]

Typo: "The idea is that I can inspect *session* name at surface creation time"

[Daniel van Vugt (vanvugt)]

OK, well since the session attribute isn't functionally required anywhere within the Mir code, I suggest a different approach: surface.set_session(s);

So you can query the surface session, but it's more clear that it's not a required parameter for surface construction, which it obviously isn't. That would reduce constructor-coupling and allow us to keep most of the simplifications proposed here.

[Daniel van Vugt (vanvugt)]

Although once again, to do that nicely we need to enable shells to override shell::Surface with their own class (to receive the virtual set_session call).

This proposal was meant to be a precursor to that feature, now the prerequisite is reversed :S

[lp:~vanvugt/mir/remove-unused-session-param]

1416. By Daniel van Vugt on 2014-02-21

Merge latest development-branch

1417. By Daniel van Vugt on 2014-02-24

Merge latest development-branch

1418. By Daniel van Vugt on 2014-02-25

Merge latest development-branch

1419. By Daniel van Vugt on 2014-02-25

Fix build failure (soft conflict) from recent landings.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1419
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/931/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1031
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1029
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/623
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/661/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/668
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/668/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/624
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/624/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/598
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4261

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/931/rebuild

review: Needs Fixing (continuous-integration)

[lp:~vanvugt/mir/remove-unused-session-param]

1420. By Daniel van Vugt on 2014-02-26

Merge latest development-branch.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1420
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/942/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1045
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1043
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/637
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/674
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/674/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/679
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/679/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/638
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/638/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/612
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4306

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/942/rebuild

review: Approve (continuous-integration)

Unmerged revisions

1420. By Daniel van Vugt on 2014-02-26

Merge latest development-branch.

1419. By Daniel van Vugt on 2014-02-25

Fix build failure (soft conflict) from recent landings.

1418. By Daniel van Vugt on 2014-02-25

Merge latest development-branch

1417. By Daniel van Vugt on 2014-02-24

Merge latest development-branch

1416. By Daniel van Vugt on 2014-02-21

Merge latest development-branch

1415. By Daniel van Vugt on 2014-02-20

Remove another unused mention of Session

1414. By Daniel van Vugt on 2014-02-20

Fix Android build too

1413. By Daniel van Vugt on 2014-02-20

Remove lots more unused Session parameters

1412. By Daniel van Vugt on 2014-02-20

Merge latest development-branch

1411. By Daniel van Vugt on 2014-02-19

Remove unused "session" parameter from PlacementStrategy::place().
Also make it const, while we can.

=== modified file 'examples/demo-shell/fullscreen_placement_strategy.cpp'
--- examples/demo-shell/fullscreen_placement_strategy.cpp	2013-08-28 03:41:48 +0000
+++ examples/demo-shell/fullscreen_placement_strategy.cpp	2014-02-26 05:35:45 +0000
@@ -31,8 +31,8 @@
 {
 }
 
-msh::SurfaceCreationParameters me::FullscreenPlacementStrategy::place(msh::Session const& /* session */,
-    msh::SurfaceCreationParameters const& request_parameters)
+msh::SurfaceCreationParameters me::FullscreenPlacementStrategy::place(
+    msh::SurfaceCreationParameters const& request_parameters) const
 {
     auto placed_parameters = request_parameters;
 

=== modified file 'examples/demo-shell/fullscreen_placement_strategy.h'
--- examples/demo-shell/fullscreen_placement_strategy.h	2013-08-28 03:41:48 +0000
+++ examples/demo-shell/fullscreen_placement_strategy.h	2014-02-26 05:35:45 +0000
@@ -38,7 +38,7 @@
     FullscreenPlacementStrategy(std::shared_ptr<shell::DisplayLayout> const& display_layout);
     ~FullscreenPlacementStrategy() = default;
     
-    shell::SurfaceCreationParameters place(shell::Session const&, shell::SurfaceCreationParameters const& request_parameters);
+    shell::SurfaceCreationParameters place(shell::SurfaceCreationParameters const& request_parameters) const override;
 
 protected:
     FullscreenPlacementStrategy(FullscreenPlacementStrategy const&) = delete;

=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp	2014-02-21 11:19:46 +0000
+++ examples/render_surfaces.cpp	2014-02-26 05:35:45 +0000
@@ -415,7 +415,6 @@
         for (auto& m : moveables)
         {
             auto const s = surface_factory->create_surface(
-                    nullptr,
                     msh::a_surface().of_size(surface_size)
                                    .of_pixel_format(surface_pf)
                                    .of_buffer_usage(mg::BufferUsage::hardware),

=== modified file 'include/server/mir/shell/placement_strategy.h'
--- include/server/mir/shell/placement_strategy.h	2013-08-28 03:41:48 +0000
+++ include/server/mir/shell/placement_strategy.h	2014-02-26 05:35:45 +0000
@@ -24,7 +24,6 @@
 
 namespace shell
 {
-class Session;
 struct SurfaceCreationParameters;
 
 class PlacementStrategy
@@ -33,7 +32,7 @@
     virtual ~PlacementStrategy() {}
     // TODO: It is strange to work in terms of SurfaceCreationParameters here,
     // perhaps a new interface is needed.
-    virtual SurfaceCreationParameters place(shell::Session const& session, SurfaceCreationParameters const& request_parameters) = 0;
+    virtual SurfaceCreationParameters place(SurfaceCreationParameters const& request_parameters) const = 0;
 
 
 protected:

=== modified file 'include/server/mir/shell/surface_factory.h'
--- include/server/mir/shell/surface_factory.h	2013-08-28 03:41:48 +0000
+++ include/server/mir/shell/surface_factory.h	2014-02-26 05:35:45 +0000
@@ -38,7 +38,6 @@
 {
 public:
     virtual std::shared_ptr<Surface> create_surface(
-        Session* session,
         SurfaceCreationParameters const& params,
         frontend::SurfaceId id,
         std::shared_ptr<frontend::EventSink> const& sink) = 0;

=== modified file 'include/test/mir_test_doubles/mock_surface_factory.h'
--- include/test/mir_test_doubles/mock_surface_factory.h	2013-08-28 03:41:48 +0000
+++ include/test/mir_test_doubles/mock_surface_factory.h	2014-02-26 05:35:45 +0000
@@ -33,8 +33,7 @@
 
 struct MockSurfaceFactory : public shell::SurfaceFactory
 {
-    MOCK_METHOD4(create_surface, std::shared_ptr<shell::Surface>(
-        shell::Session*,
+    MOCK_METHOD3(create_surface, std::shared_ptr<shell::Surface>(
         const shell::SurfaceCreationParameters&,
         frontend::SurfaceId,
         std::shared_ptr<frontend::EventSink> const&));

=== modified file 'src/server/scene/application_session.cpp'
--- src/server/scene/application_session.cpp	2014-02-24 13:45:23 +0000
+++ src/server/scene/application_session.cpp	2014-02-26 05:35:45 +0000
@@ -70,7 +70,7 @@
 mf::SurfaceId ms::ApplicationSession::create_surface(const msh::SurfaceCreationParameters& params)
 {
     auto const id = next_id();
-    auto surf = surface_factory->create_surface(this, params, id, event_sink);
+    auto surf = surface_factory->create_surface(params, id, event_sink);
 
     std::unique_lock<std::mutex> lock(surfaces_mutex);
     surfaces[id] = surf;

=== modified file 'src/server/scene/surface_source.cpp'
--- src/server/scene/surface_source.cpp	2014-01-13 06:12:33 +0000
+++ src/server/scene/surface_source.cpp	2014-02-26 05:35:45 +0000
@@ -37,7 +37,6 @@
 }
 
 std::shared_ptr<msh::Surface> ms::SurfaceSource::create_surface(
-    msh::Session* /*session*/,
     shell::SurfaceCreationParameters const& params,
     frontend::SurfaceId id,
     std::shared_ptr<mf::EventSink> const& sender)

=== modified file 'src/server/scene/surface_source.h'
--- src/server/scene/surface_source.h	2014-01-13 06:12:33 +0000
+++ src/server/scene/surface_source.h	2014-02-26 05:35:45 +0000
@@ -44,7 +44,6 @@
     virtual ~SurfaceSource() {}
 
     std::shared_ptr<shell::Surface> create_surface(
-        shell::Session* session,
         shell::SurfaceCreationParameters const& params,
         frontend::SurfaceId id,
         std::shared_ptr<frontend::EventSink> const& sink);

=== modified file 'src/server/shell/consuming_placement_strategy.cpp'
--- src/server/shell/consuming_placement_strategy.cpp	2014-01-13 06:12:33 +0000
+++ src/server/shell/consuming_placement_strategy.cpp	2014-02-26 05:35:45 +0000
@@ -35,8 +35,7 @@
 }
 
 msh::SurfaceCreationParameters msh::ConsumingPlacementStrategy::place(
-    msh::Session const& /* session */,
-    msh::SurfaceCreationParameters const& request_parameters)
+    msh::SurfaceCreationParameters const& request_parameters) const
 {
     mir::graphics::DisplayConfigurationOutputId const output_id_invalid{
         mir_display_output_id_invalid};

=== modified file 'src/server/shell/consuming_placement_strategy.h'
--- src/server/shell/consuming_placement_strategy.h	2014-01-13 06:12:33 +0000
+++ src/server/shell/consuming_placement_strategy.h	2014-02-26 05:35:45 +0000
@@ -36,7 +36,7 @@
             std::shared_ptr<DisplayLayout> const& display_layout);
     virtual ~ConsumingPlacementStrategy() {}
 
-    virtual shell::SurfaceCreationParameters place(shell::Session const& session, shell::SurfaceCreationParameters const& request_parameters);
+    virtual shell::SurfaceCreationParameters place(shell::SurfaceCreationParameters const& request_parameters) const override;
 
 protected:
     ConsumingPlacementStrategy(ConsumingPlacementStrategy const&) = delete;

=== modified file 'src/server/shell/organising_surface_factory.cpp'
--- src/server/shell/organising_surface_factory.cpp	2014-01-13 06:12:33 +0000
+++ src/server/shell/organising_surface_factory.cpp	2014-02-26 05:35:45 +0000
@@ -35,13 +35,12 @@
 }
 
 std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface(
-    Session* session,
     shell::SurfaceCreationParameters const& params,
     frontend::SurfaceId id,
     std::shared_ptr<mf::EventSink> const& sender)
 {
-    auto placed_params = placement_strategy->place(*session, params);
+    auto placed_params = placement_strategy->place(params);
 
-    return underlying_factory->create_surface(session, placed_params, id, sender);
+    return underlying_factory->create_surface(placed_params, id, sender);
 }
 

=== modified file 'src/server/shell/organising_surface_factory.h'
--- src/server/shell/organising_surface_factory.h	2014-01-13 06:12:33 +0000
+++ src/server/shell/organising_surface_factory.h	2014-02-26 05:35:45 +0000
@@ -28,7 +28,6 @@
 namespace shell
 {
 class PlacementStrategy;
-class Session;
 
 class OrganisingSurfaceFactory : public SurfaceFactory
 {
@@ -38,7 +37,6 @@
     virtual ~OrganisingSurfaceFactory();
 
     std::shared_ptr<Surface> create_surface(
-        Session* session,
         shell::SurfaceCreationParameters const& params,
         frontend::SurfaceId id,
         std::shared_ptr<frontend::EventSink> const& sink) override;

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2014-02-18 10:33:14 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2014-02-26 05:35:45 +0000
@@ -84,7 +84,7 @@
     {
     }
 
-    msh::SurfaceCreationParameters place(msh::Session const& session, msh::SurfaceCreationParameters const& request_parameters)
+    msh::SurfaceCreationParameters place(msh::SurfaceCreationParameters const& request_parameters) const override
     {
         auto placed = request_parameters;
         auto const& name = request_parameters.name;
@@ -98,9 +98,13 @@
         }
         else
         {
-            placed = underlying_strategy->place(session, placed);
+            placed = underlying_strategy->place(placed);
         }
-        placed.depth = surface_depths_by_name[name];
+
+        // Use find instead of [] here as we're limited to const operations
+        auto const d = surface_depths_by_name.find(name);
+        if (d != surface_depths_by_name.end())
+            placed.depth = d->second;
 
         return placed;
     }
@@ -378,12 +382,11 @@
     {
     }
 
-    std::shared_ptr<msh::Surface> create_surface(msh::Session* session,
-                                                 msh::SurfaceCreationParameters const& params,
+    std::shared_ptr<msh::Surface> create_surface(msh::SurfaceCreationParameters const& params,
                                                  mf::SurfaceId id,
                                                  std::shared_ptr<mf::EventSink> const& sink)
     {
-        auto surface = underlying_factory->create_surface(session, params, id, sink);
+        auto surface = underlying_factory->create_surface(params, id, sink);
 
         surface->set_input_region(input_rectangles);
 

=== modified file 'tests/integration-tests/graphics/android/test_internal_client.cpp'
--- tests/integration-tests/graphics/android/test_internal_client.cpp	2014-02-17 22:35:23 +0000
+++ tests/integration-tests/graphics/android/test_internal_client.cpp	2014-02-26 05:35:45 +0000
@@ -100,7 +100,7 @@
     auto ss = std::make_shared<ms::SurfaceStack>(surface_allocator, stub_input_registrar, scene_report);
     auto surface_controller = std::make_shared<ms::SurfaceController>(ss);
     auto surface_source = std::make_shared<ms::SurfaceSource>(surface_controller, std::make_shared<mtd::NullSurfaceConfigurator>());
-    auto surface = surface_source->create_surface(nullptr, params, id, std::shared_ptr<mf::EventSink>());
+    auto surface = surface_source->create_surface(params, id, std::shared_ptr<mf::EventSink>());
     surface->allow_framedropping(true);
     auto mir_surface = as_internal_surface(surface);
 

=== modified file 'tests/unit-tests/scene/test_application_session.cpp'
--- tests/unit-tests/scene/test_application_session.cpp	2014-02-24 13:45:23 +0000
+++ tests/unit-tests/scene/test_application_session.cpp	2014-02-26 05:35:45 +0000
@@ -86,7 +86,7 @@
     mtd::NullEventSink sender;
     mtd::MockSurfaceFactory surface_factory;
 
-    EXPECT_CALL(surface_factory, create_surface(_, _, _, _))
+    EXPECT_CALL(surface_factory, create_surface(_, _, _))
         .WillOnce(Return(mock_surface));
 
     mtd::MockSessionListener listener;
@@ -117,9 +117,9 @@
 
     mtd::NullEventSink sender;
     mtd::MockSurfaceFactory surface_factory;
-    ON_CALL(surface_factory, create_surface(_,_,_,_)).WillByDefault(Return(mock_surface));
+    ON_CALL(surface_factory, create_surface(_,_,_)).WillByDefault(Return(mock_surface));
 
-    EXPECT_CALL(surface_factory, create_surface(_, _, _, _));
+    EXPECT_CALL(surface_factory, create_surface(_, _, _));
 
     mtd::MockSessionListener listener;
     EXPECT_CALL(listener, surface_created(_, _)).Times(1);
@@ -148,11 +148,11 @@
 
     {
         InSequence seq;
-        EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
-            .WillOnce(Return(make_mock_surface()));
-        EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
-            .WillOnce(Return(make_mock_surface()));
-        EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1)
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
+            .WillOnce(Return(make_mock_surface()));
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
+            .WillOnce(Return(make_mock_surface()));
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
             .WillOnce(Return(make_mock_surface()));
     }
 
@@ -191,7 +191,7 @@
     auto mock_surface = make_mock_surface();
 
     mtd::MockSurfaceFactory surface_factory;
-    ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(mock_surface));
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(Return(mock_surface));
 
     ms::ApplicationSession app_session(
         mt::fake_shared(surface_factory),
@@ -201,7 +201,7 @@
         std::make_shared<msh::NullSessionListener>(),
         mt::fake_shared(sender));
 
-    EXPECT_CALL(surface_factory, create_surface(_, _, _, _));
+    EXPECT_CALL(surface_factory, create_surface(_, _, _));
 
     {
         InSequence seq;
@@ -271,7 +271,7 @@
         std::static_pointer_cast<msh::SurfaceBufferAccess>(default_surface);
     auto const snapshot_strategy = std::make_shared<MockSnapshotStrategy>();
 
-    EXPECT_CALL(surface_factory, create_surface(_,_,_,_))
+    EXPECT_CALL(surface_factory, create_surface(_,_,_))
         .WillOnce(Return(default_surface));
 
     EXPECT_CALL(*snapshot_strategy,

=== modified file 'tests/unit-tests/scene/test_session_manager.cpp'
--- tests/unit-tests/scene/test_session_manager.cpp	2014-02-07 12:54:45 +0000
+++ tests/unit-tests/scene/test_session_manager.cpp	2014-02-26 05:35:45 +0000
@@ -112,9 +112,9 @@
 {
     using namespace ::testing;
 
-    EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1);
+    EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);
 
-    ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(
        Return(std::make_shared<ms::SurfaceImpl>(
            mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
            msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
@@ -147,7 +147,7 @@
 TEST_F(SessionManagerSetup, create_surface_for_session_forwards_and_then_focuses_session)
 {
     using namespace ::testing;
-    ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(
         Return(std::make_shared<ms::SurfaceImpl>(
            mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
            msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
@@ -157,7 +157,7 @@
         InSequence seq;
 
         EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation
-        EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1);
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);
         EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Post Surface creation
     }
 

=== modified file 'tests/unit-tests/shell/test_consuming_placement_strategy.cpp'
--- tests/unit-tests/shell/test_consuming_placement_strategy.cpp	2014-01-13 06:12:33 +0000
+++ tests/unit-tests/shell/test_consuming_placement_strategy.cpp	2014-02-26 05:35:45 +0000
@@ -17,7 +17,6 @@
  */
 
 #include "mir_test_doubles/mock_display_layout.h"
-#include "mir_test_doubles/stub_shell_session.h"
 
 #include "src/server/shell/consuming_placement_strategy.h"
 #include "mir/shell/surface_creation_parameters.h"
@@ -43,7 +42,6 @@
     }
 
     std::shared_ptr<mtd::MockDisplayLayout> display_layout;
-    mtd::StubShellSession session;
 };
 }
 
@@ -59,7 +57,7 @@
 
     msh::ConsumingPlacementStrategy placement_strategy(display_layout);
 
-    placement_strategy.place(session, input_params);
+    placement_strategy.place(input_params);
 }
 
 TEST_F(ConsumingPlacementStrategySetup, parameters_with_geometry_are_clipped)
@@ -74,7 +72,7 @@
 
     msh::ConsumingPlacementStrategy placement_strategy(display_layout);
 
-    placement_strategy.place(session, input_params);
+    placement_strategy.place(input_params);
 }
 
 TEST_F(ConsumingPlacementStrategySetup, parameters_with_output_id_are_placed_in_output)
@@ -92,5 +90,5 @@
 
     msh::ConsumingPlacementStrategy placement_strategy(display_layout);
 
-    placement_strategy.place(session, input_params);
+    placement_strategy.place(input_params);
 }

=== modified file 'tests/unit-tests/shell/test_organising_surface_factory.cpp'
--- tests/unit-tests/shell/test_organising_surface_factory.cpp	2014-01-13 06:12:33 +0000
+++ tests/unit-tests/shell/test_organising_surface_factory.cpp	2014-02-26 05:35:45 +0000
@@ -19,10 +19,8 @@
 #include "src/server/shell/organising_surface_factory.h"
 #include "mir/shell/placement_strategy.h"
 #include "mir/shell/surface_creation_parameters.h"
-#include "mir/shell/session.h"
 
 #include "mir_test_doubles/mock_surface_factory.h"
-#include "mir_test_doubles/stub_shell_session.h"
 #include "mir_test_doubles/null_event_sink.h"
 
 #include <gtest/gtest.h>
@@ -38,7 +36,7 @@
 
 struct MockPlacementStrategy : public msh::PlacementStrategy
 {
-    MOCK_METHOD2(place, msh::SurfaceCreationParameters(msh::Session const&, msh::SurfaceCreationParameters const&));
+    MOCK_CONST_METHOD1(place, msh::SurfaceCreationParameters(msh::SurfaceCreationParameters const&));
 };
 
 struct OrganisingSurfaceFactorySetup : public testing::Test
@@ -48,7 +46,7 @@
         using namespace ::testing;
 
         underlying_surface_factory = std::make_shared<mtd::MockSurfaceFactory>();
-        ON_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).WillByDefault(Return(null_surface));
+        ON_CALL(*underlying_surface_factory, create_surface(_, _, _)).WillByDefault(Return(null_surface));
 
         placement_strategy = std::make_shared<MockPlacementStrategy>();
     }
@@ -65,14 +63,13 @@
 
     msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy);
 
-    mtd::StubShellSession session;
-    EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _, _)).Times(1);
+    EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _)).Times(1);
 
     auto params = msh::a_surface();
-    EXPECT_CALL(*placement_strategy, place(Ref(session), Ref(params))).Times(1)
+    EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
         .WillOnce(Return(msh::a_surface()));
 
-    factory.create_surface(&session, params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());
+    factory.create_surface(params, mf::SurfaceId(), std::make_shared<mtd::NullEventSink>());
 }
 
 TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory)
@@ -86,9 +83,9 @@
     auto placed_params = params;
     placed_params.size.width = geom::Width{100};
 
-    EXPECT_CALL(*placement_strategy, place(_, Ref(params))).Times(1)
+    EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
         .WillOnce(Return(placed_params));
-    EXPECT_CALL(*underlying_surface_factory, create_surface(nullptr, placed_params, mf::SurfaceId(), sink));
+    EXPECT_CALL(*underlying_surface_factory, create_surface(placed_params, mf::SurfaceId(), sink));
 
-    factory.create_surface(nullptr, params, mf::SurfaceId(), sink);
+    factory.create_surface(params, mf::SurfaceId(), sink);
 }