Mir

Merge lp:~alan-griffiths/mir/integrate-generic-shell into lp:mir

integrate-generic-shell
Merge into development-branch

Proposed by Alan Griffiths on 2015-03-03

Status:	Merged
Approved by:	Alan Griffiths on 2015-03-06
Approved revision:	2365
Merged at revision:	2375
Proposed branch:	lp:~alan-griffiths/mir/integrate-generic-shell
Merge into:	lp:mir
Prerequisite:	lp:~alan-griffiths/mir/integrate-focus-controller
Diff against target:	832 lines (+523/-56) 10 files modified examples/CMakeLists.txt (+0/-1) examples/server_example_basic_window_manager.h (+4/-5) examples/server_example_window_management.cpp (+5/-5) include/server/mir/shell/generic_shell.h (+10/-13) include/server/mir/shell/window_manager.h (+7/-9) src/server/shell/CMakeLists.txt (+1/-0) src/server/shell/generic_shell.cpp (+19/-23) src/server/symbols.map (+10/-0) tests/unit-tests/scene/CMakeLists.txt (+1/-0) tests/unit-tests/scene/test_generic_shell.cpp (+466/-0)
To merge this branch:	bzr merge lp:~alan-griffiths/mir/integrate-generic-shell
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alexandros Frantzis (community)			Approve on 2015-03-05
PS Jenkins bot	continuous-integration		Approve on 2015-03-05
Robert Carr (community)		2015-03-03	Approve on 2015-03-04
Review via email: mp+251581@code.launchpad.net

Commit Message

examples, shell: integrate the GenericShell support for writing window management policies.

Description of the Change

examples, shell: integrate the GenericShell support for writing window management policies.

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

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

review: Approve (continuous-integration)

lp:~alan-griffiths/mir/integrate-generic-shell updated on 2015-03-04

2363. By Alan Griffiths on 2015-03-04: merge lp:mir

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

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

review: Approve (continuous-integration)

Robert Carr (robertcarr) wrote on 2015-03-04:

LGTM

review: Approve

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

Looking at GenericShell, it seem to have very little logic in it, mostly forwarding calls to the WindowManager interface. It makes me wonder if the WindowManager/GenericShell is really needed and ifusers would be happy enough with AbstractShell.

98 #define MIR_EXAMPLE_GENERIC_SHELL_H_

Needs update.

Needs discussion.

review: Needs Information

lp:~alan-griffiths/mir/integrate-generic-shell updated on 2015-03-05

2364. By Alan Griffiths on 2015-03-05: Update guard in moved header

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

> Looking at GenericShell, it seem to have very little logic in it, mostly
> forwarding calls to the WindowManager interface. It makes me wonder if the
> WindowManager/GenericShell is really needed and ifusers would be happy enough
> with AbstractShell.

I contemplated rolling AbstractShell and GenericShell into one and using WindowManager as the customization point - the only real impediment to doing that is DefaultShell. (U8 makes trivial use of AbstractShell to avoid DefaultShell's behavior but I don't see that being hard to port.)

Given that DefaultShell has window management behavior that could only be described as perverse I'd like like to get rid of it as soon it is possible to change USC (and our examples) to something based on WindowManager.

> 98 #define MIR_EXAMPLE_GENERIC_SHELL_H_

Fixed.

lp:~alan-griffiths/mir/integrate-generic-shell updated on 2015-03-05

2365. By Alan Griffiths on 2015-03-05: merge lp:mir

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

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

review: Approve (continuous-integration)

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

I am not convinced that GenericShell/WindowManager is the right abstraction to expose, since the WindowManager interface is too strongly coupled conceptually to Shell, and, as mentioned above, GenericShell doesn't contain much logic. The GenericShell brings some convenience, but I am not sure the extra layer of abstraction it adds is worth it vs shell implementers just subclassing AbstractShell.

Alan mentioned in IRC an alternative approach in which WindowManager is exposed and *Shell classes are internal. I like this idea and I think it's worth pursuing it further. My only concern is that the WindowManager interface won't be enough for clients that have other shell needs (e.g. need to handle prompt sessions specially), but my experience in that area is limited.

All that being said, this MP seems like a reasonable starting point for further investigation and experimentation.

review: Approve

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

I would have been happy to take on board a different approach, but given the silence around here lets land and see where we can refactor to before the next release.

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/CMakeLists.txt'
 --- examples/CMakeLists.txt	2015-03-05 04:24:05 +0000
 +++ examples/CMakeLists.txt	2015-03-05 13:54:59 +0000
@@ -17,7 +17,6 @@
  add_library(exampleserverconfig STATIC
    server_example_canonical_window_manager.cpp
--  server_example_generic_shell.cpp
    server_example_display_configuration_policy.cpp
    server_example_input_event_filter.cpp
    server_example_log_options.cpp
 === modified file 'examples/server_example_basic_window_manager.h'
 --- examples/server_example_basic_window_manager.h	2015-03-05 05:47:28 +0000
 +++ examples/server_example_basic_window_manager.h	2015-03-05 13:54:59 +0000
@@ -19,11 +19,10 @@
  #ifndef MIR_EXAMPLE_BASIC_WINDOW_MANAGER_H_
  #define MIR_EXAMPLE_BASIC_WINDOW_MANAGER_H_
--#include "server_example_generic_shell.h"
--
  #include "mir/geometry/rectangles.h"
  #include "mir/scene/session.h"
  #include "mir/scene/surface_creation_parameters.h"
++#include "mir/shell/generic_shell.h"
  #include <map>
  #include <mutex>
@@ -105,7 +104,7 @@
  ///
  /// \tparam SurfaceInfo must be constructable from (std::shared_ptr<ms::Session>, std::shared_ptr<ms::Surface>)
  template<typename WindowManagementPolicy, typename SessionInfo, typename SurfaceInfo>
--class BasicWindowManager : public WindowManager,
++class BasicWindowManager : public shell::WindowManager,
      private BasicWindowManagerTools<SessionInfo, SurfaceInfo>
+ {
  public:
@@ -148,8 +147,8 @@
+     }
      void remove_surface(
--        std::weak_ptr<scene::Surface> const& surface,
--        std::shared_ptr<scene::Session> const& session) override
++        std::shared_ptr<scene::Session> const& session,
++        std::weak_ptr<scene::Surface> const& surface) override
+     {
          std::lock_guard<decltype(mutex)> lock(mutex);
          policy.handle_delete_surface(session, surface);
 === modified file 'examples/server_example_window_management.cpp'
 --- examples/server_example_window_management.cpp	2015-03-05 04:24:05 +0000
 +++ examples/server_example_window_management.cpp	2015-03-05 13:54:59 +0000
@@ -127,25 +127,25 @@
              auto const selection = options->get<std::string>(wm_option);
--            std::function<std::shared_ptr<WindowManager>(msh::FocusController* focus_controller)> wm_builder;
++            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<WindowManager>
++                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
+                     {
                          return std::make_shared<TilingWindowManager>(focus_controller);
                      };
+             }
              else if (selection == wm_fullscreen)
+             {
--                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<WindowManager>
++                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
+                     {
                          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<WindowManager>
++                wm_builder = [&server](msh::FocusController* focus_controller) -> std::shared_ptr<msh::WindowManager>
+                     {
                          return std::make_shared<CanonicalWindowManager>(focus_controller);
                      };
@@ -154,7 +154,7 @@
                  throw mir::AbnormalExit("Unknown window manager: " + selection);
--            auto tmp = std::make_shared<GenericShell>(
++            auto tmp = std::make_shared<msh::GenericShell>(
                  server.the_input_targeter(),
                  server.the_surface_coordinator(),
                  server.the_session_coordinator(),
 === renamed file 'examples/server_example_generic_shell.h' => 'include/server/mir/shell/generic_shell.h'
 --- examples/server_example_generic_shell.h	2015-03-05 05:47:28 +0000
 +++ include/server/mir/shell/generic_shell.h	2015-03-05 13:54:59 +0000
@@ -16,28 +16,25 @@
   * Authored By: Alan Griffiths <alan@octopull.co.uk>
   */
--#ifndef MIR_EXAMPLE_GENERIC_SHELL_H_
--#define MIR_EXAMPLE_GENERIC_SHELL_H_
--
--#include "server_example_window_manager.h"
++#ifndef MIR_SHELL_GENERIC_SHELL_H_
++#define MIR_SHELL_GENERIC_SHELL_H_
  #include "mir/shell/abstract_shell.h"
--
--///\example server_example_generic_shell.h
--/// A generic shell that supports a window manager
++#include "mir/shell/window_manager.h"
  namespace mir
+ {
  namespace geometry { class Point; }
--namespace examples
++namespace shell
+ {
--class GenericShell : public virtual shell::Shell, public virtual shell::FocusController,
--    private shell::AbstractShell
++/// A generic shell that supports a window manager
++class GenericShell : public virtual Shell, public virtual FocusController,
++    private AbstractShell
+ {
  public:
      GenericShell(
--        std::shared_ptr<shell::InputTargeter> const& input_targeter,
++        std::shared_ptr<InputTargeter> const& input_targeter,
          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,
@@ -62,13 +59,13 @@
          MirSurfaceAttrib attrib,
          int value) override;
--private:
      void add_display(geometry::Rectangle const& area) override;
      void remove_display(geometry::Rectangle const& area) override;
++private:
      std::shared_ptr<WindowManager> const window_manager;
  };
+ }
+ }
--#endif /* MIR_EXAMPLE_GENERIC_SHELL_H_ */
++#endif /* MIR_SHELL_GENERIC_SHELL_H_ */
 === renamed file 'examples/server_example_window_manager.h' => 'include/server/mir/shell/window_manager.h'
 --- examples/server_example_window_manager.h	2015-02-27 15:23:05 +0000
 +++ include/server/mir/shell/window_manager.h	2015-03-05 13:54:59 +0000
@@ -16,8 +16,8 @@
   * Authored By: Alan Griffiths <alan@octopull.co.uk>
   */
--#ifndef MIR_EXAMPLE_WINDOW_MANAGER_H_
--#define MIR_EXAMPLE_WINDOW_MANAGER_H_
++#ifndef MIR_SHELL_WINDOW_MANAGER_H_
++#define MIR_SHELL_WINDOW_MANAGER_H_
  #include "mir/frontend/surface_id.h"
  #include "mir_toolkit/common.h"
@@ -25,15 +25,13 @@
  #include <memory>
--///\example server_example_window_manager.h
--/// A window manager interface
--
  namespace mir
+ {
  namespace geometry { class Rectangle; }
  namespace scene { class Session; class Surface; class SurfaceCreationParameters; }
--namespace examples
++namespace shell
+ {
++/// interface to provide window management logic
  class WindowManager
+ {
  public:
@@ -47,8 +45,8 @@
          std::function<frontend::SurfaceId(std::shared_ptr<scene::Session> const& session, scene::SurfaceCreationParameters const& params)> const& build) = 0;
      virtual void remove_surface(
--        std::weak_ptr<scene::Surface> const& surface,
--        std::shared_ptr<scene::Session> const& session) = 0;
++        std::shared_ptr<scene::Session> const& session,
++        std::weak_ptr<scene::Surface> const& surface) = 0;
      virtual void add_display(geometry::Rectangle const& area) = 0;
@@ -70,4 +68,4 @@
+ }
+ }
--#endif /* MIR_EXAMPLE_WINDOW_MANAGER_H_ */
++#endif /* MIR_SHELL_WINDOW_MANAGER_H_ */
 === modified file 'src/server/shell/CMakeLists.txt'
 --- src/server/shell/CMakeLists.txt	2015-02-18 05:27:28 +0000
 +++ src/server/shell/CMakeLists.txt	2015-03-05 13:54:59 +0000
@@ -4,6 +4,7 @@
    abstract_shell.cpp
    default_placement_strategy.cpp
    frontend_shell.cpp
++  generic_shell.cpp
    graphics_display_layout.cpp
    default_configuration.cpp
    default_shell.cpp
 === renamed file 'examples/server_example_generic_shell.cpp' => 'src/server/shell/generic_shell.cpp'
 --- examples/server_example_generic_shell.cpp	2015-03-05 05:47:28 +0000
 +++ src/server/shell/generic_shell.cpp	2015-03-05 13:54:59 +0000
@@ -16,66 +16,62 @@
   * Authored By: Alan Griffiths <alan@octopull.co.uk>
   */
--#include "server_example_generic_shell.h"
++#include "mir/shell/generic_shell.h"
  #include "mir/geometry/point.h"
  #include "mir/scene/session.h"
  #include "mir/scene/surface_coordinator.h"
  #include "mir/scene/surface_creation_parameters.h"
--namespace me = mir::examples;
  namespace mf = mir::frontend;
  namespace ms = mir::scene;
  namespace msh = mir::shell;
--///\example server_example_generic_shell.cpp
--/// A shell that accepts a WindowManager
--
--me::GenericShell::GenericShell(
--    std::shared_ptr<msh::InputTargeter> const& input_targeter,
++msh::GenericShell::GenericShell(
++    std::shared_ptr<InputTargeter> const& input_targeter,
      std::shared_ptr<ms::SurfaceCoordinator> const& surface_coordinator,
      std::shared_ptr<ms::SessionCoordinator> const& session_coordinator,
      std::shared_ptr<ms::PromptSessionManager> const& prompt_session_manager,
--    std::function<std::shared_ptr<WindowManager>(FocusController* focus_controller)> const& wm_builder) :
++    std::function<std::shared_ptr<shell::WindowManager>(FocusController* focus_controller)> const& wm_builder) :
      AbstractShell(input_targeter, surface_coordinator, session_coordinator, prompt_session_manager),
      window_manager(wm_builder(this))
+ {
+ }
--std::shared_ptr<ms::Session> me::GenericShell::open_session(
++std::shared_ptr<ms::Session> msh::GenericShell::open_session(
      pid_t client_pid,
      std::string const& name,
      std::shared_ptr<mf::EventSink> const& sink)
+ {
--    auto const result = msh::AbstractShell::open_session(client_pid, name, sink);
++    auto const result = AbstractShell::open_session(client_pid, name, sink);
      window_manager->add_session(result);
      return result;
+ }
--void me::GenericShell::close_session(std::shared_ptr<ms::Session> const& session)
++void msh::GenericShell::close_session(std::shared_ptr<ms::Session> const& session)
+ {
      window_manager->remove_session(session);
--    msh::AbstractShell::close_session(session);
++    AbstractShell::close_session(session);
+ }
--auto me::GenericShell::create_surface(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)
++auto msh::GenericShell::create_surface(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)
  -> mf::SurfaceId
+ {
      auto const build = [this](std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& placed_params)
+         {
--            return msh::AbstractShell::create_surface(session, placed_params);
++            return AbstractShell::create_surface(session, placed_params);
          };
      return window_manager->add_surface(session, params, build);
+ }
--void me::GenericShell::destroy_surface(std::shared_ptr<ms::Session> const& session, mf::SurfaceId surface)
++void msh::GenericShell::destroy_surface(std::shared_ptr<ms::Session> const& session, mf::SurfaceId surface)
+ {
--    window_manager->remove_surface(session->surface(surface), session);
--    msh::AbstractShell::destroy_surface(session, surface);
++    window_manager->remove_surface(session, session->surface(surface));
++    AbstractShell::destroy_surface(session, surface);
+ }
--bool me::GenericShell::handle(MirEvent const& event)
++bool msh::GenericShell::handle(MirEvent const& event)
+ {
      if (mir_event_get_type(&event) != mir_event_type_input)
          return false;
@@ -97,7 +93,7 @@
      return false;
+ }
--int me::GenericShell::set_surface_attribute(
++int msh::GenericShell::set_surface_attribute(
      std::shared_ptr<ms::Session> const& session,
      std::shared_ptr<ms::Surface> const& surface,
      MirSurfaceAttrib attrib,
@@ -108,19 +104,19 @@
      case mir_surface_attrib_state:
+     {
          auto const state = window_manager->handle_set_state(surface, MirSurfaceState(value));
--        return msh::AbstractShell::set_surface_attribute(session, surface, attrib, state);
++        return AbstractShell::set_surface_attribute(session, surface, attrib, state);
+     }
      default:
--        return msh::AbstractShell::set_surface_attribute(session, surface, attrib, value);
++        return AbstractShell::set_surface_attribute(session, surface, attrib, value);
+     }
+ }
--void me::GenericShell::add_display(geometry::Rectangle const& area)
++void msh::GenericShell::add_display(geometry::Rectangle const& area)
+ {
      window_manager->add_display(area);
+ }
--void me::GenericShell::remove_display(geometry::Rectangle const& area)
++void msh::GenericShell::remove_display(geometry::Rectangle const& area)
+ {
      window_manager->remove_display(area);
+ }
 === modified file 'src/server/symbols.map'
 --- src/server/symbols.map	2015-03-05 05:47:28 +0000
 +++ src/server/symbols.map	2015-03-05 13:54:59 +0000
@@ -392,6 +392,16 @@
      mir::shell::FocusController::focused_session*;
      mir::shell::FocusController::operator*;
      mir::shell::FocusController::set_focus_to*;
++    mir::shell::FocusController::surface_at*;
++    mir::shell::GenericShell::add_display*;
++    mir::shell::GenericShell::close_session*;
++    mir::shell::GenericShell::create_surface*;
++    mir::shell::GenericShell::destroy_surface*;
++    mir::shell::GenericShell::GenericShell*;
++    mir::shell::GenericShell::handle*;
++    mir::shell::GenericShell::open_session*;
++    mir::shell::GenericShell::remove_display*;
++    mir::shell::GenericShell::set_surface_attribute*;
      mir::shell::HostLifecycleEventListener::?HostLifecycleEventListener*;
      mir::shell::HostLifecycleEventListener::HostLifecycleEventListener*;
      mir::shell::HostLifecycleEventListener::lifecycle_event_occurred*;
 === modified file 'tests/unit-tests/scene/CMakeLists.txt'
 --- tests/unit-tests/scene/CMakeLists.txt	2015-03-05 05:47:28 +0000
 +++ tests/unit-tests/scene/CMakeLists.txt	2015-03-05 13:54:59 +0000
@@ -16,6 +16,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_surface_impl.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_basic_surface.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_default_shell.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_generic_shell.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_surface_stack.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_surface_controller.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_legacy_scene_change_notification.cpp
 === added file 'tests/unit-tests/scene/test_generic_shell.cpp'
 --- tests/unit-tests/scene/test_generic_shell.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/scene/test_generic_shell.cpp	2015-03-05 13:54:59 +0000
@@ -0,0 +1,466 @@
++/*
++ * 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.griffiths@canonical.com>
++ */
++
++#include "mir/shell/generic_shell.h"
++#include "mir/shell/window_manager.h"
++
++#include "mir/events/event_builders.h"
++#include "mir/scene/session.h"
++#include "mir/scene/null_session_listener.h"
++#include "mir/scene/surface_creation_parameters.h"
++
++#include "src/server/scene/default_session_container.h"
++#include "src/server/scene/session_event_sink.h"
++#include "src/server/scene/session_manager.h"
++
++#include "mir_test_doubles/mock_surface_coordinator.h"
++#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_prompt_session_manager.h"
++#include "mir_test_doubles/stub_input_targeter.h"
++
++#include "mir_test/fake_shared.h"
++
++#include <gmock/gmock.h>
++#include <gtest/gtest.h>
++
++namespace mf = mir::frontend;
++namespace mi = mir::input;
++namespace ms = mir::scene;
++namespace msh = mir::shell;
++namespace geom = mir::geometry;
++
++namespace mt = mir::test;
++namespace mtd = mir::test::doubles;
++using namespace ::testing;
++
++namespace
++{
++MATCHER_P(WeakPtrTo, p, "")
++{
++  return !arg.owner_before(p) && !p.owner_before(arg);
++}
++
++struct MockSessionContainer : public ms::SessionContainer
++{
++    MOCK_METHOD1(insert_session, void(std::shared_ptr<ms::Session> const&));
++    MOCK_METHOD1(remove_session, void(std::shared_ptr<ms::Session> const&));
++    MOCK_CONST_METHOD1(successor_of, std::shared_ptr<ms::Session>(std::shared_ptr<ms::Session> const&));
++    MOCK_CONST_METHOD1(for_each, void(std::function<void(std::shared_ptr<ms::Session> const&)>));
++    MOCK_METHOD0(lock, void());
++    MOCK_METHOD0(unlock, void());
++    ~MockSessionContainer() noexcept {}
++};
++
++struct MockSessionEventSink : public ms::SessionEventSink
++{
++    MOCK_METHOD1(handle_focus_change, void(std::shared_ptr<ms::Session> const& session));
++    MOCK_METHOD0(handle_no_focus, void());
++    MOCK_METHOD1(handle_session_stopping, void(std::shared_ptr<ms::Session> const& session));
++};
++
++struct MockSessionManager : ms::SessionManager
++{
++    using ms::SessionManager::SessionManager;
++
++    MOCK_METHOD1(set_focus_to, void (std::shared_ptr<ms::Session> const& focus));
++
++    void unmocked_set_focus_to(std::shared_ptr<ms::Session> const& focus)
++    { ms::SessionManager::set_focus_to(focus); }
++};
++
++struct MockWindowManager : msh::WindowManager
++{
++    MockWindowManager()
++    {
++        ON_CALL(*this, add_surface(_,_,_)).WillByDefault(Invoke(
++            [](std::shared_ptr<ms::Session> const& session,
++                ms::SurfaceCreationParameters const& params,
++                std::function<mf::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& build)
++                { return build(session, params); }));
++    }
++
++    MOCK_METHOD1(add_session, void (std::shared_ptr<ms::Session> const&));
++    MOCK_METHOD1(remove_session, void (std::shared_ptr<ms::Session> const&));
++
++    MOCK_METHOD3(add_surface, mf::SurfaceId(
++        std::shared_ptr<ms::Session> const& session,
++        ms::SurfaceCreationParameters const& params,
++        std::function<mf::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& build));
++
++    MOCK_METHOD2(remove_surface, void(std::shared_ptr<ms::Session> const&, std::weak_ptr<ms::Surface> const&));
++
++    MOCK_METHOD1(add_display, void(geom::Rectangle const&));
++    MOCK_METHOD1(remove_display, void(geom::Rectangle const&));
++
++    MOCK_METHOD1(handle_key_event, bool(MirKeyInputEvent const*));
++    MOCK_METHOD1(handle_touch_event, bool(MirTouchInputEvent const*));
++    MOCK_METHOD1(handle_pointer_event, bool(MirPointerInputEvent const*));
++
++    MOCK_METHOD2(handle_set_state, int(std::shared_ptr<ms::Surface> const&, MirSurfaceState value));
++};
++
++using NiceMockWindowManager = NiceMock<MockWindowManager>;
++
++struct GenericShell : Test
++{
++    NiceMock<mtd::MockSurface> mock_surface;
++    NiceMock<mtd::MockSurfaceCoordinator> surface_coordinator;
++    NiceMock<MockSessionContainer> session_container;
++    NiceMock<MockSessionEventSink> session_event_sink;
++    NiceMock<mtd::MockSessionListener> session_listener;
++
++    NiceMock<MockSessionManager> session_manager{
++        mt::fake_shared(surface_coordinator),
++        mt::fake_shared(session_container),
++        std::make_shared<mtd::NullSnapshotStrategy>(),
++        mt::fake_shared(session_event_sink),
++        mt::fake_shared(session_listener)};
++
++    mtd::StubInputTargeter input_targeter;
++    std::shared_ptr<MockWindowManager> wm;
++
++    msh::GenericShell shell{
++        mt::fake_shared(input_targeter),
++        mt::fake_shared(surface_coordinator),
++        mt::fake_shared(session_manager),
++        std::make_shared<mtd::NullPromptSessionManager>(),
++        [this](msh::FocusController*) { return wm = std::make_shared<NiceMockWindowManager>(); }};
++
++    void SetUp() override
++    {
++        ON_CALL(session_container, successor_of(_)).WillByDefault(Return((std::shared_ptr<ms::Session>())));
++        ON_CALL(session_manager, set_focus_to(_)).
++            WillByDefault(Invoke(&session_manager, &MockSessionManager::unmocked_set_focus_to));
++        ON_CALL(surface_coordinator, add_surface(_,_))
++            .WillByDefault(Return(mt::fake_shared(mock_surface)));
++    }
++};
++}
++
++TEST_F(GenericShell, open_session_adds_session_to_window_manager)
++{
++    std::shared_ptr<ms::Session> new_session;
++
++    InSequence s;
++    EXPECT_CALL(session_container, insert_session(_)).Times(1);
++    EXPECT_CALL(*wm, add_session(_)).WillOnce(SaveArg<0>(&new_session));
++
++    auto session = shell.open_session(__LINE__, "Visual Basic Studio", std::shared_ptr<mf::EventSink>());
++    EXPECT_EQ(session, new_session);
++}
++
++TEST_F(GenericShell, close_session_removes_session_from_window_manager)
++{
++    std::shared_ptr<ms::Session> old_session;
++
++    InSequence s;
++    EXPECT_CALL(session_container, insert_session(_));
++    EXPECT_CALL(*wm, remove_session(_)).WillOnce(SaveArg<0>(&old_session));
++
++    auto session = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    shell.close_session(session);
++    EXPECT_EQ(session, old_session);
++}
++
++TEST_F(GenericShell, close_session_notifies_session_event_sink)
++{
++    auto session = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    auto session1 = shell.open_session(__LINE__, "Bla", std::shared_ptr<mf::EventSink>());
++
++    InSequence s;
++    EXPECT_CALL(session_event_sink, handle_session_stopping(session1));
++    EXPECT_CALL(session_event_sink, handle_session_stopping(session));
++
++    shell.close_session(session1);
++    shell.close_session(session);
++}
++
++TEST_F(GenericShell, create_surface_provides_create_parameters_to_window_manager)
++{
++    std::shared_ptr<ms::Session> session =
++        shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++
++    auto params = ms::a_surface();
++
++    EXPECT_CALL(*wm, add_surface(session, Ref(params), _));
++
++    shell.create_surface(session, params);
++}
++
++TEST_F(GenericShell, create_surface_allows_window_manager_to_set_create_parameters)
++{
++    std::shared_ptr<ms::Session> const session =
++        shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++
++    auto params = ms::a_surface();
++    auto placed_params = params;
++    placed_params.size.width = geom::Width{100};
++
++    EXPECT_CALL(surface_coordinator, add_surface(placed_params, _));
++
++    EXPECT_CALL(*wm, add_surface(session, Ref(params), _)).WillOnce(Invoke(
++        [&](std::shared_ptr<ms::Session> const& session,
++            ms::SurfaceCreationParameters const&,
++            std::function<mf::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const&)> const& build)
++            { return build(session, placed_params); }));
++
++    shell.create_surface(session, params);
++}
++
++TEST_F(GenericShell, destroy_surface_removes_surface_from_window_manager)
++{
++    auto const params = ms::a_surface();
++    std::shared_ptr<ms::Session> const session =
++        shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++
++    auto const surface_id = shell.create_surface(session, params);
++    auto const surface = session->surface(surface_id);
++
++    EXPECT_CALL(*wm, remove_surface(session, WeakPtrTo(surface)));
++
++    shell.destroy_surface(session, surface_id);
++}
++
++TEST_F(GenericShell, add_display_adds_display_to_window_manager)
++{
++    geom::Rectangle const arbitrary_area{{0,0}, {__LINE__,__LINE__}};
++
++    EXPECT_CALL(*wm, add_display(arbitrary_area));
++
++    shell.add_display(arbitrary_area);
++}
++
++TEST_F(GenericShell, remove_display_adds_display_to_window_manager)
++{
++    geom::Rectangle const arbitrary_area{{0,0}, {__LINE__,__LINE__}};
++
++    EXPECT_CALL(*wm, remove_display(arbitrary_area));
++
++    shell.remove_display(arbitrary_area);
++}
++
++TEST_F(GenericShell, key_input_events_are_handled_by_window_manager)
++{
++    int64_t const timestamp{0};
++    MirKeyInputEventAction const action{mir_key_input_event_action_down};
++    xkb_keysym_t const key_code{0};
++    int const scan_code{0};
++    MirInputEventModifiers const modifiers{mir_input_event_modifier_none};
++
++    auto const event = mir::events::make_event(
++        mir_input_event_type_key,
++        timestamp,
++        action,
++        key_code,
++        scan_code,
++        modifiers);
++
++    EXPECT_CALL(*wm, handle_key_event(_))
++        .WillOnce(Return(false))
++        .WillOnce(Return(true));
++
++    EXPECT_FALSE(shell.handle(*event));
++    EXPECT_TRUE(shell.handle(*event));
++}
++
++TEST_F(GenericShell, touch_input_events_are_handled_by_window_manager)
++{
++    int64_t const timestamp{0};
++    MirInputEventModifiers const modifiers{mir_input_event_modifier_none};
++
++    auto const event = mir::events::make_event(
++        mir_input_event_type_touch,
++        timestamp,
++        modifiers);
++
++    EXPECT_CALL(*wm, handle_touch_event(_))
++        .WillOnce(Return(false))
++        .WillOnce(Return(true));
++
++    EXPECT_FALSE(shell.handle(*event));
++    EXPECT_TRUE(shell.handle(*event));
++}
++
++TEST_F(GenericShell, pointer_input_events_are_handled_by_window_manager)
++{
++    int64_t const timestamp{0};
++    MirInputEventModifiers const modifiers{mir_input_event_modifier_none};
++    MirPointerInputEventAction const action{mir_pointer_input_event_action_button_down};
++    std::vector<MirPointerInputEventButton> const buttons_pressed{mir_pointer_input_button_primary};
++    float const x_axis_value{0.0};
++    float const y_axis_value{0.0};
++    float const hscroll_value{0.0};
++    float const vscroll_value{0.0};
++
++    auto const event = mir::events::make_event(
++        mir_input_event_type_pointer,
++        timestamp,
++        modifiers,
++        action,
++        buttons_pressed,
++        x_axis_value,
++        y_axis_value,
++        hscroll_value,
++        vscroll_value);
++
++    EXPECT_CALL(*wm, handle_pointer_event(_))
++        .WillOnce(Return(false))
++        .WillOnce(Return(true));
++
++    EXPECT_FALSE(shell.handle(*event));
++    EXPECT_TRUE(shell.handle(*event));
++}
++
++TEST_F(GenericShell, setting_surface_state_is_handled_by_window_manager)
++{
++    std::shared_ptr<ms::Session> const session =
++        shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++
++    auto const surface_id = shell.create_surface(session, ms::a_surface());
++    auto const surface = session->surface(surface_id);
++
++    MirSurfaceState const state{mir_surface_state_fullscreen};
++
++    EXPECT_CALL(*wm, handle_set_state(surface, state))
++        .WillOnce(Return(mir_surface_state_maximized));
++
++    EXPECT_CALL(mock_surface, configure(mir_surface_attrib_state, mir_surface_state_maximized));
++
++    shell.set_surface_attribute(session, surface, mir_surface_attrib_state, mir_surface_state_fullscreen);
++}
++
++TEST_F(GenericShell, as_focus_controller_set_focus_to_notifies_session_event_sink)
++{
++    auto session = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    auto session1 = shell.open_session(__LINE__, "Bla", std::shared_ptr<mf::EventSink>());
++
++    InSequence s;
++    EXPECT_CALL(session_event_sink, handle_focus_change(session1));
++    EXPECT_CALL(session_event_sink, handle_focus_change(session));
++    EXPECT_CALL(session_event_sink, handle_no_focus());
++
++    msh::FocusController& focus_controller = shell;
++    focus_controller.set_focus_to(session1, {});
++    focus_controller.set_focus_to(session, {});
++    focus_controller.set_focus_to({}, {});
++}
++
++TEST_F(GenericShell, as_focus_controller_focus_next_notifies_session_event_sink)
++{
++    msh::FocusController& focus_controller = shell;
++    auto session = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    auto session1 = shell.open_session(__LINE__, "Bla", std::shared_ptr<mf::EventSink>());
++    focus_controller.set_focus_to(session, {});
++
++    EXPECT_CALL(session_container, successor_of(session)).
++        WillOnce(Return(session1));
++
++    EXPECT_CALL(session_event_sink, handle_focus_change(session1));
++
++    focus_controller.focus_next();
++}
++
++TEST_F(GenericShell, as_focus_controller_focused_session_follows_focus)
++{
++    auto session = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    auto session1 = shell.open_session(__LINE__, "Bla", std::shared_ptr<mf::EventSink>());
++    EXPECT_CALL(session_container, successor_of(session1)).
++        WillOnce(Return(session));
++
++    msh::FocusController& focus_controller = shell;
++
++    focus_controller.set_focus_to(session, {});
++    EXPECT_THAT(focus_controller.focused_session(), Eq(session));
++    focus_controller.set_focus_to(session1, {});
++    EXPECT_THAT(focus_controller.focused_session(), Eq(session1));
++    focus_controller.focus_next();
++    EXPECT_THAT(focus_controller.focused_session(), Eq(session));
++}
++
++TEST_F(GenericShell, as_focus_controller_focused_surface_follows_focus)
++{
++    auto const session0 = shell.open_session(__LINE__, "XPlane", std::shared_ptr<mf::EventSink>());
++    auto const session1 = shell.open_session(__LINE__, "Bla", std::shared_ptr<mf::EventSink>());
++    NiceMock<mtd::MockSurface> dummy_surface;
++
++    EXPECT_CALL(surface_coordinator, add_surface(_,_)).Times(AnyNumber())
++        .WillOnce(Return(mt::fake_shared(dummy_surface)))
++        .WillOnce(Return(mt::fake_shared(mock_surface)));
++    EXPECT_CALL(session_container, successor_of(session1)).
++        WillOnce(Return(session0));
++
++
++    auto const surface0_id = shell.create_surface(session0, ms::a_surface());
++    auto const surface0 = session0->surface(surface0_id);
++    auto const surface1_id = shell.create_surface(session1, ms::a_surface());
++    auto const surface1 = session1->surface(surface1_id);
++
++    msh::FocusController& focus_controller = shell;
++
++    focus_controller.set_focus_to(session0, surface0);
++    EXPECT_THAT(focus_controller.focused_surface(), Eq(surface0));
++    focus_controller.set_focus_to(session1, surface1);
++    EXPECT_THAT(focus_controller.focused_surface(), Eq(surface1));
++    focus_controller.focus_next();
++    EXPECT_THAT(focus_controller.focused_surface(), Eq(surface0));
++
++    shell.destroy_surface(session0, surface0_id);
++    shell.destroy_surface(session1, surface1_id);
++    shell.close_session(session1);
++    shell.close_session(session0);
++}
++
++TEST_F(GenericShell, as_focus_controller_delegates_surface_at_to_surface_coordinator)
++{
++    auto const surface = mt::fake_shared(mock_surface);
++    geom::Point const cursor{__LINE__, __LINE__};
++
++    EXPECT_CALL(surface_coordinator, surface_at(cursor)).
++        WillOnce(Return(surface));
++
++    msh::FocusController& focus_controller = shell;
++
++    EXPECT_THAT(focus_controller.surface_at(cursor), Eq(surface));
++}
++
++namespace mir
++{
++namespace scene
++{
++// The next test is easier to write if we can compare std::weak_ptr<ms::Surface>
++inline bool operator==(
++    std::weak_ptr<ms::Surface> const& lhs,
++    std::weak_ptr<ms::Surface> const& rhs)
++{
++    return !lhs.owner_before(rhs) && !rhs.owner_before(lhs);
++}
++}
++}
++
++TEST_F(GenericShell, as_focus_controller_delegates_raise_to_surface_coordinator)
++{
++    msh::SurfaceSet const surfaces{mt::fake_shared(mock_surface)};
++
++    EXPECT_CALL(surface_coordinator, raise(surfaces));
++
++    msh::FocusController& focus_controller = shell;
++
++    focus_controller.raise(surfaces);
++}