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

chrome/browser/tabs/tab_strip_model_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <map>
 #include <string>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 252 matching lines @@
   std::map<TabContents*, int> foo_;
 
   // ProfileManager requires a base::SystemMonitor.
   base::SystemMonitor system_monitor;
 
   ProfileManager pm_;
 };
 
 class MockTabStripModelObserver : public TabStripModelObserver {
  public:
-  MockTabStripModelObserver() : empty_(true) {}
+  MockTabStripModelObserver() : empty_(true),
+                                log_tab_selection_changed_(false),
+                                model_(NULL) {}
+  explicit MockTabStripModelObserver(TabStripModel* model) :
+      empty_(true),
+      log_tab_selection_changed_(false),
+      model_(model) {}
   ~MockTabStripModelObserver() {
     STLDeleteContainerPointers(states_.begin(), states_.end());
   }
 
   enum TabStripModelObserverAction {
     INSERT,
     CLOSE,
     DETACH,
+    ACTIVATE,
     SELECT,
     MOVE,
     CHANGE,
     PINNED,
     REPLACED
   };
 
   struct State {
     State(TabContentsWrapper* a_dst_contents,
           int a_dst_index,
@@ skipping 49 matching lines @@
                              bool foreground) {
     empty_ = false;
     State* s = new State(contents, index, INSERT);
     s->foreground = foreground;
     states_.push_back(s);
   }
   virtual void ActiveTabChanged(TabContentsWrapper* old_contents,
                                 TabContentsWrapper* new_contents,
                                 int index,
                                 bool user_gesture) {
-    State* s = new State(new_contents, index, SELECT);
+    State* s = new State(new_contents, index, ACTIVATE);
     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);
     s->src_index = from_index;
     states_.push_back(s);
   }
 
   virtual void TabClosingAt(TabStripModel* tab_strip_model,
                             TabContentsWrapper* contents,
                             int index) {
@@ skipping 19 matching lines @@
   }
   virtual void TabStripEmpty() {
     empty_ = true;
   }
 
   void ClearStates() {
     STLDeleteContainerPointers(states_.begin(), states_.end());
     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);
 };
 
 TEST_F(TabStripModelTest, TestBasicAPI) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
 
   typedef MockTabStripModelObserver::State State;
 
   TabContentsWrapper* contents1 = CreateTabContents();
 
   // Note! The ordering of these tests is important, each subsequent test
   // builds on the state established in the previous. This is important if you
   // ever insert tests rather than append.
 
   // Test AppendTabContents, ContainsIndex
   {
     EXPECT_FALSE(tabstrip.ContainsIndex(0));
     tabstrip.AppendTabContents(contents1, true);
     EXPECT_TRUE(tabstrip.ContainsIndex(0));
     EXPECT_EQ(1, tabstrip.count());
     EXPECT_EQ(2, observer.GetStateCount());
     State s1(contents1, 0, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents1, 0, MockTabStripModelObserver::SELECT);
+    State s2(contents1, 0, MockTabStripModelObserver::ACTIVATE);
     s2.src_contents = NULL;
     EXPECT_TRUE(observer.StateEquals(1, s2));
     observer.ClearStates();
   }
 
   // Test InsertTabContentsAt, foreground tab.
   TabContentsWrapper* contents2 = CreateTabContents();
   {
     tabstrip.InsertTabContentsAt(1, contents2, TabStripModel::ADD_ACTIVE);
 
     EXPECT_EQ(2, tabstrip.count());
     EXPECT_EQ(2, observer.GetStateCount());
     State s1(contents2, 1, MockTabStripModelObserver::INSERT);
     s1.foreground = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::SELECT);
+    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
     s2.src_contents = contents1;
     EXPECT_TRUE(observer.StateEquals(1, s2));
     observer.ClearStates();
   }
 
   // Test InsertTabContentsAt, background tab.
   TabContentsWrapper* contents3 = CreateTabContents();
   {
     tabstrip.InsertTabContentsAt(2, contents3, TabStripModel::ADD_NONE);
 
     EXPECT_EQ(3, tabstrip.count());
     EXPECT_EQ(1, observer.GetStateCount());
     State s1(contents3, 2, MockTabStripModelObserver::INSERT);
     s1.foreground = false;
     EXPECT_TRUE(observer.StateEquals(0, s1));
     observer.ClearStates();
   }
 
   // Test ActivateTabAt
   {
     tabstrip.ActivateTabAt(2, true);
     EXPECT_EQ(1, observer.GetStateCount());
-    State s1(contents3, 2, MockTabStripModelObserver::SELECT);
+    State s1(contents3, 2, MockTabStripModelObserver::ACTIVATE);
     s1.src_contents = contents2;
     s1.user_gesture = true;
     EXPECT_TRUE(observer.StateEquals(0, s1));
     observer.ClearStates();
   }
 
   // Test DetachTabContentsAt
   {
     // Detach
     TabContentsWrapper* detached = tabstrip.DetachTabContentsAt(2);
     // ... and append again because we want this for later.
     tabstrip.AppendTabContents(detached, true);
     EXPECT_EQ(4, observer.GetStateCount());
     State s1(detached, 2, MockTabStripModelObserver::DETACH);
     EXPECT_TRUE(observer.StateEquals(0, s1));
-    State s2(contents2, 1, MockTabStripModelObserver::SELECT);
+    State s2(contents2, 1, MockTabStripModelObserver::ACTIVATE);
     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::SELECT);
+    State s4(detached, 2, MockTabStripModelObserver::ACTIVATE);
     s4.src_contents = contents2;
     s4.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(3, s4));
     observer.ClearStates();
   }
 
   // Test CloseTabContentsAt
   {
     // Let's test nothing happens when the delegate veto the close.
     delegate.set_can_close(false);
     EXPECT_FALSE(tabstrip.CloseTabContentsAt(2, TabStripModel::CLOSE_NONE));
     EXPECT_EQ(3, tabstrip.count());
     EXPECT_EQ(0, observer.GetStateCount());
 
     // Now let's close for real.
     delegate.set_can_close(true);
     EXPECT_TRUE(tabstrip.CloseTabContentsAt(2, TabStripModel::CLOSE_NONE));
     EXPECT_EQ(2, tabstrip.count());
 
     EXPECT_EQ(3, 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::SELECT);
+    State s3(contents2, 1, MockTabStripModelObserver::ACTIVATE);
     s3.src_contents = contents3;
     s3.user_gesture = false;
     EXPECT_TRUE(observer.StateEquals(2, s3));
     observer.ClearStates();
   }
 
   // Test MoveTabContentsAt, select_after_move == true
   {
     tabstrip.MoveTabContentsAt(1, 0, true);
 
@@ skipping 1153 matching lines @@
   strip.CloseTabContentsAt(strip.count() - 1, TabStripModel::CLOSE_NONE);
   EXPECT_EQ(page_d_contents, strip.GetTabContentsAt(strip.active_index()));
 
   strip.CloseAllTabs();
 }
 
 // Tests that fast shutdown is attempted appropriately.
 TEST_F(TabStripModelTest, FastShutdown) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
 
   // Make sure fast shutdown is attempted when tabs that share a RPH are shut
   // down.
   {
     TabContentsWrapper* contents1 = CreateTabContents();
     TabContentsWrapper* contents2 =
         CreateTabContentsWithSharedRPH(contents1->tab_contents());
@@ skipping 40 matching lines @@
 
     tabstrip.CloseAllTabs();
     EXPECT_TRUE(tabstrip.empty());
   }
 }
 
 // Tests various permutations of apps.
 TEST_F(TabStripModelTest, Apps) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
 
   typedef MockTabStripModelObserver::State State;
 
 #if defined(OS_WIN)
   FilePath path(FILE_PATH_LITERAL("c:\\foo"));
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
@@ skipping 105 matching lines @@
     observer.ClearStates();
   }
 
   tabstrip.CloseAllTabs();
 }
 
 // Tests various permutations of pinning tabs.
 TEST_F(TabStripModelTest, Pinning) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
+  MockTabStripModelObserver observer(&tabstrip);
   tabstrip.AddObserver(&observer);
 
   EXPECT_TRUE(tabstrip.empty());
 
   typedef MockTabStripModelObserver::State State;
 
   TabContentsWrapper* contents1 = CreateTabContents();
   TabContentsWrapper* contents2 = CreateTabContents();
   TabContentsWrapper* contents3 = CreateTabContents();
 
@@ skipping 165 matching lines @@
   delete strip.ReplaceTabContentsAt(0, new_contents);
 
   ASSERT_EQ(2, tabstrip_observer.GetStateCount());
 
   // First event should be for replaced.
   State state(new_contents, 0, MockTabStripModelObserver::REPLACED);
   state.src_contents = first_contents;
   EXPECT_TRUE(tabstrip_observer.StateEquals(0, state));
 
   // And the second for selected.
-  state = State(new_contents, 0, MockTabStripModelObserver::SELECT);
+  state = State(new_contents, 0, MockTabStripModelObserver::ACTIVATE);
   state.src_contents = first_contents;
   EXPECT_TRUE(tabstrip_observer.StateEquals(1, state));
 
   // Now add another tab and replace it, making sure we don't get a selected
   // event this time.
   TabContentsWrapper* third_contents = CreateTabContents();
   strip.AddTabContents(third_contents, 1, PageTransition::TYPED,
                        TabStripModel::ADD_NONE);
 
   tabstrip_observer.ClearStates();
@@ skipping 98 matching lines @@
   strip.AppendTabContents(contents1, true);
   strip.AppendTabContents(contents2, true);
   strip.AppendTabContents(contents3, true);
   strip.ToggleSelectionAt(1);
   strip.CloseSelectedTabs();
   EXPECT_EQ(1, strip.count());
   EXPECT_EQ(0, strip.active_index());
   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));
+
+  // Adding all tabs to selection, active tab is now at 0.
+  strip.ExtendSelectionTo(0);
+  ASSERT_EQ(4, observer.GetStateCount());
+  ASSERT_EQ(observer.GetStateAt(2)->action,
+            MockTabStripModelObserver::ACTIVATE);
+  MockTabStripModelObserver::State s2(contents0, 0,
+      MockTabStripModelObserver::SELECT);
+  s2.src_contents = contents3;
+  s2.src_index = 3;
+  EXPECT_TRUE(observer.StateEquals(3, s2));
+
+  // Closing one of the selected tabs, not the active one.
+  strip.CloseTabContentsAt(1, TabStripModel::CLOSE_NONE);
+  EXPECT_EQ(3, strip.count());
+  ASSERT_EQ(6, observer.GetStateCount());
+  ASSERT_EQ(observer.GetStateAt(4)->action,

[dpapad, 2011/06/10 17:23:55]

It seems that closing a selected tab does not result in a TabSelectionChanged
notification being sent, that needs to be fixed, right?

[sky, 2011/06/10 17:56:04]

Yes, I think its a good idea to force the notification to be sent even though
old == new.

[dpapad, 2011/06/10 18:21:37]

No, sending a TabSelectionChanged notification does not depend on "old == new"
but on 
"if (!selection_model().Equals(old_model)) {...}" (see tab_strip_model.cc:1221) 
which is true in this case. So I assume that currently
NotifyIfActiveOrSelectionChagned() is not being called from anywhere within
CloseTabContentsAt(). Adding such a call should solve this problem without
changing the if conditions.

[sky, 2011/06/10 19:25:24]

Maybe I'm confused. Lets say you have {0} active and [1-4] selected. If you
detach tab 4 then old_model becomes {0}, [1-3]. This is the same as the new
model, right?

[dpapad, 2011/06/13 21:36:18]

You are right that the selection seems to not have changed (because we store the
old_model after calling DecrementFrom()), but this is not the only condition
that causes the TabStripSelectionModel::Equals() to return false and therefore
trigger the notification. See my other comment in TabStripModel for more detail.

+            MockTabStripModelObserver::CLOSE);
+  ASSERT_EQ(observer.GetStateAt(5)->action,
+            MockTabStripModelObserver::DETACH);
+
+  // Active tab is at 0, deselecting all but the active tab.
+  strip.ToggleSelectionAt(1);
+  strip.ToggleSelectionAt(2);
+  ASSERT_EQ(8, observer.GetStateCount());
+  ASSERT_EQ(observer.GetStateAt(6)->action,
+            MockTabStripModelObserver::SELECT);
+  ASSERT_EQ(observer.GetStateAt(7)->action,
+            MockTabStripModelObserver::SELECT);
+
+  // Attempting to deselect the only selected tab, it is ignored (no
+  // notifications being sent) and tab at 0 remains selected and active.
+  strip.ToggleSelectionAt(0);
+  ASSERT_EQ(8, 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
-// ActiveTabChanged is still invoked.
+// TabSelectionChanged is 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;
+  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.
   strip.ActivateTabAt(1, true);
   ASSERT_EQ(1, observer.GetStateCount());
   MockTabStripModelObserver::State s(
       contents2, 1, MockTabStripModelObserver::SELECT);
   s.src_contents = contents2;
-  s.user_gesture = true;
+  s.src_index = 1;
+  s.user_gesture = false;
   EXPECT_TRUE(observer.StateEquals(0, s));
   strip.RemoveObserver(&observer);
   strip.CloseAllTabs();
 }