Modified TabStripModel to send the tab deactivation event before the active tab changes instead of after.

BUG=105729

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

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

3 files changed, 167 insertions, 113 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index beeb232..1a3c80e 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -164,11 +164,11 @@ 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);
+    TabStripSelectionModel new_model;
+    new_model.Copy(selection_model_);
+    new_model.SetSelectedIndex(index);
+    SetSelection(new_model, NOTIFY_DEFAULT);
   }
 }
 
@@ -263,6 +263,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
     TabStripSelectionModel old_model;
     old_model.Copy(selection_model_);
     if (index == old_active) {
+      NotifyIfTabDeactivated(removed_contents);
       if (!selection_model_.empty()) {
         // The active tab was removed, but there is still something selected.
         // Move the active and anchor to the first selected index.
@@ -273,7 +274,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
         // selection and send out notification.
         selection_model_.SetSelectedIndex(next_selected_index);
       }
-      NotifyIfActiveTabChanged(removed_contents, false);
+      NotifyIfActiveTabChanged(removed_contents, NOTIFY_DEFAULT);
     }
 
     // Sending notification in case the detached tab was selected. Using
@@ -290,20 +291,18 @@ 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_);
-  selection_model_.SetSelectedIndex(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, user_gesture, old_model);
+  TabStripSelectionModel new_model;
+  new_model.Copy(selection_model_);
+  new_model.SetSelectedIndex(index);
+  SetSelection(new_model, user_gesture ? NOTIFY_USER_GESTURE : NOTIFY_DEFAULT);
 }
 
 void TabStripModel::AddTabAtToSelection(int index) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model_);
-  selection_model_.AddIndexToSelection(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  TabStripSelectionModel new_model;
+  new_model.Copy(selection_model_);
+  new_model.AddIndexToSelection(index);
+  SetSelection(new_model, NOTIFY_DEFAULT);
 }
 
 void TabStripModel::MoveTabContentsAt(int index,
@@ -628,43 +627,40 @@ 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());
-  selection_model_.SetSelectionFromAnchorTo(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  TabStripSelectionModel new_model;
+  new_model.Copy(selection_model_);
+  new_model.SetSelectionFromAnchorTo(index);
+  SetSelection(new_model, NOTIFY_DEFAULT);
 }
 
 void TabStripModel::ToggleSelectionAt(int index) {
   DCHECK(ContainsIndex(index));
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
+  TabStripSelectionModel new_model;
+  new_model.Copy(selection_model());
   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
       // unselect it.
       return;
     }
-    selection_model_.RemoveIndexFromSelection(index);
-    selection_model_.set_anchor(index);
-    if (selection_model_.active() == index ||
-        selection_model_.active() == TabStripSelectionModel::kUnselectedIndex)
-      selection_model_.set_active(selection_model_.selected_indices()[0]);
+    new_model.RemoveIndexFromSelection(index);
+    new_model.set_anchor(index);
+    if (new_model.active() == index ||
+        new_model.active() == TabStripSelectionModel::kUnselectedIndex)
+      new_model.set_active(new_model.selected_indices()[0]);
   } else {
-    selection_model_.AddIndexToSelection(index);
-    selection_model_.set_anchor(index);
-    selection_model_.set_active(index);
+    new_model.AddIndexToSelection(index);
+    new_model.set_anchor(index);
+    new_model.set_active(index);
   }
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  SetSelection(new_model, NOTIFY_DEFAULT);
 }
 
 void TabStripModel::AddSelectionFromAnchorTo(int index) {
-  TabContentsWrapper* old_contents = GetActiveTabContents();
-  TabStripSelectionModel old_model;
-  old_model.Copy(selection_model());
-  selection_model_.AddSelectionFromAnchorTo(index);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  TabStripSelectionModel new_model;
+  new_model.Copy(selection_model_);
+  new_model.AddSelectionFromAnchorTo(index);
+  SetSelection(new_model, NOTIFY_DEFAULT);
 }
 
 bool TabStripModel::IsTabSelected(int index) const {
@@ -675,11 +671,7 @@ 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());
-  selection_model_.Copy(source);
-  NotifyIfActiveOrSelectionChanged(old_contents, false, old_model);
+  SetSelection(source, NOTIFY_DEFAULT);
 }
 
 void TabStripModel::AddTabContents(TabContentsWrapper* contents,
@@ -1233,18 +1225,22 @@ TabContentsWrapper* TabStripModel::GetContentsAt(int index) const {
   return contents_data_.at(index)->contents;
 }
 
+void TabStripModel::NotifyIfTabDeactivated(TabContentsWrapper* contents) {
+  if (contents) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      TabDeactivated(contents));
+  }
+}
+
 void TabStripModel::NotifyIfActiveTabChanged(
     TabContentsWrapper* old_contents,
-    bool user_gesture) {
+    NotifyTypes notify_types) {
   TabContentsWrapper* new_contents = GetContentsAt(active_index());
   if (old_contents != new_contents) {
-    if (old_contents) {
-      FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                        TabDeactivated(old_contents));
-    }
     FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
                       ActiveTabChanged(old_contents, new_contents,
-                                       active_index(), user_gesture));
+                                       active_index(),
+                                       notify_types == NOTIFY_USER_GESTURE));
     // Activating a discarded tab reloads it, so it is no longer discarded.
     contents_data_[active_index()]->discarded = false;
   }
@@ -1252,9 +1248,9 @@ void TabStripModel::NotifyIfActiveTabChanged(
 
 void TabStripModel::NotifyIfActiveOrSelectionChanged(
     TabContentsWrapper* old_contents,
-    bool user_gesture,
+    NotifyTypes notify_types,
     const TabStripSelectionModel& old_model) {
-  NotifyIfActiveTabChanged(old_contents, user_gesture);
+  NotifyIfActiveTabChanged(old_contents, notify_types);
 
   if (!selection_model().Equals(old_model)) {
     FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
@@ -1262,6 +1258,18 @@ void TabStripModel::NotifyIfActiveOrSelectionChanged(
   }
 }
 
+void TabStripModel::SetSelection(
+    const TabStripSelectionModel& new_model,
+    NotifyTypes notify_types) {
+  TabContentsWrapper* old_contents = GetActiveTabContents();
+  TabStripSelectionModel old_model;
+  old_model.Copy(selection_model_);
+  if (new_model.active() != selection_model_.active())
+    NotifyIfTabDeactivated(old_contents);
+  selection_model_.Copy(new_model);
+  NotifyIfActiveOrSelectionChanged(old_contents, notify_types, old_model);
+}
+
 void TabStripModel::SelectRelativeTab(bool next) {
   // This may happen during automated testing or if a user somehow buffers
   // many key accelerators.
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 1943d82..29181a4 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -492,6 +492,14 @@ class TabStripModel : public content::NotificationObserver {
   static bool ContextMenuCommandToBrowserCommand(int cmd_id, int* browser_cmd);
 
  private:
+   // Used when making selection notifications.
+  enum NotifyTypes {
+    NOTIFY_DEFAULT,
+
+    // The selection is changing from a user gesture.
+    NOTIFY_USER_GESTURE,
+  };
+
   // Gets the set of tab indices whose domain matches the tab at |index|.
   void GetIndicesWithSameDomain(int index, std::vector<int>* indices);
 
@@ -536,23 +544,28 @@ class TabStripModel : public content::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.
+  // Notifies the observers if the active tab is being deactivated.
+  void NotifyIfTabDeactivated(TabContentsWrapper* contents);
+
+  // Notifies the observers if the active tab has changed.
   void NotifyIfActiveTabChanged(TabContentsWrapper* old_contents,
-                                bool user_gesture);
+                                NotifyTypes notify_types);
 
   // 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.
+  // |old_model| is a snapshot of |selection_model_| before the change.
+  // Note: This function might end up sending 0 to 2 notifications in the
+  // following order: ActiveTabChanged, TabSelectionChanged.
   void NotifyIfActiveOrSelectionChanged(
       TabContentsWrapper* old_contents,
-      bool user_gesture,
+      NotifyTypes notify_types,
       const TabStripSelectionModel& old_model);
 
+  // Sets the selection to |new_model| and notifies any observers.
+  // Note: This function might end up sending 0 to 3 notifications in the
+  // following order: TabDeactivated, ActiveTabChanged, TabSelectionChanged.
+  void SetSelection(const TabStripSelectionModel& new_model,
+                    NotifyTypes notify_types);
+
   // Returns the number of New Tab tabs in the TabStripModel.
   int GetNewTabCount() const;
 
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index cdd7085..6cb4bbe 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -251,12 +251,11 @@ class TabStripModelTest : public ChromeRenderViewHostTestHarness {
 
 class MockTabStripModelObserver : public TabStripModelObserver {
  public:
-  MockTabStripModelObserver() : empty_(true),
-                                log_tab_selection_changed_(false),
-                                model_(NULL) {}
+  MockTabStripModelObserver()
+      : empty_(true),
+        model_(NULL) {}
   explicit MockTabStripModelObserver(TabStripModel* model)
       : empty_(true),
-        log_tab_selection_changed_(false),
         model_(model) {}
   virtual ~MockTabStripModelObserver() {
     STLDeleteContainerPointers(states_.begin(), states_.end());
@@ -267,6 +266,7 @@ class MockTabStripModelObserver : public TabStripModelObserver {
     CLOSE,
     DETACH,
     ACTIVATE,
+    DEACTIVATE,
     SELECT,
     MOVE,
     CHANGE,
@@ -343,9 +343,6 @@ class MockTabStripModelObserver : public TabStripModelObserver {
   }
   virtual void TabSelectionChanged(TabStripModel* tab_strip_model,
                                    const TabStripSelectionModel& old_model) {
-    if (!log_tab_selection_changed())
-      return;
-
     State* s = new State(model()->GetActiveTabContents(),
                          model()->active_index(),
                          SELECT);
@@ -368,6 +365,9 @@ class MockTabStripModelObserver : public TabStripModelObserver {
   virtual void TabDetachedAt(TabContentsWrapper* contents, int index) {
     states_.push_back(new State(contents, index, DETACH));
   }
+  virtual void TabDeactivated(TabContentsWrapper* contents) {
+    states_.push_back(new State(contents, model()->active_index(), DEACTIVATE));
+  }
   virtual void TabChangedAt(TabContentsWrapper* contents, int index,
                             TabChangeType change_type) {
     states_.push_back(new State(contents, index, CHANGE));
@@ -392,21 +392,13 @@ 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);
@@ -415,7 +407,7 @@ class MockTabStripModelObserver : public TabStripModelObserver {
 TEST_F(TabStripModelTest, TestBasicAPI) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
@@ -434,13 +426,16 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     tabstrip.AppendTabContents(contents1, true);
     EXPECT_TRUE(tabstrip.ContainsIndex(0));
     EXPECT_EQ(1, tabstrip.count());
-    EXPECT_EQ(2, observer.GetStateCount());
+    EXPECT_EQ(3, observer.GetStateCount());
     State s1(contents1, 0, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
     State s2(contents1, 0, MockTabStripModelObserver::ACTIVATE);
-    s2.src_contents = NULL;
     EXPECT_TRUE(observer.StateEquals(1, s2));
+    State s3(contents1, 0, MockTabStripModelObserver::SELECT);
+    s3.src_contents = NULL;
+    s3.src_index = TabStripSelectionModel::kUnselectedIndex;
+    EXPECT_TRUE(observer.StateEquals(2, s3));
     observer.ClearStates();
   }
 
@@ -450,13 +445,19 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     tabstrip.InsertTabContentsAt(1, contents2, TabStripModel::ADD_ACTIVE);
 
     EXPECT_EQ(2, tabstrip.count());
-    EXPECT_EQ(2, observer.GetStateCount());
+    EXPECT_EQ(4, observer.GetStateCount());
     State s1(contents2, 1, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
-    s2.src_contents = contents1;
+    State s2(contents1, 0, MockTabStripModelObserver::DEACTIVATE);
     EXPECT_TRUE(observer.StateEquals(1, s2));
+    State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    s3.src_contents = contents1;
+    EXPECT_TRUE(observer.StateEquals(2, s3));
+    State s4(contents2, 1, MockTabStripModelObserver::SELECT);
+    s4.src_contents = contents1;
+    s4.src_index = 0;
+    EXPECT_TRUE(observer.StateEquals(3, s4));
     observer.ClearStates();
   }
 
@@ -476,11 +477,17 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
   // Test ActivateTabAt
   {
     tabstrip.ActivateTabAt(2, true);
-    EXPECT_EQ(1, observer.GetStateCount());
-    State s1(contents3, 2, MockTabStripModelObserver::ACTIVATE);
-    s1.src_contents = contents2;
-    s1.user_gesture = true;
+    EXPECT_EQ(3, observer.GetStateCount());
+    State s1(contents2, 1, MockTabStripModelObserver::DEACTIVATE);
     EXPECT_TRUE(observer.StateEquals(0, s1));
+    State s2(contents3, 2, MockTabStripModelObserver::ACTIVATE);
+    s2.src_contents = contents2;
+    s2.user_gesture = true;
+    EXPECT_TRUE(observer.StateEquals(1, s2));
+    State s3(contents3, 2, MockTabStripModelObserver::SELECT);
+    s3.src_contents = contents2;
+    s3.src_index = 1;
+    EXPECT_TRUE(observer.StateEquals(2, s3));
     observer.ClearStates();
   }
 
@@ -490,20 +497,33 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     TabContentsWrapper* detached = tabstrip.DetachTabContentsAt(2);
     // ... and append again because we want this for later.
     tabstrip.AppendTabContents(detached, true);
-    EXPECT_EQ(4, observer.GetStateCount());
+    EXPECT_EQ(8, observer.GetStateCount());
     State s1(detached, 2, MockTabStripModelObserver::DETACH);
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
-    s2.src_contents = contents3;
-    s2.user_gesture = false;
+    State s2(detached, TabStripSelectionModel::kUnselectedIndex,
+        MockTabStripModelObserver::DEACTIVATE);
     EXPECT_TRUE(observer.StateEquals(1, s2));
-    State s3(detached, 2, MockTabStripModelObserver::INSERT);
-    s3.foreground = true;
+    State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    s3.src_contents = contents3;
+    s3.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(2, s3));
-    State s4(detached, 2, MockTabStripModelObserver::ACTIVATE);
-    s4.src_contents = contents2;
-    s4.user_gesture = false;
+    State s4(contents2, 1, MockTabStripModelObserver::SELECT);
+    s4.src_contents = NULL;
+    s4.src_index = TabStripSelectionModel::kUnselectedIndex;
     EXPECT_TRUE(observer.StateEquals(3, s4));
+    State s5(detached, 2, MockTabStripModelObserver::INSERT);
+    s5.foreground = true;
+    EXPECT_TRUE(observer.StateEquals(4, s5));
+    State s6(contents2, 1, MockTabStripModelObserver::DEACTIVATE);
+    EXPECT_TRUE(observer.StateEquals(5, s6));
+    State s7(detached, 2, MockTabStripModelObserver::ACTIVATE);
+    s7.src_contents = contents2;
+    s7.user_gesture = false;
+    EXPECT_TRUE(observer.StateEquals(6, s7));
+    State s8(detached, 2, MockTabStripModelObserver::SELECT);
+    s8.src_contents = contents2;
+    s8.src_index = 1;
+    EXPECT_TRUE(observer.StateEquals(7, s8));
     observer.ClearStates();
   }
 
@@ -520,15 +540,22 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
     EXPECT_TRUE(tabstrip.CloseTabContentsAt(2, TabStripModel::CLOSE_NONE));
     EXPECT_EQ(2, tabstrip.count());
 
-    EXPECT_EQ(3, observer.GetStateCount());
+    EXPECT_EQ(5, observer.GetStateCount());
     State s1(contents3, 2, MockTabStripModelObserver::CLOSE);
     EXPECT_TRUE(observer.StateEquals(0, s1));
     State s2(contents3, 2, MockTabStripModelObserver::DETACH);
     EXPECT_TRUE(observer.StateEquals(1, s2));
-    State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
-    s3.src_contents = contents3;
-    s3.user_gesture = false;
+    State s3(contents3, TabStripSelectionModel::kUnselectedIndex,
+        MockTabStripModelObserver::DEACTIVATE);
     EXPECT_TRUE(observer.StateEquals(2, s3));
+    State s4(contents2, 1, MockTabStripModelObserver::ACTIVATE);
+    s4.src_contents = contents3;
+    s4.user_gesture = false;
+    EXPECT_TRUE(observer.StateEquals(3, s4));
+    State s5(contents2, 1, MockTabStripModelObserver::SELECT);
+    s5.src_contents = NULL;
+    s5.src_index = TabStripSelectionModel::kUnselectedIndex;
+    EXPECT_TRUE(observer.StateEquals(4, s5));
     observer.ClearStates();
   }
 
@@ -1721,7 +1748,7 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
 TEST_F(TabStripModelTest, FastShutdown) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
@@ -1782,7 +1809,7 @@ TEST_F(TabStripModelTest, FastShutdown) {
 TEST_F(TabStripModelTest, Apps) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
@@ -1914,7 +1941,7 @@ TEST_F(TabStripModelTest, Apps) {
 TEST_F(TabStripModelTest, Pinning) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
@@ -2086,7 +2113,7 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
   strip.AddTabContents(first_contents, -1, content::PAGE_TRANSITION_TYPED,
                        TabStripModel::ADD_ACTIVE);
 
-  MockTabStripModelObserver tabstrip_observer;
+  MockTabStripModelObserver tabstrip_observer(&strip);
   strip.AddObserver(&tabstrip_observer);
 
   TabContentsWrapper* new_contents = CreateTabContents();
@@ -2140,7 +2167,7 @@ TEST_F(TabStripModelTest, DiscardTabContentsAt) {
 
   // Start watching for events after the appends to avoid observing state
   // transitions that aren't relevant to this test.
-  MockTabStripModelObserver tabstrip_observer;
+  MockTabStripModelObserver tabstrip_observer(&tabstrip);
   tabstrip.AddObserver(&tabstrip_observer);
 
   // Discard one of the tabs.
@@ -2291,7 +2318,6 @@ TEST_F(TabStripModelTest, MultipleSelection) {
   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.
@@ -2306,14 +2332,16 @@ TEST_F(TabStripModelTest, MultipleSelection) {
 
   // Adding all tabs to selection, active tab is now at 0.
   strip.ExtendSelectionTo(0);
-  ASSERT_EQ(2, observer.GetStateCount());
+  ASSERT_EQ(3, observer.GetStateCount());
   ASSERT_EQ(observer.GetStateAt(0)->action,
+            MockTabStripModelObserver::DEACTIVATE);
+  ASSERT_EQ(observer.GetStateAt(1)->action,
             MockTabStripModelObserver::ACTIVATE);
   MockTabStripModelObserver::State s2(contents0, 0,
       MockTabStripModelObserver::SELECT);
   s2.src_contents = contents3;
   s2.src_index = 3;
-  EXPECT_TRUE(observer.StateEquals(1, s2));
+  EXPECT_TRUE(observer.StateEquals(2, s2));
   observer.ClearStates();
 
   // Toggle the active tab, should make the next index active.
@@ -2321,10 +2349,12 @@ TEST_F(TabStripModelTest, MultipleSelection) {
   EXPECT_EQ(1, strip.active_index());
   EXPECT_EQ(3U, strip.selection_model().size());
   EXPECT_EQ(4, strip.count());
-  ASSERT_EQ(2, observer.GetStateCount());
+  ASSERT_EQ(3, observer.GetStateCount());
   ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::ACTIVATE);
+            MockTabStripModelObserver::DEACTIVATE);
   ASSERT_EQ(observer.GetStateAt(1)->action,
+            MockTabStripModelObserver::ACTIVATE);
+  ASSERT_EQ(observer.GetStateAt(2)->action,
             MockTabStripModelObserver::SELECT);
   observer.ClearStates();
 
@@ -2333,10 +2363,12 @@ TEST_F(TabStripModelTest, MultipleSelection) {
   EXPECT_EQ(0, strip.active_index());
   EXPECT_EQ(4U, strip.selection_model().size());
   EXPECT_EQ(4, strip.count());
-  ASSERT_EQ(2, observer.GetStateCount());
+  ASSERT_EQ(3, observer.GetStateCount());
   ASSERT_EQ(observer.GetStateAt(0)->action,
-            MockTabStripModelObserver::ACTIVATE);
+            MockTabStripModelObserver::DEACTIVATE);
   ASSERT_EQ(observer.GetStateAt(1)->action,
+            MockTabStripModelObserver::ACTIVATE);
+  ASSERT_EQ(observer.GetStateAt(2)->action,
             MockTabStripModelObserver::SELECT);
   observer.ClearStates();
 
@@ -2355,14 +2387,16 @@ TEST_F(TabStripModelTest, MultipleSelection) {
   // 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(5, 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);
+            MockTabStripModelObserver::DEACTIVATE);
   ASSERT_EQ(observer.GetStateAt(3)->action,
+            MockTabStripModelObserver::ACTIVATE);
+  ASSERT_EQ(observer.GetStateAt(4)->action,
             MockTabStripModelObserver::SELECT);
   observer.ClearStates();
 
@@ -2397,7 +2431,6 @@ TEST_F(TabStripModelTest, MultipleToSingle) {
   strip.ToggleSelectionAt(1);
 
   MockTabStripModelObserver observer(&strip);
-  observer.set_log_tab_selection_changed(true);
   strip.AddObserver(&observer);
   // This changes the selection (0 is no longer selected) but the selected_index
   // still remains at 1.