Merge lp:~alan-griffiths/mir/death-to-SurfaceConfigurator into lp:mir

[Alan Griffiths]

scene: Remove the useless (and unused) SurfaceConfigurator customization point

scene: Remove the useless (and unused) SurfaceConfigurator customization point

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2445
http://jenkins.qa.ubuntu.com/job/mir-ci/3414/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1899
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1898
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1849
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1411
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1411/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1849
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1849/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4802
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19315

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Cool.

I implemented and almost proposed this a while back, but then found it was used downstream somewhere. Are all downstream users weaned off it yet?

review: Needs Information

[Alexandros Frantzis (afrantzis)]

Looks good. A quick grep shows a reference to the_surface_configurator in qtmir, but it's just a using declaration and the introduced name does not seem to be used, so it should be safe to just delete the declaration.

review: Approve

[Alan Griffiths (alan-griffiths)]

> Cool.
>
> I implemented and almost proposed this a while back, but then found it was
> used downstream somewhere. Are all downstream users weaned off it yet?

Nearly: https://code.launchpad.net/~mir-team/qtmir/compatibility-with-mir-API-changes/+merge/252589 line 161

[Daniel van Vugt (vanvugt)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1289/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1908/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1907/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1858/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-autolanding/469/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1858/console

review: Needs Fixing (continuous-integration)

[lp:~alan-griffiths/mir/death-to-SurfaceConfigurator]

2446. By Alan Griffiths on 2015-04-01

merge lp:mir

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'playground/demo-shell/demo_shell.cpp'
--- playground/demo-shell/demo_shell.cpp	2015-03-31 14:05:53 +0000
+++ playground/demo-shell/demo_shell.cpp	2015-04-01 10:12:47 +0000
@@ -107,8 +107,7 @@
             { return std::make_shared<shell::DefaultWindowManager>(
                 focus_controller,
                 the_placement_strategy(),
-                the_session_coordinator(),
-                the_surface_configurator()); };
+                the_session_coordinator()); };
     }
 
     std::shared_ptr<msh::HostLifecycleEventListener> the_host_lifecycle_event_listener() override

=== modified file 'src/include/server/mir/default_server_configuration.h'
--- src/include/server/mir/default_server_configuration.h	2015-03-31 02:35:42 +0000
+++ src/include/server/mir/default_server_configuration.h	2015-04-01 10:12:47 +0000
@@ -104,7 +104,6 @@
 class SessionCoordinator;
 class SnapshotStrategy;
 class SurfaceCoordinator;
-class SurfaceConfigurator;
 class SurfaceStackModel;
 class SurfaceStack;
 class SceneReport;
@@ -283,7 +282,6 @@
     virtual std::shared_ptr<scene::SurfaceStackModel> the_surface_stack_model();
     virtual std::shared_ptr<scene::SurfaceFactory>    the_surface_factory();
     virtual std::shared_ptr<scene::SurfaceCoordinator>the_surface_coordinator();
-    virtual std::shared_ptr<scene::SurfaceConfigurator> the_surface_configurator();
     /** @} */
 
     /** @name scene configuration - dependencies
@@ -424,7 +422,6 @@
     CachedPtr<scene::PixelBuffer>       pixel_buffer;
     CachedPtr<scene::SnapshotStrategy>  snapshot_strategy;
     CachedPtr<shell::DisplayLayout>     shell_display_layout;
-    CachedPtr<scene::SurfaceConfigurator> surface_configurator;
     CachedPtr<compositor::DisplayBufferCompositorFactory> display_buffer_compositor_factory;
     CachedPtr<compositor::Compositor> compositor;
     CachedPtr<compositor::CompositorReport> compositor_report;

=== removed file 'src/include/server/mir/scene/surface_configurator.h'
--- src/include/server/mir/scene/surface_configurator.h	2015-03-24 15:18:19 +0000
+++ src/include/server/mir/scene/surface_configurator.h	1970-01-01 00:00:00 +0000
@@ -1,54 +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_SCENE_SURFACE_CONFIGURATOR_H_
-#define MIR_SCENE_SURFACE_CONFIGURATOR_H_
-
-#include "mir_toolkit/common.h"
-
-#include <memory>
-
-namespace mir
-{
-
-namespace scene
-{
-class Surface;
-
-class SurfaceConfigurator
-{
-public:
-    virtual ~SurfaceConfigurator() = default;
-
-    /// Returns the selected value.
-    virtual int select_attribute_value(Surface const& surface, MirSurfaceAttrib attrib,
-                                       int requested_value) = 0;
-
-    virtual void attribute_set(Surface const& surface, MirSurfaceAttrib attrib,
-                               int new_value) = 0;
-
-protected:
-    SurfaceConfigurator() = default;
-    SurfaceConfigurator(SurfaceConfigurator const&) = delete;
-    SurfaceConfigurator& operator=(SurfaceConfigurator const&) = delete;
-};
-
-}
-} // namespace mir
-
-#endif // MIR_SCENE_SURFACE_CONFIGURATOR_H_

=== modified file 'src/include/server/mir/shell/default_window_manager.h'
--- src/include/server/mir/shell/default_window_manager.h	2015-03-31 11:18:41 +0000
+++ src/include/server/mir/shell/default_window_manager.h	2015-04-01 10:12:47 +0000
@@ -23,7 +23,7 @@
 
 namespace mir
 {
-namespace scene { class PlacementStrategy; class SurfaceConfigurator; class SessionCoordinator; }
+namespace scene { class PlacementStrategy; class SessionCoordinator; }
 
 namespace shell
 {
@@ -34,8 +34,7 @@
 public:
     explicit DefaultWindowManager(FocusController* focus_controller,
         std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
-        std::shared_ptr<scene::SessionCoordinator> const& session_coordinator,
-        std::shared_ptr<scene::SurfaceConfigurator> const& surface_configurator);
+        std::shared_ptr<scene::SessionCoordinator> const& session_coordinator);
 
     void add_session(std::shared_ptr<scene::Session> const& session) override;
 
@@ -70,7 +69,6 @@
     FocusController* const focus_controller;
     std::shared_ptr<scene::PlacementStrategy> const placement_strategy;
     std::shared_ptr<scene::SessionCoordinator> const session_coordinator;
-    std::shared_ptr<scene::SurfaceConfigurator> const surface_configurator;
 };
 }
 }

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2015-01-21 07:34:50 +0000
+++ src/server/default_server_configuration.cpp	2015-04-01 10:12:47 +0000
@@ -29,7 +29,6 @@
 #include "mir/options/program_option.h"
 #include "mir/frontend/session_credentials.h"
 #include "mir/frontend/session_authorizer.h"
-#include "mir/scene/surface_configurator.h"
 #include "mir/graphics/cursor.h"
 #include "mir/scene/null_session_listener.h"
 #include "mir/graphics/display.h"
@@ -101,25 +100,6 @@
         });
 }
 
-std::shared_ptr<ms::SurfaceConfigurator> mir::DefaultServerConfiguration::the_surface_configurator()
-{
-    struct DefaultSurfaceConfigurator : public ms::SurfaceConfigurator
-    {
-        int select_attribute_value(ms::Surface const&, MirSurfaceAttrib, int requested_value)
-        {
-            return requested_value;
-        }
-        void attribute_set(ms::Surface const&, MirSurfaceAttrib, int)
-        {
-        }
-    };
-    return surface_configurator(
-        [this]()
-        {
-            return std::make_shared<DefaultSurfaceConfigurator>();
-        });
-}
-
 std::shared_ptr<mf::SessionAuthorizer>
 mir::DefaultServerConfiguration::the_session_authorizer()
 {

=== modified file 'src/server/scene/surface_allocator.h'
--- src/server/scene/surface_allocator.h	2015-03-31 02:35:42 +0000
+++ src/server/scene/surface_allocator.h	2015-04-01 10:12:47 +0000
@@ -36,7 +36,6 @@
 {
 class BufferStreamFactory;
 class SceneReport;
-class SurfaceConfigurator;
 
 class SurfaceAllocator : public SurfaceFactory
 {

=== modified file 'src/server/shell/default_window_manager.cpp'
--- src/server/shell/default_window_manager.cpp	2015-03-31 11:18:41 +0000
+++ src/server/shell/default_window_manager.cpp	2015-04-01 10:12:47 +0000
@@ -23,7 +23,6 @@
 #include "mir/scene/session.h"
 #include "mir/scene/session_coordinator.h"
 #include "mir/scene/surface.h"
-#include "mir/scene/surface_configurator.h"
 #include "mir/scene/surface_creation_parameters.h"
 #include "mir/shell/focus_controller.h"
 
@@ -34,12 +33,10 @@
 msh::DefaultWindowManager::DefaultWindowManager(
     FocusController* focus_controller,
     std::shared_ptr<scene::PlacementStrategy> const& placement_strategy,
-    std::shared_ptr<scene::SessionCoordinator> const& session_coordinator,
-    std::shared_ptr<scene::SurfaceConfigurator> const& surface_configurator) :
+    std::shared_ptr<scene::SessionCoordinator> const& session_coordinator) :
     focus_controller{focus_controller},
     placement_strategy{placement_strategy},
-    session_coordinator{session_coordinator},
-    surface_configurator{surface_configurator}
+    session_coordinator{session_coordinator}
 {
 }
 
@@ -138,11 +135,5 @@
     MirSurfaceAttrib attrib,
     int value)
 {
-    auto const configured_value = surface_configurator->select_attribute_value(*surface, attrib, value);
-
-    auto const result = surface->configure(attrib, configured_value);
-
-    surface_configurator->attribute_set(*surface, attrib, result);
-
-    return result;
+    return surface->configure(attrib, value);
 }

=== removed file 'tests/include/mir_test_doubles/mock_surface_configurator.h'
--- tests/include/mir_test_doubles/mock_surface_configurator.h	2015-01-21 07:34:50 +0000
+++ tests/include/mir_test_doubles/mock_surface_configurator.h	1970-01-01 00:00:00 +0000
@@ -1,43 +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_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_
-#define MIR_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_
-
-#include "mir/scene/surface_configurator.h"
-
-#include <gmock/gmock.h>
-
-namespace mir
-{
-namespace test
-{
-namespace doubles
-{
-
-struct MockSurfaceConfigurator : public scene::SurfaceConfigurator
-{
-    MOCK_METHOD3(select_attribute_value, int(scene::Surface const&, MirSurfaceAttrib, int));
-    MOCK_METHOD3(attribute_set, void(scene::Surface const&, MirSurfaceAttrib, int));
-};
-
-}
-}
-}
-
-#endif /* MIR_TEST_DOUBLES_MOCK_SURFACE_CONFIGURATOR_H_ */

=== removed file 'tests/include/mir_test_doubles/null_surface_configurator.h'
--- tests/include/mir_test_doubles/null_surface_configurator.h	2015-01-21 07:34:50 +0000
+++ tests/include/mir_test_doubles/null_surface_configurator.h	1970-01-01 00:00:00 +0000
@@ -1,48 +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_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_
-#define MIR_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_
-
-#include "mir/scene/surface_configurator.h"
-
-namespace mir
-{
-namespace test
-{
-namespace doubles
-{
-
-struct NullSurfaceConfigurator : public scene::SurfaceConfigurator
-{
-    int select_attribute_value(scene::Surface const&,
-                               MirSurfaceAttrib, int requested_value)
-    {
-        return requested_value;
-    }
-    void attribute_set(scene::Surface const&,
-                       MirSurfaceAttrib, int)
-    {
-    }
-};
-
-}
-}
-}
-
-#endif /* MIR_TEST_DOUBLES_NULL_SURFACE_CONFIGURATOR_H_ */

=== modified file 'tests/integration-tests/test_default_window_manager.cpp'
--- tests/integration-tests/test_default_window_manager.cpp	2015-03-31 11:18:41 +0000
+++ tests/integration-tests/test_default_window_manager.cpp	2015-04-01 10:12:47 +0000
@@ -33,7 +33,6 @@
 #include "mir_test_doubles/null_snapshot_strategy.h"
 #include "mir_test_doubles/null_event_sink.h"
 #include "mir_test_doubles/null_session_event_sink.h"
-#include "mir_test_doubles/mock_surface_configurator.h"
 #include "mir_test_doubles/null_prompt_session_manager.h"
 #include "mir_test_doubles/mock_input_targeter.h"
 #include "mir_test_doubles/stub_buffer_stream_factory.h"
@@ -82,8 +81,6 @@
             mt::fake_shared(session_listener)
         };
 
-    NiceMock<mtd::MockSurfaceConfigurator> surface_configurator;
-
     msh::AbstractShell shell{
         mt::fake_shared(input_targeter),
         mt::fake_shared(surface_coordinator),
@@ -93,8 +90,7 @@
             { return std::make_shared<msh::DefaultWindowManager>(
                 focus_controller,
                 std::make_shared<NullPlacementStrategy>(),
-                mt::fake_shared(session_manager),
-                mt::fake_shared(surface_configurator));
+                mt::fake_shared(session_manager));
             }};
 };
 }
@@ -183,49 +179,3 @@
 
     shell.set_focus_to(mt::fake_shared(app), mock_surface);
 }
-
-TEST_F(TestDefaultWindowManager, configurator_selects_attribute_values)
-{
-    mtd::StubSceneSession app;
-    auto const session = mt::fake_shared(app);
-    auto const surface = std::make_shared<NiceMock<mtd::MockSurface>>();
-    ON_CALL(*surface, configure(_, _)).WillByDefault(ReturnArg<1>());
-
-    InSequence seq;
-
-    EXPECT_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_state, mir_surface_state_restored))
-        .WillOnce(Return(mir_surface_state_minimized));
-
-    EXPECT_CALL(*surface, configure(mir_surface_attrib_state, mir_surface_state_minimized));
-
-    EXPECT_CALL(surface_configurator, attribute_set(_, mir_surface_attrib_state, mir_surface_state_minimized));
-
-    EXPECT_THAT(
-        shell.set_surface_attribute(session, surface, mir_surface_attrib_state, mir_surface_state_restored),
-        Eq(mir_surface_state_minimized));
-}
-
-TEST_F(TestDefaultWindowManager, set_surface_attribute_returns_value_set_by_configurator)
-{
-    mtd::StubSceneSession app;
-    auto const session = mt::fake_shared(app);
-    auto const surface = std::make_shared<NiceMock<mtd::MockSurface>>();
-    ON_CALL(*surface, configure(_, _)).WillByDefault(ReturnArg<1>());
-
-    ON_CALL(surface_configurator, select_attribute_value(_, Not(mir_surface_attrib_focus), _))
-        .WillByDefault(ReturnArg<1>());
-
-    ON_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_focus, mir_surface_focused))
-        .WillByDefault(Return(mir_surface_unfocused));
-
-    ON_CALL(surface_configurator, select_attribute_value(_, mir_surface_attrib_focus, Not(mir_surface_focused)))
-        .WillByDefault(Return(mir_surface_focused));
-
-    EXPECT_THAT(
-        shell.set_surface_attribute(session, surface, mir_surface_attrib_focus, mir_surface_focused),
-        Eq(mir_surface_unfocused));
-
-    EXPECT_THAT(
-        shell.set_surface_attribute(session, surface, mir_surface_attrib_focus, mir_surface_unfocused),
-        Eq(mir_surface_focused));
-}

=== modified file 'tests/integration-tests/test_surface_stack_with_compositor.cpp'
--- tests/integration-tests/test_surface_stack_with_compositor.cpp	2015-03-31 02:35:42 +0000
+++ tests/integration-tests/test_surface_stack_with_compositor.cpp	2015-04-01 10:12:47 +0000
@@ -31,7 +31,6 @@
 #include "mir_test_doubles/stub_display_buffer.h"
 #include "mir_test_doubles/stub_buffer.h"
 #include "mir_test_doubles/stub_input_sender.h"
-#include "mir_test_doubles/null_surface_configurator.h"
 #include "mir_test_doubles/null_display_sync_group.h"
 
 #include <condition_variable>

=== modified file 'tests/mir_test_framework/fake_event_hub_server_configuration.cpp'
--- tests/mir_test_framework/fake_event_hub_server_configuration.cpp	2015-03-31 11:18:41 +0000
+++ tests/mir_test_framework/fake_event_hub_server_configuration.cpp	2015-04-01 10:12:47 +0000
@@ -69,7 +69,6 @@
         { return std::make_shared<ms::DefaultWindowManager>(
             focus_controller,
             the_placement_strategy(),
-            the_session_coordinator(),
-            the_surface_configurator()); };
+            the_session_coordinator()); };
 }
 

=== modified file 'tests/unit-tests/scene/test_surface.cpp'
--- tests/unit-tests/scene/test_surface.cpp	2015-03-31 02:35:42 +0000
+++ tests/unit-tests/scene/test_surface.cpp	2015-04-01 10:12:47 +0000
@@ -23,7 +23,6 @@
 #include "src/server/report/null_report_factory.h"
 #include "mir/frontend/event_sink.h"
 #include "mir/scene/surface_creation_parameters.h"
-#include "mir/scene/surface_configurator.h"
 #include "mir/scene/surface_event_source.h"
 #include "mir/input/input_channel.h"
 

=== modified file 'tests/unit-tests/shell/test_default_window_manager.cpp'
--- tests/unit-tests/shell/test_default_window_manager.cpp	2015-03-31 11:18:41 +0000
+++ tests/unit-tests/shell/test_default_window_manager.cpp	2015-04-01 10:12:47 +0000
@@ -34,7 +34,6 @@
 #include "mir_test_doubles/mock_session_listener.h"
 #include "mir_test_doubles/mock_surface.h"
 #include "mir_test_doubles/null_snapshot_strategy.h"
-#include "mir_test_doubles/null_surface_configurator.h"
 #include "mir_test_doubles/null_prompt_session_manager.h"
 #include "mir_test_doubles/stub_input_targeter.h"
 #include "mir_test_doubles/stub_buffer_stream_factory.h"
@@ -127,8 +126,7 @@
             { return std::make_shared<msh::DefaultWindowManager>(
                 focus_controller,
                 mt::fake_shared(placement_strategy),
-                mt::fake_shared(session_manager),
-                std::make_shared<mtd::NullSurfaceConfigurator>());
+                mt::fake_shared(session_manager));
             }};
 
     void SetUp() override