Add context menu submenu to add tab to existing tab group.
Reuses (and slightly simplifies) some of the logic used to add tabs to
new tab groups.
This CL is part of the prototype of the above-described tab groups
feature. The prototype will allow users to create and manipulate groups
primarily via tab context menus, and will display tab group affiliation
in the tabstrip. Future work will include persisting and syncing groups,
manipulating groups via tab dragging, and a dropdown menu for the group
headers. See go/chrome-tab-groups-design
Bug: 905491
Change-Id: Id36b896ab5e1cfb51f85b6e1f4fb931670fa2ff1
Reviewed-on: https://chromium-review.googlesource.com/c/1406292
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623533}diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 683f83a..b3b6844 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -5755,9 +5755,12 @@
<message name="IDS_TAB_CXMENU_SEND_TO_MY_DEVICES" desc="The label of the tab context menu item for share this tab to other devices.">
Send to my devices
</message>
- <message name="IDS_TAB_CXMENU_ADD_TAB_TO_NEW_GROUP" desc="The label of the tab context menu item for creating a new tab group and adding a single tab to it.">
+ <message name="IDS_TAB_CXMENU_ADD_TAB_TO_NEW_GROUP" desc="The label of the tab context menu item for creating a new tab group and adding one or more tabs to it.">
Add to new group
</message>
+ <message name="IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP" desc="The label of the tab context menu submenu for adding one or more tabs to an existing tab group.">
+ Add to existing group
+ </message>
</if>
<if expr="use_titlecase">
<message name="IDS_TAB_CXMENU_NEWTAB" desc="In Title Case: The label of the 'New Tab' Tab context menu item.">
@@ -5799,8 +5802,11 @@
<message name="IDS_TAB_CXMENU_BOOKMARK_ALL_TABS" desc="In Title Case: The label of the tab context menu item for creating a bookmark folder containing an entry for each open tab.">
Bookmark All Tabs...
</message>
- <message name="IDS_TAB_CXMENU_ADD_TAB_TO_NEW_GROUP" desc="In Title Case: The label of the tab context menu item for creating a new tab group and adding a single tab to it.">
- Add Tab to New Group
+ <message name="IDS_TAB_CXMENU_ADD_TAB_TO_NEW_GROUP" desc="In Title Case: The label of the tab context menu item for creating a new tab group and adding one or more tabs to it.">
+ Add to New Group
+ </message>
+ <message name="IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP" desc="In Title Case: The label of the tab context menu submenu for adding one or more tabs to an existing tab group.">
+ Add to Existing Group
</message>
<message name="IDS_TAB_CXMENU_SEND_TO_MY_DEVICES" desc="In Title Case: The label of the tab context menu item for share this tab to other devices.">
Send To My Devices
diff --git a/chrome/app/generated_resources_grd/IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP.png.sha1 b/chrome/app/generated_resources_grd/IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP.png.sha1
new file mode 100644
index 0000000..c3607b4
--- /dev/null
+++ b/chrome/app/generated_resources_grd/IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP.png.sha1
@@ -0,0 +1 @@
+19feaf7abe591f018878302c0ac3c2958a07c430
\ No newline at end of file
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index a49c40a..0fd706c 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -1007,6 +1007,8 @@
"tab_contents/tab_contents_iterator.h",
"tab_modal_confirm_dialog_delegate.cc",
"tab_modal_confirm_dialog_delegate.h",
+ "tabs/existing_tab_group_sub_menu_model.cc",
+ "tabs/existing_tab_group_sub_menu_model.h",
"tabs/hover_tab_selector.cc",
"tabs/hover_tab_selector.h",
"tabs/pinned_tab_codec.cc",
diff --git a/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.cc b/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.cc
new file mode 100644
index 0000000..7cec602
--- /dev/null
+++ b/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.cc
@@ -0,0 +1,77 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.h"
+
+#include "chrome/browser/ui/tabs/tab_group_data.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+
+constexpr int kFirstCommandIndex =
+ TabStripModel::ContextMenuCommand::CommandLast + 1;
+
+ExistingTabGroupSubMenuModel::ExistingTabGroupSubMenuModel(TabStripModel* model,
+ int context_index)
+ : SimpleMenuModel(this) {
+ model_ = model;
+ context_index_ = context_index;
+ Build();
+}
+
+void ExistingTabGroupSubMenuModel::Build() {
+ // Start command ids after the parent menu's ids to avoid collisions.
+ int group_index = kFirstCommandIndex;
+ for (TabGroupData* group : model_->ListTabGroups()) {
+ if (ShouldShowGroup(model_, context_index_, group)) {
+ AddItem(group_index, group->title());
+ }
+ group_index++;
+ }
+}
+
+bool ExistingTabGroupSubMenuModel::IsCommandIdChecked(int command_id) const {
+ return false;
+}
+
+bool ExistingTabGroupSubMenuModel::IsCommandIdEnabled(int command_id) const {
+ return true;
+}
+
+void ExistingTabGroupSubMenuModel::ExecuteCommand(int command_id,
+ int event_flags) {
+ const int groupId = command_id - kFirstCommandIndex;
+ // TODO(https://crbug.com/922736): If a group has been deleted, groupId may
+ // refer to a different group than it did when the menu was created.
+ DCHECK((size_t)groupId < model_->ListTabGroups().size());
+ model_->ExecuteAddToExistingGroupCommand(context_index_,
+ model_->ListTabGroups()[groupId]);
+}
+
+// static
+bool ExistingTabGroupSubMenuModel::ShouldShowSubmenu(TabStripModel* model,
+ int context_index) {
+ for (TabGroupData* group : model->ListTabGroups()) {
+ if (ShouldShowGroup(model, context_index, group)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+// static
+bool ExistingTabGroupSubMenuModel::ShouldShowGroup(TabStripModel* model,
+ int context_index,
+ TabGroupData* group) {
+ if (!model->IsTabSelected(context_index)) {
+ if (group != nullptr && group != model->GetTabGroupForTab(context_index)) {
+ return true;
+ }
+ } else {
+ for (int index : model->selection_model().selected_indices()) {
+ if (group != nullptr && group != model->GetTabGroupForTab(index)) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
diff --git a/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.h b/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.h
new file mode 100644
index 0000000..7901738e
--- /dev/null
+++ b/chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.h
@@ -0,0 +1,50 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_TABS_EXISTING_TAB_GROUP_SUB_MENU_MODEL_H_
+#define CHROME_BROWSER_UI_TABS_EXISTING_TAB_GROUP_SUB_MENU_MODEL_H_
+
+#include <stddef.h>
+
+#include "base/macros.h"
+#include "chrome/browser/ui/tabs/tab_group_data.h"
+#include "ui/base/models/simple_menu_model.h"
+
+class TabStripModel;
+
+class ExistingTabGroupSubMenuModel : public ui::SimpleMenuModel,
+ ui::SimpleMenuModel::Delegate {
+ public:
+ ExistingTabGroupSubMenuModel(TabStripModel* model, int context_index);
+ ~ExistingTabGroupSubMenuModel() override = default;
+
+ bool IsCommandIdChecked(int command_id) const override;
+
+ bool IsCommandIdEnabled(int command_id) const override;
+
+ void ExecuteCommand(int command_id, int event_flags) override;
+
+ // Whether the submenu should be shown in the provided context. True iff
+ // the submenu would show at least one group. Does not assume ownership of
+ // |model|; |model| must outlive this instance.
+ static bool ShouldShowSubmenu(TabStripModel* model, int context_index_);
+
+ private:
+ void Build();
+
+ // Unowned; |model_| must outlive this instance.
+ TabStripModel* model_;
+
+ int context_index_;
+
+ // Whether the submenu should contain the group |group|. True iff at least
+ // one tab that would be affected by the command is not in |group|.
+ static bool ShouldShowGroup(TabStripModel* model,
+ int context_index,
+ TabGroupData* group);
+
+ DISALLOW_COPY_AND_ASSIGN(ExistingTabGroupSubMenuModel);
+};
+
+#endif // CHROME_BROWSER_UI_TABS_EXISTING_TAB_GROUP_SUB_MENU_MODEL_H_
diff --git a/chrome/browser/ui/tabs/tab_group_data.cc b/chrome/browser/ui/tabs/tab_group_data.cc
index 745c6d3..aced56e 100644
--- a/chrome/browser/ui/tabs/tab_group_data.cc
+++ b/chrome/browser/ui/tabs/tab_group_data.cc
@@ -7,7 +7,9 @@
#include "third_party/skia/include/utils/SkRandom.h"
TabGroupData::TabGroupData() {
- title_ = base::ASCIIToUTF16("Group");
+ static int groupCount = 0;
+ title_ = base::ASCIIToUTF16("Group " + std::to_string(groupCount));
+ groupCount++;
static SkRandom rand;
stroke_color_ = rand.nextU() | 0xff000000;
}
diff --git a/chrome/browser/ui/tabs/tab_menu_model.cc b/chrome/browser/ui/tabs/tab_menu_model.cc
index c784cf8..f6fe2d9 100644
--- a/chrome/browser/ui/tabs/tab_menu_model.cc
+++ b/chrome/browser/ui/tabs/tab_menu_model.cc
@@ -6,6 +6,7 @@
#include "base/command_line.h"
#include "chrome/browser/browser_features.h"
+#include "chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
#include "chrome/browser/ui/tabs/tab_utils.h"
@@ -21,6 +22,8 @@
Build(tab_strip, index);
}
+TabMenuModel::~TabMenuModel() {}
+
void TabMenuModel::Build(TabStripModel* tab_strip, int index) {
std::vector<int> affected_indices =
tab_strip->IsTabSelected(index)
@@ -31,6 +34,15 @@
if (base::FeatureList::IsEnabled(features::kTabGroups)) {
AddItemWithStringId(TabStripModel::CommandAddToNewGroup,
IDS_TAB_CXMENU_ADD_TAB_TO_NEW_GROUP);
+
+ // Create submenu with existing groups
+ if (ExistingTabGroupSubMenuModel::ShouldShowSubmenu(tab_strip, index)) {
+ add_to_existing_group_submenu_ =
+ std::make_unique<ExistingTabGroupSubMenuModel>(tab_strip, index);
+ AddSubMenuWithStringId(TabStripModel::CommandAddToExistingGroup,
+ IDS_TAB_CXMENU_ADD_TAB_TO_EXISTING_GROUP,
+ add_to_existing_group_submenu_.get());
+ }
}
AddSeparator(ui::NORMAL_SEPARATOR);
AddItemWithStringId(TabStripModel::CommandReload, IDS_TAB_CXMENU_RELOAD);
diff --git a/chrome/browser/ui/tabs/tab_menu_model.h b/chrome/browser/ui/tabs/tab_menu_model.h
index 48b9ec9..fd7c13a 100644
--- a/chrome/browser/ui/tabs/tab_menu_model.h
+++ b/chrome/browser/ui/tabs/tab_menu_model.h
@@ -18,11 +18,13 @@
TabMenuModel(ui::SimpleMenuModel::Delegate* delegate,
TabStripModel* tab_strip,
int index);
- ~TabMenuModel() override {}
+ ~TabMenuModel() override;
private:
void Build(TabStripModel* tab_strip, int index);
+ std::unique_ptr<ui::SimpleMenuModel> add_to_existing_group_submenu_;
+
DISALLOW_COPY_AND_ASSIGN(TabMenuModel);
};
diff --git a/chrome/browser/ui/tabs/tab_strip_model.cc b/chrome/browser/ui/tabs/tab_strip_model.cc
index 7dc8104..c6d6d4f 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model.cc
@@ -156,8 +156,8 @@
void set_pinned(bool value) { pinned_ = value; }
bool blocked() const { return blocked_; }
void set_blocked(bool value) { blocked_ = value; }
- TabGroupData* group() const { return group_; }
- void set_group(TabGroupData* value) { group_ = value; }
+ const TabGroupData* group() const { return group_; }
+ void set_group(const TabGroupData* value) { group_ = value; }
private:
// Make sure that if someone deletes this WebContents out from under us, it
@@ -192,7 +192,7 @@
// break that guarantee, with undefined results.
// - The exact shape of the group-related changes to the TabStripModel API
// (and the relevant bits of the extension API) are TBD.
- TabGroupData* group_ = nullptr;
+ const TabGroupData* group_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(WebContentsData);
};
@@ -762,6 +762,15 @@
return contents_data_[index]->group();
}
+std::vector<TabGroupData*> TabStripModel::ListTabGroups() const {
+ std::vector<TabGroupData*> groups;
+ for (std::unique_ptr<TabGroupData> const& group : group_data_) {
+ groups.push_back(group.get());
+ }
+
+ return groups;
+}
+
int TabStripModel::IndexOfFirstNonPinnedTab() const {
for (size_t i = 0; i < contents_data_.size(); ++i) {
if (!IsTabPinned(static_cast<int>(i)))
@@ -936,13 +945,53 @@
if (ContainsIndex(destination_index - 1)) {
const TabGroupData* split_group = GetTabGroupForTab(destination_index - 1);
if (split_group != nullptr) {
- while (ContainsIndex(destination_index + 1) &&
- GetTabGroupForTab(destination_index + 1) == split_group) {
+ while (ContainsIndex(destination_index) &&
+ GetTabGroupForTab(destination_index) == split_group) {
destination_index++;
}
}
}
+ std::vector<int> new_indices =
+ IsTabPinned(indices[0]) ? SetTabsPinned(indices, true) : indices;
+
+ MoveTabsIntoGroup(new_indices, destination_index, group);
+}
+
+void TabStripModel::AddToExistingGroup(const std::vector<int>& indices,
+ const TabGroupData* group) {
+ // TODO(https://crbug.com/915956): Tabs should be ungrouped before they are
+ // moved (once ungrouping is a thing) so that groups never get split up.
+
+ int destination_index = -1;
+ bool pin = false;
+ for (int i = contents_data_.size() - 1; i >= 0; i--) {
+ if (contents_data_[i]->group() == group) {
+ destination_index = i + 1;
+ pin = IsTabPinned(i);
+ break;
+ }
+ }
+ // TODO(https://crbug.com/915956): No tab already exists in that group.
+ // This state will be unreachable once tab groups are deleted when their
+ // last member is ungrouped. DCHECK for now.
+ DCHECK_NE(destination_index, -1);
+
+ // Ignore indices that are already in the group.
+ std::vector<int> new_indices;
+ for (size_t i = 0; i < indices.size(); i++) {
+ if (GetTabGroupForTab(indices[i]) != group) {
+ new_indices.push_back(indices[i]);
+ }
+ }
+ new_indices = SetTabsPinned(new_indices, pin);
+
+ MoveTabsIntoGroup(new_indices, destination_index, group);
+}
+
+void TabStripModel::MoveTabsIntoGroup(const std::vector<int>& indices,
+ int destination_index,
+ const TabGroupData* group) {
// Some tabs will need to be moved to the right, some to the left. We need to
// handle those separately. First, move tabs to the right, starting with the
// rightmost tab so we don't cause other tabs we are about to move to shift.
@@ -952,35 +1001,49 @@
numTabsMovingRight++;
}
for (int i = numTabsMovingRight - 1; i >= 0; i--) {
- int insertion_index = destination_index - numTabsMovingRight + i + 1;
+ int insertion_index = destination_index - numTabsMovingRight + i;
MoveWebContentsAt(indices[i], insertion_index, false);
contents_data_[insertion_index]->set_group(group);
}
- // Collect indices for tabs moving to the left, pinning them if any tabs in
- // |indices| are pinned (or, equivalently, if the first tab is). Any tabs
- // pinned here will no longer be in the position indicated in |indices|, so
- // we need to record adjusted indices in |move_left_indices|. If we aren't
- // pinning a tab, we can just collect its unmodified index.
+ // Collect indices for tabs moving to the left.
std::vector<int> move_left_indices;
for (size_t i = numTabsMovingRight; i < indices.size(); i++) {
- if (IsTabPinned(indices[0]) && !IsTabPinned(indices[i])) {
- SetTabPinned(indices[i], true);
- move_left_indices.push_back(IndexOfFirstNonPinnedTab() - 1);
- } else {
- move_left_indices.push_back(indices[i]);
- }
+ move_left_indices.push_back(indices[i]);
}
// Move tabs to the left, starting with the leftmost tab.
- int move_left_starting_index =
- numTabsMovingRight == 0 ? destination_index : destination_index + 1;
for (size_t i = 0; i < move_left_indices.size(); i++) {
- MoveWebContentsAt(move_left_indices[i], move_left_starting_index + i,
- false);
- contents_data_[move_left_starting_index + i]->set_group(group);
+ MoveWebContentsAt(move_left_indices[i], destination_index + i, false);
+ contents_data_[destination_index + i]->set_group(group);
}
}
+std::vector<int> TabStripModel::SetTabsPinned(const std::vector<int>& indices,
+ bool pinned) {
+ std::vector<int> new_indices;
+ if (pinned) {
+ for (size_t i = 0; i < indices.size(); i++) {
+ if (IsTabPinned(indices[i])) {
+ new_indices.push_back(indices[i]);
+ } else {
+ SetTabPinned(indices[i], true);
+ new_indices.push_back(IndexOfFirstNonPinnedTab() - 1);
+ }
+ }
+ } else {
+ for (size_t i = indices.size() - 1; i < indices.size(); i--) {
+ if (!IsTabPinned(indices[i])) {
+ new_indices.push_back(indices[i]);
+ } else {
+ SetTabPinned(indices[i], false);
+ new_indices.push_back(IndexOfFirstNonPinnedTab());
+ }
+ }
+ std::reverse(new_indices.begin(), new_indices.end());
+ }
+ return new_indices;
+}
+
// Context menu functions.
bool TabStripModel::IsContextMenuCommandEnabled(
int context_index,
@@ -1044,6 +1107,9 @@
case CommandAddToNewGroup:
return true;
+ case CommandAddToExistingGroup:
+ return true;
+
default:
NOTREACHED();
}
@@ -1153,7 +1219,7 @@
}
case CommandToggleTabAudioMuted: {
- const std::vector<int>& indices = GetIndicesForCommand(context_index);
+ std::vector<int> indices = GetIndicesForCommand(context_index);
const bool mute = WillContextMenuMute(context_index);
if (mute)
base::RecordAction(UserMetricsAction("TabContextMenu_MuteTabs"));
@@ -1167,7 +1233,6 @@
}
case CommandToggleSiteMuted: {
- const std::vector<int>& indices = GetIndicesForCommand(context_index);
const bool mute = WillContextMenuMuteSites(context_index);
if (mute) {
base::RecordAction(
@@ -1176,7 +1241,7 @@
base::RecordAction(
UserMetricsAction("SoundContentSetting.UnmuteBy.TabStrip"));
}
- SetSitesMuted(indices, mute);
+ SetSitesMuted(GetIndicesForCommand(context_index), mute);
break;
}
@@ -1190,8 +1255,13 @@
case CommandAddToNewGroup: {
base::RecordAction(UserMetricsAction("TabContextMenu_AddToNewGroup"));
- const std::vector<int>& indices = GetIndicesForCommand(context_index);
- AddToNewGroup(indices);
+ AddToNewGroup(GetIndicesForCommand(context_index));
+ break;
+ }
+
+ case CommandAddToExistingGroup: {
+ // Do nothing. The submenu's delegate will invoke
+ // ExecuteAddToExistingGroupCommand with the correct group later.
break;
}
@@ -1200,6 +1270,14 @@
}
}
+void TabStripModel::ExecuteAddToExistingGroupCommand(
+ int context_index,
+ const TabGroupData* group) {
+ base::RecordAction(UserMetricsAction("TabContextMenu_AddToExistingGroup"));
+
+ AddToExistingGroup(GetIndicesForCommand(context_index), group);
+}
+
bool TabStripModel::WillContextMenuMute(int index) {
std::vector<int> indices = GetIndicesForCommand(index);
return !chrome::AreAllTabsMuted(*this, indices);
diff --git a/chrome/browser/ui/tabs/tab_strip_model.h b/chrome/browser/ui/tabs/tab_strip_model.h
index aeca460e..f4e8b20 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.h
+++ b/chrome/browser/ui/tabs/tab_strip_model.h
@@ -290,6 +290,9 @@
// https://crbug.com/915956.
const TabGroupData* GetTabGroupForTab(int index) const;
+ // Returns the list of tab groups that contain at least one tab in this strip.
+ std::vector<TabGroupData*> ListTabGroups() const;
+
// Returns the index of the first tab that is not a pinned tab. This returns
// |count()| if all of the tabs are pinned tabs, and 0 if none of the tabs are
// pinned tabs.
@@ -347,9 +350,16 @@
// https://crbug.com/915956.
void AddToNewGroup(const std::vector<int>& indices);
+ // Add the set of tabs pointed to by |indices| to the tab group |group|. The
+ // tabs take on the pinnedness of the tabs already in the group, and are moved
+ // to immediately follow the tabs already in the group.
+ void AddToExistingGroup(const std::vector<int>& indices,
+ const TabGroupData* group);
+
// View API //////////////////////////////////////////////////////////////////
- // Context menu functions.
+ // Context menu functions. Tab groups uses command ids following CommandLast
+ // for entries in the 'Add to existing group' submenu.
enum ContextMenuCommand {
CommandFirst,
CommandNewTab,
@@ -365,6 +375,7 @@
CommandSendToMyDevices,
CommandBookmarkAllTabs,
CommandAddToNewGroup,
+ CommandAddToExistingGroup,
CommandLast
};
@@ -379,6 +390,11 @@
void ExecuteContextMenuCommand(int context_index,
ContextMenuCommand command_id);
+ // Adds the tab at |context_index| to the given tab group |group|. If
+ // |context_index| is selected the command applies to all selected tabs.
+ void ExecuteAddToExistingGroupCommand(int context_index,
+ const TabGroupData* group);
+
// Returns true if 'CommandToggleTabAudioMuted' will mute. |index| is the
// index supplied to |ExecuteContextMenuCommand|.
bool WillContextMenuMute(int index);
@@ -526,6 +542,17 @@
// starting at |start| to |index|. See MoveSelectedTabsTo for more details.
void MoveSelectedTabsToImpl(int index, size_t start, size_t length);
+ // Moves the set of tabs indicated by |indices| to precede the tab at index
+ // |destination_index|, maintaining their order and the order of tabs not
+ // being moved, and adds them to the tab group |group|.
+ void MoveTabsIntoGroup(const std::vector<int>& indices,
+ int destination_index,
+ const TabGroupData* group);
+
+ // Ensures all tabs indicated by |indices| are pinned, moving them in the
+ // process if necessary. Returns the new locations of all of those tabs.
+ std::vector<int> SetTabsPinned(const std::vector<int>& indices, bool pinned);
+
// Sets the sound content setting for each site at the |indices|.
void SetSitesMuted(const std::vector<int>& indices, bool mute) const;
diff --git a/chrome/browser/ui/tabs/tab_strip_model_unittest.cc b/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
index d5a3afb55..e6f725a 100644
--- a/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
@@ -2774,3 +2774,156 @@
strip.ActivateTabAt(0, true);
strip.CloseAllTabs();
}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupIdempotent) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AddToNewGroup({0});
+ const TabGroupData* group = strip.GetTabGroupForTab(0);
+
+ strip.AddToExistingGroup({0}, group);
+
+ EXPECT_EQ(strip.GetTabGroupForTab(0), group);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroup) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> in{strip.GetWebContentsAt(0),
+ strip.GetWebContentsAt(1)};
+ strip.AddToNewGroup({0});
+ const TabGroupData* group = strip.GetTabGroupForTab(0);
+
+ strip.AddToExistingGroup({1}, group);
+
+ EXPECT_EQ(strip.GetTabGroupForTab(1), group);
+ EXPECT_EQ(strip.GetWebContentsAt(0), in[0]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), in[1]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupReordersToTheRight) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> orig{strip.GetWebContentsAt(0),
+ strip.GetWebContentsAt(1)};
+ strip.AddToNewGroup({1});
+ const TabGroupData* group = strip.GetTabGroupForTab(1);
+
+ strip.AddToExistingGroup({0}, group);
+
+ EXPECT_EQ(strip.GetTabGroupForTab(0), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(1), group);
+ EXPECT_EQ(strip.GetWebContentsAt(0), orig[1]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), orig[0]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupReordersToTheLeft) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> orig{strip.GetWebContentsAt(0),
+ strip.GetWebContentsAt(1),
+ strip.GetWebContentsAt(2)};
+ strip.AddToNewGroup({0});
+ const TabGroupData* group = strip.GetTabGroupForTab(0);
+
+ strip.AddToExistingGroup({2}, group);
+
+ EXPECT_EQ(strip.GetTabGroupForTab(0), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(1), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(2), nullptr);
+ EXPECT_EQ(strip.GetWebContentsAt(0), orig[0]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), orig[2]);
+ EXPECT_EQ(strip.GetWebContentsAt(2), orig[1]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupReorders) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> orig{
+ strip.GetWebContentsAt(0), strip.GetWebContentsAt(1),
+ strip.GetWebContentsAt(2), strip.GetWebContentsAt(3)};
+ strip.AddToNewGroup({1});
+ const TabGroupData* group = strip.GetTabGroupForTab(1);
+
+ strip.AddToExistingGroup({0, 3}, group);
+
+ EXPECT_EQ(strip.GetTabGroupForTab(0), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(1), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(2), group);
+ EXPECT_EQ(strip.GetTabGroupForTab(3), nullptr);
+ EXPECT_EQ(strip.GetWebContentsAt(0), orig[1]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), orig[0]);
+ EXPECT_EQ(strip.GetWebContentsAt(2), orig[3]);
+ EXPECT_EQ(strip.GetWebContentsAt(3), orig[2]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupPins) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> orig{strip.GetWebContentsAt(0),
+ strip.GetWebContentsAt(1)};
+ strip.SetTabPinned(0, true);
+ strip.AddToNewGroup({0});
+ const TabGroupData* group = strip.GetTabGroupForTab(0);
+
+ strip.AddToExistingGroup({1}, group);
+
+ EXPECT_TRUE(strip.IsTabPinned(1));
+ EXPECT_EQ(strip.GetTabGroupForTab(1), group);
+ EXPECT_EQ(strip.GetWebContentsAt(0), orig[0]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), orig[1]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
+
+TEST_F(TabStripModelTest, AddTabToExistingGroupUnpins) {
+ TabStripDummyDelegate delegate;
+ TabStripModel strip(&delegate, profile());
+ strip.AppendWebContents(CreateWebContents(), false);
+ strip.AppendWebContents(CreateWebContents(), false);
+ std::vector<WebContents*> orig{strip.GetWebContentsAt(0),
+ strip.GetWebContentsAt(1)};
+ strip.SetTabPinned(0, true);
+ strip.AddToNewGroup({1});
+ const TabGroupData* group = strip.GetTabGroupForTab(1);
+
+ strip.AddToExistingGroup({0}, group);
+
+ EXPECT_FALSE(strip.IsTabPinned(0));
+ EXPECT_EQ(strip.GetTabGroupForTab(0), group);
+ EXPECT_EQ(strip.GetWebContentsAt(0), orig[1]);
+ EXPECT_EQ(strip.GetWebContentsAt(1), orig[0]);
+
+ strip.ActivateTabAt(0, true);
+ strip.CloseAllTabs();
+}
diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml
index 42f94aa..5a25c9dd 100644
--- a/tools/metrics/actions/actions.xml
+++ b/tools/metrics/actions/actions.xml
@@ -19789,6 +19789,15 @@
</description>
</action>
+<action name="TabContextMenu_AddToExistingGroup">
+ <owner>bsep@google.com</owner>
+ <owner>tbergquist@google.com</owner>
+ <description>
+ User selected an entry in the Add to existing group submenu from the tab
+ context menu.
+ </description>
+</action>
+
<action name="TabContextMenu_AddToNewGroup">
<owner>bsep@google.com</owner>
<owner>tbergquist@google.com</owner>
