QtMir

Merge lp:~dandrader/qtmir/fixActiveFocusUpdate into lp:qtmir

fixActiveFocusUpdate
Merge into trunk

Proposed by Daniel d'Andrada on 2016-05-24

Status:	Merged
Approved by:	Nick Dedekind on 2016-06-01
Approved revision:	491
Merged at revision:	499
Proposed branch:	lp:~dandrader/qtmir/fixActiveFocusUpdate
Merge into:	lp:qtmir
Diff against target:	980 lines (+401/-217) 17 files modified src/modules/Unity/Application/mirsurface.cpp (+21/-7) src/modules/Unity/Application/mirsurface.h (+7/-3) src/modules/Unity/Application/mirsurfaceinterface.h (+4/-3) src/modules/Unity/Application/mirsurfaceitem.cpp (+5/-11) src/modules/Unity/Application/mirsurfaceitem.h (+1/-1) tests/framework/CMakeLists.txt (+1/-1) tests/framework/fake_mirsurface.cpp (+0/-2) tests/framework/fake_mirsurface.h (+2/-1) tests/framework/fake_session.cpp (+2/-1) tests/framework/fake_session.h (+2/-0) tests/framework/fake_surface.h (+89/-69) tests/framework/mock_shell.h (+111/-0) tests/framework/stub_scene_surface.cpp (+0/-103) tests/modules/SessionManager/session_test.cpp (+0/-3) tests/modules/SurfaceManager/CMakeLists.txt (+1/-0) tests/modules/SurfaceManager/mirsurface_test.cpp (+10/-11) tests/modules/SurfaceManager/mirsurfaceitem_test.cpp (+145/-1)
To merge this branch:	bzr merge lp:~dandrader/qtmir/fixActiveFocusUpdate
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Nick Dedekind (community)		2016-05-24	Approve on 2016-06-01
Unity8 CI Bot	continuous-integration	2016-05-24	Approve on 2016-05-24
Review via email: mp+295624@code.launchpad.net

This proposal supersedes a proposal from 2016-05-09.

Commit Message

Fix mir::scene::Surface focus attribute updates

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes.

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-05-09: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Nick Dedekind (nick-dedekind) wrote on 2016-05-11: Posted in a previous version of this proposal

Looks fine.

review: Approve

Nick Dedekind (nick-dedekind) wrote on 2016-05-11: Posted in a previous version of this proposal

I don't believe there's any real way to test this at the moment, since consumes input doesn't currently change, but was tested in mock by branch:
https://code.launchpad.net/~nick-dedekind/unity8/sidestage-fixes

Nick Dedekind (nick-dedekind) wrote on 2016-05-23: Posted in a previous version of this proposal

I think this may be wrong.
We encounter an issue with this in unity8 shell tests (FUNCTION=Shell::test_lifecyclePolicyForNonTouchApp) when the consumesInput is bound to interactive in https://code.launchpad.net/~unity-team/unity8/sidestage-fixes/+merge/295130.

Problem arises from the active focus not being passed through when consumesInput=false, even if the surfaceItem does not have active focus.

review: Needs Fixing

Nick Dedekind (nick-dedekind) wrote on 2016-05-23: Posted in a previous version of this proposal

perhaps:

void MirSurfaceItem::updateMirSurfaceActiveFocus()
{
    if (m_surface && m_surface->live()) {
        m_surface->setActiveFocus(hasActiveFocus() && m_consumesInput);
    }
}

Nick Dedekind (nick-dedekind) wrote on 2016-05-23: Posted in a previous version of this proposal

and updateMirSurfaceActiveFocus always needs calling when consumeInput changes.

Daniel d'Andrada (dandrader) wrote on 2016-05-23: Posted in a previous version of this proposal

> and updateMirSurfaceActiveFocus always needs calling when consumeInput
> changes.

Imagine multiple views per surface:

- View A: hasActiveFocus()=true, m_consumesInput=true (e.g. main view in desktop stage)
- View B: hasActiveFocus()=false, m_consumesInput=true (e.g. spread view in desktop stage)
- Surface: activeFocus=true (because of View A)

It's a wrong situation (two view consuming input for the same surface) but possible with the current API.

- m_consumesInput in View B turns false.

With the code you suggest, Surface would lose active focus despite of View A. That's why I only call update when m_consumesInput turns true (not obvious).

Looks like we might need a more involved solution...

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-05-23: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Daniel d'Andrada (dandrader) wrote on 2016-05-24: Posted in a previous version of this proposal

An all-new patch. Should work in all situations now.

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

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

review: Needs Fixing (continuous-integration)

lp:~dandrader/qtmir/fixActiveFocusUpdate updated on 2016-05-24

491. By Daniel d'Andrada on 2016-05-24: Remove dead code

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

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

review: Approve (continuous-integration)

Nick Dedekind (nick-dedekind) wrote on 2016-06-01:

Looks fine.

review: Approve

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:

Daniel d'Andrada

Mir development team

 === modified file 'src/modules/Unity/Application/mirsurface.cpp'
 --- src/modules/Unity/Application/mirsurface.cpp	2016-05-17 19:18:44 +0000
 +++ src/modules/Unity/Application/mirsurface.cpp	2016-05-24 21:32:26 +0000
@@ -441,7 +441,18 @@
      Q_EMIT focusedChanged(value);
+ }
--void MirSurface::setActiveFocus(bool focus)
++void MirSurface::setViewActiveFocus(qintptr viewId, bool value)
++{
++    if (value && !m_activelyFocusedViews.contains(viewId)) {
++        m_activelyFocusedViews.insert(viewId);
++        updateActiveFocus();
++    } else if (!value && (m_activelyFocusedViews.contains(viewId) || m_neverSetSurfaceFocus)) {
++        m_activelyFocusedViews.remove(viewId);
++        updateActiveFocus();
++    }
++}
++
++void MirSurface::updateActiveFocus()
+ {
      if (!m_session) {
          return;
@@ -450,17 +461,19 @@
      // Temporary hotfix for http://pad.lv/1483752
      if (m_session->childSessions()->rowCount() > 0) {
          // has child trusted session, ignore any focus change attempts
--        DEBUG_MSG << "(" << focus << ") - has child trusted session, ignore any focus change attempts";
++        DEBUG_MSG << "() has child trusted session, ignore any focus change attempts";
          return;
+     }
--    DEBUG_MSG << "(" << focus << ")";
--
--    if (focus) {
++    if (m_activelyFocusedViews.isEmpty()) {
++        DEBUG_MSG << "() unfocused";
++        m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_unfocused);
++    } else {
++        DEBUG_MSG << "() focused";
          m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_focused);
--    } else {
--        m_shell->set_surface_attribute(m_session->session(), m_surface, mir_surface_attrib_focus, mir_surface_unfocused);
+     }
++
++    m_neverSetSurfaceFocus = false;
+ }
  void MirSurface::close()
@@ -739,6 +752,7 @@
+         }
+     }
      updateVisibility();
++    setViewActiveFocus(viewId, false);
+ }
  void MirSurface::setViewVisibility(qintptr viewId, bool visible)
 === modified file 'src/modules/Unity/Application/mirsurface.h'
 --- src/modules/Unity/Application/mirsurface.h	2016-05-17 19:18:44 +0000
 +++ src/modules/Unity/Application/mirsurface.h	2016-05-24 21:32:26 +0000
@@ -25,9 +25,8 @@
  #include <QMutex>
  #include <QPointer>
  #include <QSharedPointer>
--#include <QSGTextureProvider>
  #include <QWeakPointer>
--#include <QPair>
++#include <QSet>
  #include "mirbuffersgtexture.h"
  #include "session.h"
@@ -117,7 +116,8 @@
      // end of methods called from the rendering (scene graph) thread
      void setFocused(bool focus) override;
--    void setActiveFocus(bool focus) override;
++
++    void setViewActiveFocus(qintptr viewId, bool value) override;
      void mousePressEvent(QMouseEvent *event) override;
      void mouseMoveEvent(QMouseEvent *event) override;
@@ -179,6 +179,7 @@
      bool clientIsRunning() const;
      void updateVisibility();
      void applyKeymap();
++    void updateActiveFocus();
      std::shared_ptr<mir::scene::Surface> m_surface;
      QPointer<SessionInterface> m_session;
@@ -203,6 +204,9 @@
      };
      QHash<qintptr, View> m_views;
++    QSet<qintptr> m_activelyFocusedViews;
++    bool m_neverSetSurfaceFocus{true};
++
      std::shared_ptr<SurfaceObserver> m_surfaceObserver;
      QSize m_size;
 === modified file 'src/modules/Unity/Application/mirsurfaceinterface.h'
 --- src/modules/Unity/Application/mirsurfaceinterface.h	2016-04-29 15:41:00 +0000
 +++ src/modules/Unity/Application/mirsurfaceinterface.h	2016-05-24 21:32:26 +0000
@@ -72,10 +72,11 @@
      /*
          Defines the "focus" attribute of the underlying mir::scene::Surface, which is what
          the client application sees.
--        Set by a MirSurfaceItem and used to inform the client application about the actual
--        focus, pretty much a one-to-one mapping with QML's active focus concept, hence the name.
++        Set by MirSurfaceItems (aka views) and used to inform the client application about the
++        actual focus, pretty much a one-to-one mapping with QML's active focus concept, hence
++        the name.
       */
--    virtual void setActiveFocus(bool focus) = 0;
++    virtual void setViewActiveFocus(qintptr viewId, bool value) = 0;
      virtual void mousePressEvent(QMouseEvent *event) = 0;
      virtual void mouseMoveEvent(QMouseEvent *event) = 0;
 === modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
 --- src/modules/Unity/Application/mirsurfaceitem.cpp	2016-04-29 15:43:46 +0000
 +++ src/modules/Unity/Application/mirsurfaceitem.cpp	2016-05-24 21:32:26 +0000
@@ -566,10 +566,10 @@
      m_surface->setViewVisibility((qintptr)this, isVisible());
+ }
--void MirSurfaceItem::updateMirSurfaceActiveFocus(bool focused)
++void MirSurfaceItem::updateMirSurfaceActiveFocus()
+ {
--    if (m_surface && m_consumesInput && m_surface->live()) {
--        m_surface->setActiveFocus(focused);
++    if (m_surface && m_surface->live()) {
++        m_surface->setViewActiveFocus(qintptr(this), m_consumesInput && hasActiveFocus());
+     }
+ }
@@ -619,6 +619,7 @@
          setAcceptHoverEvents(false);
+     }
++    updateMirSurfaceActiveFocus();
      Q_EMIT consumesInputChanged(value);
+ }
@@ -640,11 +641,6 @@
      if (m_surface) {
          disconnect(m_surface, nullptr, this, nullptr);
--
--        if (hasActiveFocus() && m_consumesInput && m_surface->live()) {
--            m_surface->setActiveFocus(false);
--        }
--
          m_surface->unregisterView((qintptr)this);
          unsetCursor();
+     }
@@ -690,9 +686,7 @@
              Q_EMIT orientationAngleChanged(m_surface->orientationAngle());
+         }
--        if (m_consumesInput) {
--            m_surface->setActiveFocus(hasActiveFocus());
--        }
++        updateMirSurfaceActiveFocus();
+     }
      update();
 === modified file 'src/modules/Unity/Application/mirsurfaceitem.h'
 --- src/modules/Unity/Application/mirsurfaceitem.h	2016-04-29 15:43:46 +0000
 +++ src/modules/Unity/Application/mirsurfaceitem.h	2016-05-24 21:32:26 +0000
@@ -116,7 +116,7 @@
      void scheduleMirSurfaceSizeUpdate();
      void updateMirSurfaceSize();
--    void updateMirSurfaceActiveFocus(bool focused);
++    void updateMirSurfaceActiveFocus();
      void updateMirSurfaceVisibility();
      void onActualSurfaceSizeChanged(const QSize &size);
 === modified file 'tests/framework/CMakeLists.txt'
 --- tests/framework/CMakeLists.txt	2016-02-18 17:27:58 +0000
 +++ tests/framework/CMakeLists.txt	2016-05-24 21:32:26 +0000
@@ -29,7 +29,7 @@
      mock_surface.cpp
      mock_task_controller.cpp
      stub_input_channel.cpp
--    stub_scene_surface.cpp
++    fake_surface.h
      qtmir_test.cpp
+ )
 === modified file 'tests/framework/fake_mirsurface.cpp'
 --- tests/framework/fake_mirsurface.cpp	2016-04-05 18:58:38 +0000
 +++ tests/framework/fake_mirsurface.cpp	2016-05-24 21:32:26 +0000
@@ -152,8 +152,6 @@
  void FakeMirSurface::setFocused(bool focus) { m_focused = focus; }
--void FakeMirSurface::setActiveFocus(bool) {}
--
  void FakeMirSurface::mousePressEvent(QMouseEvent *) {}
  void FakeMirSurface::mouseMoveEvent(QMouseEvent *) {}
 === modified file 'tests/framework/fake_mirsurface.h'
 --- tests/framework/fake_mirsurface.h	2016-05-17 19:18:44 +0000
 +++ tests/framework/fake_mirsurface.h	2016-05-24 21:32:26 +0000
@@ -110,7 +110,8 @@
      // end of methods called from the rendering (scene graph) thread
      void setFocused(bool focus) override;
--    void setActiveFocus(bool focus) override;
++
++    void setViewActiveFocus(qintptr, bool) override {};
      void mousePressEvent(QMouseEvent *) override;
      void mouseMoveEvent(QMouseEvent *) override;
 === modified file 'tests/framework/fake_session.cpp'
 --- tests/framework/fake_session.cpp	2016-05-17 19:18:44 +0000
 +++ tests/framework/fake_session.cpp	2016-05-24 21:32:26 +0000
@@ -28,6 +28,7 @@
  FakeSession::~FakeSession()
+ {
++    delete m_childSessions;
+ }
  QString FakeSession::name() const { return QString("foo-session"); }
@@ -38,7 +39,7 @@
  MirSurfaceListModel* FakeSession::promptSurfaceList() { return &m_promptSurfaceList; }
--SessionModel *FakeSession::childSessions() const { return nullptr; }
++SessionModel *FakeSession::childSessions() const { return m_childSessions; }
  SessionInterface::State FakeSession::state() const { return m_state; }
 === modified file 'tests/framework/fake_session.h'
 --- tests/framework/fake_session.h	2016-05-17 19:18:44 +0000
 +++ tests/framework/fake_session.h	2016-05-24 21:32:26 +0000
@@ -15,6 +15,7 @@
   */
  #include <Unity/Application/session_interface.h>
++#include <Unity/Application/sessionmodel.h>
  #include <Unity/Application/mirsurfacelistmodel.h>
  #include <QDebug>
@@ -83,6 +84,7 @@
      std::shared_ptr<mir::scene::Session> m_session;
      MirSurfaceListModel m_surfaceList;
      MirSurfaceListModel m_promptSurfaceList;
++    SessionModel *m_childSessions{new SessionModel};
  };
  } // namespace qtmi
 === renamed file 'tests/framework/stub_scene_surface.h' => 'tests/framework/fake_surface.h'
 --- tests/framework/stub_scene_surface.h	2016-02-01 12:18:00 +0000
 +++ tests/framework/fake_surface.h	2016-05-24 21:32:26 +0000
@@ -1,88 +1,108 @@
  /*
-- * Copyright (C) 2014-2015 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
-- * the Free Software Foundation.
-- *
-- * This program is distributed in the hope that it will be useful, but WITHOUT
-- * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
-- * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-- * Lesser General Public License for more details.
-- *
-- * You should have received a copy of the GNU Lesser General Public License
++ * Copyright © 2014,2016 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/>.
++ *
++ * Original file authored by: Andreas Pokorny <andreas.pokorny@canonical.com>
   */
--#ifndef MIR_TEST_DOUBLES_STUB_SCENE_SURFACE_H_
--#define MIR_TEST_DOUBLES_STUB_SCENE_SURFACE_H_
++#ifndef MIR_SCENE_FAKESURFACE_H_
++#define MIR_SCENE_FAKESURFACE_H_
--#include "mir/scene/surface.h"
++#include <mir/scene/surface.h>
  #include "stub_input_channel.h"
  #include <memory>
  #include <gmock/gmock.h>
--namespace mir
--{
--namespace test
--{
--namespace doubles
--{
++namespace mir {
++namespace scene {
--class StubSceneSurface :
--    public mir::scene::Surface
++class FakeSurface : public Surface
+ {
  public:
--    std::shared_ptr<StubInputChannel> channel;
++    std::shared_ptr<mir::test::doubles::StubInputChannel> channel;
      int fd;
      mir::input::InputReceptionMode input_mode{mir::input::InputReceptionMode::normal};
--    StubSceneSurface(int fd);
--    virtual ~StubSceneSurface();
--
--    std::shared_ptr<mir::input::InputChannel> input_channel() const override;
--
--    mir::input::InputReceptionMode reception_mode() const override;
--
--    std::string name() const override;
--    geometry::Point top_left() const override;
--    geometry::Size client_size() const override;
--    geometry::Size size() const override;
--    geometry::Rectangle input_bounds() const override;
--    bool input_area_contains(mir::geometry::Point const&) const override;
--
--    graphics::RenderableList generate_renderables(compositor::CompositorID) const override;
--    float alpha() const override;
--    MirSurfaceType type() const override;
--    MirSurfaceState state() const override;
--
--    void hide() override;
--    void show() override;
--    void move_to(geometry::Point const&) override;
--    void set_input_region(std::vector<geometry::Rectangle> const&) override;
--    void resize(geometry::Size const&) override;
--    void set_transformation(glm::mat4 const&) override;
--    void set_alpha(float) override;
--    void set_orientation(MirOrientation) override;
--
--    void add_observer(std::shared_ptr<scene::SurfaceObserver> const&) override;
--    void remove_observer(std::weak_ptr<scene::SurfaceObserver> const&) override;
--
--    void set_reception_mode(input::InputReceptionMode mode) override;
--    void consume(MirEvent const*) override;
--
--    void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& /* image */) override;
--    std::shared_ptr<graphics::CursorImage> cursor_image() const override;
--
--    bool supports_input() const override;
--    int client_input_fd() const override;
--    int configure(MirSurfaceAttrib, int) override;
--    int query(MirSurfaceAttrib) const override;
++    FakeSurface(int fd=123)
++        : channel(std::make_shared<mir::test::doubles::StubInputChannel>(fd)), fd(fd)
++    {
++    }
++
++    std::shared_ptr<mir::input::InputChannel> input_channel() const override
++    {
++        return channel;
++    }
++
++    mir::input::InputReceptionMode reception_mode() const override
++    {
++        return input_mode;
++    }
++
++    std::string name() const override { return {}; }
++    geometry::Point top_left() const override { return {}; }
++    geometry::Size client_size() const override { return {};}
++    geometry::Size size() const override { return {}; }
++    geometry::Rectangle input_bounds() const override { return {{},{}}; }
++    bool input_area_contains(mir::geometry::Point const&) const override { return false; }
++
++    void set_streams(std::list<scene::StreamInfo> const&) override {}
++    graphics::RenderableList generate_renderables(compositor::CompositorID) const override { return {}; }
++    int buffers_ready_for_compositor(void const*) const override { return 0; }
++
++    float alpha() const override { return 0.0f;}
++    MirSurfaceType type() const override { return mir_surface_type_normal; }
++    MirSurfaceState state() const override { return mir_surface_state_unknown; }
++
++    void hide() override {}
++    void show() override {}
++    bool visible() const override { return true; }
++    void move_to(geometry::Point const&) override {}
++    void set_input_region(std::vector<geometry::Rectangle> const&) override {}
++    void resize(geometry::Size const&) override {}
++    void set_transformation(glm::mat4 const&) override {}
++    void set_alpha(float) override {}
++    void set_orientation(MirOrientation) {}
++
++    void add_observer(std::shared_ptr<scene::SurfaceObserver> const&) override {}
++    void remove_observer(std::weak_ptr<scene::SurfaceObserver> const&) override {}
++
++    void set_reception_mode(input::InputReceptionMode mode) override { input_mode = mode; }
++    void consume(MirEvent const*) override {}
++
++    void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& /* image */) {}
++    std::shared_ptr<graphics::CursorImage> cursor_image() const { return {}; }
++
++    void request_client_surface_close() override {}
++
++    bool supports_input() const override { return true;}
++    int client_input_fd() const override { return fd;}
++    int configure(MirSurfaceAttrib, int) override { return 0; }
++    int query(MirSurfaceAttrib) const override { return 0; }
++    void with_most_recent_buffer_do(std::function<void(graphics::Buffer&)> const&) {}
++
++    std::shared_ptr<Surface> parent() const override { return nullptr; }
++
++    void set_keymap(MirInputDeviceId, std::string const&, std::string const&, std::string const&, std::string const&) override
++    {}
++
++    void set_cursor_stream(std::shared_ptr<frontend::BufferStream> const&, geometry::Displacement const&) {}
++    void rename(std::string const&) {}
++    std::shared_ptr<frontend::BufferStream> primary_buffer_stream() const override { return nullptr; }
  };
--}
--}
--}
++} // namespace scene
++} // namescape mir
--#endif
++#endif // MIR_SCENE_FAKESURFACE_H_
 === added file 'tests/framework/mock_shell.h'
 --- tests/framework/mock_shell.h	1970-01-01 00:00:00 +0000
 +++ tests/framework/mock_shell.h	2016-05-24 21:32:26 +0000
@@ -0,0 +1,111 @@
++/*
++ * Copyright (C) 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
++ * the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful, but WITHOUT
++ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
++ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
++ * Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++#ifndef MOCK_MIR_SHELL_SHELL_H
++#define MOCK_MIR_SHELL_SHELL_H
++
++#include <mir/shell/shell.h>
++
++#include <mir/scene/prompt_session_creation_parameters.h>
++#include <mir/scene/surface_creation_parameters.h>
++#include <mir/shell/surface_specification.h>
++
++#include <gmock/gmock.h>
++
++namespace mir {
++namespace shell {
++
++class MockShell : public Shell
++{
++public:
++
++    // From mir::shell::Shell
++
++    MOCK_METHOD3(open_session, std::shared_ptr<scene::Session>(
++        pid_t client_pid,
++        std::string const& name,
++        std::shared_ptr<frontend::EventSink> const& sink));
++
++    MOCK_METHOD1(close_session, void(std::shared_ptr<scene::Session> const& session));
++
++    MOCK_METHOD2(start_prompt_session_for, std::shared_ptr<scene::PromptSession>(
++        std::shared_ptr<scene::Session> const& session,
++        scene::PromptSessionCreationParameters const& params));
++
++    MOCK_METHOD2(add_prompt_provider_for, void (
++        std::shared_ptr<scene::PromptSession> const& prompt_session,
++        std::shared_ptr<scene::Session> const& session));
++
++    MOCK_METHOD1(stop_prompt_session, void (std::shared_ptr<scene::PromptSession> const& prompt_session));
++
++    MOCK_METHOD3(create_surface, frontend::SurfaceId (
++        std::shared_ptr<scene::Session> const& session,
++        scene::SurfaceCreationParameters const& params,
++        std::shared_ptr<frontend::EventSink> const& sink));
++
++    MOCK_METHOD3(modify_surface, void (
++        std::shared_ptr<scene::Session> const& session,
++        std::shared_ptr<scene::Surface> const& surface,
++        shell::SurfaceSpecification  const& modifications));
++
++    MOCK_METHOD2(destroy_surface, void (std::shared_ptr<scene::Session> const& session, frontend::SurfaceId surface));
++
++    MOCK_METHOD4(set_surface_attribute, int (
++        std::shared_ptr<scene::Session> const& session,
++        std::shared_ptr<scene::Surface> const& surface,
++        MirSurfaceAttrib attrib,
++        int value));
++
++    MOCK_METHOD2(get_surface_attribute, int (
++        std::shared_ptr<scene::Surface> const& surface,
++        MirSurfaceAttrib attrib));
++
++    MOCK_METHOD3(raise_surface, void (
++        std::shared_ptr<scene::Session> const& session,
++        std::shared_ptr<scene::Surface> const& surface,
++        uint64_t timestamp));
++
++    // From mir::shell::FocusController
++
++    MOCK_METHOD0(focus_next_session, void ());
++
++    MOCK_CONST_METHOD0(focused_session, std::shared_ptr<scene::Session>());
++
++    MOCK_METHOD2(set_focus_to, void (
++        std::shared_ptr<scene::Session> const& focus_session,
++        std::shared_ptr<scene::Surface> const& focus_surface));
++
++    MOCK_CONST_METHOD0(focused_surface, std::shared_ptr<scene::Surface>());
++
++    MOCK_CONST_METHOD1(surface_at, std::shared_ptr<scene::Surface> (geometry::Point cursor));
++
++    MOCK_METHOD1(raise, void (SurfaceSet const& surfaces));
++
++    // From mir::input::EventFilter
++
++    MOCK_METHOD1(handle, bool (MirEvent const& event));
++
++    // From mir::compositor::DisplayListener
++
++    MOCK_METHOD1(add_display, void (geometry::Rectangle const& area));
++
++    MOCK_METHOD1(remove_display, void (geometry::Rectangle const& area));
++};
++
++} // namespace shell
++} // namespace mir
++
++#endif // MOCK_MIR_SHELL_SHELL_H
 === removed file 'tests/framework/stub_scene_surface.cpp'
 --- tests/framework/stub_scene_surface.cpp	2016-02-01 12:18:00 +0000
 +++ tests/framework/stub_scene_surface.cpp	1970-01-01 00:00:00 +0000
@@ -1,103 +0,0 @@
--/*
-- * Copyright (C) 2015 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
-- * the Free Software Foundation.
-- *
-- * This program is distributed in the hope that it will be useful, but WITHOUT
-- * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
-- * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-- * Lesser General Public License for more details.
-- *
-- * You should have received a copy of the GNU Lesser General Public License
-- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- */
--
--#include "stub_scene_surface.h"
--
--namespace mir
--{
--namespace test
--{
--namespace doubles
--{
--
--StubSceneSurface::StubSceneSurface(int fd)
--    : channel(std::make_shared<StubInputChannel>(fd)), fd(fd)
--{
--}
--
--StubSceneSurface::~StubSceneSurface()
--{
--}
--
--std::shared_ptr<mir::input::InputChannel> StubSceneSurface::input_channel() const
--{
--    return channel;
--}
--
--mir::input::InputReceptionMode StubSceneSurface::reception_mode() const
--{
--    return input_mode;
--}
--
--std::string StubSceneSurface::name() const { return {}; }
--
--mir::geometry::Point StubSceneSurface::top_left() const { return {}; }
--
--mir::geometry::Size StubSceneSurface::client_size() const { return {};}
--
--mir::geometry::Size StubSceneSurface::size() const { return {}; }
--
--mir::geometry::Rectangle StubSceneSurface::input_bounds() const { return {{},{}}; }
--
--bool StubSceneSurface::input_area_contains(mir::geometry::Point const&) const { return false; }
--
--mir::graphics::RenderableList StubSceneSurface::generate_renderables(mir::compositor::CompositorID) const { return {};}
--
--float StubSceneSurface::alpha() const { return 0.0f; }
--
--MirSurfaceType StubSceneSurface::type() const { return mir_surface_type_normal; }
--
--MirSurfaceState StubSceneSurface::state() const { return mir_surface_state_unknown; }
--
--void StubSceneSurface::hide() {}
--
--void StubSceneSurface::show() {}
--
--void StubSceneSurface::move_to(const mir::geometry::Point &) {}
--
--void StubSceneSurface::set_input_region(const std::vector<mir::geometry::Rectangle> &) {}
--
--void StubSceneSurface::resize(const mir::geometry::Size &) {}
--
--void StubSceneSurface::set_transformation(const glm::mat4 &) {}
--
--void StubSceneSurface::set_alpha(float) {}
--
--void StubSceneSurface::set_orientation(MirOrientation) {}
--
--void StubSceneSurface::add_observer(const std::shared_ptr<mir::scene::SurfaceObserver> &) {}
--
--void StubSceneSurface::remove_observer(const std::weak_ptr<mir::scene::SurfaceObserver> &) {}
--
--void StubSceneSurface::set_reception_mode(mir::input::InputReceptionMode mode) { input_mode = mode; }
--
--void StubSceneSurface::consume(const MirEvent *) {}
--
--void StubSceneSurface::set_cursor_image(const std::shared_ptr<mir::graphics::CursorImage> &) {}
--
--std::shared_ptr<mir::graphics::CursorImage> StubSceneSurface::cursor_image() const { return {}; }
--
--bool StubSceneSurface::supports_input() const { return true; }
--
--int StubSceneSurface::client_input_fd() const { return fd;}
--
--int StubSceneSurface::configure(MirSurfaceAttrib, int) { return 0; }
--
--int StubSceneSurface::query(MirSurfaceAttrib) const { return 0; }
--
--} // namespace doubles
--} // namespace test
--} // namespace mir
 === modified file 'tests/modules/SessionManager/session_test.cpp'
 --- tests/modules/SessionManager/session_test.cpp	2016-05-17 19:18:44 +0000
 +++ tests/modules/SessionManager/session_test.cpp	2016-05-24 21:32:26 +0000
@@ -20,13 +20,10 @@
  #include <Unity/Application/application.h>
  #include <Unity/Application/mirsurfaceitem.h>
--#include "stub_scene_surface.h"
--
  using namespace qtmir;
  using mir::scene::MockSession;
  namespace ms = mir::scene;
--namespace mtd = mir::test::doubles;
  class SessionTests : public ::testing::QtMirTest
+ {
 === modified file 'tests/modules/SurfaceManager/CMakeLists.txt'
 --- tests/modules/SurfaceManager/CMakeLists.txt	2015-11-20 16:47:09 +0000
 +++ tests/modules/SurfaceManager/CMakeLists.txt	2016-05-24 21:32:26 +0000
@@ -10,6 +10,7 @@
    ${CMAKE_SOURCE_DIR}/src/platforms/mirserver
    ${CMAKE_SOURCE_DIR}/tests/framework
    ${MIRSERVER_INCLUDE_DIRS}
++  ${Qt5Quick_PRIVATE_INCLUDE_DIRS}
+ )
  add_executable(surfacemanager_test ${MIR_SURFACE_MANAGER_TEST_SOURCES})
 === modified file 'tests/modules/SurfaceManager/mirsurface_test.cpp'
 --- tests/modules/SurfaceManager/mirsurface_test.cpp	2016-03-28 18:02:26 +0000
 +++ tests/modules/SurfaceManager/mirsurface_test.cpp	2016-05-24 21:32:26 +0000
@@ -16,6 +16,8 @@
  #define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
++struct MirEvent {}; // otherwise won't compile otherwise due to incomplete type
++
  #include <gtest/gtest.h>
  #include <QLoggingCategory>
@@ -30,6 +32,7 @@
  // tests/framework
  #include <fake_session.h>
  #include <mock_mir_session.h>
++#include <mock_shell.h>
  #include <mock_surface.h>
  // tests/modules/common
@@ -41,6 +44,7 @@
  using namespace qtmir;
  namespace ms = mir::scene;
++using namespace testing;
  class MirSurfaceTest : public ::testing::Test
+ {
@@ -50,12 +54,12 @@
          // We don't want the logging spam cluttering the test results
          QLoggingCategory::setFilterRules(QStringLiteral("qtmir.surfaces=false"));
+     }
++
++    NiceMock<mir::shell::MockShell> m_mockShell;
  };
  TEST_F(MirSurfaceTest, UpdateTextureBeforeDraw)
+ {
--    using namespace testing;
--
      int argc = 0;
      char* argv[0];
      QCoreApplication qtApp(argc, argv); // app for deleteLater event
@@ -67,7 +71,7 @@
      EXPECT_CALL(*mockSurface.get(),buffers_ready_for_compositor(_))
          .WillRepeatedly(Return(1));
--    MirSurface *surface = new MirSurface(mockSurface, fakeSession, nullptr, surfaceObserver, CreationHints());
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &m_mockShell, surfaceObserver, CreationHints());
      surfaceObserver->frame_posted(1, mir::geometry::Size{1,1});
      QSignalSpy spyFrameDropped(surface, SIGNAL(frameDropped()));
@@ -81,8 +85,6 @@
  TEST_F(MirSurfaceTest, DeleteMirSurfaceOnLastNonLiveUnregisterView)
+ {
--    using namespace testing;
--
      int argc = 0;
      char* argv[0];
      QCoreApplication qtApp(argc, argv); // app for deleteLater event
@@ -90,7 +92,7 @@
      auto fakeSession = new FakeSession();
      auto mockSurface = std::make_shared<NiceMock<ms::MockSurface>>();
--    MirSurface *surface = new MirSurface(mockSurface, fakeSession, nullptr, nullptr, CreationHints());
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &m_mockShell, nullptr, CreationHints());
      bool surfaceDeleted = false;
      QObject::connect(surface, &QObject::destroyed, surface, [&surfaceDeleted](){ surfaceDeleted = true; });
@@ -114,8 +116,6 @@
  TEST_F(MirSurfaceTest, DoNotDeleteMirSurfaceOnLastLiveUnregisterView)
+ {
--    using namespace testing;
--
      int argc = 0;
      char* argv[0];
      QCoreApplication qtApp(argc, argv); // app for deleteLater event
@@ -123,7 +123,7 @@
      auto fakeSession = new FakeSession();
      auto mockSurface = std::make_shared<NiceMock<ms::MockSurface>>();
--    MirSurface *surface = new MirSurface(mockSurface, fakeSession, nullptr, nullptr, CreationHints());
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &m_mockShell, nullptr, CreationHints());
      bool surfaceDeleted = false;
      QObject::connect(surface, &QObject::destroyed, surface, [&surfaceDeleted](){ surfaceDeleted = true; });
@@ -148,7 +148,6 @@
   */
  TEST_F(MirSurfaceTest, failedSurfaceCloseEventualyDestroysSurface)
+ {
--    using namespace ::testing;
      const pid_t pid = 1234;
      const char appId[] = "test-app";
@@ -162,7 +161,7 @@
      auto mockSurface = std::make_shared<NiceMock<ms::MockSurface>>();
--    MirSurface *surface = new MirSurface(mockSurface, fakeSession, nullptr, nullptr, CreationHints());
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &m_mockShell, nullptr, CreationHints());
      bool surfaceDeleted = false;
      QObject::connect(surface, &QObject::destroyed, surface, [&surfaceDeleted](){ surfaceDeleted = true; });
 === modified file 'tests/modules/SurfaceManager/mirsurfaceitem_test.cpp'
 --- tests/modules/SurfaceManager/mirsurfaceitem_test.cpp	2015-12-15 10:03:32 +0000
 +++ tests/modules/SurfaceManager/mirsurfaceitem_test.cpp	2016-05-24 21:32:26 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014-2015 Canonical, Ltd.
++ * Copyright (C) 2014-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
@@ -16,16 +16,27 @@
  #define MIR_INCLUDE_DEPRECATED_EVENT_HEADER
++struct MirEvent {}; // otherwise won't compile otherwise due to incomplete type
++
  #include <gtest/gtest.h>
  #include <QLoggingCategory>
  #include <QTest>
++#include <private/qquickitem_p.h>
  // the test subject
  #include <Unity/Application/mirsurfaceitem.h>
++// and friends
++#include <Unity/Application/mirsurface.h>
  // tests/framework
  #include <fake_mirsurface.h>
++#include <fake_session.h>
++#include <mock_shell.h>
++#include <fake_surface.h>
++
++// tests/modules/common
++#include <surfaceobserver.h>
  using namespace qtmir;
@@ -152,3 +163,136 @@
      delete surfaceItem2;
      delete fakeSurface;
+ }
++
++TEST_F(MirSurfaceItemTest, NoSurfaceActiveFocusIfItemDoesNotConsumeInput)
++{
++    using namespace testing;
++
++    // All the stuff qtmir::MirSurface needs
++    auto fakeSession = new FakeSession();
++    std::shared_ptr<mir::scene::Surface> mockSurface = std::make_shared<mir::scene::FakeSurface>();
++    auto surfaceObserver = std::make_shared<SurfaceObserver>();
++    mir::shell::MockShell mockShell;
++
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &mockShell, surfaceObserver, CreationHints());
++
++    MirSurfaceItem *surfaceItem = new MirSurfaceItem;
++    QQuickItemPrivate *surfaceItemPrivate = QQuickItemPrivate::get(surfaceItem);
++
++    surfaceItem->setConsumesInput(true);
++    surfaceItemPrivate->activeFocus = true;
++
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(AtLeast(1));
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(0);
++
++    surfaceItem->setSurface(surface);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(0);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(AtLeast(1));
++
++    surfaceItem->setConsumesInput(false);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(AtLeast(1));
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(0);
++
++    surfaceItem->setConsumesInput(true);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(0);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(AtLeast(1));
++
++    delete surfaceItem;
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++
++    // clean up
++    delete surface;
++    delete fakeSession;
++}
++
++TEST_F(MirSurfaceItemTest, AggregateSurfaceActiveFocus)
++{
++    using namespace testing;
++
++    // All the stuff qtmir::MirSurface needs
++    auto fakeSession = new FakeSession();
++    std::shared_ptr<mir::scene::Surface> mockSurface = std::make_shared<mir::scene::FakeSurface>();
++    auto surfaceObserver = std::make_shared<SurfaceObserver>();
++    mir::shell::MockShell mockShell;
++
++    MirSurface *surface = new MirSurface(mockSurface, fakeSession, &mockShell, surfaceObserver, CreationHints());
++
++    MirSurfaceItem *surfaceItem1 = new MirSurfaceItem;
++    QQuickItemPrivate *surfaceItem1Private = QQuickItemPrivate::get(surfaceItem1);
++
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(0);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused));
++
++    surfaceItem1->setConsumesInput(true);
++    surfaceItem1Private->activeFocus = false;
++    surfaceItem1->setSurface(surface);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(AtLeast(1));
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(0);
++
++    surfaceItem1Private->activeFocus = true;
++    Q_EMIT surfaceItem1->activeFocusChanged(true);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(AtLeast(0)); // no harm in calling it unnecessarily
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(0);
++
++    MirSurfaceItem *surfaceItem2 = new MirSurfaceItem;
++    QQuickItemPrivate *surfaceItem2Private = QQuickItemPrivate::get(surfaceItem2);
++
++    surfaceItem2->setConsumesInput(true);
++    surfaceItem2Private->activeFocus = false;
++    surfaceItem2->setSurface(surface);
++
++    surfaceItem2Private->activeFocus = true;
++    Q_EMIT surfaceItem2->activeFocusChanged(true);
++    surfaceItem1Private->activeFocus = false;
++    Q_EMIT surfaceItem1->activeFocusChanged(false);
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(AtLeast(0)); // no harm in calling it unnecessarily
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(0);
++
++    surfaceItem1Private->activeFocus = true;
++    Q_EMIT surfaceItem1->activeFocusChanged(true);
++    surfaceItem1->setConsumesInput(false);
++
++    delete surfaceItem1;
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_focused))
++        .Times(0);
++    EXPECT_CALL(mockShell, set_surface_attribute(fakeSession->session(), mockSurface, mir_surface_attrib_focus, (int)mir_surface_unfocused))
++        .Times(AtLeast(1));
++
++    delete surfaceItem2;
++
++    Mock::VerifyAndClearExpectations(&mockShell);
++
++    // clean up
++    delete surface;
++    delete fakeSession;
++}