Re-add "Close Other Tabs" to tabstrip context menu.

Bug: 1020399
TBR: asvitkine
Change-Id: Ib85e84b08d8a39d860b9e87d1eb0f2b1a991fa23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894758
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712018}

chrome/app/generated_resources.grd

@@ -6177,6 +6177,9 @@ the Bookmarks menu.">
         <message name="IDS_TAB_CXMENU_CLOSETAB" desc="The label of the tab context menu item for closing one or more tabs.">
           Close
         </message>
+        <message name="IDS_TAB_CXMENU_CLOSEOTHERTABS" desc="The label of the 'Close Other Tabs' Tab context menu item.">
+          Close other tabs
+        </message>
         <message name="IDS_TAB_CXMENU_CLOSETABSTORIGHT" desc="The label of the 'Close Tabs to the Right' Tab context menu item.">
           Close tabs to the right
         </message>
@@ -6221,6 +6224,9 @@ the Bookmarks menu.">
         <message name="IDS_TAB_CXMENU_CLOSETAB" desc="In Title Case: The label of the tab context menu item for closing one or more tabs.">
           Close
         </message>
+        <message name="IDS_TAB_CXMENU_CLOSEOTHERTABS" desc="In Title Case: The label of the 'Close Other Tabs' Tab context menu item.">
+          Close Other Tabs
+        </message>
         <message name="IDS_TAB_CXMENU_CLOSETABSTORIGHT" desc="In Title Case: The label of the 'Close Tabs to the Right' Tab context menu item.">
           Close Tabs to the Right
         </message>

chrome/browser/ui/tabs/tab_menu_model.cc

@@ -128,6 +128,8 @@ void TabMenuModel::Build(TabStripModel* tab_strip, int index) {
 
   AddSeparator(ui::NORMAL_SEPARATOR);
   AddItemWithStringId(TabStripModel::CommandCloseTab, IDS_TAB_CXMENU_CLOSETAB);
+  AddItemWithStringId(TabStripModel::CommandCloseOtherTabs,
+                      IDS_TAB_CXMENU_CLOSEOTHERTABS);
   AddItemWithStringId(TabStripModel::CommandCloseTabsToRight,
                       IDS_TAB_CXMENU_CLOSETABSTORIGHT);
 }

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -1193,6 +1193,7 @@ bool TabStripModel::IsContextMenuCommandEnabled(
       return false;
     }
 
+    case CommandCloseOtherTabs:
     case CommandCloseTabsToRight:
       return !GetIndicesClosedByCommand(context_index, command_id).empty();
 
@@ -1302,6 +1303,16 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index,
       break;
     }
 
+    case CommandCloseOtherTabs: {
+      ReentrancyCheck reentrancy_check(&reentrancy_guard_);
+
+      base::RecordAction(UserMetricsAction("TabContextMenu_CloseOtherTabs"));
+      InternalCloseTabs(GetWebContentsesByIndices(GetIndicesClosedByCommand(
+                            context_index, command_id)),
+                        CLOSE_CREATE_HISTORICAL_TAB);
+      break;
+    }
+
     case CommandCloseTabsToRight: {
       ReentrancyCheck reentrancy_check(&reentrancy_guard_);
 
@@ -1498,15 +1509,18 @@ std::vector<int> TabStripModel::GetIndicesClosedByCommand(
     int index,
     ContextMenuCommand id) const {
   DCHECK(ContainsIndex(index));
-  DCHECK_EQ(CommandCloseTabsToRight, id);
+  DCHECK(id == CommandCloseTabsToRight || id == CommandCloseOtherTabs);
   bool is_selected = IsTabSelected(index);
-  int last_unclosed_tab =
-      is_selected ? selection_model_.selected_indices().back() : index;
+  int last_unclosed_tab = -1;
+  if (id == CommandCloseTabsToRight) {
+    last_unclosed_tab =
+        is_selected ? selection_model_.selected_indices().back() : index;
+  }
 
   // NOTE: callers expect the vector to be sorted in descending order.
   std::vector<int> indices;
   for (int i = count() - 1; i > last_unclosed_tab; --i) {
-    if (!IsTabPinned(i))
+    if (i != index && !IsTabPinned(i) && (!is_selected || !IsTabSelected(i)))
       indices.push_back(i);
   }
   return indices;

chrome/browser/ui/tabs/tab_strip_model.h

@@ -466,6 +466,7 @@ class TabStripModel {
     CommandReload,
     CommandDuplicate,
     CommandCloseTab,
+    CommandCloseOtherTabs,
     CommandCloseTabsToRight,
     CommandTogglePinned,
     CommandFocusMode,

chrome/browser/ui/tabs/tab_strip_model_unittest.cc

@@ -1199,6 +1199,68 @@ TEST_F(TabStripModelTest, CommandCloseTab) {
   EXPECT_TRUE(tabstrip.empty());
 }
 
+// Tests IsContextMenuCommandEnabled and ExecuteContextMenuCommand with
+// CommandCloseOtherTabs.
+TEST_F(TabStripModelTest, CommandCloseOtherTabs) {
+  TestTabStripModelDelegate delegate;
+  TabStripModel tabstrip(&delegate, profile());
+  EXPECT_TRUE(tabstrip.empty());
+
+  // Create three tabs, select two tabs, CommandCloseOtherTabs should be enabled
+  // and close two tabs.
+  ASSERT_NO_FATAL_FAILURE(
+      PrepareTabstripForSelectionTest(&tabstrip, 3, 0, "0 1"));
+  EXPECT_TRUE(tabstrip.IsContextMenuCommandEnabled(
+      0, TabStripModel::CommandCloseOtherTabs));
+  tabstrip.ExecuteContextMenuCommand(0, TabStripModel::CommandCloseOtherTabs);
+  EXPECT_EQ("0 1", GetTabStripStateString(tabstrip));
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+
+  // Select two tabs, CommandCloseOtherTabs should be enabled and invoking it
+  // with a non-selected index should close the two other tabs.
+  ASSERT_NO_FATAL_FAILURE(
+      PrepareTabstripForSelectionTest(&tabstrip, 3, 0, "0 1"));
+  EXPECT_TRUE(tabstrip.IsContextMenuCommandEnabled(
+      2, TabStripModel::CommandCloseOtherTabs));
+  tabstrip.ExecuteContextMenuCommand(0, TabStripModel::CommandCloseOtherTabs);
+  EXPECT_EQ("0 1", GetTabStripStateString(tabstrip));
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+
+  // Select all, CommandCloseOtherTabs should not be enabled.
+  ASSERT_NO_FATAL_FAILURE(
+      PrepareTabstripForSelectionTest(&tabstrip, 3, 0, "0 1 2"));
+  EXPECT_FALSE(tabstrip.IsContextMenuCommandEnabled(
+      2, TabStripModel::CommandCloseOtherTabs));
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+
+  // Three tabs, pin one, select the two non-pinned.
+  ASSERT_NO_FATAL_FAILURE(
+      PrepareTabstripForSelectionTest(&tabstrip, 3, 1, "1 2"));
+  EXPECT_FALSE(tabstrip.IsContextMenuCommandEnabled(
+      1, TabStripModel::CommandCloseOtherTabs));
+  // If we don't pass in the pinned index, the command should be enabled.
+  EXPECT_TRUE(tabstrip.IsContextMenuCommandEnabled(
+      0, TabStripModel::CommandCloseOtherTabs));
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+
+  // 3 tabs, one pinned.
+  ASSERT_NO_FATAL_FAILURE(
+      PrepareTabstripForSelectionTest(&tabstrip, 3, 1, "1"));
+  EXPECT_TRUE(tabstrip.IsContextMenuCommandEnabled(
+      1, TabStripModel::CommandCloseOtherTabs));
+  EXPECT_TRUE(tabstrip.IsContextMenuCommandEnabled(
+      0, TabStripModel::CommandCloseOtherTabs));
+  tabstrip.ExecuteContextMenuCommand(1, TabStripModel::CommandCloseOtherTabs);
+  // The pinned tab shouldn't be closed.
+  EXPECT_EQ("0p 1", GetTabStripStateString(tabstrip));
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+}
+
 // Tests IsContextMenuCommandEnabled and ExecuteContextMenuCommand with
 // CommandCloseTabsToRight.
 TEST_F(TabStripModelTest, CommandCloseTabsToRight) {
@@ -1255,6 +1317,7 @@ TEST_F(TabStripModelTest, CommandTogglePinned) {
 
 // Tests the following context menu commands:
 //  - Close Tab
+//  - Close Other Tabs
 //  - Close Tabs To Right
 TEST_F(TabStripModelTest, TestContextMenuCloseCommands) {
   TestTabStripModelDelegate delegate;
@@ -1280,6 +1343,26 @@ TEST_F(TabStripModelTest, TestContextMenuCloseCommands) {
   EXPECT_EQ(1, tabstrip.count());
   EXPECT_EQ(raw_opener, tabstrip.GetActiveWebContents());
 
+  std::unique_ptr<WebContents> dummy = CreateWebContents();
+  WebContents* raw_dummy = dummy.get();
+  tabstrip.AppendWebContents(std::move(dummy), false);
+
+  contents1 = CreateWebContents();
+  contents2 = CreateWebContents();
+  contents3 = CreateWebContents();
+  InsertWebContentses(&tabstrip, std::move(contents1), std::move(contents2),
+                      std::move(contents3));
+  EXPECT_EQ(5, tabstrip.count());
+
+  int dummy_index = tabstrip.count() - 1;
+  tabstrip.ActivateTabAt(dummy_index, {TabStripModel::GestureType::kOther});
+  EXPECT_EQ(raw_dummy, tabstrip.GetActiveWebContents());
+
+  tabstrip.ExecuteContextMenuCommand(dummy_index,
+                                     TabStripModel::CommandCloseOtherTabs);
+  EXPECT_EQ(1, tabstrip.count());
+  EXPECT_EQ(raw_dummy, tabstrip.GetActiveWebContents());
+
   tabstrip.CloseAllTabs();
   EXPECT_TRUE(tabstrip.empty());
 }
@@ -1310,6 +1393,11 @@ TEST_F(TabStripModelTest, GetIndicesClosedByCommand) {
   EXPECT_EQ("4 3 2",
             indicesClosedAsString(1, TabStripModel::CommandCloseTabsToRight));
 
+  EXPECT_EQ("4 3 2 1",
+            indicesClosedAsString(0, TabStripModel::CommandCloseOtherTabs));
+  EXPECT_EQ("4 3 2 0",
+            indicesClosedAsString(1, TabStripModel::CommandCloseOtherTabs));
+
   // Pin the first two tabs. Pinned tabs shouldn't be closed by the close other
   // commands.
   tabstrip.SetTabPinned(0, true);
@@ -1320,6 +1408,11 @@ TEST_F(TabStripModelTest, GetIndicesClosedByCommand) {
   EXPECT_EQ("4 3",
             indicesClosedAsString(2, TabStripModel::CommandCloseTabsToRight));
 
+  EXPECT_EQ("4 3 2",
+            indicesClosedAsString(0, TabStripModel::CommandCloseOtherTabs));
+  EXPECT_EQ("4 3",
+            indicesClosedAsString(2, TabStripModel::CommandCloseOtherTabs));
+
   tabstrip.CloseAllTabs();
   EXPECT_TRUE(tabstrip.empty());
 }

tools/metrics/actions/actions.xml

@@ -21812,9 +21812,11 @@ should be able to be added at any place in this file.
 </action>
 
 <action name="TabContextMenu_CloseOtherTabs">
-  <obsolete>Deprecated due to removal.</obsolete>
-  <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
-  <description>Please enter the description of this user action.</description>
+  <owner>pbos@chromium.org</owner>
+  <description>
+    User selected Close Other Tabs from the tab context menu. Temporarily
+    removed in M78.
+  </description>
 </action>
 
 <action name="TabContextMenu_CloseTab">