Merge lp:~mir-team/miral/move_workspace_content into lp:miral

[Nick Dedekind]

Added move WindowManagerTools::move_workspace_content_to_workspace

Added move WindowManagerTools::move_workspace_content_to_workspace to move all the windows from one workspace to another.

[lp:~mir-team/miral/move_workspace_content]

524. By Nick Dedekind on 2017-03-01

merged with trunk

[Alan Griffiths (alan-griffiths)]

Just a nit: I see the logic is there (but not the test) for windows that already exist in the target workspace.

Can we add a test that windows in both to and from only appear once after?

review: Needs Fixing

[lp:~mir-team/miral/move_workspace_content]

525. By Nick Dedekind on 2017-03-02

added test for moving workspaces with pre-existing

[Alan Griffiths (alan-griffiths)]

Great. Thanks!

review: Approve

=== modified file 'debian/libmiral2.symbols'
--- debian/libmiral2.symbols	2017-02-15 17:32:34 +0000
+++ debian/libmiral2.symbols	2017-03-02 10:25:41 +0000
@@ -375,6 +375,7 @@
  (c++)"miral::WindowManagerTools::create_workspace()@MIRAL_1.3" 1.3.0
  (c++)"miral::WindowManagerTools::add_tree_to_workspace(miral::Window const&, std::shared_ptr<miral::Workspace> const&)@MIRAL_1.3" 1.3.0
  (c++)"miral::WindowManagerTools::remove_tree_from_workspace(miral::Window const&, std::shared_ptr<miral::Workspace> const&)@MIRAL_1.3" 1.3.0
+ (c++)"miral::WindowManagerTools::move_workspace_content_to_workspace(std::shared_ptr<miral::Workspace> const&, std::shared_ptr<miral::Workspace> const&)@MIRAL_1.3" 1.3.0
  (c++)"miral::WorkspacePolicy::advise_adding_to_workspace(std::shared_ptr<miral::Workspace> const&, std::vector<miral::Window, std::allocator<miral::Window> > const&)@MIRAL_1.3" 1.3.0
  (c++)"miral::WorkspacePolicy::advise_removing_from_workspace(std::shared_ptr<miral::Workspace> const&, std::vector<miral::Window, std::allocator<miral::Window> > const&)@MIRAL_1.3" 1.3.0
  (c++)"miral::WindowManagerTools::for_each_window_in_workspace(std::shared_ptr<miral::Workspace> const&, std::function<void (miral::Window const&)> const&)@MIRAL_1.3" 1.3.0

=== modified file 'include/miral/window_manager_tools.h'
--- include/miral/window_manager_tools.h	2017-02-14 17:11:07 +0000
+++ include/miral/window_manager_tools.h	2017-03-02 10:25:41 +0000
@@ -193,6 +193,15 @@
     void remove_tree_from_workspace(Window const& window, std::shared_ptr<Workspace> const& workspace);
 
     /**
+     * Moves all the content from one workspace to another
+     * @param from_workspace the workspace to move the windows from;
+     * @param to_workspace the workspace to move the windows to;
+     */
+    void move_workspace_content_to_workspace(
+        std::shared_ptr<Workspace> const& to_workspace,
+        std::shared_ptr<Workspace> const& from_workspace);
+
+    /**
      * invoke callback with each workspace containing window
      * \warning it is unsafe to add or remove windows from workspaces from the callback during enumeration
      * @param window

=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2017-02-21 15:02:23 +0000
+++ miral/basic_window_manager.cpp	2017-03-02 10:25:41 +0000
@@ -1951,6 +1951,39 @@
         workspace_policy->advise_removing_from_workspace(workspace, windows_removed);
 }
 
+void miral::BasicWindowManager::move_workspace_content_to_workspace(
+    std::shared_ptr<Workspace> const& to_workspace, std::shared_ptr<Workspace> const& from_workspace)
+{
+    std::vector<Window> windows_removed;
+
+    auto const iter_pair_from = workspaces_to_windows.left.equal_range(from_workspace);
+    for (auto kv = iter_pair_from.first; kv != iter_pair_from.second;)
+    {
+        auto const current = kv++;
+        windows_removed.push_back(current->second);
+        workspaces_to_windows.left.erase(current);
+    }
+
+    if (!windows_removed.empty())
+        workspace_policy->advise_removing_from_workspace(from_workspace, windows_removed);
+
+    std::vector<Window> windows_added;
+
+    auto const iter_pair_to = workspaces_to_windows.left.equal_range(to_workspace);
+    for (auto& w : windows_removed)
+    {
+        if (!std::count_if(iter_pair_to.first, iter_pair_to.second,
+                           [&w](wwbimap_t::left_value_type const& kv) { return kv.second == w; }))
+        {
+            workspaces_to_windows.left.insert(wwbimap_t::left_value_type{to_workspace, w});
+            windows_added.push_back(w);
+        }
+    }
+
+    if (!windows_added.empty())
+        workspace_policy->advise_adding_to_workspace(to_workspace, windows_added);
+}
+
 void miral::BasicWindowManager::for_each_workspace_containing(
     miral::Window const& window, std::function<void(std::shared_ptr<miral::Workspace> const&)> const& callback)
 {

=== modified file 'miral/basic_window_manager.h'
--- miral/basic_window_manager.h	2017-02-27 12:38:27 +0000
+++ miral/basic_window_manager.h	2017-03-02 10:25:41 +0000
@@ -107,6 +107,10 @@
 
     void remove_tree_from_workspace(Window const& window, std::shared_ptr<Workspace> const& workspace) override;
 
+    void move_workspace_content_to_workspace(
+        std::shared_ptr<Workspace> const& to_workspace,
+        std::shared_ptr<Workspace> const& from_workspace);
+
     void for_each_workspace_containing(
         Window const& window,
         std::function<void(std::shared_ptr<Workspace> const& workspace)> const& callback) override;

=== modified file 'miral/symbols.map'
--- miral/symbols.map	2017-02-15 17:32:34 +0000
+++ miral/symbols.map	2017-03-02 10:25:41 +0000
@@ -376,6 +376,7 @@
     miral::WindowManagerTools::for_each_window_in_workspace*;
     miral::WindowManagerTools::for_each_workspace_containing*;
     miral::WindowManagerTools::remove_tree_from_workspace*;
+    miral::WindowManagerTools::move_workspace_content_to_workspace*;
     miral::WorkspacePolicy::?WorkspacePolicy*;
     miral::WorkspacePolicy::WorkspacePolicy*;
     miral::WorkspacePolicy::advise_adding_to_workspace*;

=== modified file 'miral/window_management_trace.cpp'
--- miral/window_management_trace.cpp	2017-02-17 15:36:54 +0000
+++ miral/window_management_trace.cpp	2017-03-02 10:25:41 +0000
@@ -555,6 +555,14 @@
 }
 MIRAL_TRACE_EXCEPTION
 
+void miral::WindowManagementTrace::move_workspace_content_to_workspace(
+    std::shared_ptr<Workspace> const& to_workspace, std::shared_ptr<Workspace> const& from_workspace)
+try {
+    mir::log_info("%s to_workspace=%p, from_workspace=%p", __func__, to_workspace.get(), from_workspace.get());
+    wrapped.move_workspace_content_to_workspace(to_workspace, from_workspace);
+}
+MIRAL_TRACE_EXCEPTION
+
 void miral::WindowManagementTrace::for_each_workspace_containing(
     miral::Window const& window, std::function<void(std::shared_ptr<miral::Workspace> const&)> const& callback)
 try {

=== modified file 'miral/window_management_trace.h'
--- miral/window_management_trace.h	2017-02-03 17:49:34 +0000
+++ miral/window_management_trace.h	2017-03-02 10:25:41 +0000
@@ -96,6 +96,10 @@
 
     void remove_tree_from_workspace(Window const& window, std::shared_ptr<Workspace> const& workspace) override;
 
+    void move_workspace_content_to_workspace(
+        std::shared_ptr<Workspace> const& to_workspace,
+        std::shared_ptr<Workspace> const& from_workspace) override;
+
     void for_each_workspace_containing(
         Window const& window,
         std::function<void(std::shared_ptr<Workspace> const& workspace)> const& callback) override;

=== modified file 'miral/window_manager_tools.cpp'
--- miral/window_manager_tools.cpp	2017-02-03 17:49:34 +0000
+++ miral/window_manager_tools.cpp	2017-03-02 10:25:41 +0000
@@ -112,6 +112,11 @@
     std::shared_ptr<miral::Workspace> const& workspace)
 { tools->remove_tree_from_workspace(window, workspace); }
 
+void miral::WindowManagerTools::move_workspace_content_to_workspace(
+    std::shared_ptr<Workspace> const& to_workspace,
+    std::shared_ptr<Workspace> const& from_workspace)
+{ tools->move_workspace_content_to_workspace(to_workspace, from_workspace); }
+
 void miral::WindowManagerTools::for_each_workspace_containing(
     miral::Window const& window,
     std::function<void(std::shared_ptr<miral::Workspace> const&)> const& callback)

=== modified file 'miral/window_manager_tools_implementation.h'
--- miral/window_manager_tools_implementation.h	2017-02-03 17:49:34 +0000
+++ miral/window_manager_tools_implementation.h	2017-03-02 10:25:41 +0000
@@ -73,6 +73,9 @@
     virtual auto create_workspace() -> std::shared_ptr<Workspace> = 0;
     virtual void add_tree_to_workspace(Window const& window, std::shared_ptr<Workspace> const& workspace) = 0;
     virtual void remove_tree_from_workspace(Window const& window, std::shared_ptr<Workspace> const& workspace) = 0;
+    virtual void move_workspace_content_to_workspace(
+        std::shared_ptr<Workspace> const& to_workspace,
+        std::shared_ptr<Workspace> const& from_workspace) = 0;
     virtual void for_each_workspace_containing(
         Window const& window,
         std::function<void(std::shared_ptr<Workspace> const& workspace)> const& callback) = 0;

=== modified file 'test/workspaces.cpp'
--- test/workspaces.cpp	2017-02-14 17:11:07 +0000
+++ test/workspaces.cpp	2017-03-02 10:25:41 +0000
@@ -601,3 +601,69 @@
                 << "server_window(a_window): " << tools.info_for(server_window(a_window)).name();
         });
 }
+
+TEST_F(Workspaces, move_windows_from_one_workspace_to_another)
+{
+    auto const pre_workspace = create_workspace();
+    auto const from_workspace = create_workspace();
+    auto const to_workspace = create_workspace();
+    auto const post_workspace = create_workspace();
+
+    create_window(a_window);
+    create_window(another_window);
+
+    invoke_tools([&, this](WindowManagerTools& tools)
+        {
+            tools.add_tree_to_workspace(server_window(a_window), pre_workspace);
+            tools.add_tree_to_workspace(server_window(top_level), from_workspace);
+            tools.add_tree_to_workspace(server_window(another_window), post_workspace);
+
+            tools.move_workspace_content_to_workspace(to_workspace, from_workspace);
+        });
+
+    EXPECT_THAT(windows_in_workspace(from_workspace).size(), Eq(0u));
+
+    EXPECT_THAT(workspaces_containing_window(server_window(a_window)), ElementsAre(pre_workspace));
+    EXPECT_THAT(workspaces_containing_window(server_window(top_level)), ElementsAre(to_workspace));
+    EXPECT_THAT(workspaces_containing_window(server_window(dialog)), ElementsAre(to_workspace));
+    EXPECT_THAT(workspaces_containing_window(server_window(tip)), ElementsAre(to_workspace));
+    EXPECT_THAT(workspaces_containing_window(server_window(another_window)), ElementsAre(post_workspace));
+}
+
+TEST_F(Workspaces, when_moving_windows_from_one_workspace_to_another_windows_only_appear_once_in_target_workspace)
+{
+    auto const from_workspace = create_workspace();
+    auto const to_workspace = create_workspace();
+
+    create_window(a_window);
+    create_window(another_window);
+
+    invoke_tools([&, this](WindowManagerTools& tools)
+        {
+            tools.add_tree_to_workspace(server_window(a_window), from_workspace);
+            tools.add_tree_to_workspace(server_window(another_window), from_workspace);
+            tools.add_tree_to_workspace(server_window(a_window), to_workspace);
+
+            tools.move_workspace_content_to_workspace(to_workspace, from_workspace);
+        });
+
+    EXPECT_THAT(windows_in_workspace(to_workspace), ElementsAre(server_window(a_window), server_window(another_window)));
+}
+
+TEST_F(Workspaces, when_workspace_content_is_moved_the_policy_is_notified)
+{
+    auto const from_workspace = create_workspace();
+    auto const to_workspace = create_workspace();
+
+    EXPECT_CALL(policy(), advise_removing_from_workspace(from_workspace,
+         ElementsAre(server_window(top_level), server_window(dialog), server_window(tip))));
+
+     EXPECT_CALL(policy(), advise_adding_to_workspace(to_workspace,
+          ElementsAre(server_window(top_level), server_window(dialog), server_window(tip))));
+
+    invoke_tools([&, this](WindowManagerTools& tools)
+        {
+            tools.add_tree_to_workspace(server_window(dialog), from_workspace);
+            tools.move_workspace_content_to_workspace(to_workspace, from_workspace);
+        });
+}