Fixes bug where highlighting the close XXX menu items in the context

menu should flash mini-tabs.

BUG=none
TEST=create three tabs, pin the first two, right click on the first 
and hover over the context menu item close tabs to right,
make sure the second tab (which is pinned) doesn't highlight.

Review URL: http://codereview.chromium.org/1439001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42820 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 125 insertions, 29 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index ebebb463..b5095a2 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -611,40 +611,24 @@ void TabStripModel::ExecuteContextMenuCommand(
       UserMetrics::RecordAction(
           UserMetricsAction("TabContextMenu_CloseOtherTabs"),
           profile_);
-      TabContents* contents = GetTabContentsAt(context_index);
-      std::vector<int> closing_tabs;
-      for (int i = count() - 1; i >= 0; --i) {
-        if (GetTabContentsAt(i) != contents && !IsMiniTab(i))
-          closing_tabs.push_back(i);
-      }
-      InternalCloseTabs(closing_tabs, true);
+      InternalCloseTabs(GetIndicesClosedByCommand(context_index, command_id),
+                        true);
       break;
     }
     case CommandCloseTabsToRight: {
       UserMetrics::RecordAction(
           UserMetricsAction("TabContextMenu_CloseTabsToRight"),
           profile_);
-      std::vector<int> closing_tabs;
-      for (int i = count() - 1; i > context_index; --i) {
-        if (!IsMiniTab(i))
-          closing_tabs.push_back(i);
-      }
-      InternalCloseTabs(closing_tabs, true);
+      InternalCloseTabs(GetIndicesClosedByCommand(context_index, command_id),
+                        true);
       break;
     }
     case CommandCloseTabsOpenedBy: {
       UserMetrics::RecordAction(
           UserMetricsAction("TabContextMenu_CloseTabsOpenedBy"),
           profile_);
-      std::vector<int> closing_tabs = GetIndexesOpenedBy(context_index);
-      for (std::vector<int>::iterator i = closing_tabs.begin();
-           i != closing_tabs.end();) {
-        if (IsMiniTab(*i))
-          i = closing_tabs.erase(i);
-        else
-          ++i;
-      }
-      InternalCloseTabs(closing_tabs, true);
+      InternalCloseTabs(GetIndicesClosedByCommand(context_index, command_id),
+                        true);
       break;
     }
     case CommandRestoreTab: {
@@ -676,11 +660,30 @@ void TabStripModel::ExecuteContextMenuCommand(
   }
 }
 
-std::vector<int> TabStripModel::GetIndexesOpenedBy(int index) const {
+
+std::vector<int> TabStripModel::GetIndicesClosedByCommand(
+    int index,
+    ContextMenuCommand id) const {
+  DCHECK(ContainsIndex(index));
+
+  // NOTE: some callers assume indices are sorted in reverse order.
   std::vector<int> indices;
-  NavigationController* opener = &GetTabContentsAt(index)->controller();
-  for (int i = count() - 1; i >= 0; --i) {
-    if (OpenerMatches(contents_data_.at(i), opener, true))
+
+  if (id == CommandCloseTabsOpenedBy) {
+    NavigationController* opener = &GetTabContentsAt(index)->controller();
+    for (int i = count() - 1; i >= 0; --i) {
+      if (OpenerMatches(contents_data_[i], opener, true) && !IsMiniTab(i))
+        indices.push_back(i);
+    }
+    return indices;
+  }
+
+  if (id != CommandCloseTabsToRight && id != CommandCloseOtherTabs)
+    return indices;
+
+  int start = (id == CommandCloseTabsToRight) ? index + 1 : 0;
+  for (int i = count() - 1; i >= start; --i) {
+    if (i != index && !IsMiniTab(i))
       indices.push_back(i);
   }
   return indices;
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index a4c3641..427fb6e 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -559,9 +559,11 @@ class TabStripModel : public NotificationObserver {
   void ExecuteContextMenuCommand(int context_index,
                                  ContextMenuCommand command_id);
 
-  // Returns a vector of indices of TabContentses opened from the TabContents
-  // at the specified |index|.
-  std::vector<int> GetIndexesOpenedBy(int index) const;
+  // Returns a vector of indices of the tabs that will close when executing the
+  // command |id| for the tab at |index|. The returned indices are sorted in
+  // descending order.
+  std::vector<int> GetIndicesClosedByCommand(int index,
+                                             ContextMenuCommand id) const;
 
   // Overridden from notificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index ac5881a2..451cc81 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/stl_util-inl.h"
+#include "base/string_util.h"
 #include "chrome/browser/dom_ui/new_tab_ui.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/profile_manager.h"
@@ -152,6 +153,20 @@ class TabStripModelTest : public RenderViewHostTestHarness {
     return actual;
   }
 
+  std::string GetIndicesClosedByCommandAsString(
+      const TabStripModel& model,
+      int index,
+      TabStripModel::ContextMenuCommand id) const {
+    std::vector<int> indices = model.GetIndicesClosedByCommand(index, id);
+    std::string result;
+    for (size_t i = 0; i < indices.size(); ++i) {
+      if (i != 0)
+        result += " ";
+      result += IntToString(indices[i]);
+    }
+    return result;
+  }
+
  private:
   PropertyAccessor<int>* GetIDAccessor() {
     static PropertyAccessor<int> accessor;
@@ -808,6 +823,82 @@ TEST_F(TabStripModelTest, TestContextMenuCloseCommands) {
   EXPECT_TRUE(tabstrip.empty());
 }
 
+// Tests GetIndicesClosedByCommand.
+TEST_F(TabStripModelTest, GetIndicesClosedByCommand) {
+  TabStripDummyDelegate delegate(NULL);
+  TabStripModel tabstrip(&delegate, profile());
+  EXPECT_TRUE(tabstrip.empty());
+
+  TabContents* contents1 = CreateTabContents();
+  TabContents* contents2 = CreateTabContents();
+  TabContents* contents3 = CreateTabContents();
+  TabContents* contents4 = CreateTabContents();
+  TabContents* contents5 = CreateTabContents();
+
+  tabstrip.AppendTabContents(contents1, true);
+  tabstrip.AppendTabContents(contents2, true);
+  tabstrip.AppendTabContents(contents3, true);
+  tabstrip.AppendTabContents(contents4, true);
+  tabstrip.AppendTabContents(contents5, true);
+
+  EXPECT_EQ("4 3 2 1", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseTabsToRight));
+  EXPECT_EQ("4 3 2", GetIndicesClosedByCommandAsString(
+                tabstrip, 1, TabStripModel::CommandCloseTabsToRight));
+
+  EXPECT_EQ("4 3 2 1", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseOtherTabs));
+  EXPECT_EQ("4 3 2 0", GetIndicesClosedByCommandAsString(
+                tabstrip, 1, TabStripModel::CommandCloseOtherTabs));
+
+  // Pin the first two tabs. Pinned tabs shouldn't be closed by the close other
+  // commands.
+  tabstrip.SetTabPinned(0, true);
+  tabstrip.SetTabPinned(1, true);
+
+  EXPECT_EQ("4 3 2", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseTabsToRight));
+  EXPECT_EQ("4 3", GetIndicesClosedByCommandAsString(
+                tabstrip, 2, TabStripModel::CommandCloseTabsToRight));
+
+  EXPECT_EQ("4 3 2", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseOtherTabs));
+  EXPECT_EQ("4 3", GetIndicesClosedByCommandAsString(
+                tabstrip, 2, TabStripModel::CommandCloseOtherTabs));
+
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+}
+
+// Tests GetIndicesClosedByCommand.
+TEST_F(TabStripModelTest, GetIndicesClosedByCommandWithOpener) {
+  TabStripDummyDelegate delegate(NULL);
+  TabStripModel tabstrip(&delegate, profile());
+  EXPECT_TRUE(tabstrip.empty());
+
+  TabContents* contents1 = CreateTabContents();
+  TabContents* contents2 = CreateTabContents();
+  TabContents* contents3 = CreateTabContents();
+  TabContents* contents4 = CreateTabContents();
+
+  tabstrip.AppendTabContents(contents1, true);
+  InsertTabContentses(&tabstrip, contents2, contents3, contents4);
+
+  EXPECT_EQ("3 2 1", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseTabsOpenedBy));
+
+  // Pin the first two tabs and make sure the index isn't returned when asking
+  // for the openner.
+  tabstrip.SetTabPinned(0, true);
+  tabstrip.SetTabPinned(1, true);
+
+  EXPECT_EQ("3 2", GetIndicesClosedByCommandAsString(
+                tabstrip, 0, TabStripModel::CommandCloseTabsOpenedBy));
+
+  tabstrip.CloseAllTabs();
+  EXPECT_TRUE(tabstrip.empty());
+}
+
 // Tests whether or not TabContentses are inserted in the correct position
 // using this "smart" function with a simulated middle click action on a series
 // of links on the home page.