Fixes bug in tabstrip that resulted in not repainting when we

should. This was the result of the tabstrip not notifying observers
when the selection changed. For example, if tabs 1 and 2 were selected
and 1 active, and 2 was removed observers weren't notified.

BUG=none
TEST=none
R=ben@chromium.org

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

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

2 files changed, 40 insertions, 2 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 2925e53..407bb00 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -242,11 +242,21 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
 
 void TabStripModel::SelectTabContentsAt(int index, bool user_gesture) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old =
+  bool had_multi = selection_model_.selected_indices().size() > 1;
+  TabContentsWrapper* old_contents =
       (selected_index() == TabStripSelectionModel::kUnselectedIndex) ?
       NULL : GetSelectedTabContents();
   selection_model_.SetSelectedIndex(index);
-  NotifyTabSelectedIfChanged(old, index, user_gesture);
+  TabContentsWrapper* new_contents = GetContentsAt(index);
+  if (old_contents != new_contents && old_contents) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      TabDeselected(old_contents));
+  }
+  if (old_contents != new_contents || had_multi) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      TabSelectedAt(old_contents, new_contents,
+                                    selected_index(), user_gesture));
+  }
 }
 
 void TabStripModel::MoveTabContentsAt(int index,
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index cf734ab..6f51e02 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -2173,3 +2173,31 @@ TEST_F(TabStripModelTest, CloseSelectedTabs) {
   EXPECT_EQ(0, strip.selected_index());
   strip.CloseAllTabs();
 }
+
+// Verifies that if we change the selection from a multi selection to a single
+// selection, but not in a way that changes the selected_index that
+// TabSelectedAt is still invoked.
+TEST_F(TabStripModelTest, MultipleToSingle) {
+  TabStripDummyDelegate delegate(NULL);
+  TabStripModel strip(&delegate, profile());
+  TabContentsWrapper* contents1 = CreateTabContents();
+  TabContentsWrapper* contents2 = CreateTabContents();
+  strip.AppendTabContents(contents1, false);
+  strip.AppendTabContents(contents2, false);
+  strip.ToggleSelectionAt(0);
+  strip.ToggleSelectionAt(1);
+
+  MockTabStripModelObserver observer;
+  strip.AddObserver(&observer);
+  // This changes the selection (0 is no longer selected) but the selected_index
+  // still remains at 1.
+  strip.SelectTabContentsAt(1, true);
+  ASSERT_EQ(1, observer.GetStateCount());
+  MockTabStripModelObserver::State s(
+      contents2, 1, MockTabStripModelObserver::SELECT);
+  s.src_contents = contents2;
+  s.user_gesture = true;
+  EXPECT_TRUE(observer.StateEquals(0, s));
+  strip.RemoveObserver(&observer);
+  strip.CloseAllTabs();
+}