Revert 89752 - Multi-tab: Adding new Notification when tab selection changes.

In this CL
1) TabStripModelObserver::ActiveTabChanged is only called when the active tab actually changes
   (where as before it was called to also signal tab selection changes).
2) TabStripModelObserver::TabSelectionChanged is called when the tab selection changes.
3) BaseTabStrip::SelectTabAt() is replaced by BaseTabStrip::SetSelection().

BUG=NONE
TEST=NONE

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

TBR=dpapad@chromium.org
Review URL: http://codereview.chromium.org/7200044

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

25 files changed, 131 insertions, 284 deletions

diff --git a/chrome/browser/aeropeek_manager.cc b/chrome/browser/aeropeek_manager.cc
index 57df43c..f4a2da6 100644
--- a/chrome/browser/aeropeek_manager.cc
+++ b/chrome/browser/aeropeek_manager.cc
@@ -1115,6 +1115,9 @@ void AeroPeekManager::ActiveTabChanged(TabContentsWrapper* old_contents,
                                        TabContentsWrapper* new_contents,
                                        int index,
                                        bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   // Deactivate the old window in the thumbnail list and activate the new one
   // to synchronize the thumbnail list with TabStrip.
   if (old_contents) {
diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc
index 32bd2a1..15c6c0f 100644
--- a/chrome/browser/extensions/extension_browser_event_router.cc
+++ b/chrome/browser/extensions/extension_browser_event_router.cc
@@ -303,6 +303,9 @@ void ExtensionBrowserEventRouter::ActiveTabChanged(
     TabContentsWrapper* new_contents,
     int index,
     bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   ListValue args;
   args.Append(Value::CreateIntegerValue(
       ExtensionTabUtil::GetTabId(new_contents->tab_contents())));
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 13c1b2e..35aa0d1 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -157,11 +157,10 @@ void TabStripModel::InsertTabContentsAt(int index,
 
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
                     TabInsertedAt(contents, index, active));
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model_);
+
   if (active) {
     selection_model_.SetSelectedIndex(index);
-    NotifyIfActiveOrSelectionChanged(selected_contents, false, old_model);
+    NotifyTabSelectedIfChanged(selected_contents, index, false);
   }
 }
 
@@ -207,7 +206,6 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
   DCHECK(ContainsIndex(index));
 
   TabContentsWrapper* removed_contents = GetContentsAt(index);
-  bool was_selected = IsTabSelected(index);
   int next_selected_index = order_controller_->DetermineNewSelectedIndex(index);
   delete contents_data_.at(index);
   contents_data_.erase(contents_data_.begin() + index);
@@ -217,36 +215,26 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
       TabDetachedAt(removed_contents, index));
   if (empty()) {
-    selection_model_.Clear();
     // TabDetachedAt() might unregister observers, so send |TabStripEmtpy()| in
     // a second pass.
     FOR_EACH_OBSERVER(TabStripModelObserver, observers_, TabStripEmpty());
   } else {
     int old_active = active_index();
     selection_model_.DecrementFrom(index);
-    TabStripSelectionModel old_model;
-    old_model.Copy(selection_model_);
     if (index == old_active) {
       if (!selection_model_.empty()) {
-        // The active tab was removed, but there is still something selected.
+        // A selected tab was removed, but there is still something selected.
         // Move the active and anchor to the first selected index.
         selection_model_.set_active(selection_model_.selected_indices()[0]);
         selection_model_.set_anchor(selection_model_.active());
+        NotifyTabSelectedIfChanged(removed_contents, active_index(), false);
       } else {
         // The active tab was removed and nothing is selected. Reset the
         // selection and send out notification.
         selection_model_.SetSelectedIndex(next_selected_index);
+        NotifyTabSelectedIfChanged(removed_contents, next_selected_index,
+                                   false);
       }
-      NotifyIfActiveTabChanged(removed_contents, false);
-    }
-
-    // Sending notification in case the detached tab was selected. Using
-    // NotifyIfActiveOrSelectionChanged() here would not guarantee that a
-    // notification is sent even though the tab selection has changed because
-    // |old_model| is stored after calling DecrementFrom().
-    if (was_selected) {
-      FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                        TabSelectionChanged(old_model));
     }
   }
   return removed_contents;
@@ -254,11 +242,21 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
 
 void TabStripModel::ActivateTabAt(int index, bool user_gesture) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model_);
+  bool had_multi = selection_model_.selected_indices().size() > 1;
+  TabContentsWrapper* old_contents =
+      (active_index() == TabStripSelectionModel::kUnselectedIndex) ?
+      NULL : GetActiveTabContents();
   selection_model_.SetSelectedIndex(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, user_gesture, old_model);
+  TabContentsWrapper* new_contents = GetContentsAt(index);
+  if (old_contents != new_contents && old_contents) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      TabDeactivated(old_contents));
+  }
+  if (old_contents != new_contents || had_multi) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      ActiveTabChanged(old_contents, new_contents,
+                                       active_index(), user_gesture));
+  }
 }
 
 void TabStripModel::MoveTabContentsAt(int index,
@@ -570,18 +568,15 @@ int TabStripModel::ConstrainInsertionIndex(int index, bool mini_tab) {
 
 void TabStripModel::ExtendSelectionTo(int index) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
+  int old_active = active_index();
   selection_model_.SetSelectionFromAnchorTo(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  // This may not have resulted in a change, but we assume it did.
+  NotifyActiveTabChanged(old_active);
 }
 
 void TabStripModel::ToggleSelectionAt(int index) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
+  int old_active = active_index();
   if (selection_model_.IsSelected(index)) {
     if (selection_model_.size() == 1) {
       // One tab must be selected and this tab is currently selected so we can't
@@ -597,15 +592,13 @@ void TabStripModel::ToggleSelectionAt(int index) {
     selection_model_.set_anchor(index);
     selection_model_.set_active(index);
   }
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  NotifyActiveTabChanged(old_active);
 }
 
 void TabStripModel::AddSelectionFromAnchorTo(int index) {
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
+  int old_active = active_index();
   selection_model_.AddSelectionFromAnchorTo(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  NotifyActiveTabChanged(old_active);
 }
 
 bool TabStripModel::IsTabSelected(int index) const {
@@ -616,11 +609,10 @@ bool TabStripModel::IsTabSelected(int index) const {
 void TabStripModel::SetSelectionFromModel(
     const TabStripSelectionModel& source) {
   DCHECK_NE(TabStripSelectionModel::kUnselectedIndex, source.active());
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
+  int old_active_index = active_index();
   selection_model_.Copy(source);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  // This may not have resulted in a change, but we assume it did.
+  NotifyActiveTabChanged(old_active_index);
 }
 
 void TabStripModel::AddTabContents(TabContentsWrapper* contents,
@@ -1215,31 +1207,33 @@ TabContentsWrapper* TabStripModel::GetContentsAt(int index) const {
   return contents_data_.at(index)->contents;
 }
 
-void TabStripModel::NotifyIfActiveTabChanged(
-    TabContentsWrapper* old_contents,
-    bool user_gesture) {
-  TabContentsWrapper* new_contents = GetContentsAt(active_index());
-  if (old_contents != new_contents) {
-    if (old_contents) {
-      FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                        TabDeactivated(old_contents));
-    }
+void TabStripModel::NotifyTabSelectedIfChanged(TabContentsWrapper* old_contents,
+                                               int to_index,
+                                               bool user_gesture) {
+  TabContentsWrapper* new_contents = GetContentsAt(to_index);
+  if (old_contents == new_contents)
+    return;
+
+  TabContentsWrapper* last_selected_contents = old_contents;
+  if (last_selected_contents) {
     FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                      ActiveTabChanged(old_contents, new_contents,
-                                       active_index(), user_gesture));
+                      TabDeactivated(last_selected_contents));
   }
-}
 
-void TabStripModel::NotifyIfActiveOrSelectionChanged(
-    TabContentsWrapper* old_contents,
-    bool user_gesture,
-    const TabStripSelectionModel& old_model) {
-  NotifyIfActiveTabChanged(old_contents, user_gesture);
+  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                    ActiveTabChanged(last_selected_contents, new_contents,
+                                     active_index(), user_gesture));
+}
 
-  if (!selection_model().Equals(old_model)) {
-    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                      TabSelectionChanged(old_model));
-  }
+void TabStripModel::NotifyActiveTabChanged(int old_active_index) {
+  TabContentsWrapper* old_tab =
+      old_active_index == TabStripSelectionModel::kUnselectedIndex ?
+      NULL : GetTabContentsAt(old_active_index);
+  TabContentsWrapper* new_tab =
+      active_index() == TabStripSelectionModel::kUnselectedIndex ?
+      NULL : GetTabContentsAt(active_index());
+  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                    ActiveTabChanged(old_tab, new_tab, active_index(), true));
 }
 
 void TabStripModel::SelectRelativeTab(bool next) {
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 21270cd..df4d83b 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -514,22 +514,15 @@ class TabStripModel : public NotificationObserver {
 
   TabContentsWrapper* GetContentsAt(int index) const;
 
-  // Notifies the observers if the active tab has changed. If |old_contents| is
-  // non-null a TabDeactivated notification is sent right before sending
-  // ActiveTabChanged notification.
-  void NotifyIfActiveTabChanged(TabContentsWrapper* old_contents,
-                                bool user_gesture);
-
-  // Notifies the observers if the active tab or the tab selection has changed.
-  // If |old_contents| is non-null a TabDeactivated notification is sent right
-  // before sending ActiveTabChanged notification. |old_model| is a snapshot of
-  // |selection_model_| before the change.
-  // Note: This function might end up sending 0 to 3 notifications in the
-  // following order: TabDeactivated, ActiveTabChanged, TabSelectionChanged.
-  void NotifyIfActiveOrSelectionChanged(
-      TabContentsWrapper* old_contents,
-      bool user_gesture,
-      const TabStripSelectionModel& old_model);
+  // If the TabContentsWrapper at |to_index| differs from |old_contents|
+  // notifies observers.
+  void NotifyTabSelectedIfChanged(TabContentsWrapper* old_contents,
+                                  int to_index,
+                                  bool user_gesture);
+
+  // Notifies the observers the active tab changed. |old_active_index| gives
+  // the old active index.
+  void NotifyActiveTabChanged(int old_active_index);
 
   // Returns the number of New Tab tabs in the TabStripModel.
   int GetNewTabCount() const;
diff --git a/chrome/browser/tabs/tab_strip_model_observer.cc b/chrome/browser/tabs/tab_strip_model_observer.cc
index cf0d1eb..83b0cf9 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.cc
+++ b/chrome/browser/tabs/tab_strip_model_observer.cc
@@ -27,10 +27,6 @@ void TabStripModelObserver::ActiveTabChanged(TabContentsWrapper* old_contents,
                                              bool user_gesture) {
 }
 
-void TabStripModelObserver::TabSelectionChanged(
-    const TabStripSelectionModel& model) {
-}
-
 void TabStripModelObserver::TabMoved(TabContentsWrapper* contents,
                                      int from_index,
                                      int to_index) {
diff --git a/chrome/browser/tabs/tab_strip_model_observer.h b/chrome/browser/tabs/tab_strip_model_observer.h
index 5a69eb5..b275350 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.h
+++ b/chrome/browser/tabs/tab_strip_model_observer.h
@@ -8,7 +8,6 @@
 
 class TabContentsWrapper;
 class TabStripModel;
-class TabStripSelectionModel;
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -62,26 +61,24 @@ class TabStripModelObserver {
   // happens.
   virtual void TabDeactivated(TabContentsWrapper* contents);
 
-  // Sent when the active tab changes. The previously active tab is identified
-  // by |old_contents| and the newly active tab by |new_contents|. |index| is
-  // the index of |new_contents|. |user_gesture| specifies whether or not this
+  // Sent when the selection changes. The previously selected tab is identified
+  // by |old_contents| and the newly selected tab by |new_contents|. |index| is
+  // the index of |new_contents|. When using multiple selection this may be sent
+  // even when the active tab has not changed. For example, if the selection is
+  // extended this method is invoked to inform observers the selection has
+  // changed, but |old_contents| and |new_contents| are the same.  If you only
+  // care about when the active tab changes, check for when |old_contents|
+  // differs from |new_contents|. |user_gesture| specifies whether or not this
   // was done by a user input event (e.g. clicking on a tab, keystroke) or as a
   // side-effect of some other function.
-  // Note: It is possible for the selection to change while the active tab
-  // remains unchanged. For example, control-click may not change the active tab
-  // but does change the selection. In this case |ActiveTabChanged| is not sent.
-  // If you care about any changes to the selection, override
-  // TabSelectionChanged.
+  //
+  // TODO(dpapad): Add TabSelectionChanged method for when the selected tabs
+  // change.
   virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
                                 TabContentsWrapper* new_contents,
                                 int index,
                                 bool user_gesture);
 
-  // Sent when the selection changes. More precisely when selected tabs, anchor
-  // tab or active tab change. |old_model| is a snapshot of the selection model
-  // before the change. See also ActiveTabChanged for details.
-  virtual void TabSelectionChanged(const TabStripSelectionModel& old_model);
-
   // The specified TabContents at |from_index| was moved to |to_index|.
   virtual void TabMoved(TabContentsWrapper* contents,
                         int from_index,
diff --git a/chrome/browser/tabs/tab_strip_model_order_controller.cc b/chrome/browser/tabs/tab_strip_model_order_controller.cc
index c336279..d26f97b 100644
--- a/chrome/browser/tabs/tab_strip_model_order_controller.cc
+++ b/chrome/browser/tabs/tab_strip_model_order_controller.cc
@@ -111,6 +111,9 @@ void TabStripModelOrderController::ActiveTabChanged(
     TabContentsWrapper* new_contents,
     int index,
     bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   NavigationController* old_opener = NULL;
   if (old_contents) {
     int index = tabstrip_->GetIndexOfTabContents(old_contents);
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index b988941..6643bbc 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -270,13 +270,7 @@ class TabStripModelTest : public RenderViewHostTestHarness {
 
 class MockTabStripModelObserver : public TabStripModelObserver {
  public:
-  MockTabStripModelObserver() : empty_(true),
-                                log_tab_selection_changed_(false),
-                                model_(NULL) {}
-  explicit MockTabStripModelObserver(TabStripModel* model)
-      : empty_(true),
-        log_tab_selection_changed_(false),
-        model_(model) {}
+  MockTabStripModelObserver() : empty_(true) {}
   ~MockTabStripModelObserver() {
     STLDeleteContainerPointers(states_.begin(), states_.end());
   }
@@ -285,7 +279,6 @@ class MockTabStripModelObserver : public TabStripModelObserver {
     INSERT,
     CLOSE,
     DETACH,
-    ACTIVATE,
     SELECT,
     MOVE,
     CHANGE,
@@ -355,22 +348,11 @@ class MockTabStripModelObserver : public TabStripModelObserver {
                                 TabContentsWrapper* new_contents,
                                 int index,
                                 bool user_gesture) {
-    State* s = new State(new_contents, index, ACTIVATE);
+    State* s = new State(new_contents, index, SELECT);
     s->src_contents = old_contents;
     s->user_gesture = user_gesture;
     states_.push_back(s);
   }
-  virtual void TabSelectionChanged(const TabStripSelectionModel& old_model) {
-    if (!log_tab_selection_changed())
-      return;
-
-    State* s = new State(model()->GetActiveTabContents(),
-                         model()->active_index(),
-                         SELECT);
-    s->src_contents = model()->GetTabContentsAt(old_model.active());
-    s->src_index = old_model.active();
-    states_.push_back(s);
-  }
   virtual void TabMoved(
       TabContentsWrapper* contents, int from_index, int to_index) {
     State* s = new State(contents, to_index, MOVE);
@@ -410,22 +392,12 @@ class MockTabStripModelObserver : public TabStripModelObserver {
     states_.clear();
   }
 
-  void set_log_tab_selection_changed(bool flag) {
-    log_tab_selection_changed_ = flag;
-  }
-
   bool empty() const { return empty_; }
-  bool log_tab_selection_changed() const { return log_tab_selection_changed_; }
-  TabStripModel* model() { return model_; }
 
  private:
   std::vector<State*> states_;
 
   bool empty_;
-  // TODO(dpapad): Remove this variable and update TestBasicAPI so that it takes
-  // into account TabSelectionChanged notifications.
-  bool log_tab_selection_changed_;
-  TabStripModel* model_;
 
   DISALLOW_COPY_AND_ASSIGN(MockTabStripModelObserver);
 };
@@ -456,7 +428,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     State s1(contents1, 0, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents1, 0, MockTabStripModelObserver::ACTIVATE);
+    State s2(contents1, 0, MockTabStripModelObserver::SELECT);
     s2.src_contents = NULL;
     EXPECT_TRUE(observer.StateEquals(1, s2));
     observer.ClearStates();
@@ -472,7 +444,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     State s1(contents2, 1, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    State s2(contents2, 1, MockTabStripModelObserver::SELECT);
     s2.src_contents = contents1;
     EXPECT_TRUE(observer.StateEquals(1, s2));
     observer.ClearStates();
@@ -495,7 +467,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
   {
     tabstrip.ActivateTabAt(2, true);
     EXPECT_EQ(1, observer.GetStateCount());
-    State s1(contents3, 2, MockTabStripModelObserver::ACTIVATE);
+    State s1(contents3, 2, MockTabStripModelObserver::SELECT);
     s1.src_contents = contents2;
     s1.user_gesture = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
@@ -511,14 +483,14 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     EXPECT_EQ(4, observer.GetStateCount());
     State s1(detached, 2, MockTabStripModelObserver::DETACH);
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    State s2(contents2, 1, MockTabStripModelObserver::SELECT);
     s2.src_contents = contents3;
     s2.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(1, s2));
     State s3(detached, 2, MockTabStripModelObserver::INSERT);
     s3.foreground = true;
     EXPECT_TRUE(observer.StateEquals(2, s3));
-    State s4(detached, 2, MockTabStripModelObserver::ACTIVATE);
+    State s4(detached, 2, MockTabStripModelObserver::SELECT);
     s4.src_contents = contents2;
     s4.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(3, s4));
@@ -543,7 +515,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     EXPECT_TRUE(observer.StateEquals(0, s1));
     State s2(contents3, 2, MockTabStripModelObserver::DETACH);
     EXPECT_TRUE(observer.StateEquals(1, s2));
-    State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    State s3(contents2, 1, MockTabStripModelObserver::SELECT);
     s3.src_contents = contents3;
     s3.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(2, s3));
@@ -2090,7 +2062,7 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
   EXPECT_TRUE(tabstrip_observer.StateEquals(0, state));
 
   // And the second for selected.
-  state = State(new_contents, 0, MockTabStripModelObserver::ACTIVATE);
+  state = State(new_contents, 0, MockTabStripModelObserver::SELECT);
   state.src_contents = first_contents;
   EXPECT_TRUE(tabstrip_observer.StateEquals(1, state));
 
@@ -2209,89 +2181,9 @@ TEST_F(TabStripModelTest, CloseSelectedTabs) {
   strip.CloseAllTabs();
 }
 
-TEST_F(TabStripModelTest, MultipleSelection) {
-  TabStripDummyDelegate delegate(NULL);
-  TabStripModel strip(&delegate, profile());
-  MockTabStripModelObserver observer(&strip);
-  TabContentsWrapper* contents0 = CreateTabContents();
-  TabContentsWrapper* contents1 = CreateTabContents();
-  TabContentsWrapper* contents2 = CreateTabContents();
-  TabContentsWrapper* contents3 = CreateTabContents();
-  strip.AppendTabContents(contents0, false);
-  strip.AppendTabContents(contents1, false);
-  strip.AppendTabContents(contents2, false);
-  strip.AppendTabContents(contents3, false);
-  observer.set_log_tab_selection_changed(true);
-  strip.AddObserver(&observer);
-
-  // Selection and active tab change.
-  strip.ActivateTabAt(3, true);
-  ASSERT_EQ(2, observer.GetStateCount());
-  ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::ACTIVATE);
-  MockTabStripModelObserver::State s1(contents3, 3,
-      MockTabStripModelObserver::SELECT);
-  EXPECT_TRUE(observer.StateEquals(1, s1));
-  observer.ClearStates();
-
-  // Adding all tabs to selection, active tab is now at 0.
-  strip.ExtendSelectionTo(0);
-  ASSERT_EQ(2, observer.GetStateCount());
-  ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::ACTIVATE);
-  MockTabStripModelObserver::State s2(contents0, 0,
-      MockTabStripModelObserver::SELECT);
-  s2.src_contents = contents3;
-  s2.src_index = 3;
-  EXPECT_TRUE(observer.StateEquals(1, s2));
-  observer.ClearStates();
-
-  // Closing one of the selected tabs, not the active one.
-  strip.CloseTabContentsAt(1, TabStripModel::CLOSE_NONE);
-  EXPECT_EQ(3, strip.count());
-  ASSERT_EQ(3, observer.GetStateCount());
-  ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::CLOSE);
-  ASSERT_EQ(observer.GetStateAt(1)->action,
-            MockTabStripModelObserver::DETACH);
-  ASSERT_EQ(observer.GetStateAt(2)->action,
-            MockTabStripModelObserver::SELECT);
-  observer.ClearStates();
-
-  // Closing the active tab, while there are others tabs selected.
-  strip.CloseTabContentsAt(0, TabStripModel::CLOSE_NONE);
-  EXPECT_EQ(2, strip.count());
-  ASSERT_EQ(4, observer.GetStateCount());
-  ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::CLOSE);
-  ASSERT_EQ(observer.GetStateAt(1)->action,
-            MockTabStripModelObserver::DETACH);
-  ASSERT_EQ(observer.GetStateAt(2)->action,
-            MockTabStripModelObserver::ACTIVATE);
-  ASSERT_EQ(observer.GetStateAt(3)->action,
-            MockTabStripModelObserver::SELECT);
-  observer.ClearStates();
-
-  // Active tab is at 0, deselecting all but the active tab.
-  strip.ToggleSelectionAt(1);
-  ASSERT_EQ(1, observer.GetStateCount());
-  ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::SELECT);
-  observer.ClearStates();
-
-  // Attempting to deselect the only selected and therefore active tab,
-  // it is ignored (no notifications being sent) and tab at 0 remains selected
-  // and active.
-  strip.ToggleSelectionAt(0);
-  ASSERT_EQ(0, observer.GetStateCount());
-
-  strip.RemoveObserver(&observer);
-  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
-// TabSelectionChanged is invoked.
+// ActiveTabChanged is still invoked.
 TEST_F(TabStripModelTest, MultipleToSingle) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel strip(&delegate, profile());
@@ -2302,8 +2194,7 @@ TEST_F(TabStripModelTest, MultipleToSingle) {
   strip.ToggleSelectionAt(0);
   strip.ToggleSelectionAt(1);
 
-  MockTabStripModelObserver observer(&strip);
-  observer.set_log_tab_selection_changed(true);
+  MockTabStripModelObserver observer;
   strip.AddObserver(&observer);
   // This changes the selection (0 is no longer selected) but the selected_index
   // still remains at 1.
@@ -2312,8 +2203,7 @@ TEST_F(TabStripModelTest, MultipleToSingle) {
   MockTabStripModelObserver::State s(
       contents2, 1, MockTabStripModelObserver::SELECT);
   s.src_contents = contents2;
-  s.src_index = 1;
-  s.user_gesture = false;
+  s.user_gesture = true;
   EXPECT_TRUE(observer.StateEquals(0, s));
   strip.RemoveObserver(&observer);
   strip.CloseAllTabs();
diff --git a/chrome/browser/tabs/tab_strip_selection_model.cc b/chrome/browser/tabs/tab_strip_selection_model.cc
index f2921dc..43757c3 100644
--- a/chrome/browser/tabs/tab_strip_selection_model.cc
+++ b/chrome/browser/tabs/tab_strip_selection_model.cc
@@ -141,9 +141,3 @@ void TabStripSelectionModel::Copy(const TabStripSelectionModel& source) {
   active_ = source.active_;
   anchor_ = source.anchor_;
 }
-
-bool TabStripSelectionModel::Equals(const TabStripSelectionModel& rhs) const {
-  return active_ == rhs.active() &&
-      anchor_ == rhs.anchor() &&
-      selected_indices() == rhs.selected_indices();
-}
diff --git a/chrome/browser/tabs/tab_strip_selection_model.h b/chrome/browser/tabs/tab_strip_selection_model.h
index 15a0afd..9ce7618 100644
--- a/chrome/browser/tabs/tab_strip_selection_model.h
+++ b/chrome/browser/tabs/tab_strip_selection_model.h
@@ -95,9 +95,6 @@ class TabStripSelectionModel {
   // Copies the selection from |source| to this.
   void Copy(const TabStripSelectionModel& source);
 
-  // Compares this selection with |rhs|.
-  bool Equals(const TabStripSelectionModel& rhs) const;
-
  private:
   SelectedIndices selected_indices_;
 
diff --git a/chrome/browser/tabs/tab_strip_selection_model_unittest.cc b/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
index ec18932..121ef65 100644
--- a/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_selection_model_unittest.cc
@@ -173,20 +173,3 @@ TEST_F(TabStripSelectionModelTest, AddSelectionFromAnchorTo) {
   model.AddSelectionFromAnchorTo(0);
   EXPECT_EQ("active=0 anchor=2 selection=0 1 2 3 4", StateAsString(model));
 }
-
-TEST_F(TabStripSelectionModelTest, Equals) {
-  TabStripSelectionModel model1;
-  model1.SetSelectedIndex(0);
-  model1.AddSelectionFromAnchorTo(4);
-
-  TabStripSelectionModel model2;
-  model2.SetSelectedIndex(0);
-  model2.AddSelectionFromAnchorTo(4);
-
-  EXPECT_TRUE(model1.Equals(model2));
-  EXPECT_TRUE(model2.Equals(model1));
-
-  model2.SetSelectedIndex(0);
-  EXPECT_FALSE(model1.Equals(model2));
-  EXPECT_FALSE(model2.Equals(model1));
-}
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index bfa8761..372865a 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -2907,6 +2907,9 @@ void Browser::ActiveTabChanged(TabContentsWrapper* old_contents,
                                TabContentsWrapper* new_contents,
                                int index,
                                bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   // On some platforms we want to automatically reload tabs that are
   // killed when the user selects them.
   if (user_gesture && new_contents->tab_contents()->crashed_status() ==
diff --git a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
index d94ba3d..95c9cae 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
@@ -1215,7 +1215,7 @@ class NotificationBridge : public NotificationObserver {
   // Take closing tabs into account.
   NSInteger activeIndex = [self indexFromModelIndex:modelIndex];
 
-  if (oldContents) {
+  if (oldContents && oldContents != newContents) {
     int oldModelIndex =
         browser_->GetIndexOfController(&(oldContents->controller()));
     if (oldModelIndex != -1) {  // When closing a tab, the old tab may be gone.
diff --git a/chrome/browser/ui/gtk/browser_window_gtk.cc b/chrome/browser/ui/gtk/browser_window_gtk.cc
index 46212a4..1dc46d4 100644
--- a/chrome/browser/ui/gtk/browser_window_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_window_gtk.cc
@@ -1175,6 +1175,9 @@ void BrowserWindowGtk::ActiveTabChanged(TabContentsWrapper* old_contents,
                                         TabContentsWrapper* new_contents,
                                         int index,
                                         bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   if (old_contents && !old_contents->tab_contents()->is_being_destroyed())
     old_contents->view()->StoreFocus();
 
diff --git a/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc b/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
index 29aed09..50d3367 100644
--- a/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
+++ b/chrome/browser/ui/touch/frame/touch_browser_frame_view.cc
@@ -241,6 +241,9 @@ void TouchBrowserFrameView::ActiveTabChanged(TabContentsWrapper* old_contents,
                                              TabContentsWrapper* new_contents,
                                              int index,
                                              bool user_gesture) {
+  if (new_contents == old_contents)
+    return;
+
   TabContents* contents = new_contents->tab_contents();
   if (!TabContentsHasFocus(contents))
     return;
diff --git a/chrome/browser/ui/touch/tabs/touch_tab_strip.cc b/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
index 80cc07a..46bfd71 100644
--- a/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
+++ b/chrome/browser/ui/touch/tabs/touch_tab_strip.cc
@@ -86,8 +86,7 @@ void TouchTabStrip::RemoveTabAt(int model_index) {
   StartRemoveTabAnimation(model_index);
 }
 
-void SetSelection(const TabStripSelectionModel& old_selection,
-                  const TabStripSelectionModel& new_selection) {
+void TouchTabStrip::SelectTabAt(int old_model_index, int new_model_index) {
   SchedulePaint();
 }
 
diff --git a/chrome/browser/ui/touch/tabs/touch_tab_strip.h b/chrome/browser/ui/touch/tabs/touch_tab_strip.h
index cf4c686..f6dadb3 100644
--- a/chrome/browser/ui/touch/tabs/touch_tab_strip.h
+++ b/chrome/browser/ui/touch/tabs/touch_tab_strip.h
@@ -12,7 +12,6 @@ namespace ui {
 enum TouchStatus;
 }
 
-class TabStripSelectionModel;
 class TouchTab;
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -41,8 +40,7 @@ class TouchTabStrip : public BaseTabStrip {
   virtual void StopAllHighlighting();
   virtual BaseTab* CreateTabForDragging();
   virtual void RemoveTabAt(int model_index);
-  virtual void SetSelection(const TabStripSelectionModel& old_selection,
-                            const TabStripSelectionModel& new_selection);
+  virtual void SelectTabAt(int old_model_index, int new_model_index);
   virtual void TabTitleChangedNotLoading(int model_index);
   virtual BaseTab* CreateTab();
   virtual void StartInsertTabAnimation(int model_index);
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 78a91cd..3f57cd6 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1419,6 +1419,9 @@ void BrowserView::ActiveTabChanged(TabContentsWrapper* old_contents,
                                    TabContentsWrapper* new_contents,
                                    int index,
                                    bool user_gesture) {
+  if (old_contents == new_contents)
+    return;
+
   ProcessTabSelected(new_contents, true);
 }
 
diff --git a/chrome/browser/ui/views/tabs/base_tab_strip.h b/chrome/browser/ui/views/tabs/base_tab_strip.h
index 691ed07..0d5f1a4 100644
--- a/chrome/browser/ui/views/tabs/base_tab_strip.h
+++ b/chrome/browser/ui/views/tabs/base_tab_strip.h
@@ -18,7 +18,6 @@
 class BaseTab;
 class DraggedTabController;
 class TabStripController;
-class TabStripSelectionModel;
 
 // Base class for the view tab strip implementations.
 class BaseTabStrip : public AbstractTabStripView,
@@ -63,8 +62,7 @@ class BaseTabStrip : public AbstractTabStripView,
 
   // Selects a tab at the specified index. |old_model_index| is the selected
   // index prior to the selection change.
-  virtual void SetSelection(const TabStripSelectionModel& old_selection,
-                            const TabStripSelectionModel& new_selection) = 0;
+  virtual void SelectTabAt(int old_model_index, int new_model_index) = 0;
 
   // Moves a tab.
   virtual void MoveTab(int from_model_index, int to_model_index);
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
index 8cf63f9..bef8148 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -10,7 +10,6 @@
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
-#include "chrome/browser/tabs/tab_strip_selection_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/ui/tabs/tab_menu_model.h"
@@ -342,9 +341,13 @@ void BrowserTabStripController::TabDetachedAt(TabContentsWrapper* contents,
   tabstrip_->RemoveTabAt(model_index);
 }
 
-void BrowserTabStripController::TabSelectionChanged(
-    const TabStripSelectionModel& old_model) {
-  tabstrip_->SetSelection(old_model, model_->selection_model());
+void BrowserTabStripController::ActiveTabChanged(
+    TabContentsWrapper* old_contents,
+    TabContentsWrapper* contents,
+    int model_index,
+    bool user_gesture) {
+  tabstrip_->SelectTabAt(model_->GetIndexOfTabContents(old_contents),
+                         model_index);
 }
 
 void BrowserTabStripController::TabMoved(TabContentsWrapper* contents,
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
index 8d5622f..71ca58a 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
@@ -16,7 +16,6 @@
 class BaseTab;
 class BaseTabStrip;
 class Browser;
-class TabStripSelectionModel;
 
 struct TabRendererData;
 
@@ -72,8 +71,10 @@ class BrowserTabStripController : public TabStripController,
                              bool active) OVERRIDE;
   virtual void TabDetachedAt(TabContentsWrapper* contents,
                              int model_index) OVERRIDE;
-  virtual void TabSelectionChanged(
-      const TabStripSelectionModel& old_model) OVERRIDE;
+  virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
+                                TabContentsWrapper* contents,
+                                int model_index,
+                                bool user_gesture) OVERRIDE;
   virtual void TabMoved(TabContentsWrapper* contents,
                         int from_model_index,
                         int to_model_index) OVERRIDE;
diff --git a/chrome/browser/ui/views/tabs/side_tab_strip.cc b/chrome/browser/ui/views/tabs/side_tab_strip.cc
index 5e099e3..86cc393 100644
--- a/chrome/browser/ui/views/tabs/side_tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/side_tab_strip.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/views/tabs/side_tab_strip.h"
 
-#include "chrome/browser/tabs/tab_strip_selection_model.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/tabs/side_tab.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
@@ -176,11 +175,11 @@ void SideTabStrip::RemoveTabAt(int model_index) {
   StartRemoveTabAnimation(model_index);
 }
 
-void SideTabStrip::SetSelection(const TabStripSelectionModel& old_selection,
-                                const TabStripSelectionModel& new_selection) {
-  GetBaseTabAtModelIndex(new_selection.active())->SchedulePaint();
-  if (old_selection.active() != new_selection.active())
-    MakeTabVisible(ModelIndexToTabIndex(new_selection.active()));
+void SideTabStrip::SelectTabAt(int old_model_index, int new_model_index) {
+  GetBaseTabAtModelIndex(new_model_index)->SchedulePaint();
+
+  if (controller()->IsActiveTab(new_model_index))
+    MakeTabVisible(ModelIndexToTabIndex(new_model_index));
 }
 
 void SideTabStrip::TabTitleChangedNotLoading(int model_index) {
diff --git a/chrome/browser/ui/views/tabs/side_tab_strip.h b/chrome/browser/ui/views/tabs/side_tab_strip.h
index 6a5ef7e..7dd9805 100644
--- a/chrome/browser/ui/views/tabs/side_tab_strip.h
+++ b/chrome/browser/ui/views/tabs/side_tab_strip.h
@@ -10,7 +10,6 @@
 #include "views/controls/button/button.h"
 
 struct TabRendererData;
-class TabStripSelectionModel;
 
 class SideTabStrip : public BaseTabStrip, public views::ButtonListener {
  public:
@@ -29,9 +28,7 @@ class SideTabStrip : public BaseTabStrip, public views::ButtonListener {
   virtual void StopAllHighlighting() OVERRIDE;
   virtual BaseTab* CreateTabForDragging() OVERRIDE;
   virtual void RemoveTabAt(int model_index) OVERRIDE;
-  virtual void SetSelection(
-      const TabStripSelectionModel& old_selection,
-      const TabStripSelectionModel& new_selection) OVERRIDE;
+  virtual void SelectTabAt(int old_model_index, int new_model_index) OVERRIDE;
   virtual void TabTitleChangedNotLoading(int model_index) OVERRIDE;
 
   // views::View overrides:
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 40d7fa4..e35af1d 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -11,7 +11,6 @@
 #include "base/stl_util-inl.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/defaults.h"
-#include "chrome/browser/tabs/tab_strip_selection_model.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
@@ -237,8 +236,7 @@ void TabStrip::RemoveTabAt(int model_index) {
     StartRemoveTabAnimation(model_index);
 }
 
-void TabStrip::SetSelection(const TabStripSelectionModel& old_selection,
-                            const TabStripSelectionModel& new_selection) {
+void TabStrip::SelectTabAt(int old_model_index, int new_model_index) {
   // We have "tiny tabs" if the tabs are so tiny that the unselected ones are
   // a different size to the selected ones.
   bool tiny_tabs = current_unselected_width_ != current_selected_width_;
@@ -248,16 +246,8 @@ void TabStrip::SetSelection(const TabStripSelectionModel& old_selection,
     SchedulePaint();
   }
 
-  TabStripSelectionModel::SelectedIndices no_longer_selected;
-  std::insert_iterator<TabStripSelectionModel::SelectedIndices>
-      it(no_longer_selected, no_longer_selected.begin());
-  std::set_difference(old_selection.selected_indices().begin(),
-                      old_selection.selected_indices().end(),
-                      new_selection.selected_indices().begin(),
-                      new_selection.selected_indices().end(),
-                      it);
-  for (size_t i = 0; i < no_longer_selected.size(); ++i) {
-    GetTabAtTabDataIndex(ModelIndexToTabIndex(no_longer_selected[i]))->
+  if (old_model_index >= 0) {
+    GetTabAtTabDataIndex(ModelIndexToTabIndex(old_model_index))->
         StopMiniTabTitleAnimation();
   }
 }
diff --git a/chrome/browser/ui/views/tabs/tab_strip.h b/chrome/browser/ui/views/tabs/tab_strip.h
index 4d6dbf9..679f17a 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.h
+++ b/chrome/browser/ui/views/tabs/tab_strip.h
@@ -16,7 +16,6 @@
 #include "views/mouse_watcher.h"
 
 class Tab;
-class TabStripSelectionModel;
 
 namespace views {
 class ImageView;
@@ -58,9 +57,7 @@ class TabStrip : public BaseTabStrip,
   // BaseTabStrip implementation:
   virtual void PrepareForCloseAt(int model_index) OVERRIDE;
   virtual void RemoveTabAt(int model_index) OVERRIDE;
-  virtual void SetSelection(
-      const TabStripSelectionModel& old_selection,
-      const TabStripSelectionModel& new_selection) OVERRIDE;
+  virtual void SelectTabAt(int old_model_index, int new_model_index) OVERRIDE;
   virtual void TabTitleChangedNotLoading(int model_index) OVERRIDE;
   virtual void StartHighlight(int model_index) OVERRIDE;
   virtual void StopAllHighlighting() OVERRIDE;