QtMir

Merge lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy into lp:qtmir

introducing-QtmirWindowManagementPolicy
Merge into trunk

Proposed by Alan Griffiths on 2015-11-26

Status:	Rejected
Rejected by:	Alan Griffiths on 2016-04-29
Proposed branch:	lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy
Merge into:	lp:qtmir
Prerequisite:	lp:~alan-griffiths/qtmir/import-WindowManagement-code-from-Mir-examples
Diff against target:	601 lines (+309/-135) 5 files modified src/platforms/mirserver/CMakeLists.txt (+18/-5) src/platforms/mirserver/mirserver.cpp (+5/-3) src/platforms/mirserver/mirwindowmanager.cpp (+242/-122) src/platforms/mirserver/mirwindowmanager.h (+23/-4) tests/mirserver/WindowManager/window_manager.cpp (+21/-1)
To merge this branch:	bzr merge lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot (community)	continuous-integration		Needs Fixing on 2016-03-24
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2016-01-20
Daniel d'Andrada (community)		2015-11-26	Abstain on 2015-11-27
Review via email: mp+278736@code.launchpad.net

Commit message

Wire up the draft Window Management framework to a (currently trivial) QtmirWindowManagementPolicy

Description of the change

Wire up the draft Window Management framework to a (currently trivial) QtmirWindowManagementPolicy

Revision history for this message

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

FAILED: Continuous integration, rev:419
http://jenkins.qa.ubuntu.com/job/qtmir-ci/579/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/275
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/275
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/275/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/157
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/312/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/312/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/157/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-11-26:

"""
void QtmirWindowManagementPolicy::generate_decorations_for(
"""

Why would Mir or even QtMir have anything to do with window decorations?

review: Needs Information

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-11-26:

Don't think this approach is top-down enough. Looks too disconnected with unity8's/QML "reality". It should probably start in unity8, changing the mock implementations of the interfaces it use to talk to QtMir and the interfaces themselves (Unity.Application presently). Once you have something that makes sense over there then you move down to QtMir to write the real implementation of it. Then you propose the unity8, unity-api and qtmir branches in tandem.

Afterall, if I got the plan right, the idea is that QtMir provides unity8 with a tree of surfaces which unity8 them wraps or syncs with QML items in the QML scene.

Otherwise you run the risk of laboring something in QtMir just to find out later that it's unusable in practice, up in unity8.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-27:

> """
> void QtmirWindowManagementPolicy::generate_decorations_for(
> """
>
> Why would Mir or even QtMir have anything to do with window decorations?

If as you suggest QtMir does not have anything to do with window decorations this function can keep the proposed null implementation. However, other clients of Mir *do* work with decorations. E.g. mir_demo_server and mir_proving_server draw titlebars.

Alternatively, we can come up with a more general approach then we can change the code here and port it back to Mir.

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-11-27:

On 27/11/2015 08:18, Alan Griffiths wrote:
>> """
>> void QtmirWindowManagementPolicy::generate_decorations_for(
>> """
>>
>> Why would Mir or even QtMir have anything to do with window decorations?
> If as you suggest QtMir does not have anything to do with window decorations this function can keep the proposed null implementation. However, other clients of Mir *do* work with decorations. E.g. mir_demo_server and mir_proving_server draw titlebars.
>
> Alternatively, we can come up with a more general approach then we can change the code here and port it back to Mir.

But those barebones pure Mir+OpenGL are not very realistic examples of
shell implementations (apart from, maybe, some dumb kiosks or the like).
Any minimally complex shell will likely use a toolkit to help out with
animations etc. And they most likely have their own scene
implementation, like Qt/QML.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-27:

> But those barebones pure Mir+OpenGL are not very realistic examples of
> shell implementations (apart from, maybe, some dumb kiosks or the like).
> Any minimally complex shell will likely use a toolkit to help out with
> animations etc. And they most likely have their own scene
> implementation, like Qt/QML.

There are a lot of "dumb kiosks" about and that's a valid usecase for Mir (possibly with only S/W rendering).

While I understand these scenarios are of little interest to Unity8, the cost of supporting them is small: a vtab entry for this trivial function (which doesn't even need to be exposed by qtmir).

Revision history for this message

Daniel d'Andrada (dandrader) on 2015-11-27:

review: Abstain

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-13:

FAILED: Continuous integration, rev:419
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/7/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/7/rebuild

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy updated on 2016-01-18

420. By Alan Griffiths on 2016-01-18: merge lp:~alan-griffiths/qtmir/import-WindowManagement-code-from-Mir-examples

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-18:

FAILED: Continuous integration, rev:420
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/19/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/19/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-01-18:

FAILED: Continuous integration, rev:420
http://jenkins.qa.ubuntu.com/job/qtmir-ci/643/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/337/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/339/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/219/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/374/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/376/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/219/console

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

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy updated on 2016-01-18

421. By Alan Griffiths on 2016-01-18: Resolve merge better

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-18:

FAILED: Continuous integration, rev:421
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/20/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/20/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-01-18:

FAILED: Continuous integration, rev:421
http://jenkins.qa.ubuntu.com/job/qtmir-ci/644/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/338
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/340
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/340/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/220
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/375/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/377/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/220/console

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

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy updated on 2016-01-20

422. By Alan Griffiths on 2016-01-20: merge lp:~alan-griffiths/qtmir/import-WindowManagement-code-from-Mir-examples/

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-20:

FAILED: Continuous integration, rev:422
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/27/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/27/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-01-20:

FAILED: Continuous integration, rev:422
http://jenkins.qa.ubuntu.com/job/qtmir-ci/649/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/343
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/345
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/345/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/225
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/380/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/382/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/225/console

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

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy updated on 2016-03-09

423. By Alan Griffiths on 2016-03-07: Rather brutal merge\ lp:qtmir
424. By Alan Griffiths on 2016-03-07: TODO{arg} comments to flage stuff that needs sorting
425. By Alan Griffiths on 2016-03-09: Refactor to get all of the state and most of the logic out of MirWindowManagerImpl

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-03-09:

> """
> void QtmirWindowManagementPolicy::generate_decorations_for(
> """
>
> Why would Mir or even QtMir have anything to do with window decorations?

Having thought about this some more - the need for this customization point in the Mir examples comes from the rather inelegant way that surfaces get constructed.

It's far from trivial to clean up without breaking downstream code, but getting everything to use a common structure (so as to deprecate legacy mechanisms) will help.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-09:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/102/rebuild

review: Approve (continuous-integration)

lp:~alan-griffiths/qtmir/introducing-QtmirWindowManagementPolicy updated on 2016-03-14

426. By Alan Griffiths on 2016-03-14: merge lp:qtmir and resolve conflicts

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/115/rebuild

review: Approve (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

FAILED: Continuous integration, rev:426
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/138/
Executed test runs:
FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1119/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/138/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

426. By Alan Griffiths on 2016-03-14: merge lp:qtmir and resolve conflicts
425. By Alan Griffiths on 2016-03-09: Refactor to get all of the state and most of the logic out of MirWindowManagerImpl
424. By Alan Griffiths on 2016-03-07: TODO{arg} comments to flage stuff that needs sorting
423. By Alan Griffiths on 2016-03-07: Rather brutal merge\ lp:qtmir
422. By Alan Griffiths on 2016-01-20: merge lp:~alan-griffiths/qtmir/import-WindowManagement-code-from-Mir-examples/
421. By Alan Griffiths on 2016-01-18: Resolve merge better
420. By Alan Griffiths on 2016-01-18: merge lp:~alan-griffiths/qtmir/import-WindowManagement-code-from-Mir-examples
419. By Alan Griffiths on 2015-11-26: Introducing QtmirWindowManagementPolicy

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

Daniel d'Andrada

Mir development team

 === modified file 'src/platforms/mirserver/CMakeLists.txt'
 --- src/platforms/mirserver/CMakeLists.txt	2016-03-10 09:28:57 +0000
 +++ src/platforms/mirserver/CMakeLists.txt	2016-03-14 09:55:02 +0000
@@ -17,11 +17,23 @@
     list(APPEND QT5_PLATFORMSUPPORT_INCLUDE_DIRS ${newitem})
  endforeach(item ${Qt5Gui_PRIVATE_INCLUDE_DIRS})
++# Use the local copy of the work-in-progress Window Management code.
++# (Will remove this once Mir support is published.)
++set(QTMIR_USE_WINDOW_MANAGEMENT_WIP on)
++
  include_directories(
--    ${CMAKE_SOURCE_DIR}/src/common
--
      ${MIRCOMMON_INCLUDE_DIRS}
      ${MIRSERVER_INCLUDE_DIRS}
++)
++
++if (QTMIR_USE_WINDOW_MANAGEMENT_WIP)
++add_subdirectory(wm-wip)
++include_directories(${CMAKE_CURRENT_SOURCE_DIR}/wm-wip)
++endif()
++
++include_directories(
++    ${CMAKE_SOURCE_DIR}/src/common
++
      ${MIRRENDERERGLDEV_INCLUDE_DIRS}
      ${URL_DISPATCHER_INCLUDE_DIRS}
@@ -36,8 +48,6 @@
      ${APPLICATION_API_INCLUDE_DIRS}
+ )
--add_subdirectory(wm-wip)
--
  # We have to remove -pedantic for tracepoints.c
  string (REPLACE " -pedantic " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
  # Needed to compile tracepoints in C99 mode.
@@ -82,9 +92,12 @@
      ${MIRSERVER_QPA_PLUGIN_SRC}
+ )
++if (QTMIR_USE_WINDOW_MANAGEMENT_WIP)
++target_link_libraries(qpa-mirserver experimentalwindowmanager)
++endif()
++
  target_link_libraries(
      qpa-mirserver
--
      ${MIRSERVER_LDFLAGS}
      ${MIRCLIENT_LDFLAGS}
      ${URL_DISPATCHER_LDFLAGS}
 === modified file 'src/platforms/mirserver/mirserver.cpp'
 --- src/platforms/mirserver/mirserver.cpp	2016-02-15 17:18:08 +0000
 +++ src/platforms/mirserver/mirserver.cpp	2016-03-14 09:55:02 +0000
@@ -107,11 +107,13 @@
              return std::make_shared<MirServerStatusListener>();
          });
--    override_the_window_manager_builder([this](mir::shell::FocusController*)
++    override_the_window_manager_builder([this](mir::shell::FocusController* focus_controller)
          -> std::shared_ptr<mir::shell::WindowManager>
+         {
--            auto windowManager = MirWindowManager::create(the_shell_display_layout(),
--                    std::static_pointer_cast<::SessionListener>(the_session_listener()));
++            std::shared_ptr<MirWindowManager> const windowManager{MirWindowManager::create(focus_controller,
++                the_shell_display_layout(),
++                std::static_pointer_cast<::SessionListener>(the_session_listener()))};
++
              m_windowManager = windowManager;
              return windowManager;
          });
 === modified file 'src/platforms/mirserver/mirwindowmanager.cpp'
 --- src/platforms/mirserver/mirwindowmanager.cpp	2016-02-11 11:58:16 +0000
 +++ src/platforms/mirserver/mirwindowmanager.cpp	2016-03-14 09:55:02 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2015 Canonical, Ltd.
++ * Copyright (C) 2015-2016 Canonical, Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it under
   * the terms of the GNU Lesser General Public License version 3, as published by
@@ -24,36 +24,49 @@
  #include <mir/scene/surface_creation_parameters.h>
  #include <mir/scene/surface.h>
  #include <mir/shell/display_layout.h>
++#include <mir/shell/surface_ready_observer.h>
++
++#include "server_example_basic_window_manager.h"
  #include <QMutexLocker>
++namespace me = mir::examples;
++namespace mf = mir::frontend;
  namespace ms = mir::scene;
++namespace msh = mir::shell;
++using namespace mir::geometry;
  namespace
+ {
--class MirWindowManagerImpl : public MirWindowManager
++class QtmirWindowManagementPolicy  : public me::WindowManagementPolicy
+ {
  public:
--    MirWindowManagerImpl(const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout,
--            std::shared_ptr<::SessionListener> sessionListener);
--
--    void add_session(std::shared_ptr<mir::scene::Session> const& session) override;
--
--    void remove_session(std::shared_ptr<mir::scene::Session> const& session) override;
--
--    mir::frontend::SurfaceId add_surface(
--        std::shared_ptr<mir::scene::Session> const& session,
--        mir::scene::SurfaceCreationParameters const& params,
--        std::function<mir::frontend::SurfaceId(std::shared_ptr<mir::scene::Session> const& session, mir::scene::SurfaceCreationParameters const& params)> const& build) override;
--
--    void remove_surface(
--        std::shared_ptr<mir::scene::Session> const& session,
--        std::weak_ptr<mir::scene::Surface> const& surface) override;
--
--    void add_display(mir::geometry::Rectangle const& area) override;
--
--    void remove_display(mir::geometry::Rectangle const& area) override;
++    explicit QtmirWindowManagementPolicy(
++        me::WindowManagerTools* const tools,
++        std::shared_ptr<msh::DisplayLayout> const& display_layout,
++        std::shared_ptr<::SessionListener> const& session_listener,
++        qtmir::WindowManagementAdvisor* advisor);
++
++    void handle_session_info_updated(SessionInfoMap& session_info, Rectangles const& displays) override;
++
++    void handle_displays_updated(SessionInfoMap& session_info, Rectangles const& displays) override;
++
++    auto handle_place_new_surface(
++        std::shared_ptr<ms::Session> const& session,
++        ms::SurfaceCreationParameters const& request_parameters)
++        -> ms::SurfaceCreationParameters override;
++
++    void handle_new_surface(std::shared_ptr<ms::Session> const& session, std::shared_ptr<ms::Surface> const& surface) override;
++
++    void handle_modify_surface(
++        std::shared_ptr<ms::Session> const& session,
++        std::shared_ptr<ms::Surface> const& surface,
++        msh::SurfaceSpecification const& modifications) override;
++
++    void handle_delete_surface(std::shared_ptr<ms::Session> const& session, std::weak_ptr<ms::Surface> const& surface) override;
++
++    int handle_set_state(std::shared_ptr<ms::Surface> const& surface, MirSurfaceState value) override;
      bool handle_keyboard_event(MirKeyboardEvent const* event) override;
@@ -61,132 +74,165 @@
      bool handle_pointer_event(MirPointerEvent const* event) override;
--    int set_surface_attribute(
--        std::shared_ptr<mir::scene::Session> const& session,
--        std::shared_ptr<mir::scene::Surface> const& surface,
--        MirSurfaceAttrib attrib,
--        int value) override;
--
      void handle_raise_surface(
++        std::shared_ptr<ms::Session> const& session,
++        std::shared_ptr<ms::Surface> const& surface) override;
++
++    void generate_decorations_for(
++        std::shared_ptr<ms::Session> const& session,
++        std::shared_ptr<ms::Surface> const& surface,
++        SurfaceInfoMap& surface_map,
++        std::function<mir::frontend::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& build) override;
++
++private:
++    static const int modifier_mask =
++        mir_input_event_modifier_alt |
++        mir_input_event_modifier_shift |
++        mir_input_event_modifier_sym |
++        mir_input_event_modifier_ctrl |
++        mir_input_event_modifier_meta;
++
++    me::WindowManagerTools* const tools;
++    std::shared_ptr<msh::DisplayLayout> const display_layout;
++    std::shared_ptr<::SessionListener> const session_listener;
++    qtmir::WindowManagementAdvisor* const advisor;
++};
++
++template <typename Target>
++inline Target implicit_cast(Target target) { return target; }
++
++// TODO This is likely not the best place to implement WindowManagementAdvisor
++// but it works for now, and avoids re-plumbing the MirWindowManager Q_SIGNAL
++class MirWindowManagerImpl : public MirWindowManager,
++     public me::WindowManagerBuilder<QtmirWindowManagementPolicy>,
++     qtmir::WindowManagementAdvisor
++{
++public:
++
++    template <typename... PolicyArgs>
++    MirWindowManagerImpl(msh::FocusController* focus_controller, PolicyArgs&&... policy_args) :
++        me::WindowManagerBuilder<QtmirWindowManagementPolicy>(
++            focus_controller,
++            std::forward<PolicyArgs>(policy_args)...,
++            implicit_cast<qtmir::WindowManagementAdvisor*>(this))
++    {
++    }
++
++private:
++    mir::scene::SurfaceCreationParameters proposePlacementForNewSurface(
++        mir::scene::SurfaceCreationParameters const &params,
          std::shared_ptr<mir::scene::Session> const& session,
--        std::shared_ptr<mir::scene::Surface> const& surface,
--        uint64_t timestamp) override;
--
--    void modify_surface(
--        const std::shared_ptr<mir::scene::Session>&,
--        const std::shared_ptr<mir::scene::Surface>& surface,
--        const mir::shell::SurfaceSpecification& modifications) override;
--
--private:
--    std::shared_ptr<mir::shell::DisplayLayout> const m_displayLayout;
--    std::shared_ptr<::SessionListener> m_sessionListener;
++        mir::shell::DisplayLayout &displayLayout,
++        ::SessionListener &sessionListener) override;
  };
+ }
--MirWindowManagerImpl::MirWindowManagerImpl(const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout,
--        std::shared_ptr<::SessionListener> sessionListener) :
--    m_displayLayout{displayLayout},
--    m_sessionListener(sessionListener)
--{
--    qCDebug(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::MirWindowManagerImpl";
--}
--
--void MirWindowManagerImpl::add_session(std::shared_ptr<ms::Session> const& /*session*/)
--{
--}
--
--void MirWindowManagerImpl::remove_session(std::shared_ptr<ms::Session> const& /*session*/)
--{
--}
--
--mir::frontend::SurfaceId MirWindowManagerImpl::add_surface(
--    std::shared_ptr<ms::Session> const& session,
--    ms::SurfaceCreationParameters const& requestParameters,
--    std::function<mir::frontend::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& build)
--{
++mir::scene::SurfaceCreationParameters MirWindowManagerImpl::proposePlacementForNewSurface(
++    mir::scene::SurfaceCreationParameters const &request_parameters,
++    std::shared_ptr<mir::scene::Session> const& session,
++    mir::shell::DisplayLayout &displayLayout,
++    ::SessionListener &sessionListener)
++{
++    auto placedParameters = request_parameters;
++
      tracepoint(qtmirserver, surfacePlacementStart);
--    m_sessionListener->surfaceAboutToBeCreated(*session.get(), qtmir::SizeHints(requestParameters));
++    sessionListener.surfaceAboutToBeCreated(*session.get(), qtmir::SizeHints(request_parameters));
      QSize initialSize;
      // can be connected to via Qt::BlockingQueuedConnection to alter surface initial size
+     {
--        int surfaceType = requestParameters.type.is_set() ? requestParameters.type.value() : -1;
++        int surfaceType = request_parameters.type.is_set() ? request_parameters.type.value() : -1;
          Q_EMIT sessionAboutToCreateSurface(session, surfaceType, initialSize);
+     }
--    ms::SurfaceCreationParameters placedParameters = requestParameters;
      if (initialSize.isValid()) {
          placedParameters.size.width = mir::geometry::Width(initialSize.width());
          placedParameters.size.height = mir::geometry::Height(initialSize.height());
      } else {
          qCWarning(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): didn't get a initial surface"
--            " size from shell. Falling back to fullscreen placement";
++                " size from shell. Falling back to fullscreen placement";
          // This is bad. Fallback to fullscreen
--        mir::geometry::Rectangle rect{requestParameters.top_left, requestParameters.size};
--        m_displayLayout->size_to_output(rect);
++        mir::geometry::Rectangle rect{request_parameters.top_left, request_parameters.size};
++        displayLayout.size_to_output(rect);
          placedParameters.size = rect.size;
+     }
--
--    qCDebug(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): size requested ("
--                                << requestParameters.size.width.as_int() << "," << requestParameters.size.height.as_int() << ") and placed ("
++    qCDebug(QTMIR_MIR_MESSAGES) << "CanonicalWindowManagerPolicy::handle_place_new_surface(): size requested ("
++                                << request_parameters.size.width.as_int() << "," << request_parameters.size.height.as_int() << ") and placed ("
                                  << placedParameters.size.width.as_int() << "," << placedParameters.size.height.as_int() << ")";
      tracepoint(qtmirserver, surfacePlacementEnd);
--    return build(session, placedParameters);
--}
--
--void MirWindowManagerImpl::remove_surface(
--    std::shared_ptr<ms::Session> const& /*session*/,
--    std::weak_ptr<ms::Surface> const& /*surface*/)
--{
--}
--
--void MirWindowManagerImpl::add_display(mir::geometry::Rectangle const& /*area*/)
--{
--}
--
--void MirWindowManagerImpl::remove_display(mir::geometry::Rectangle const& /*area*/)
--{
--}
--
--bool MirWindowManagerImpl::handle_keyboard_event(MirKeyboardEvent const* /*event*/)
--{
--    return false;
--}
--
--bool MirWindowManagerImpl::handle_touch_event(MirTouchEvent const* /*event*/)
--{
--    return false;
--}
--
--bool MirWindowManagerImpl::handle_pointer_event(MirPointerEvent const* /*event*/)
--{
--    return false;
--}
--
--void MirWindowManagerImpl::handle_raise_surface(
--    std::shared_ptr<mir::scene::Session> const& /*session*/,
--    std::shared_ptr<mir::scene::Surface> const& /*surface*/,
--    uint64_t /*timestamp*/)
--{
--}
--
--int MirWindowManagerImpl::set_surface_attribute(
++    return placedParameters;
++}
++
++std::unique_ptr<MirWindowManager> MirWindowManager::create(
++    msh::FocusController* focus_controller,
++    const std::shared_ptr<msh::DisplayLayout> &displayLayout,
++    const std::shared_ptr<::SessionListener> &sessionListener)
++{
++    return std::make_unique<MirWindowManagerImpl>(focus_controller, displayLayout, sessionListener);
++}
++
++QtmirWindowManagementPolicy::QtmirWindowManagementPolicy(
++    me::WindowManagerTools* const tools,
++    std::shared_ptr<msh::DisplayLayout> const& display_layout,
++    std::shared_ptr<::SessionListener> const& session_listener,
++    qtmir::WindowManagementAdvisor* advisor) :
++    tools(tools),
++    display_layout(display_layout),
++    session_listener(session_listener),
++    advisor(advisor)
++{
++}
++
++void QtmirWindowManagementPolicy::handle_session_info_updated(SessionInfoMap& /*session_info*/, Rectangles const& /*displays*/)
++{
++}
++
++void QtmirWindowManagementPolicy::handle_displays_updated(SessionInfoMap& /*session_info*/, Rectangles const& /*displays*/)
++{
++}
++
++auto QtmirWindowManagementPolicy::handle_place_new_surface(
++    std::shared_ptr<ms::Session> const& session,
++    ms::SurfaceCreationParameters const& parameters)
++-> ms::SurfaceCreationParameters
++{
++    return advisor->proposePlacementForNewSurface(parameters, session, *display_layout, *session_listener);
++}
++
++void QtmirWindowManagementPolicy::handle_new_surface(std::shared_ptr<ms::Session> const& session, std::shared_ptr<ms::Surface> const& surface)
++{
++    auto& surface_info = tools->info_for(surface);
++    if (auto const parent = surface_info.parent.lock())
++    {
++        tools->info_for(parent).children.push_back(surface);
++    }
++
++    tools->info_for(session).surfaces.push_back(surface);
++
++    if (surface_info.can_be_active())
++    {
++        surface->add_observer(
++            std::make_shared<msh::SurfaceReadyObserver>(
++                [this](
++                    std::shared_ptr<ms::Session> const& /*session*/,
++                    std::shared_ptr<ms::Surface> const& /*surface*/)
++                    {
++                    // TODO select_active_surface(surface);
++                    },
++                session,
++                surface));
++    }
++}
++
++void QtmirWindowManagementPolicy::handle_modify_surface(
      std::shared_ptr<ms::Session> const& /*session*/,
      std::shared_ptr<ms::Surface> const& surface,
--    MirSurfaceAttrib attrib,
--    int value)
--{
--    return surface->configure(attrib, value);
--}
--
--void MirWindowManagerImpl::modify_surface(const std::shared_ptr<mir::scene::Session>&,
--                                          const std::shared_ptr<mir::scene::Surface>& surface,
--                                          const mir::shell::SurfaceSpecification& modifications)
++    msh::SurfaceSpecification const& modifications)
+ {
      if (modifications.name.is_set()) {
          surface->rename(modifications.name.value());
@@ -199,9 +245,83 @@
+     }
+ }
--std::shared_ptr<MirWindowManager> MirWindowManager::create(
--    const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout,
--    std::shared_ptr<::SessionListener> sessionListener)
--{
--    return std::make_shared<MirWindowManagerImpl>(displayLayout, sessionListener);
++void QtmirWindowManagementPolicy::handle_delete_surface(std::shared_ptr<ms::Session> const& session, std::weak_ptr<ms::Surface> const& surface)
++{
++    auto& info = tools->info_for(surface);
++
++    if (auto const parent = info.parent.lock())
++    {
++        auto& siblings = tools->info_for(parent).children;
++
++        for (auto i = begin(siblings); i != end(siblings); ++i)
++        {
++            if (surface.lock() == i->lock())
++            {
++                siblings.erase(i);
++                break;
++            }
++        }
++    }
++
++    session->destroy_surface(surface);
++
++    if (info.titlebar)
++    {
++        session->destroy_surface(info.titlebar_id);
++        tools->forget(info.titlebar);
++    }
++
++    auto& surfaces = tools->info_for(session).surfaces;
++
++    for (auto i = begin(surfaces); i != end(surfaces); ++i)
++    {
++        if (surface.lock() == i->lock())
++        {
++            surfaces.erase(i);
++            break;
++        }
++    }
++
++    if (surfaces.empty() && session == tools->focused_session())
++    {
++//        active_surface_.reset();
++        tools->focus_next_session();
++//        select_active_surface(tools->focused_surface());
++    }
++}
++
++int QtmirWindowManagementPolicy::handle_set_state(std::shared_ptr<ms::Surface> const& surface, MirSurfaceState value)
++{
++    auto& info = tools->info_for(surface);
++    return info.state = value;
++}
++
++bool QtmirWindowManagementPolicy::handle_keyboard_event(MirKeyboardEvent const* /*event*/)
++{
++    return false;
++}
++
++bool QtmirWindowManagementPolicy::handle_touch_event(MirTouchEvent const* /*event*/)
++{
++    return false;
++}
++
++bool QtmirWindowManagementPolicy::handle_pointer_event(MirPointerEvent const* /*event*/)
++{
++    return false;
++}
++
++void QtmirWindowManagementPolicy::handle_raise_surface(
++    std::shared_ptr<ms::Session> const& /*session*/,
++    std::shared_ptr<ms::Surface> const& /*surface*/)
++{
++
++}
++
++void QtmirWindowManagementPolicy::generate_decorations_for(
++    std::shared_ptr<ms::Session> const& /*session*/,
++    std::shared_ptr<ms::Surface> const& /*surface*/,
++    SurfaceInfoMap& /*surface_map*/,
++    std::function<mir::frontend::SurfaceId(std::shared_ptr<ms::Session> const& session, ms::SurfaceCreationParameters const& params)> const& /*build*/)
++{
+ }
 === modified file 'src/platforms/mirserver/mirwindowmanager.h'
 --- src/platforms/mirserver/mirwindowmanager.h	2016-02-15 17:18:08 +0000
 +++ src/platforms/mirserver/mirwindowmanager.h	2016-03-14 09:55:02 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2015 Canonical, Ltd.
++ * Copyright (C) 2015-2016 Canonical, Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it under
   * the terms of the GNU Lesser General Public License version 3, as published by
@@ -31,14 +31,33 @@
+     }
+ }
--class MirWindowManager : public QObject, public mir::shell::WindowManager
++namespace qtmir
++{
++class WindowManagementAdvisor
++{
++public:
++    virtual mir::scene::SurfaceCreationParameters proposePlacementForNewSurface(
++        mir::scene::SurfaceCreationParameters const &params,
++        std::shared_ptr<mir::scene::Session> const& session,
++        mir::shell::DisplayLayout &displayLayout,
++        ::SessionListener &sessionListener) = 0;
++
++    WindowManagementAdvisor() = default;
++    virtual ~WindowManagementAdvisor() = default;
++    WindowManagementAdvisor(WindowManagementAdvisor const&) = delete;
++    WindowManagementAdvisor& operator=(WindowManagementAdvisor const&) = delete;
++};
++}
++
++class MirWindowManager : public QObject, public virtual mir::shell::WindowManager
+ {
      Q_OBJECT
  public:
--    static std::shared_ptr<MirWindowManager> create(
++    static std::unique_ptr<MirWindowManager> create(
++        mir::shell::FocusController* focus_controller,
          const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout,
--        std::shared_ptr<::SessionListener> sessionListener);
++        std::shared_ptr<::SessionListener> const& sessionListener);
  Q_SIGNALS:
      // requires Qt::BlockingQueuedConnection!!
 === modified file 'tests/mirserver/WindowManager/window_manager.cpp'
 --- tests/mirserver/WindowManager/window_manager.cpp	2016-02-11 11:58:16 +0000
 +++ tests/mirserver/WindowManager/window_manager.cpp	2016-03-14 09:55:02 +0000
@@ -22,6 +22,7 @@
  #include <mir/events/event_builders.h>
  #include <mir/scene/surface_creation_parameters.h>
  #include <mir/shell/display_layout.h>
++#include <mir/shell/focus_controller.h>
  #include "gtest/gtest.h"
  #include "gmock/gmock.h"
@@ -54,15 +55,34 @@
      MOCK_CONST_METHOD1(surface, std::shared_ptr<ms::Surface> (mir::frontend::SurfaceId surface));
  };
++struct StubFocusController : msh::FocusController
++{
++public:
++    void focus_next_session() override {}
++
++    std::shared_ptr<ms::Session> focused_session() const override { return {}; }
++
++    void set_focus_to(
++        std::shared_ptr<ms::Session> const& /*focus_session*/,
++        std::shared_ptr<ms::Surface> const& /*focus_surface*/) override {}
++
++    std::shared_ptr<ms::Surface> focused_surface() const override { return {}; }
++
++    std::shared_ptr<ms::Surface> surface_at(Point /*cursor*/) const override { return {}; }
++
++    void raise(msh::SurfaceSet const& /*surfaces*/) override {}
++};
++
  struct WindowManager : Test
+ {
      const std::shared_ptr<MockDisplayLayout> mock_display_layout =
          std::make_shared<NiceMock<MockDisplayLayout>>();
++    StubFocusController focus_controller;
      std::shared_ptr<SessionListener> sessionListener = std::make_shared<SessionListener>();
      const std::shared_ptr<MirWindowManager> window_manager =
--        MirWindowManager::create(mock_display_layout, sessionListener);
++        MirWindowManager::create(&focus_controller, mock_display_layout, sessionListener);
      const Rectangle arbitrary_display{{0, 0}, {97, 101}};
      const std::shared_ptr<MockSession> arbitrary_session = std::make_shared<NiceMock<MockSession>>();