Changes instant so that the newly created tab has a new id. Doing

otherwise causes subtle problems with extensions and breaks the
invariant that the tab id is unique.

BUG=68508
TEST=none

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

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

23 files changed, 61 insertions, 49 deletions

diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc
index 6df9ede..7993949 100644
--- a/chrome/browser/extensions/extension_browser_event_router.cc
+++ b/chrome/browser/extensions/extension_browser_event_router.cc
@@ -466,11 +466,13 @@ void ExtensionBrowserEventRouter::TabChangedAt(TabContentsWrapper* contents,
 }
 
 void ExtensionBrowserEventRouter::TabReplacedAt(
+    TabStripModel* tab_strip_model,
     TabContentsWrapper* old_contents,
     TabContentsWrapper* new_contents,
     int index) {
-  UnregisterForTabNotifications(old_contents->tab_contents());
-  RegisterForTabNotifications(new_contents->tab_contents());
+  TabClosingAt(tab_strip_model, old_contents, index);
+  TabInsertedAt(new_contents, index,
+                tab_strip_model->selected_index() == index);
 }
 
 void ExtensionBrowserEventRouter::TabPinnedStateChanged(
diff --git a/chrome/browser/extensions/extension_browser_event_router.h b/chrome/browser/extensions/extension_browser_event_router.h
index cdf2efe..e42f831 100644
--- a/chrome/browser/extensions/extension_browser_event_router.h
+++ b/chrome/browser/extensions/extension_browser_event_router.h
@@ -73,7 +73,8 @@ class ExtensionBrowserEventRouter : public TabStripModelObserver,
                         int to_index);
   virtual void TabChangedAt(TabContentsWrapper* contents, int index,
                             TabChangeType change_type);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabPinnedStateChanged(TabContentsWrapper* contents, int index);
diff --git a/chrome/browser/gtk/tabs/tab_strip_gtk.cc b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
index 8cadca9..233ad58 100644
--- a/chrome/browser/gtk/tabs/tab_strip_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
@@ -1061,7 +1061,8 @@ void TabStripGtk::TabChangedAt(TabContentsWrapper* contents, int index,
   tab->UpdateFromModel();
 }
 
-void TabStripGtk::TabReplacedAt(TabContentsWrapper* old_contents,
+void TabStripGtk::TabReplacedAt(TabStripModel* tab_strip_model,
+                                TabContentsWrapper* old_contents,
                                 TabContentsWrapper* new_contents,
                                 int index) {
   TabChangedAt(new_contents, index, ALL);
diff --git a/chrome/browser/gtk/tabs/tab_strip_gtk.h b/chrome/browser/gtk/tabs/tab_strip_gtk.h
index 20d9ee0..716a793 100644
--- a/chrome/browser/gtk/tabs/tab_strip_gtk.h
+++ b/chrome/browser/gtk/tabs/tab_strip_gtk.h
@@ -115,7 +115,8 @@ class TabStripGtk : public TabStripModelObserver,
                         int to_index);
   virtual void TabChangedAt(TabContentsWrapper* contents, int index,
                             TabChangeType change_type);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabMiniStateChanged(TabContentsWrapper* contents, int index);
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index 9439f21..f294535 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -975,14 +975,6 @@ void NavigationController::CopyStateFromAndPrune(NavigationController* source) {
     if (last_committed_entry_index_ != -1)
       last_committed_entry_index_--;
   }
-
-  // Take over the session id from source.
-  session_id_ = source->session_id_;
-
-  // Reset source's session id as we're taking it over. We give it a new id in
-  // case source is added later on, which can happen with instant enabled if the
-  // tab has a before unload handler.
-  source->session_id_ = SessionID();
 }
 
 void NavigationController::PruneAllButActive() {
diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h
index d86b587..adbc348 100644
--- a/chrome/browser/tab_contents/navigation_controller.h
+++ b/chrome/browser/tab_contents/navigation_controller.h
@@ -364,10 +364,8 @@ class NavigationController {
 
   // A variant of CopyStateFrom. Removes all entries from this except the last
   // entry, inserts all entries from |source| before and including the active
-  // entry and resets the |session_id_| of this to match |source|.  This is
-  // intended for use when you're going to throw away |source| and replace it
-  // with this. This method is intended for use when the last entry of |this|
-  // is the active entry. For example:
+  // entry. This method is intended for use when the last entry of |this| is the
+  // active entry. For example:
   // source: A B *C* D
   // this:   E F *G*   (last must be active or pending)
   // result: A B *G*
diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc
index 91c23eaa..1935457 100644
--- a/chrome/browser/tab_contents/navigation_controller_unittest.cc
+++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc
@@ -1725,6 +1725,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune) {
 
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
+  SessionID other_id(other_controller.session_id());
   other_contents->NavigateAndCommit(url3);
   other_controller.CopyStateFromAndPrune(&controller());
 
@@ -1738,10 +1739,9 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune) {
   EXPECT_EQ(url2, other_controller.GetEntryAtIndex(1)->url());
   EXPECT_EQ(url3, other_controller.GetEntryAtIndex(2)->url());
 
-  // The session id of the new tab should be that of the old, and the old should
-  // get a new id.
-  EXPECT_EQ(id.id(), other_controller.session_id().id());
-  EXPECT_NE(id.id(), controller().session_id().id());
+  // Make sure session ids didn't change.
+  EXPECT_EQ(id.id(), controller().session_id().id());
+  EXPECT_EQ(other_id.id(), other_controller.session_id().id());
 }
 
 // Test CopyStateFromAndPrune with 2 urls, the first selected and nothing in
@@ -1758,6 +1758,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune2) {
 
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
+  SessionID other_id(other_controller.session_id());
   other_controller.CopyStateFromAndPrune(&controller());
 
   // other_controller should now contain the 1 url: url1.
@@ -1768,10 +1769,9 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune2) {
 
   EXPECT_EQ(url1, other_controller.GetEntryAtIndex(0)->url());
 
-  // The session id of the new tab should be that of the old, and the old should
-  // get a new id.
-  EXPECT_EQ(id.id(), other_controller.session_id().id());
-  EXPECT_NE(id.id(), controller().session_id().id());
+  // Make sure session ids didn't change.
+  EXPECT_EQ(id.id(), controller().session_id().id());
+  EXPECT_EQ(other_id.id(), other_controller.session_id().id());
 }
 
 // Test CopyStateFromAndPrune with 2 urls, the first selected and nothing in
@@ -1788,6 +1788,7 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) {
 
   scoped_ptr<TestTabContents> other_contents(CreateTestTabContents());
   NavigationController& other_controller = other_contents->controller();
+  SessionID other_id(other_controller.session_id());
   other_controller.LoadURL(url3, GURL(), PageTransition::TYPED);
   other_controller.CopyStateFromAndPrune(&controller());
 
@@ -1805,10 +1806,9 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) {
 
   EXPECT_EQ(url3, other_controller.pending_entry()->url());
 
-  // The session id of the new tab should be that of the old, and the old should
-  // be get a new id.
-  EXPECT_EQ(id.id(), other_controller.session_id().id());
-  EXPECT_NE(id.id(), controller().session_id().id());
+  // Make sure session ids didn't change.
+  EXPECT_EQ(id.id(), controller().session_id().id());
+  EXPECT_EQ(other_id.id(), other_controller.session_id().id());
 }
 
 // Tests that navigations initiated from the page (with the history object)
diff --git a/chrome/browser/tabs/default_tab_handler.cc b/chrome/browser/tabs/default_tab_handler.cc
index bf8b0ee..f25a526 100644
--- a/chrome/browser/tabs/default_tab_handler.cc
+++ b/chrome/browser/tabs/default_tab_handler.cc
@@ -171,10 +171,12 @@ void DefaultTabHandler::TabMoved(TabContentsWrapper* contents,
   delegate_->AsBrowser()->TabMoved(contents, from_index, to_index);
 }
 
-void DefaultTabHandler::TabReplacedAt(TabContentsWrapper* old_contents,
+void DefaultTabHandler::TabReplacedAt(TabStripModel* tab_strip_model,
+                                      TabContentsWrapper* old_contents,
                                       TabContentsWrapper* new_contents,
                                       int index) {
-  delegate_->AsBrowser()->TabReplacedAt(old_contents, new_contents, index);
+  delegate_->AsBrowser()->TabReplacedAt(tab_strip_model, old_contents,
+                                        new_contents, index);
 }
 
 void DefaultTabHandler::TabPinnedStateChanged(TabContentsWrapper* contents,
diff --git a/chrome/browser/tabs/default_tab_handler.h b/chrome/browser/tabs/default_tab_handler.h
index 5d433fb..9a365fa 100644
--- a/chrome/browser/tabs/default_tab_handler.h
+++ b/chrome/browser/tabs/default_tab_handler.h
@@ -71,7 +71,8 @@ class DefaultTabHandler : public TabHandler,
   virtual void TabMoved(TabContentsWrapper* contents,
                         int from_index,
                         int to_index);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabPinnedStateChanged(TabContentsWrapper* contents, int index);
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 0e1f65f..c127254 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -176,7 +176,7 @@ TabContentsWrapper* TabStripModel::ReplaceTabContentsAt(
   contents_data_[index]->contents = new_contents;
 
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                    TabReplacedAt(old_contents, new_contents, index));
+                    TabReplacedAt(this, old_contents, new_contents, index));
 
   // 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
diff --git a/chrome/browser/tabs/tab_strip_model_observer.cc b/chrome/browser/tabs/tab_strip_model_observer.cc
index 8ba8814..dec6f41 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.cc
+++ b/chrome/browser/tabs/tab_strip_model_observer.cc
@@ -38,7 +38,8 @@ void TabStripModelObserver::TabChangedAt(TabContentsWrapper* contents,
                                          TabChangeType change_type) {
 }
 
-void TabStripModelObserver::TabReplacedAt(TabContentsWrapper* old_contents,
+void TabStripModelObserver::TabReplacedAt(TabStripModel* tab_strip_model,
+                                          TabContentsWrapper* old_contents,
                                           TabContentsWrapper* new_contents,
                                           int index) {
 }
diff --git a/chrome/browser/tabs/tab_strip_model_observer.h b/chrome/browser/tabs/tab_strip_model_observer.h
index c681ae0..d123961 100644
--- a/chrome/browser/tabs/tab_strip_model_observer.h
+++ b/chrome/browser/tabs/tab_strip_model_observer.h
@@ -85,9 +85,9 @@ class TabStripModelObserver {
                             TabChangeType change_type);
 
   // The tab contents was replaced at the specified index. This is invoked when
-  // a tab becomes phantom. See description of phantom tabs in class description
-  // of TabStripModel for details.
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  // instant is enabled and the user navigates by way of instant.
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
 
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index b33aaa6a..c13b38c 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -341,8 +341,10 @@ class MockTabStripModelObserver : public TabStripModelObserver {
                             TabChangeType change_type) {
     states_.push_back(new State(contents, index, CHANGE));
   }
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
-                             TabContentsWrapper* new_contents, int index) {
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
+                             TabContentsWrapper* new_contents,
+                             int index) {
     State* s = new State(new_contents, index, REPLACED);
     s ->src_contents = old_contents;
     states_.push_back(s);
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 5bdda30..49e694b 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -2673,7 +2673,8 @@ void Browser::TabMoved(TabContentsWrapper* contents,
   SyncHistoryWithTabs(std::min(from_index, to_index));
 }
 
-void Browser::TabReplacedAt(TabContentsWrapper* old_contents,
+void Browser::TabReplacedAt(TabStripModel* tab_strip_model,
+                            TabContentsWrapper* old_contents,
                             TabContentsWrapper* new_contents,
                             int index) {
   TabDetachedAtImpl(old_contents, index, DETACH_TYPE_REPLACE);
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index 1c09b28..16b0aba 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -708,7 +708,8 @@ class Browser : public TabHandlerDelegate,
   virtual void TabMoved(TabContentsWrapper* contents,
                         int from_index,
                         int to_index);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabPinnedStateChanged(TabContentsWrapper* contents, int index);
diff --git a/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.h b/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.h
index 534fcfb..f3ff14e 100644
--- a/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.h
+++ b/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.h
@@ -40,7 +40,8 @@ class TabStripModelObserverBridge : public TabStripModelObserver {
                         int to_index);
   virtual void TabChangedAt(TabContentsWrapper* contents, int index,
                             TabChangeType change_type);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabMiniStateChanged(TabContentsWrapper* contents, int index);
diff --git a/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.mm b/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.mm
index a998d23..8d1679a 100644
--- a/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.mm
+++ b/chrome/browser/ui/cocoa/tab_strip_model_observer_bridge.mm
@@ -86,6 +86,7 @@ void TabStripModelObserverBridge::TabChangedAt(TabContentsWrapper* contents,
 }
 
 void TabStripModelObserverBridge::TabReplacedAt(
+    TabStripModel* tab_strip_model,
     TabContentsWrapper* old_contents,
     TabContentsWrapper* new_contents,
     int index) {
diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.cc b/chrome/browser/ui/toolbar/wrench_menu_model.cc
index e45db37..7a9a9ba 100644
--- a/chrome/browser/ui/toolbar/wrench_menu_model.cc
+++ b/chrome/browser/ui/toolbar/wrench_menu_model.cc
@@ -334,7 +334,8 @@ void WrenchMenuModel::TabSelectedAt(TabContentsWrapper* old_contents,
   UpdateZoomControls();
 }
 
-void WrenchMenuModel::TabReplacedAt(TabContentsWrapper* old_contents,
+void WrenchMenuModel::TabReplacedAt(TabStripModel* tab_strip_model,
+                                    TabContentsWrapper* old_contents,
                                     TabContentsWrapper* new_contents,
                                     int index) {
   UpdateZoomControls();
diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.h b/chrome/browser/ui/toolbar/wrench_menu_model.h
index 2690ea9..5ebf7ee 100644
--- a/chrome/browser/ui/toolbar/wrench_menu_model.h
+++ b/chrome/browser/ui/toolbar/wrench_menu_model.h
@@ -98,8 +98,10 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
                              TabContentsWrapper* new_contents,
                              int index,
                              bool user_gesture);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
-                             TabContentsWrapper* new_contents, int index);
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
+                             TabContentsWrapper* new_contents,
+                             int index);
   virtual void TabStripModelDeleted();
 
   // Overridden from NotificationObserver:
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index ed6f48b..1993374 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1491,7 +1491,8 @@ void BrowserView::TabSelectedAt(TabContentsWrapper* old_contents,
   ProcessTabSelected(new_contents, true);
 }
 
-void BrowserView::TabReplacedAt(TabContentsWrapper* old_contents,
+void BrowserView::TabReplacedAt(TabStripModel* tab_strip_model,
+                                TabContentsWrapper* old_contents,
                                 TabContentsWrapper* new_contents,
                                 int index) {
   if (index != browser_->tabstrip_model()->selected_index())
diff --git a/chrome/browser/ui/views/frame/browser_view.h b/chrome/browser/ui/views/frame/browser_view.h
index c40f06b..79b0859 100644
--- a/chrome/browser/ui/views/frame/browser_view.h
+++ b/chrome/browser/ui/views/frame/browser_view.h
@@ -353,7 +353,8 @@ class BrowserView : public BrowserBubbleHost,
                              TabContentsWrapper* new_contents,
                              int index,
                              bool user_gesture);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int index);
   virtual void TabStripEmpty();
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 31f8021..5389ec8 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -335,7 +335,8 @@ void BrowserTabStripController::TabChangedAt(TabContentsWrapper* contents,
   SetTabDataAt(contents, model_index);
 }
 
-void BrowserTabStripController::TabReplacedAt(TabContentsWrapper* old_contents,
+void BrowserTabStripController::TabReplacedAt(TabStripModel* tab_strip_model,
+                                              TabContentsWrapper* old_contents,
                                               TabContentsWrapper* new_contents,
                                               int model_index) {
   SetTabDataAt(new_contents, model_index);
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 f36febd..e7e9915 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
@@ -71,7 +71,8 @@ class BrowserTabStripController : public TabStripController,
   virtual void TabChangedAt(TabContentsWrapper* contents,
                             int model_index,
                             TabChangeType change_type);
-  virtual void TabReplacedAt(TabContentsWrapper* old_contents,
+  virtual void TabReplacedAt(TabStripModel* tab_strip_model,
+                             TabContentsWrapper* old_contents,
                              TabContentsWrapper* new_contents,
                              int model_index);
   virtual void TabPinnedStateChanged(TabContentsWrapper* contents,