Changes TabStripModel to send out selection notification when the

selected tab is replaced.

BUG=none
TEST=none

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

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

2 files changed, 64 insertions, 1 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 608f9a9..4f6cfde 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -244,7 +244,17 @@ void TabStripModel::ReplaceTabContentsAt(
     int index,
     TabContents* new_contents,
     TabStripModelObserver::TabReplaceType type) {
-  delete ReplaceTabContentsAtImpl(index, new_contents, type);
+  scoped_ptr<TabContents> old_contents(
+      ReplaceTabContentsAtImpl(index, new_contents, type));
+
+  // When the selected tab contents is replaced send out selected notification
+  // too. We do this as nearly all observers need to treat a replace of the
+  // selected contents as selection changing.
+  if (selected_index_ == index) {
+    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                      TabSelectedAt(old_contents.get(), new_contents,
+                                    selected_index_, false));
+  }
 }
 
 void TabStripModel::ReplaceNavigationControllerAt(
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 47cb0f1..557aadf 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -1938,3 +1938,56 @@ TEST_F(TabStripModelTest, DeletePhantomTabContents) {
   // And there should have been no methods sent to the TabStripModelObserver.
   EXPECT_EQ(0, tabstrip_observer.GetStateCount());
 }
+
+// Makes sure the TabStripModel calls the right observer methods during a
+// replace.
+TEST_F(TabStripModelTest, ReplaceSendsSelected) {
+  typedef MockTabStripModelObserver::State State;
+
+  TabStripDummyDelegate delegate(NULL);
+  TabStripModel strip(&delegate, profile());
+
+  TabContents* first_contents = CreateTabContents();
+  strip.AddTabContents(first_contents, -1, PageTransition::TYPED,
+                       TabStripModel::ADD_SELECTED);
+
+  MockTabStripModelObserver tabstrip_observer;
+  strip.AddObserver(&tabstrip_observer);
+
+  TabContents* new_contents = CreateTabContents();
+  strip.ReplaceTabContentsAt(0, new_contents,
+                             TabStripModelObserver::REPLACE_MATCH_PREVIEW);
+
+  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.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.
+  TabContents* third_contents = CreateTabContents();
+  strip.AddTabContents(third_contents, 1, PageTransition::TYPED,
+                       TabStripModel::ADD_NONE);
+
+  tabstrip_observer.ClearStates();
+
+  // And replace it.
+  new_contents = CreateTabContents();
+  strip.ReplaceTabContentsAt(1, new_contents,
+                             TabStripModelObserver::REPLACE_MATCH_PREVIEW);
+
+  ASSERT_EQ(1, tabstrip_observer.GetStateCount());
+
+  state = State(new_contents, 1, MockTabStripModelObserver::REPLACED);
+  state.src_contents = third_contents;
+  EXPECT_TRUE(tabstrip_observer.StateEquals(0, state));
+
+  strip.CloseAllTabs();
+}