Merge lp:~alan-griffiths/mir/remove-PlacementStrategy into lp:mir

[Alan Griffiths]

scene, tests, playground: remove PlacementStrategy hierarchy

scene, tests, playground: remove PlacementStrategy hierarchy

There were two isolated uses of this strategy interface. By moving the two implementations to the two points of use we can get rid of the whole abstraction.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2693
http://jenkins.qa.ubuntu.com/job/mir-ci/4163/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2946
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/461
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2894
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/319
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/319/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2894
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2894/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5718
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21368

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Fields of red... More pleasant than it sounds.

review: Approve

=== modified file 'playground/demo-shell/CMakeLists.txt'
--- playground/demo-shell/CMakeLists.txt	2015-06-17 15:19:52 +0000
+++ playground/demo-shell/CMakeLists.txt	2015-06-22 16:27:03 +0000
@@ -3,7 +3,6 @@
   demo_renderer.cpp
   window_manager.cpp
   default_window_manager.cpp
-  default_placement_strategy.cpp
 )
 
 add_subdirectory(typo)

=== removed file 'playground/demo-shell/default_placement_strategy.cpp'
--- playground/demo-shell/default_placement_strategy.cpp	2015-06-17 15:26:26 +0000
+++ playground/demo-shell/default_placement_strategy.cpp	1970-01-01 00:00:00 +0000
@@ -1,58 +0,0 @@
-/*
- * Copyright © 2013 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by: Robert Carr <[e-mail]>
- */
-
-#include "default_placement_strategy.h"
-#include "mir/scene/surface_creation_parameters.h"
-#include "mir/shell/display_layout.h"
-#include "mir/geometry/rectangle.h"
-
-#include "mir_toolkit/client_types.h"
-
-#include <algorithm>
-
-namespace me = mir::examples;
-namespace ms = mir::scene;
-namespace msh = mir::shell;
-namespace geom = mir::geometry;
-
-me::DefaultPlacementStrategy::DefaultPlacementStrategy(
-    std::shared_ptr<msh::DisplayLayout> const& display_layout)
-    : display_layout(display_layout)
-{
-}
-
-ms::SurfaceCreationParameters me::DefaultPlacementStrategy::place(
-    ms::Session const& /* session */,
-    ms::SurfaceCreationParameters const& request_parameters)
-{
-    mir::graphics::DisplayConfigurationOutputId const output_id_invalid{
-        mir_display_output_id_invalid};
-    auto placed_parameters = request_parameters;
-
-    geom::Rectangle rect{request_parameters.top_left, request_parameters.size};
-
-    if (request_parameters.output_id != output_id_invalid)
-    {
-        display_layout->place_in_output(request_parameters.output_id, rect);
-    }
-
-    placed_parameters.top_left = rect.top_left;
-    placed_parameters.size = rect.size;
-
-    return placed_parameters;
-}

=== removed file 'playground/demo-shell/default_placement_strategy.h'
--- playground/demo-shell/default_placement_strategy.h	2015-06-17 15:26:26 +0000
+++ playground/demo-shell/default_placement_strategy.h	1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-/*
- * Copyright © 2013 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by: Robert Carr <[e-mail]>
- */
-
-#ifndef MIR_EXAMPLES_DEFAULT_PLACEMENT_STRATEGY_H_
-#define MIR_EXAMPLES_DEFAULT_PLACEMENT_STRATEGY_H_
-
-#include "mir/scene/placement_strategy.h"
-
-#include <memory>
-
-namespace mir
-{
-namespace shell
-{
-class DisplayLayout;
-}
-
-namespace examples
-{
-class DefaultPlacementStrategy : public scene::PlacementStrategy
-{
-public:
-    explicit DefaultPlacementStrategy(
-            std::shared_ptr<shell::DisplayLayout> const& display_layout);
-    virtual ~DefaultPlacementStrategy() {}
-
-    virtual scene::SurfaceCreationParameters place(scene::Session const& session, scene::SurfaceCreationParameters const& request_parameters);
-
-protected:
-    DefaultPlacementStrategy(DefaultPlacementStrategy const&) = delete;
-    DefaultPlacementStrategy& operator=(DefaultPlacementStrategy const&) = delete;
-
-private:
-    std::shared_ptr<shell::DisplayLayout> const display_layout;
-};
-
-}
-} // namespace mir
-
-#endif // MIR_SHELL_DEFAULT_PLACEMENT_STRATEGY_H_

=== modified file 'playground/demo-shell/default_window_manager.cpp'
--- playground/demo-shell/default_window_manager.cpp	2015-06-17 14:41:13 +0000
+++ playground/demo-shell/default_window_manager.cpp	2015-06-22 16:27:03 +0000
@@ -18,26 +18,29 @@
 
 #include "default_window_manager.h"
 
-#include "mir/scene/placement_strategy.h"
 #include "mir/scene/session.h"
 #include "mir/scene/session_coordinator.h"
 #include "mir/scene/surface.h"
 #include "mir/scene/surface_creation_parameters.h"
+#include "mir/shell/display_layout.h"
 #include "mir/shell/focus_controller.h"
 #include "mir/shell/surface_ready_observer.h"
 #include "mir/shell/surface_specification.h"
 
+#include "mir_toolkit/client_types.h"
+
 namespace mf = mir::frontend;
 namespace ms = mir::scene;
 namespace msh = mir::shell;
 namespace me = mir::examples;
+namespace geom = mir::geometry;
 
 me::DefaultWindowManager::DefaultWindowManager(
     msh::FocusController* focus_controller,
-    std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
+    std::shared_ptr<shell::DisplayLayout> const& display_layout,
     std::shared_ptr<scene::SessionCoordinator> const& session_coordinator) :
     focus_controller{focus_controller},
-    placement_strategy{placement_strategy},
+    display_layout{display_layout},
     session_coordinator{session_coordinator}
 {
 }
@@ -58,11 +61,25 @@
 
 auto me::DefaultWindowManager::add_surface(
     std::shared_ptr<scene::Session> const& session,
-    scene::SurfaceCreationParameters const& params,
+    scene::SurfaceCreationParameters const& request_parameters,
     std::function<frontend::SurfaceId(std::shared_ptr<scene::Session> const& session, scene::SurfaceCreationParameters const& params)> const& build)
 -> frontend::SurfaceId
 {
-    auto const result = build(session, placement_strategy->place(*session, params));
+    mir::graphics::DisplayConfigurationOutputId const output_id_invalid{
+        mir_display_output_id_invalid};
+    auto placed_parameters = request_parameters;
+
+    geom::Rectangle rect{request_parameters.top_left, request_parameters.size};
+
+    if (request_parameters.output_id != output_id_invalid)
+    {
+        display_layout->place_in_output(request_parameters.output_id, rect);
+    }
+
+    placed_parameters.top_left = rect.top_left;
+    placed_parameters.size = rect.size;
+
+    auto const result = build(session, placed_parameters);
     auto const surface = session->surface(result);
 
     surface->add_observer(std::make_shared<msh::SurfaceReadyObserver>(

=== modified file 'playground/demo-shell/default_window_manager.h'
--- playground/demo-shell/default_window_manager.h	2015-06-17 14:41:13 +0000
+++ playground/demo-shell/default_window_manager.h	2015-06-22 16:27:03 +0000
@@ -23,9 +23,9 @@
 
 namespace mir
 {
-namespace scene { class PlacementStrategy; class SessionCoordinator; }
+namespace scene { class SessionCoordinator; }
 
-namespace shell { class FocusController; }
+namespace shell { class FocusController; class DisplayLayout; }
 
 namespace examples
 {
@@ -33,7 +33,7 @@
 {
 public:
     explicit DefaultWindowManager(shell::FocusController* focus_controller,
-        std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
+        std::shared_ptr<shell::DisplayLayout> const& display_layout,
         std::shared_ptr<scene::SessionCoordinator> const& session_coordinator);
 
     void add_session(std::shared_ptr<scene::Session> const& session) override;
@@ -72,7 +72,7 @@
 
 private:
     shell::FocusController* const focus_controller;
-    std::shared_ptr<scene::PlacementStrategy> const placement_strategy;
+    std::shared_ptr<shell::DisplayLayout> const display_layout;
     std::shared_ptr<scene::SessionCoordinator> const session_coordinator;
 };
 }

=== modified file 'playground/demo-shell/demo_shell.cpp'
--- playground/demo-shell/demo_shell.cpp	2015-06-17 15:26:26 +0000
+++ playground/demo-shell/demo_shell.cpp	2015-06-22 16:27:03 +0000
@@ -30,7 +30,6 @@
 #include "mir/compositor/renderer_factory.h"
 #include "mir/options/option.h"
 #include "default_window_manager.h"
-#include "default_placement_strategy.h"
 #include "server_example_tiling_window_manager.h"
 #include "mir/shell/canonical_window_manager.h"
 #include "server_example_host_lifecycle_event_listener.h"
@@ -128,7 +127,7 @@
 
                 return std::make_shared<DefaultWindowManager>(
                     focus_controller,
-                    the_placement_strategy(),
+                    the_shell_display_layout(),
                     the_session_coordinator());
             };
     }
@@ -142,15 +141,6 @@
            });
     }
 
-    std::shared_ptr<ms::PlacementStrategy> the_placement_strategy()
-    {
-        return placement_strategy([this]
-            {
-                return std::make_shared<DefaultPlacementStrategy>(
-                    the_shell_display_layout());
-            });
-    }
-
 private:
     std::vector<std::shared_ptr<mi::EventFilter>> const filter_list;
 };

=== removed file 'src/include/server/mir/scene/placement_strategy.h'
--- src/include/server/mir/scene/placement_strategy.h	2015-06-17 05:20:42 +0000
+++ src/include/server/mir/scene/placement_strategy.h	1970-01-01 00:00:00 +0000
@@ -1,45 +0,0 @@
-/*
- * Copyright © 2013-2014 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by: Robert Carr <[e-mail]>
- */
-
-#ifndef MIR_SCENE_PLACEMENT_STRATEGY_H_
-#define MIR_SCENE_PLACEMENT_STRATEGY_H_
-
-namespace mir
-{
-namespace scene
-{
-class Session;
-struct SurfaceCreationParameters;
-
-// TODO Can be subsumed by DefaultShell
-class PlacementStrategy
-{
-public:
-    virtual ~PlacementStrategy() = default;
-
-    virtual SurfaceCreationParameters place(Session const& session, SurfaceCreationParameters const& request_parameters) = 0;
-
-protected:
-    PlacementStrategy() = default;
-    PlacementStrategy(PlacementStrategy const&) = delete;
-    PlacementStrategy& operator=(PlacementStrategy const&) = delete;
-};
-}
-} // namespace mir
-
-#endif // MIR_SCENE_PLACEMENT_STRATEGY_H_

=== modified file 'tests/acceptance-tests/throwback/test_client_cursor_api.cpp'
--- tests/acceptance-tests/throwback/test_client_cursor_api.cpp	2015-06-17 15:19:52 +0000
+++ tests/acceptance-tests/throwback/test_client_cursor_api.cpp	2015-06-22 16:27:03 +0000
@@ -20,13 +20,10 @@
 #include "mir/graphics/cursor.h"
 #include "mir/graphics/cursor_image.h"
 #include "mir/input/cursor_images.h"
-#include "mir/shell/canonical_window_manager.h"
-#include "mir/scene/placement_strategy.h"
-
 
 #include "mir_test_framework/in_process_server.h"
 #include "mir_test_framework/fake_event_hub_server_configuration.h"
-#include "mir_test_framework/declarative_placement_strategy.h"
+#include "mir_test_framework/declarative_placement_window_manage_policy.h"
 #include "mir_test_framework/using_stub_client_platform.h"
 #include "mir_test_framework/headless_nested_server_runner.h"
 #include "mir_test_doubles/mock_egl.h"
@@ -215,47 +212,18 @@
     std::string const cursor_name;
 };
 
-class PlacementWindowManagerPolicy : public msh::CanonicalWindowManagerPolicy
-{
-public:
-    PlacementWindowManagerPolicy(
-        Tools* const tools,
-        std::shared_ptr<ms::PlacementStrategy> const& placement_strategy,
-        std::shared_ptr<msh::DisplayLayout> const& display_layout) :
-        msh::CanonicalWindowManagerPolicy{tools, display_layout},
-        placement_strategy{placement_strategy}
-    {}
-
-    auto handle_place_new_surface(
-        std::shared_ptr<ms::Session> const& session,
-        ms::SurfaceCreationParameters const& request_parameters)
-        -> ms::SurfaceCreationParameters
-    {
-        return placement_strategy->place(*session, request_parameters);
-    }
-
-
-private:
-    std::shared_ptr<ms::PlacementStrategy> const placement_strategy;
-};
-
 struct TestServerConfiguration : mtf::FakeEventHubServerConfiguration
 {
-    std::shared_ptr<mir::scene::PlacementStrategy> the_placement_strategy()
-    {
-        return std::make_shared<mtf::DeclarativePlacementStrategy>(
-            client_geometries, client_depths);
-    }
-
     auto the_window_manager_builder() -> msh::WindowManagerBuilder override
     {
-        using PlacementWindowManager = msh::BasicWindowManager<PlacementWindowManagerPolicy, msh::CanonicalSessionInfo, msh::CanonicalSurfaceInfo>;
+        using PlacementWindowManager = msh::BasicWindowManager<mtf::DeclarativePlacementWindowManagerPolicy, msh::CanonicalSessionInfo, msh::CanonicalSurfaceInfo>;
 
         return [&](msh::FocusController* focus_controller)
             {
                 return std::make_shared<PlacementWindowManager>(
                     focus_controller,
-                    the_placement_strategy(),
+                    client_geometries,
+                    client_depths,
                     the_shell_display_layout());
             };
     }

=== renamed file 'tests/include/mir_test_framework/declarative_placement_strategy.h' => 'tests/include/mir_test_framework/declarative_placement_window_manage_policy.h'
--- tests/include/mir_test_framework/declarative_placement_strategy.h	2015-06-17 15:19:52 +0000
+++ tests/include/mir_test_framework/declarative_placement_window_manage_policy.h	2015-06-22 16:27:03 +0000
@@ -16,10 +16,10 @@
  * Authored by: Robert Carr <[e-mail]>
  */
 
-#ifndef MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_STRATEGY_H_
-#define MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_STRATEGY_H_
+#ifndef MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_WINDOW_MANAGER_POLICY_H_
+#define MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_WINDOW_MANAGER_POLICY_H_
 
-#include "mir/scene/placement_strategy.h"
+#include "mir/shell/canonical_window_manager.h"
 #include "mir/geometry/rectangle.h"
 #include "mir/scene/depth_id.h"
 
@@ -32,31 +32,29 @@
 typedef std::map<std::string, mir::geometry::Rectangle> SurfaceGeometries;
 typedef std::map<std::string, mir::scene::DepthId> SurfaceDepths;
 
-/// DeclarativePlacementStrategy is a test utility server component for specifying
+/// DeclarativePlacementWindowManagerPolicy is a test utility server component for specifying
 /// a static list of surface geometries and relative depths. Used, for example,
 /// in input tests where it is necessary to set up scenarios depending on
 /// multiple surfaces geometry and stacking.
-class DeclarativePlacementStrategy : public mir::scene::PlacementStrategy
+class DeclarativePlacementWindowManagerPolicy : public mir::shell::CanonicalWindowManagerPolicy
 {
- public:
-    // Placement requests will be passed through to default strategy, and then overriden if the surface appears
-    // in the geometry or depth map. This allows for the convenience of leaving some surfaces geometries unspecified
-    // and receiving the default behavior.
-    DeclarativePlacementStrategy(
-        SurfaceGeometries const& positions_by_name, SurfaceDepths const& depths_by_name);
-
-    virtual ~DeclarativePlacementStrategy() = default;
-    
-    mir::scene::SurfaceCreationParameters place(mir::scene::Session const& session, mir::scene::SurfaceCreationParameters const& request_parameters) override;
-
-protected:
-    DeclarativePlacementStrategy(const DeclarativePlacementStrategy&) = delete;
-    DeclarativePlacementStrategy& operator=(const DeclarativePlacementStrategy&) = delete;
+public:
+    DeclarativePlacementWindowManagerPolicy(
+        Tools* const tools,
+        SurfaceGeometries const& positions_by_name, 
+        SurfaceDepths const& depths_by_name,
+        std::shared_ptr<mir::shell::DisplayLayout> const& display_layout);
+
+    auto handle_place_new_surface(
+        std::shared_ptr<mir::scene::Session> const& session,
+        mir::scene::SurfaceCreationParameters const& request_parameters)
+    -> mir::scene::SurfaceCreationParameters;
 
 private:
     SurfaceGeometries const& surface_geometries_by_name;
     SurfaceDepths const& surface_depths_by_name;
 };
+
 }
 
-#endif // MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_STRATEGY_H_
+#endif // MIR_TEST_FRAMEWORK_DECLARATIVE_PLACEMENT_WINDOW_MANAGER_POLICY_H_

=== modified file 'tests/integration-tests/test_session.cpp'
--- tests/integration-tests/test_session.cpp	2015-06-17 05:20:42 +0000
+++ tests/integration-tests/test_session.cpp	2015-06-22 16:27:03 +0000
@@ -21,7 +21,6 @@
 #include "mir/compositor/compositor.h"
 #include "src/server/scene/application_session.h"
 #include "src/server/scene/pixel_buffer.h"
-#include "mir/scene/placement_strategy.h"
 #include "mir/scene/surface.h"
 #include "mir/scene/surface_creation_parameters.h"
 #include "mir/scene/null_session_listener.h"

=== modified file 'tests/mir_test_framework/CMakeLists.txt'
--- tests/mir_test_framework/CMakeLists.txt	2015-06-17 05:20:42 +0000
+++ tests/mir_test_framework/CMakeLists.txt	2015-06-22 16:27:03 +0000
@@ -41,7 +41,7 @@
   stub_client_platform_factory.cpp
   stub_server_platform_factory.cpp
   udev_environment.cpp
-  declarative_placement_strategy.cpp
+  declarative_placement_window_manage_policy.cpp
   fake_event_hub_server_configuration.cpp
   any_surface.cpp
   headless_nested_server_runner.cpp

=== renamed file 'tests/mir_test_framework/declarative_placement_strategy.cpp' => 'tests/mir_test_framework/declarative_placement_window_manage_policy.cpp'
--- tests/mir_test_framework/declarative_placement_strategy.cpp	2015-06-17 15:26:26 +0000
+++ tests/mir_test_framework/declarative_placement_window_manage_policy.cpp	2015-06-22 16:27:03 +0000
@@ -16,23 +16,27 @@
  * Authored by: Robert Carr <[e-mail]>
  */
 
+#include "mir_test_framework/declarative_placement_window_manage_policy.h"
 #include "mir/scene/surface_creation_parameters.h"
 
-#include "mir_test_framework/declarative_placement_strategy.h"
-
 namespace ms = mir::scene;
 
 namespace mtf = mir_test_framework;
 
-mtf::DeclarativePlacementStrategy::DeclarativePlacementStrategy(
-    SurfaceGeometries const& positions,
-    SurfaceDepths const& depths) :
-    surface_geometries_by_name(positions),
-    surface_depths_by_name(depths)
+mtf::DeclarativePlacementWindowManagerPolicy::DeclarativePlacementWindowManagerPolicy(
+    Tools* const tools,
+    SurfaceGeometries const& positions_by_name,
+    SurfaceDepths const& depths_by_name,
+    std::shared_ptr<mir::shell::DisplayLayout> const& display_layout) :
+    mir::shell::CanonicalWindowManagerPolicy{tools, display_layout},
+    surface_geometries_by_name{positions_by_name},
+    surface_depths_by_name{depths_by_name}
 {
 }
 
-ms::SurfaceCreationParameters mtf::DeclarativePlacementStrategy::place(ms::Session const& /*session*/, ms::SurfaceCreationParameters const& request_parameters)
+ms::SurfaceCreationParameters mtf::DeclarativePlacementWindowManagerPolicy::handle_place_new_surface(
+    std::shared_ptr<mir::scene::Session> const& /*session*/,
+    ms::SurfaceCreationParameters const& request_parameters)
 {
     auto placed = request_parameters;
 

=== modified file 'tests/unit-tests/scene/test_surface_controller.cpp'
--- tests/unit-tests/scene/test_surface_controller.cpp	2015-06-17 05:20:42 +0000
+++ tests/unit-tests/scene/test_surface_controller.cpp	2015-06-22 16:27:03 +0000
@@ -19,7 +19,6 @@
 #include "src/server/scene/surface_controller.h"
 #include "src/server/scene/surface_stack_model.h"
 #include "mir/scene/surface_factory.h"
-#include "mir/scene/placement_strategy.h"
 #include "mir/scene/surface_creation_parameters.h"
 #include "mir_test_doubles/stub_scene_session.h"