Add Move function to Tab Groups extension API

This CL adds the implementation and tests for the Move function, which
rounds out the functions in the Tab Groups API.

Bug: 1106846
Change-Id: If943b628272aed06b700162dab85ccab4091b842
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419754
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826003}

chrome/browser/extensions/api/tab_groups/tab_groups_api.cc

@@ -5,8 +5,8 @@
 #include "chrome/browser/extensions/api/tab_groups/tab_groups_api.h"
 
 #include <memory>
-#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/strings/pattern.h"
 #include "base/strings/string16.h"
@@ -35,6 +35,37 @@
 
 namespace extensions {
 
+namespace {
+
+// Returns true if a group could be moved into the |target_index| of the given
+// |tab_strip|. Sets the |error| string otherwise.
+bool IndexSupportsGroupMove(TabStripModel* tab_strip,
+                            int target_index,
+                            std::string* error) {
+  // A group can always be moved to the end of the tabstrip.
+  if (target_index >= tab_strip->count() || target_index < 0)
+    return true;
+
+  if (tab_strip->IsTabPinned(target_index)) {
+    *error = tab_groups_constants::kCannotMoveGroupIntoMiddleOfPinnedTabsError;
+    return false;
+  }
+
+  base::Optional<tab_groups::TabGroupId> target_group =
+      tab_strip->GetTabGroupForTab(target_index);
+  base::Optional<tab_groups::TabGroupId> adjacent_group =
+      tab_strip->GetTabGroupForTab(target_index - 1);
+
+  if (target_group.has_value() && target_group == adjacent_group) {
+    *error = tab_groups_constants::kCannotMoveGroupIntoMiddleOfOtherGroupError;
+    return false;
+  }
+
+  return true;
+}
+
+}  // namespace
+
 ExtensionFunction::ResponseAction TabGroupsGetFunction::Run() {
   std::unique_ptr<api::tab_groups::Get::Params> params(
       api::tab_groups::Get::Params::Create(*args_));
@@ -166,4 +197,131 @@ ExtensionFunction::ResponseAction TabGroupsUpdateFunction::Run() {
                                              *tab_group->visual_data()))));
 }
 
+ExtensionFunction::ResponseAction TabGroupsMoveFunction::Run() {
+  std::unique_ptr<api::tab_groups::Move::Params> params(
+      api::tab_groups::Move::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params);
+
+  int group_id = params->group_id;
+  int new_index = params->move_properties.index;
+  int* window_id = params->move_properties.window_id.get();
+
+  tab_groups::TabGroupId group = tab_groups::TabGroupId::CreateEmpty();
+  std::string error;
+  if (!MoveGroup(group_id, new_index, window_id, &group, &error))
+    return RespondNow(Error(std::move(error)));
+
+  if (!has_callback())
+    return RespondNow(NoArguments());
+
+  return RespondNow(ArgumentList(api::tab_groups::Get::Results::Create(
+      *tab_groups_util::CreateTabGroupObject(group))));
+}
+
+bool TabGroupsMoveFunction::MoveGroup(int group_id,
+                                      int new_index,
+                                      int* window_id,
+                                      tab_groups::TabGroupId* group,
+                                      std::string* error) {
+  Browser* source_browser = nullptr;
+  const tab_groups::TabGroupVisualData* visual_data = nullptr;
+  if (!tab_groups_util::GetGroupById(
+          group_id, browser_context(), include_incognito_information(),
+          &source_browser, group, &visual_data, error)) {
+    return false;
+  }
+
+  if (!source_browser->window()->IsTabStripEditable()) {
+    *error = tabs_constants::kTabStripNotEditableError;
+    return false;
+  }
+
+  TabStripModel* source_tab_strip = source_browser->tab_strip_model();
+  std::vector<int> tabs =
+      source_tab_strip->group_model()->GetTabGroup(*group)->ListTabs();
+
+  if (window_id) {
+    Browser* target_browser = nullptr;
+
+    if (!windows_util::GetBrowserFromWindowID(
+            this, *window_id, WindowController::GetAllWindowFilter(),
+            &target_browser, error)) {
+      return false;
+    }
+
+    if (!target_browser->window()->IsTabStripEditable()) {
+      *error = tabs_constants::kTabStripNotEditableError;
+      return false;
+    }
+
+    // TODO(crbug.com/990158): Rather than calling is_type_normal(), should
+    // this call SupportsWindowFeature(Browser::FEATURE_TABSTRIP)?
+    if (!target_browser->is_type_normal()) {
+      *error = tabs_constants::kCanOnlyMoveTabsWithinNormalWindowsError;
+      return false;
+    }
+
+    if (target_browser->profile() != source_browser->profile()) {
+      *error = tabs_constants::kCanOnlyMoveTabsWithinSameProfileError;
+      return false;
+    }
+
+    // If windowId is different from the current window, move between windows.
+    if (target_browser != source_browser) {
+      TabStripModel* target_tab_strip = target_browser->tab_strip_model();
+
+      if (new_index > target_tab_strip->count() || new_index < 0)
+        new_index = target_tab_strip->count();
+
+      if (!IndexSupportsGroupMove(target_tab_strip, new_index, error))
+        return false;
+
+      target_tab_strip->group_model()->AddTabGroup(*group, *visual_data);
+
+      for (size_t i = 0; i < tabs.size(); ++i) {
+        // Detach tabs from the same index each time, since each detached tab is
+        // removed from the model, and groups are always contiguous.
+        std::unique_ptr<content::WebContents> web_contents =
+            source_tab_strip->DetachWebContentsAt(tabs.front());
+
+        // Attach tabs in consecutive indices, to insert them in the same order.
+        target_tab_strip->InsertWebContentsAt(new_index + i,
+                                              std::move(web_contents),
+                                              TabStripModel::ADD_NONE, *group);
+      }
+
+      return true;
+    }
+  }
+
+  // Perform a move within the same window.
+
+  // When moving to the right, adjust the target index for the size of the
+  // group, since the group itself may occupy several indices to the right.
+  const int start_index = tabs.front();
+  if (new_index > start_index)
+    new_index += tabs.size() - 1;
+
+  // Unlike when moving between windows, IndexSupportsGroupMove should be called
+  // before clamping the index to count()-1 instead of after. Since the current
+  // group being moved could occupy index count()-1, IndexSupportsGroupMove
+  // could return a false negative for the current group.
+  if (!IndexSupportsGroupMove(source_tab_strip, new_index, error))
+    return false;
+
+  // Unlike when moving between windows, the index should be clamped to
+  // count()-1 instead of count(). Since the current tab(s) being moved are
+  // within the same tabstrip, they can't be added beyond the end of the
+  // occupied indices, but rather just shifted among them.
+  if (new_index >= source_tab_strip->count() || new_index < 0)
+    new_index = source_tab_strip->count() - 1;
+
+  if (new_index == start_index)
+    return true;
+
+  source_tab_strip->MoveGroupTo(*group, new_index);
+
+  return true;
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/tab_groups/tab_groups_api.h

@@ -5,8 +5,14 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_TAB_GROUPS_TAB_GROUPS_API_H_
 #define CHROME_BROWSER_EXTENSIONS_API_TAB_GROUPS_TAB_GROUPS_API_H_
 
+#include <string>
+
 #include "extensions/browser/extension_function.h"
 
+namespace tab_groups {
+class TabGroupId;
+}
+
 namespace extensions {
 
 class TabGroupsGetFunction : public ExtensionFunction {
@@ -51,6 +57,31 @@ class TabGroupsUpdateFunction : public ExtensionFunction {
   ~TabGroupsUpdateFunction() override = default;
 };
 
+class TabGroupsMoveFunction : public ExtensionFunction {
+ public:
+  DECLARE_EXTENSION_FUNCTION("tabGroups.move", TAB_GROUPS_MOVE)
+  TabGroupsMoveFunction() = default;
+  TabGroupsMoveFunction(const TabGroupsMoveFunction&) = delete;
+  TabGroupsMoveFunction& operator=(const TabGroupsMoveFunction&) = delete;
+
+  // ExtensionFunction:
+  ResponseAction Run() override;
+
+ protected:
+  ~TabGroupsMoveFunction() override = default;
+
+ private:
+  // Moves the group with ID |group_id| to the |new_index| in the window with ID
+  // |window_id|. If |window_id| is not specified, moves the group within its
+  // current window. |group| is populated with the group's TabGroupId, and
+  // |error| is populated if the group cannot be found or moved.
+  bool MoveGroup(int group_id,
+                 int new_index,
+                 int* window_id,
+                 tab_groups::TabGroupId* group,
+                 std::string* error);
+};
+
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_TAB_GROUPS_TAB_GROUPS_API_H_

chrome/browser/extensions/api/tab_groups/tab_groups_api_unittest.cc

@@ -297,4 +297,169 @@ TEST_F(TabGroupsApiUnitTest, TabGroupsUpdateError) {
             error);
 }
 
+// Test that moving a group to the right results in the correct tab order.
+TEST_F(TabGroupsApiUnitTest, TabGroupsMoveRight) {
+  scoped_refptr<const Extension> extension = CreateTabGroupsExtension();
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  // Create a group with multiple tabs.
+  tab_groups::TabGroupId group = tab_strip_model->AddToNewGroup({1, 2, 3});
+  int group_id = tab_groups_util::GetGroupId(group);
+
+  // Use the TabGroupsMoveFunction to move the group to index 2.
+  auto function = base::MakeRefCounted<TabGroupsMoveFunction>();
+  function->set_extension(extension);
+  constexpr char kFormatArgs[] = R"([%d, {"index": 2}])";
+  const std::string args = base::StringPrintf(kFormatArgs, group_id);
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      function.get(), args, browser(), api_test_utils::NONE));
+
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(0), web_contents(0));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(1), web_contents(4));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(2), web_contents(1));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(3), web_contents(2));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(4), web_contents(3));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(5), web_contents(5));
+
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(2).value());
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(3).value());
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(4).value());
+}
+
+// Test that moving a group to the left results in the correct tab order.
+TEST_F(TabGroupsApiUnitTest, TabGroupsMoveLeft) {
+  scoped_refptr<const Extension> extension = CreateTabGroupsExtension();
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  // Create a group with multiple tabs.
+  tab_groups::TabGroupId group = tab_strip_model->AddToNewGroup({2, 3, 4});
+  int group_id = tab_groups_util::GetGroupId(group);
+
+  // Use the TabGroupsMoveFunction to move the group to index 0.
+  auto function = base::MakeRefCounted<TabGroupsMoveFunction>();
+  function->set_extension(extension);
+  constexpr char kFormatArgs[] = R"([%d, {"index": 0}])";
+  const std::string args = base::StringPrintf(kFormatArgs, group_id);
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      function.get(), args, browser(), api_test_utils::NONE));
+
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(0), web_contents(2));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(1), web_contents(3));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(2), web_contents(4));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(3), web_contents(0));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(4), web_contents(1));
+  EXPECT_EQ(tab_strip_model->GetWebContentsAt(5), web_contents(5));
+
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(0).value());
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(1).value());
+  EXPECT_EQ(group, tab_strip_model->GetTabGroupForTab(2).value());
+}
+
+// Test that moving a group to another window works as expected.
+TEST_F(TabGroupsApiUnitTest, TabGroupsMoveAcrossWindows) {
+  scoped_refptr<const Extension> extension = CreateTabGroupsExtension();
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  // Create a group with multiple tabs.
+  tab_groups::TabGroupId group = tab_strip_model->AddToNewGroup({2, 3, 4});
+  int group_id = tab_groups_util::GetGroupId(group);
+
+  // Create a new window and add a few tabs.
+  TestBrowserWindow* window2 = new TestBrowserWindow;
+  // TestBrowserWindowOwner handles its own lifetime, and also cleans up
+  // |window2|.
+  new TestBrowserWindowOwner(window2);
+  Browser::CreateParams params(profile(), /* user_gesture */ true);
+  params.type = Browser::TYPE_NORMAL;
+  params.window = window2;
+  std::unique_ptr<Browser> browser2(Browser::Create(params));
+  BrowserList::SetLastActive(browser2.get());
+  int window_id2 = ExtensionTabUtil::GetWindowId(browser2.get());
+
+  TabStripModel* tab_strip_model2 = browser2->tab_strip_model();
+  constexpr int kNumTabs2 = 3;
+  for (int i = 0; i < kNumTabs2; ++i) {
+    std::unique_ptr<content::WebContents> contents(
+        content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
+    CreateSessionServiceTabHelper(contents.get());
+    tab_strip_model2->AppendWebContents(std::move(contents),
+                                        /* foreground */ true);
+  }
+  ASSERT_EQ(kNumTabs2, tab_strip_model2->count());
+
+  // Use the TabGroupsMoveFunction to move the group to index 1 in the other
+  // window.
+  constexpr int kNumTabsMovedAcrossWindows = 3;
+  auto function = base::MakeRefCounted<TabGroupsMoveFunction>();
+  function->set_extension(extension);
+  constexpr char kFormatArgs[] = R"([%d, {"windowId": %d, "index": 1}])";
+  const std::string args =
+      base::StringPrintf(kFormatArgs, group_id, window_id2);
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      function.get(), args, browser(), api_test_utils::NONE));
+
+  ASSERT_EQ(kNumTabs2 + kNumTabsMovedAcrossWindows, tab_strip_model2->count());
+  EXPECT_EQ(tab_strip_model2->GetWebContentsAt(1), web_contents(2));
+  EXPECT_EQ(tab_strip_model2->GetWebContentsAt(2), web_contents(3));
+  EXPECT_EQ(tab_strip_model2->GetWebContentsAt(3), web_contents(4));
+
+  EXPECT_EQ(group, tab_strip_model2->GetTabGroupForTab(1).value());
+  EXPECT_EQ(group, tab_strip_model2->GetTabGroupForTab(2).value());
+  EXPECT_EQ(group, tab_strip_model2->GetTabGroupForTab(3).value());
+
+  // Clean up.
+  browser2->tab_strip_model()->CloseAllTabs();
+}
+
+// Test that a group is cannot be moved into the pinned tabs region.
+TEST_F(TabGroupsApiUnitTest, TabGroupsMoveToPinnedError) {
+  scoped_refptr<const Extension> extension = CreateTabGroupsExtension();
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  // Pin the first 3 tabs.
+  tab_strip_model->SetTabPinned(0, /* pinned */ true);
+  tab_strip_model->SetTabPinned(1, /* pinned */ true);
+  tab_strip_model->SetTabPinned(2, /* pinned */ true);
+
+  // Create a group with an unpinned tab.
+  tab_groups::TabGroupId group = tab_strip_model->AddToNewGroup({4});
+  int group_id = tab_groups_util::GetGroupId(group);
+
+  // Try to move the group to index 1 and expect an error.
+  auto function = base::MakeRefCounted<TabGroupsMoveFunction>();
+  function->set_extension(extension);
+  constexpr char kFormatArgs[] = R"([%d, {"index": 1}])";
+  const std::string args = base::StringPrintf(kFormatArgs, group_id);
+  std::string error = extension_function_test_utils::RunFunctionAndReturnError(
+      function.get(), args, browser(), api_test_utils::NONE);
+  EXPECT_EQ(tab_groups_constants::kCannotMoveGroupIntoMiddleOfPinnedTabsError,
+            error);
+}
+
+// Test that a group cannot be moved into the middle of another group.
+TEST_F(TabGroupsApiUnitTest, TabGroupsMoveToOtherGroupError) {
+  scoped_refptr<const Extension> extension = CreateTabGroupsExtension();
+
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  // Create two tab groups, one with multiple tabs and the other to move.
+  tab_strip_model->AddToNewGroup({0, 1, 2});
+  tab_groups::TabGroupId group = tab_strip_model->AddToNewGroup({4});
+  int group_id = tab_groups_util::GetGroupId(group);
+
+  // Try to move the second group to index 1 and expect an error.
+  auto function = base::MakeRefCounted<TabGroupsMoveFunction>();
+  function->set_extension(extension);
+  constexpr char kFormatArgs[] = R"([%d, {"index": 1}])";
+  const std::string args = base::StringPrintf(kFormatArgs, group_id);
+  std::string error = extension_function_test_utils::RunFunctionAndReturnError(
+      function.get(), args, browser(), api_test_utils::NONE);
+  EXPECT_EQ(tab_groups_constants::kCannotMoveGroupIntoMiddleOfOtherGroupError,
+            error);
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/tab_groups/tab_groups_constants.cc

@@ -11,6 +11,10 @@ const char kCollapsedKey[] = "collapsed";
 const char kColorKey[] = "color";
 const char kTitleKey[] = "title";
 
+const char kCannotMoveGroupIntoMiddleOfOtherGroupError[] =
+    "Cannot move the group to an index that is in the middle of another group.";
+const char kCannotMoveGroupIntoMiddleOfPinnedTabsError[] =
+    "Cannot move the group to an index that is in the middle of pinned tabs.";
 const char kGroupNotFoundError[] = "No group with id: *.";
 
 }  // namespace tab_groups_constants

chrome/browser/extensions/api/tab_groups/tab_groups_constants.h

@@ -16,6 +16,8 @@ extern const char kColorKey[];
 extern const char kTitleKey[];
 
 // Error messages.
+extern const char kCannotMoveGroupIntoMiddleOfOtherGroupError[];
+extern const char kCannotMoveGroupIntoMiddleOfPinnedTabsError[];
 extern const char kGroupNotFoundError[];
 
 }  // namespace tab_groups_constants

chrome/common/extensions/api/tab_groups.json

@@ -144,6 +144,49 @@
             ]
           }
         ]
+      },
+      {
+        "name": "move",
+        "type": "function",
+        "description": "Moves the group and all its tabs within its window, or to a new window.",
+        "parameters": [
+          {
+            "type": "integer",
+            "name": "groupId",
+            "minimum": 0,
+            "description": "The ID of the group to move."
+          },
+          {
+            "type": "object",
+            "name": "moveProperties",
+            "properties": {
+              "windowId": {
+                "type": "integer",
+                "minimum": -2,
+                "optional": true,
+                "description": "The window to move the group to. Defaults to the window the group is currently in. Note that groups can only be moved to and from windows with $(ref:windows.WindowType) type <code>\"normal\"</code>."
+              },
+              "index": {
+                "type": "integer",
+                "minimum": -1,
+                "description": "The position to move the group to. Use <code>-1</code> to place the group at the end of the window."
+              }
+            }
+          },
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "name": "group",
+                "$ref": "TabGroup",
+                "optional": true,
+                "description": "Details about the moved group."
+              }
+            ]
+          }
+        ]
       }
     ]
   }

extensions/browser/extension_function_histogram_value.h

@@ -1591,6 +1591,7 @@ enum HistogramValue {
   TAB_GROUPS_QUERY = 1528,
   TAB_GROUPS_UPDATE = 1529,
   ACCESSIBILITY_PRIVATE_UPDATESELECTTOSPEAKPANEL = 1530,
+  TAB_GROUPS_MOVE = 1531,
   // Last entry: Add new entries above, then run:
   // python tools/metrics/histograms/update_extension_histograms.py
   ENUM_BOUNDARY

third_party/closure_compiler/externs/tab_groups.js

@@ -83,3 +83,15 @@ chrome.tabGroups.query = function(queryInfo, callback) {};
  * @see https://developer.chrome.com/extensions/tabGroups#method-update
  */
 chrome.tabGroups.update = function(groupId, updateProperties, callback) {};
+
+/**
+ * Moves the group and all its tabs within its window, or to a new window.
+ * @param {number} groupId The ID of the group to move.
+ * @param {{
+ *   windowId: (number|undefined),
+ *   index: number
+ * }} moveProperties
+ * @param {function((!chrome.tabGroups.TabGroup|undefined)): void=} callback
+ * @see https://developer.chrome.com/extensions/tabGroups#method-move
+ */
+chrome.tabGroups.move = function(groupId, moveProperties, callback) {};

tools/metrics/histograms/enums.xml

@@ -25326,6 +25326,7 @@ Called by update_extension_histograms.py.-->
   <int value="1528" label="TAB_GROUPS_QUERY"/>
   <int value="1529" label="TAB_GROUPS_UPDATE"/>
   <int value="1530" label="ACCESSIBILITY_PRIVATE_UPDATESELECTTOSPEAKPANEL"/>
+  <int value="1531" label="TAB_GROUPS_MOVE"/>
 </enum>
 
 <enum name="ExtensionIconState">