Merge lp:~alan-griffiths/miral/BasicWindowManager-provides-active_window-logic into lp:miral

[Alan Griffiths]

Provide standard "active window" support in BasicWindowManager, just calling the policies for stuff that can reasonably differ.

Provide standard "active window" support in BasicWindowManager, just calling the policies for stuff that can reasonably differ.

[lp:~alan-griffiths/miral/BasicWindowManager-provides-active_window-logic]

200. By Alan Griffiths on 2016-05-27

Use MRU list when switching applications

201. By Alan Griffiths on 2016-05-27

Extract common code

=== modified file 'include/miral/window_management_policy.h'
--- include/miral/window_management_policy.h	2016-05-06 15:36:23 +0000
+++ include/miral/window_management_policy.h	2016-05-28 07:54:16 +0000
@@ -50,7 +50,8 @@
     virtual bool handle_pointer_event(MirPointerEvent const* event) = 0;
     virtual void handle_raise_window(WindowInfo& window_info) = 0;
 
-    virtual auto select_active_window(Window const& hint) -> Window = 0;
+    virtual void handle_focus_lost(miral::WindowInfo const& info) = 0;
+    virtual void handle_focus_gained(miral::WindowInfo const& info) = 0;
 
     virtual ~WindowManagementPolicy() = default;
     WindowManagementPolicy() = default;
 
=== modified file 'include/miral/window_manager_tools.h'
--- include/miral/window_manager_tools.h	2016-05-06 16:04:34 +0000
+++ include/miral/window_manager_tools.h	2016-05-28 07:54:16 +0000
@@ -55,10 +55,10 @@
     virtual auto info_for(std::weak_ptr<mir::scene::Session> const& session) const -> ApplicationInfo& = 0;
     virtual auto info_for(std::weak_ptr<mir::scene::Surface> const& surface) const -> WindowInfo& = 0;
     virtual auto info_for(Window const& window) const -> WindowInfo& = 0;
-    virtual auto focused_application() const -> Application = 0;
-    virtual auto focused_window() const -> Window = 0;
+    virtual void kill_active_application(int sig) = 0;
+    virtual auto active_window() const -> Window = 0;
+    virtual auto select_active_window(Window const& hint) -> Window = 0;
     virtual void focus_next_application() = 0;
-    virtual void set_focus_to(Window const& window) = 0;
     virtual auto window_at(mir::geometry::Point cursor) const -> Window = 0;
     virtual auto active_display() -> mir::geometry::Rectangle const = 0;
     virtual void destroy(Window& window) = 0;
 
=== modified file 'miral-kiosk/kiosk_window_manager.cpp'
--- miral-kiosk/kiosk_window_manager.cpp	2016-05-28 07:54:16 +0000
+++ miral-kiosk/kiosk_window_manager.cpp	2016-05-28 07:54:16 +0000
@@ -116,7 +116,7 @@
 
 void KioskWindowManagerPolicy::handle_window_ready(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools->select_active_window(window_info.window());
 }
 
 void KioskWindowManagerPolicy::handle_modify_window(
@@ -147,7 +147,7 @@
 
 void KioskWindowManagerPolicy::handle_raise_window(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools->select_active_window(window_info.window());
 }
 
 bool KioskWindowManagerPolicy::handle_keyboard_event(MirKeyboardEvent const* event)
@@ -168,18 +168,18 @@
             modifiers == mir_input_event_modifier_alt &&
             scan_code == KEY_GRAVE)
     {
-        if (auto const prev = tools->focused_window())
+        if (auto const prev = tools->active_window())
         {
             auto const& siblings = tools->info_for(prev.application()).windows();
             auto current = find(begin(siblings), end(siblings), prev);
 
-            while (current != end(siblings) && prev == select_active_window(*current))
+            while (current != end(siblings) && prev == tools->select_active_window(*current))
                 ++current;
 
             if (current == end(siblings))
             {
                 current = begin(siblings);
-                while (prev != *current && prev == select_active_window(*current))
+                while (prev != *current && prev == tools->select_active_window(*current))
                     ++current;
             }
         }
@@ -218,41 +218,12 @@
 
     if (action == mir_pointer_action_button_down)
     {
-        select_active_window(tools->window_at(cursor));
+        tools->select_active_window(tools->window_at(cursor));
     }
 
     return false;
 }
 
-auto KioskWindowManagerPolicy::select_active_window(Window const& window) -> Window
-{
-    if (!window)
-    {
-        tools->set_focus_to({});
-        return window;
-    }
-
-    auto const& info_for = tools->info_for(window);
-
-    if (info_for.can_be_active())
-    {
-        tools->set_focus_to(info_for.window());
-        tools->raise_tree(window);
-
-        raise_splash_session();
-
-        return window;
-    }
-    else
-    {
-        // Cannot have input focus - try the parent
-        if (auto const parent = info_for.parent())
-            return select_active_window(parent);
-
-        return {};
-    }
-}
-
 void KioskWindowManagerPolicy::raise_splash_session() const
 {
     if (auto session = splash.session().lock())
@@ -263,3 +234,12 @@
             tools->raise_tree(s);
     }
 }
+
+void KioskWindowManagerPolicy::handle_focus_gained(WindowInfo const& info)
+{
+    tools->raise_tree(info.window());
+}
+
+void KioskWindowManagerPolicy::handle_focus_lost(WindowInfo const& /*info*/)
+{
+}
 
=== modified file 'miral-kiosk/kiosk_window_manager.h'
--- miral-kiosk/kiosk_window_manager.h	2016-05-06 14:38:26 +0000
+++ miral-kiosk/kiosk_window_manager.h	2016-05-28 07:54:16 +0000
@@ -57,7 +57,9 @@
 
     void handle_raise_window(miral::WindowInfo& window_info) override;
 
-    auto select_active_window(miral::Window const& window) -> miral::Window override;
+    void handle_focus_lost(miral::WindowInfo const& info) override;
+
+    void handle_focus_gained(miral::WindowInfo const& info) override;
 
 private:
     static const int modifier_mask =
 
=== modified file 'miral-shell/canonical_window_manager.cpp'
--- miral-shell/canonical_window_manager.cpp	2016-05-28 07:54:16 +0000
+++ miral-shell/canonical_window_manager.cpp	2016-05-28 07:54:16 +0000
@@ -58,7 +58,7 @@
 void CanonicalWindowManagerPolicy::click(Point cursor)
 {
     if (auto const window = tools->window_at(cursor))
-        select_active_window(window);
+        tools->select_active_window(window);
 }
 
 void CanonicalWindowManagerPolicy::handle_app_info_updated(Rectangles const& /*displays*/)
@@ -86,8 +86,8 @@
 bool CanonicalWindowManagerPolicy::resize(Point cursor)
 {
     if (!resizing)
-        select_active_window(tools->window_at(old_cursor));
-    return resize(active_window(), cursor, old_cursor);
+        tools->select_active_window(tools->window_at(old_cursor));
+    return resize(tools->active_window(), cursor, old_cursor);
 }
 
 
@@ -279,7 +279,7 @@
 
 void CanonicalWindowManagerPolicy::handle_window_ready(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools->select_active_window(window_info.window());
 }
 
 void CanonicalWindowManagerPolicy::handle_modify_window(
@@ -370,9 +370,6 @@
     {
         tools->destroy(titlebar->window);
     }
-
-    if (active_window() == window_info.window())
-        active_window_.reset();
 }
 
 auto CanonicalWindowManagerPolicy::handle_set_state(WindowInfo& window_info, MirSurfaceState value)
@@ -505,13 +502,13 @@
 
 void CanonicalWindowManagerPolicy::drag(Point cursor)
 {
-    select_active_window(tools->window_at(old_cursor));
-    drag(active_window(), cursor, old_cursor, display_area);
+    tools->select_active_window(tools->window_at(old_cursor));
+    drag(tools->active_window(), cursor, old_cursor, display_area);
 }
 
 void CanonicalWindowManagerPolicy::handle_raise_window(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools->select_active_window(window_info.window());
 }
 
 bool CanonicalWindowManagerPolicy::handle_keyboard_event(MirKeyboardEvent const* event)
@@ -545,13 +542,11 @@
         switch (modifiers & modifier_mask)
         {
         case mir_input_event_modifier_alt:
-            if (auto const application = tools->focused_application())
-                miral::kill(application, SIGTERM);
-
+            tools->kill_active_application(SIGTERM);
             return true;
 
         case mir_input_event_modifier_ctrl:
-            if (auto const window = tools->focused_window())
+            if (auto const window = tools->active_window())
                 window.request_client_surface_close();
 
             return true;
@@ -572,18 +567,18 @@
             modifiers == mir_input_event_modifier_alt &&
             scan_code == KEY_GRAVE)
     {
-        if (auto const prev = tools->focused_window())
+        if (auto const prev = tools->active_window())
         {
             auto const& siblings = tools->info_for(prev.application()).windows();
             auto current = find(begin(siblings), end(siblings), prev);
 
-            while (current != end(siblings) && prev == select_active_window(*current))
+            while (current != end(siblings) && prev == tools->select_active_window(*current))
                 ++current;
 
             if (current == end(siblings))
             {
                 current = begin(siblings);
-                while (prev != *current && prev == select_active_window(*current))
+                while (prev != *current && prev == tools->select_active_window(*current))
                     ++current;
             }
         }
@@ -702,7 +697,7 @@
 
 void CanonicalWindowManagerPolicy::toggle(MirSurfaceState state)
 {
-    if (auto window = active_window())
+    if (auto const window = tools->active_window())
     {
         auto& info = tools->info_for(window);
 
@@ -713,54 +708,13 @@
     }
 }
 
-auto CanonicalWindowManagerPolicy::select_active_window(Window const& hint) -> miral::Window
-{
-    auto const prev_window = active_window();
-
-    if (hint == prev_window)
-        return hint;
-
-    if (!hint)
-    {
-        if (prev_window)
-        {
-            tools->set_focus_to({});
-            handle_focus_lost(tools->info_for(prev_window));
-        }
-
-        return hint;
-    }
-
-    auto const& info_for_hint = tools->info_for(hint);
-
-    if (info_for_hint.can_be_active())
-    {
-        tools->set_focus_to(hint);
-        tools->raise_tree(hint);
-
-        if (prev_window)
-            handle_focus_lost(tools->info_for(prev_window));
-
-        handle_focus_gained(info_for_hint);
-        return hint;
-    }
-    else
-    {
-        // Cannot have input focus - try the parent
-        if (auto const parent = info_for_hint.parent())
-            return select_active_window(parent);
-    }
-
-    return {};
-}
-
 void CanonicalWindowManagerPolicy::handle_focus_gained(WindowInfo const& info)
 {
+    tools->raise_tree(info.window());
+
     if (auto const titlebar = std::static_pointer_cast<CanonicalWindowManagementPolicyData>(info.userdata()))
         titlebar->paint_titlebar(0xFF);
 
-    active_window_ = info.window();
-
     // Frig to force the spinner to the top
     if (auto const spinner_session = spinner.session())
     {
@@ -775,14 +729,6 @@
 {
     if (auto const titlebar = std::static_pointer_cast<CanonicalWindowManagementPolicyData>(info.userdata()))
         titlebar->paint_titlebar(0x3F);
-
-    active_window_.reset();
-}
-
-auto CanonicalWindowManagerPolicy::active_window() const
--> Window
-{
-    return active_window_;
 }
 
 bool CanonicalWindowManagerPolicy::resize(Window const& window, Point cursor, Point old_cursor)
 
=== modified file 'miral-shell/canonical_window_manager.h'
--- miral-shell/canonical_window_manager.h	2016-05-28 07:54:16 +0000
+++ miral-shell/canonical_window_manager.h	2016-05-28 07:54:16 +0000
@@ -74,7 +74,9 @@
 
     void handle_raise_window(miral::WindowInfo& window_info) override;
 
-    auto select_active_window(miral::Window const& hint) -> miral::Window override;
+    void handle_focus_lost(miral::WindowInfo const& info) override;
+
+    void handle_focus_gained(miral::WindowInfo const& info) override;
 
 private:
     static const int modifier_mask =
@@ -89,9 +91,6 @@
     bool resize(Point cursor);
     void toggle(MirSurfaceState state);
 
-    auto active_window() const -> miral::Window;
-    void handle_focus_lost(miral::WindowInfo const& info);
-    void handle_focus_gained(miral::WindowInfo const& info);
 
     bool resize(miral::Window const& window, Point cursor, Point old_cursor);
     bool drag(miral::Window window, Point to, Point from, Rectangle bounds);
@@ -105,7 +104,6 @@
 
     Rectangle display_area;
     Point old_cursor{};
-    miral::Window active_window_;
     using FullscreenSurfaces = std::set<miral::Window>;
 
     FullscreenSurfaces fullscreen_surfaces;
 
=== modified file 'miral-shell/tiling_window_manager.cpp'
--- miral-shell/tiling_window_manager.cpp	2016-05-28 07:54:16 +0000
+++ miral-shell/tiling_window_manager.cpp	2016-05-28 07:54:16 +0000
@@ -57,7 +57,7 @@
 void TilingWindowManagerPolicy::click(Point cursor)
 {
     auto const window = tools->window_at(cursor);
-    select_active_window(window);
+    tools->select_active_window(window);
 }
 
 void TilingWindowManagerPolicy::handle_app_info_updated(Rectangles const& displays)
@@ -76,7 +76,7 @@
     {
         if (application == application_under(old_cursor))
         {
-            if (auto const window = select_active_window(tools->window_at(old_cursor)))
+            if (auto const window = tools->select_active_window(tools->window_at(old_cursor)))
             {
                 resize(window, cursor, old_cursor, tile_for(tools->info_for(application)));
             }
@@ -154,7 +154,7 @@
 
 void TilingWindowManagerPolicy::handle_window_ready(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools->select_active_window(window_info.window());
 }
 
 void TilingWindowManagerPolicy::handle_modify_window(
@@ -240,7 +240,7 @@
     {
         if (application == application_under(old_cursor))
         {
-            if (auto const window = select_active_window(tools->window_at(old_cursor)))
+            if (auto const window = tools->select_active_window(tools->window_at(old_cursor)))
             {
                 drag(tools->info_for(window), cursor, old_cursor, tile_for(tools->info_for(application)));
             }
@@ -250,7 +250,7 @@
 
 void TilingWindowManagerPolicy::handle_raise_window(WindowInfo& window_info)
 {
-    select_active_window(window_info.window());
+    tools-> select_active_window(window_info.window());
 }
 
 bool TilingWindowManagerPolicy::handle_keyboard_event(MirKeyboardEvent const* event)
@@ -284,13 +284,12 @@
         switch (modifiers & modifier_mask)
         {
         case mir_input_event_modifier_alt:
-            if (auto const application = tools->focused_application())
-                miral::kill(application, SIGTERM);
+            tools->kill_active_application(SIGTERM);
 
             return true;
 
         case mir_input_event_modifier_ctrl:
-            if (auto const window = tools->focused_window())
+            if (auto const window = tools->active_window())
                 window.request_client_surface_close();
 
             return true;
@@ -311,18 +310,18 @@
             modifiers == mir_input_event_modifier_alt &&
             scan_code == KEY_GRAVE)
     {
-        if (auto const prev = tools->focused_window())
+        if (auto const prev = tools->active_window())
         {
             auto const& siblings = tools->info_for(prev.application()).windows();
             auto current = find(begin(siblings), end(siblings), prev);
 
-            while (current != end(siblings) && prev == select_active_window(*current))
+            while (current != end(siblings) && prev == tools->select_active_window(*current))
                 ++current;
 
             if (current == end(siblings))
             {
                 current = begin(siblings);
-                while (prev != *current && prev == select_active_window(*current))
+                while (prev != *current && prev == tools->select_active_window(*current))
                     ++current;
             }
         }
@@ -420,7 +419,7 @@
 
 void TilingWindowManagerPolicy::toggle(MirSurfaceState state)
 {
-    if (auto window = tools->focused_window())
+    if (auto window = tools->active_window())
     {
         auto& window_info = tools->info_for(window);
 
@@ -583,28 +582,11 @@
     }
 }
 
-auto TilingWindowManagerPolicy::select_active_window(Window const& window) -> Window
-{
-    if (!window)
-    {
-        tools->set_focus_to({});
-        return window;
-    }
-
-    auto const& info_for = tools->info_for(window);
-
-    if (info_for.can_be_active())
-    {
-        tools->set_focus_to(info_for.window());
-        tools->raise_tree(window);
-        return window;
-    }
-    else
-    {
-        // Cannot have input focus - try the parent
-        if (auto const parent = info_for.parent())
-            return select_active_window(parent);
-
-        return {};
-    }
+void TilingWindowManagerPolicy::handle_focus_gained(WindowInfo const& info)
+{
+    tools->raise_tree(info.window());
+}
+
+void TilingWindowManagerPolicy::handle_focus_lost(WindowInfo const& /*info*/)
+{
 }
 
=== modified file 'miral-shell/tiling_window_manager.h'
--- miral-shell/tiling_window_manager.h	2016-05-06 14:38:26 +0000
+++ miral-shell/tiling_window_manager.h	2016-05-28 07:54:16 +0000
@@ -68,7 +68,9 @@
 
     void handle_raise_window(miral::WindowInfo& window_info) override;
 
-    auto select_active_window(miral::Window const& window) -> miral::Window override;
+    void handle_focus_lost(miral::WindowInfo const& info) override;
+
+    void handle_focus_gained(miral::WindowInfo const& info) override;
 
 private:
     static const int modifier_mask =
 
=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2016-05-28 07:54:16 +0000
+++ miral/basic_window_manager.cpp	2016-05-28 07:54:16 +0000
@@ -151,38 +151,21 @@
     if (is_active_window)
     {
         // Try to make the parent active
-        if (parent)
-        {
-            if (policy->select_active_window(parent))
-                return;
-        }
-
-        // TODO the policy for choosing a window is mixed with implementing the choice.
-        // I.e. select_active_window() calls set_focus_to() which updates mru_active_windows
-        // during the iteration. There must be a better way to sequence the logic.
-        // Until then we copy the list.
-        auto const copy_mru_windows = mru_active_windows;
-
-        // Try to activate to recently active window of same application
-        {
-            Window new_focus;
-
-            copy_mru_windows.enumerate([&](Window& window)
-                {
-                    return window.application() != session ||
-                        !(new_focus = policy->select_active_window(window));
-                });
-
-            if (new_focus) return;
-        }
+        if (parent && select_active_window(parent))
+            return;
+
+        if (can_activate_window_for_session(session))
+            return;
 
         // Try to activate to recently active window of any application
         {
             Window new_focus;
 
-            copy_mru_windows.enumerate([&](Window& window)
+            mru_active_windows.enumerate([&](Window& window)
             {
-                return !(new_focus = policy->select_active_window(window));
+                // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window
+                auto const w = window;
+                return !(new_focus = select_active_window(w));
             });
 
             if (new_focus) return;
@@ -315,29 +298,27 @@
     return info_for(std::weak_ptr<mir::scene::Surface>(window));
 }
 
-auto miral::BasicWindowManager::focused_application() const
--> Application
+void miral::BasicWindowManager::kill_active_application(int sig)
 {
-    return focus_controller->focused_session();
+    if (auto const application = focus_controller->focused_session())
+        miral::kill(application, sig);
 }
 
-auto miral::BasicWindowManager::focused_window() const
--> Window
+auto miral::BasicWindowManager::active_window() const -> Window
 {
-    auto focussed_surface = focus_controller->focused_surface();
-    return focussed_surface ? info_for(focussed_surface).window() : Window{};
+    return mru_active_windows.top();
 }
 
 void miral::BasicWindowManager::focus_next_application()
 {
     focus_controller->focus_next_session();
-    policy->select_active_window(focused_window());
-}
-
-void miral::BasicWindowManager::set_focus_to(Window const& window)
-{
-    if (window) mru_active_windows.push(window);
-    focus_controller->set_focus_to(window.application(), window);
+
+    if (can_activate_window_for_session(focus_controller->focused_session()))
+        return;
+
+    // Last resort: accept wherever focus_controller placed focus
+    auto const focussed_surface = focus_controller->focused_surface();
+    select_active_window(focussed_surface ? info_for(focussed_surface).window() : Window{});
 }
 
 auto miral::BasicWindowManager::window_at(geometry::Point cursor) const
@@ -462,3 +443,55 @@
     std::lock_guard<decltype(mutex)> lock(mutex);
     callback();
 }
+
+auto miral::BasicWindowManager::select_active_window(Window const& hint) -> miral::Window
+{
+    auto const prev_window = active_window();
+
+    if (!hint)
+    {
+        if (prev_window)
+        {
+            focus_controller->set_focus_to(hint.application(), hint);
+            policy->handle_focus_lost(info_for(prev_window));
+        }
+
+        return hint;
+    }
+
+    auto const& info_for_hint = info_for(hint);
+
+    if (info_for_hint.can_be_active())
+    {
+        mru_active_windows.push(hint);
+        focus_controller->set_focus_to(hint.application(), hint);
+
+        if (prev_window && prev_window != hint)
+            policy->handle_focus_lost(info_for(prev_window));
+
+        policy->handle_focus_gained(info_for_hint);
+        return hint;
+    }
+    else
+    {
+        // Cannot have input focus - try the parent
+        if (auto const parent = info_for_hint.parent())
+            return select_active_window(parent);
+    }
+
+    return {};
+}
+
+auto miral::BasicWindowManager::can_activate_window_for_session(miral::Application const& session) -> bool
+{
+    miral::Window new_focus;
+
+    mru_active_windows.enumerate([&](miral::Window& window)
+        {
+            // select_active_window() calls set_focus_to() which updates mru_active_windows and changes window
+            auto const w = window;
+            return w.application() != session || !(new_focus = miral::BasicWindowManager::select_active_window(w));
+        });
+
+    return new_focus;
+}
 \ No newline at end of file
 
=== modified file 'miral/basic_window_manager.h'
--- miral/basic_window_manager.h	2016-05-28 07:54:16 +0000
+++ miral/basic_window_manager.h	2016-05-28 07:54:16 +0000
@@ -114,14 +114,14 @@
 
     auto info_for(Window const& window) const -> WindowInfo& override;
 
-    auto focused_application() const -> Application override;
-
-    auto focused_window() const -> Window override;
+    void kill_active_application(int sig) override;
+
+    auto active_window() const -> Window override;
+
+    auto select_active_window(Window const& hint) -> Window override;
 
     void focus_next_application() override;
 
-    void set_focus_to(Window const& window) override;
-
     auto window_at(mir::geometry::Point cursor) const -> Window override;
 
     auto active_display() -> mir::geometry::Rectangle const override;
@@ -156,6 +156,8 @@
     void update_event_timestamp(MirKeyboardEvent const* kev);
     void update_event_timestamp(MirPointerEvent const* pev);
     void update_event_timestamp(MirTouchEvent const* tev);
+
+    auto can_activate_window_for_session(miral::Application const& session) -> bool;
 };
 }