Mir

Merge lp:~alan-griffiths/mir/make-window-management-configurable into lp:mir

make-window-management-configurable
Merge into development-branch

Proposed by Alan Griffiths on 2015-03-20

Status:	Merged
Approved by:	Alan Griffiths on 2015-03-26
Approved revision:	2416
Merged at revision:	2429
Proposed branch:	lp:~alan-griffiths/mir/make-window-management-configurable
Merge into:	lp:mir
Diff against target:	322 lines (+92/-66) 9 files modified examples/server_example_window_management.cpp (+8/-37) include/server/mir/server.h (+4/-0) include/server/mir/shell/abstract_shell.h (+2/-1) include/server/mir/shell/window_manager_builder.h (+37/-0) src/include/server/mir/default_server_configuration.h (+2/-0) src/server/server.cpp (+14/-0) src/server/shell/default_configuration.cpp (+21/-18) src/server/symbols.map (+2/-0) tests/acceptance-tests/test_custom_window_management.cpp (+2/-10)
To merge this branch:	bzr merge lp:~alan-griffiths/mir/make-window-management-configurable
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-03-26
Andreas Pokorny (community)			Approve on 2015-03-25
Daniel van Vugt			Abstain on 2015-03-25
Alexandros Frantzis (community)		2015-03-20	Approve on 2015-03-24
Review via email: mp+253725@code.launchpad.net

Commit Message

shell: Provide a configuration point for the WindowManagement strategy

Description of the Change

shell: Provide a configuration point for the WindowManagement strategy

PS Jenkins bot (ps-jenkins) wrote on 2015-03-21:

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

review: Needs Fixing (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2015-03-24:

Looks good.

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-03-24:

FAILED: Continuous integration, rev:2414
http://jenkins.qa.ubuntu.com/job/mir-ci/3336/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1786
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1785
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1737/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1333/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1737/console

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-24:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-24:

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

review: Approve (continuous-integration)

Daniel van Vugt (vanvugt) wrote on 2015-03-25:

"WindowManager" ideally should not be a class if we're doing a good job. Because window management is just one of many behaviours of a Shell.

But I don't think this proposal makes the situation any worse.

review: Abstain

Andreas Pokorny (andreas-pokorny) wrote on 2015-03-25:

looks good.

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-03-25:

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-26:

review: Needs Fixing (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2015-03-26:

I'm pretty sure that neither failure is introduced by this MP...

> Build timed out (after 60 minutes). Marking the build as failed.
> Build was aborted

https://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4741/console
(mako-05)

> 27 - mir_acceptance_tests.NestedServer.* (SEGFAULT)

https://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1831/console

PS Jenkins bot (ps-jenkins) on 2015-03-26:

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'examples/server_example_window_management.cpp'
 --- examples/server_example_window_management.cpp	2015-03-24 16:02:48 +0000
 +++ examples/server_example_window_management.cpp	2015-03-24 17:20:59 +0000
@@ -115,56 +115,27 @@
  void me::add_window_manager_option_to(Server& server)
+ {
--    server.add_configuration_option(wm_option, wm_description, mir::OptionType::string);
++    server.add_configuration_option(wm_option, wm_description, wm_canonical);
--    server.override_the_shell([&server]()
--        -> std::shared_ptr<msh::Shell>
++    server.override_the_window_manager_builder([&server](msh::FocusController* focus_controller)
++        -> std::shared_ptr<msh::WindowManager>
+         {
              auto const options = server.get_options();
--
--            if (!options->is_set(wm_option))
--                return std::shared_ptr<msh::Shell>{};
--
              auto const selection = options->get<std::string>(wm_option);
--            std::function<std::shared_ptr<msh::WindowManager>(msh::FocusController* focus_controller)> wm_builder;
--
              if (selection == wm_tiling)
+             {
--                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
--                    {
--                        return std::make_shared<TilingWindowManager>(focus_controller);
--                    };
++                return std::make_shared<TilingWindowManager>(focus_controller);
+             }
              else if (selection == wm_fullscreen)
+             {
--                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
--                    {
--                        return std::make_shared<FullscreenWindowManager>(focus_controller, server.the_shell_display_layout());
--                    };
++                return std::make_shared<FullscreenWindowManager>(focus_controller, server.the_shell_display_layout());
+             }
              else if (selection == wm_canonical)
+             {
--                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
--                    {
--                        return std::make_shared<CanonicalWindowManager>(
--                            focus_controller,
--                            server.the_shell_display_layout());
--                    };
++                return std::make_shared<CanonicalWindowManager>(focus_controller, server.the_shell_display_layout());
+             }
--            else
--                throw mir::AbnormalExit("Unknown window manager: " + selection);
--
--
--            auto tmp = std::make_shared<msh::AbstractShell>(
--                server.the_input_targeter(),
--                server.the_surface_coordinator(),
--                server.the_session_coordinator(),
--                server.the_prompt_session_manager(),
--                wm_builder);
--
--            server.the_composite_event_filter()->prepend(tmp);
--
--            return tmp;
++
++            throw mir::AbnormalExit("Unknown window manager: " + selection);
          });
+ }
 === modified file 'include/server/mir/server.h'
 --- include/server/mir/server.h	2015-03-24 16:02:48 +0000
 +++ include/server/mir/server.h	2015-03-24 17:20:59 +0000
@@ -19,6 +19,7 @@
  #ifndef MIR_SERVER_H_
  #define MIR_SERVER_H_
++#include "mir/shell/window_manager_builder.h"
  #include "mir_toolkit/common.h"
  #include <functional>
@@ -254,6 +255,9 @@
      /// Sets an override functor for creating the surface configurator.
      void override_the_surface_configurator(Builder<scene::SurfaceConfigurator> const& surface_configurator_builder);
++    /// Sets an override functor for creating the window manager.
++    void override_the_window_manager_builder(shell::WindowManagerBuilder const wmb);
++
      /// Each of the wrap functions takes a wrapper functor of the same form
      template<typename T> using Wrapper = std::function<std::shared_ptr<T>(std::shared_ptr<T> const&)>;
 === modified file 'include/server/mir/shell/abstract_shell.h'
 --- include/server/mir/shell/abstract_shell.h	2015-03-24 16:02:48 +0000
 +++ include/server/mir/shell/abstract_shell.h	2015-03-24 17:20:59 +0000
@@ -20,6 +20,7 @@
  #define MIR_SHELL_ABSTRACT_SHELL_H_
  #include "mir/shell/shell.h"
++#include "mir/shell/window_manager_builder.h"
  #include <mutex>
@@ -38,7 +39,7 @@
          std::shared_ptr<scene::SurfaceCoordinator> const& surface_coordinator,
          std::shared_ptr<scene::SessionCoordinator> const& session_coordinator,
          std::shared_ptr<scene::PromptSessionManager> const& prompt_session_manager,
--        std::function<std::shared_ptr<WindowManager>(FocusController* focus_controller)> const& wm_builder);
++        WindowManagerBuilder const& wm_builder);
      ~AbstractShell() noexcept;
 === added file 'include/server/mir/shell/window_manager_builder.h'
 --- include/server/mir/shell/window_manager_builder.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/shell/window_manager_builder.h	2015-03-24 17:20:59 +0000
@@ -0,0 +1,37 @@
++/*
++ * Copyright © 2015 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: Alan Griffiths <alan@octopull.co.uk>
++ */
++
++#ifndef INCLUDE_SERVER_MIR_SHELL_WINDOW_MANAGER_BUILDER_H_
++#define INCLUDE_SERVER_MIR_SHELL_WINDOW_MANAGER_BUILDER_H_
++
++#include <functional>
++#include <memory>
++
++namespace mir
++{
++namespace shell
++{
++class WindowManager;
++class FocusController;
++
++/// WindowManagers are built while initializing an AbstractShell, so a builder functor is needed
++using WindowManagerBuilder =
++    std::function<std::shared_ptr<WindowManager>(FocusController* focus_controller)>;
++}
++}
++#endif /* INCLUDE_SERVER_MIR_SHELL_WINDOW_MANAGER_BUILDER_H_ */
 === modified file 'src/include/server/mir/default_server_configuration.h'
 --- src/include/server/mir/default_server_configuration.h	2015-03-24 16:02:48 +0000
 +++ src/include/server/mir/default_server_configuration.h	2015-03-24 17:20:59 +0000
@@ -20,6 +20,7 @@
  #include "mir/cached_ptr.h"
  #include "mir/server_configuration.h"
++#include "mir/shell/window_manager_builder.h"
  #include <memory>
  #include <string>
@@ -251,6 +252,7 @@
       * configurable interfaces for modifying shell
       *  @{ */
      virtual auto the_shell() -> std::shared_ptr<shell::Shell>;
++    virtual auto the_window_manager_builder() -> shell::WindowManagerBuilder;
      virtual std::shared_ptr<scene::PlacementStrategy>   the_placement_strategy();
      virtual std::shared_ptr<scene::SessionListener>     the_session_listener();
      virtual std::shared_ptr<shell::DisplayLayout>       the_shell_display_layout();
 === modified file 'src/server/server.cpp'
 --- src/server/server.cpp	2015-03-24 16:02:48 +0000
 +++ src/server/server.cpp	2015-03-24 17:20:59 +0000
@@ -119,6 +119,8 @@
      FOREACH_OVERRIDE(MIR_SERVER_BUILDER)
++    shell::WindowManagerBuilder wmb;
++
      FOREACH_WRAPPER(MIR_SERVER_WRAPPER)
  };
@@ -209,6 +211,12 @@
      FOREACH_WRAPPER(MIR_SERVER_CONFIG_WRAP)
++    auto the_window_manager_builder() -> shell::WindowManagerBuilder override
++    {
++        if (self->wmb) return self->wmb;
++        return mir::DefaultServerConfiguration::the_window_manager_builder();
++    }
++
      Self* const self;
  };
@@ -450,6 +458,12 @@
  #undef MIR_SERVER_OVERRIDE
++void mir::Server::override_the_window_manager_builder(shell::WindowManagerBuilder const wmb)
++{
++    verify_setting_allowed(self->server_config);
++    self->wmb = wmb;
++}
++
  #define MIR_SERVER_WRAP(name)\
  void mir::Server::wrap_##name(decltype(Self::name##_wrapper) const& value)\
  {\
 === modified file 'src/server/shell/default_configuration.cpp'
 --- src/server/shell/default_configuration.cpp	2015-03-24 16:02:48 +0000
 +++ src/server/shell/default_configuration.cpp	2015-03-24 17:20:59 +0000
@@ -22,6 +22,7 @@
  #include "default_placement_strategy.h"
  #include "default_window_manager.h"
++#include "mir/input/composite_event_filter.h"
  #include "mir/shell/abstract_shell.h"
  #include "frontend_shell.h"
  #include "graphics_display_layout.h"
@@ -35,27 +36,29 @@
+ {
      return shell([this]
+         {
--            auto const input_targeter = the_input_targeter();
--            auto const surface_coordinator = the_surface_coordinator();
--            auto const session_coordinator = the_session_coordinator();
--            auto const prompt_session_manager = the_prompt_session_manager();
--
--            auto const builder = [&](msh::FocusController* focus_controller)
--                { return std::make_shared<msh::DefaultWindowManager>(
--                    focus_controller,
--                    the_placement_strategy(),
--                    session_coordinator,
--                    the_surface_configurator()); };
--
--            return wrap_shell(std::make_shared<msh::AbstractShell>(
--                input_targeter,
--                surface_coordinator,
--                session_coordinator,
--                prompt_session_manager,
--                builder));
++            auto const result = wrap_shell(std::make_shared<msh::AbstractShell>(
++                the_input_targeter(),
++                the_surface_coordinator(),
++                the_session_coordinator(),
++                the_prompt_session_manager(),
++                the_window_manager_builder()));
++
++            the_composite_event_filter()->prepend(result);
++
++            return result;
          });
+ }
++auto mir::DefaultServerConfiguration::the_window_manager_builder() -> shell::WindowManagerBuilder
++{
++    return [this](msh::FocusController* focus_controller)
++        { return std::make_shared<msh::DefaultWindowManager>(
++            focus_controller,
++            the_placement_strategy(),
++            the_session_coordinator(),
++            the_surface_configurator()); };
++}
++
  auto mir::DefaultServerConfiguration::wrap_shell(std::shared_ptr<msh::Shell> const& wrapped) -> std::shared_ptr<msh::Shell>
+ {
      return wrapped;
 === modified file 'src/server/symbols.map'
 --- src/server/symbols.map	2015-03-24 16:02:48 +0000
 +++ src/server/symbols.map	2015-03-24 17:20:59 +0000
@@ -152,6 +152,7 @@
      mir::Server::override_the_session_mediator_report*;
      mir::Server::override_the_shell*;
      mir::Server::override_the_surface_configurator*;
++    mir::Server::override_the_window_manager_builder*;
      mir::Server::run*;
      mir::Server::Server*;
      mir::Server::set_command_line*;
@@ -597,6 +598,7 @@
      mir::DefaultServerConfiguration::the_surface_factory*;
      mir::DefaultServerConfiguration::the_surface_stack_model*;
      mir::DefaultServerConfiguration::the_touch_visualizer*;
++    mir::DefaultServerConfiguration::the_window_manager_builder*;
      mir::DefaultServerConfiguration::wrap_cursor_listener*;
      mir::DefaultServerConfiguration::wrap_display_buffer_compositor_factory*;
      mir::DefaultServerConfiguration::wrap_display_configuration_policy*;
 === modified file 'tests/acceptance-tests/test_custom_window_management.cpp'
 --- tests/acceptance-tests/test_custom_window_management.cpp	2015-03-24 16:02:48 +0000
 +++ tests/acceptance-tests/test_custom_window_management.cpp	2015-03-24 17:20:59 +0000
@@ -16,7 +16,6 @@
   * Authored by: Alan Griffiths <alan@octopull.co.uk>
   */
--#include "mir/shell/abstract_shell.h"
  #include "mir/geometry/rectangle.h"
  #include "mir/scene/session.h"
@@ -105,15 +104,8 @@
          initial_display_layout(display_geometry);
--        server.override_the_shell([this]
--           {
--                return std::make_shared<msh::AbstractShell>(
--                    server.the_input_targeter(),
--                    server.the_surface_coordinator(),
--                    server.the_session_coordinator(),
--                    server.the_prompt_session_manager(),
--                    [this](msh::FocusController*) { return mt::fake_shared(window_manager); });
--           });
++        server.override_the_window_manager_builder([this]
++            (msh::FocusController*) { return mt::fake_shared(window_manager); });
+     }
      void TearDown() override