Remove TabContents from TabStripModel::ReplaceTabContentsAt and TabStripModel::DiscardTabContentsAt.

BUG=107201
TEST=no visible change


Review URL: https://chromiumcodereview.appspot.com/11443039

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

10 files changed, 55 insertions, 73 deletions

diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.cc b/chrome/browser/chromeos/memory/oom_priority_manager.cc
index a0f38aa..39d6d6a 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.cc
@@ -27,7 +27,6 @@
 #include "chrome/browser/memory_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
@@ -78,7 +77,7 @@ const int kFocusedTabScoreAdjustIntervalMs = 500;
 
 // Returns a unique ID for a WebContents.  Do not cast back to a pointer, as
 // the WebContents could be deleted if the user closed the tab.
-int64 IdFromTabContents(WebContents* web_contents) {
+int64 IdFromWebContents(WebContents* web_contents) {
   return reinterpret_cast<int64>(web_contents);
 }
 
@@ -233,15 +232,15 @@ bool OomPriorityManager::DiscardTabById(int64 target_web_contents_id) {
       // Can't discard tabs that are already discarded or active.
       if (model->IsTabDiscarded(idx) || (model->active_index() == idx))
         continue;
-      WebContents* web_contents = model->GetTabContentsAt(idx)->web_contents();
-      int64 web_contents_id = IdFromTabContents(web_contents);
+      WebContents* web_contents = model->GetWebContentsAt(idx);
+      int64 web_contents_id = IdFromWebContents(web_contents);
       if (web_contents_id == target_web_contents_id) {
         LOG(WARNING) << "Discarding tab " << idx
             << " id " << target_web_contents_id;
         // Record statistics before discarding because we want to capture the
         // memory state that lead to the discard.
         RecordDiscardStatistics();
-        model->DiscardTabContentsAt(idx);
+        model->DiscardWebContentsAt(idx);
         return true;
       }
     }
@@ -444,7 +443,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
     bool is_browser_for_app = browser->is_app();
     const TabStripModel* model = browser->tab_strip_model();
     for (int i = 0; i < model->count(); i++) {
-      WebContents* contents = model->GetTabContentsAt(i)->web_contents();
+      WebContents* contents = model->GetWebContentsAt(i);
       if (!contents->IsCrashed()) {
         TabStats stats;
         stats.is_app = is_browser_for_app;
@@ -454,7 +453,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
         stats.last_selected = contents->GetLastSelectedTime();
         stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
         stats.title = contents->GetTitle();
-        stats.tab_contents_id = IdFromTabContents(contents);
+        stats.tab_contents_id = IdFromWebContents(contents);
         stats_list.push_back(stats);
       }
     }
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager_browsertest.cc b/chrome/browser/chromeos/memory/oom_priority_manager_browsertest.cc
index 981fa56d..35aaddb 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager_browsertest.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager_browsertest.cc
@@ -103,7 +103,7 @@ IN_PROC_BROWSER_TEST_F(OomPriorityManagerTest, OomPriorityManagerBasics) {
   browser()->tab_strip_model()->ActivateTabAt(1, true);
   EXPECT_EQ(1, browser()->active_index());
   EXPECT_FALSE(browser()->tab_strip_model()->IsTabDiscarded(1));
-  browser()->tab_strip_model()->DiscardTabContentsAt(2);
+  browser()->tab_strip_model()->DiscardWebContentsAt(2);
   EXPECT_TRUE(browser()->tab_strip_model()->IsTabDiscarded(2));
 
   // Force creation of the FindBarController.
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index 7e8346e4..6510921 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -24,7 +24,6 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
@@ -878,7 +877,7 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, SessionStorage) {
 }
 
 IN_PROC_BROWSER_TEST_F(SessionRestoreTest, SessionStorageAfterTabReplace) {
-  // Simulate what prerendering does: create a new TabContents with the same
+  // Simulate what prerendering does: create a new WebContents with the same
   // SessionStorageNamespace as an existing tab, then replace the tab with it.
   {
     content::NavigationController* controller =
@@ -894,15 +893,12 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, SessionStorageAfterTabReplace) {
             session_storage_namespace_map));
 
     TabStripModel* tab_strip_model = browser()->tab_strip_model();
-    // We only need to create a TabContents because ReplaceTabContentsAt wants
-    // one. We don't need any of the helpers TabContents provides.
-    scoped_ptr<TabContents> old_tab_contents(
-        tab_strip_model->ReplaceTabContentsAt(
-            tab_strip_model->active_index(),
-            TabContents::Factory::CreateTabContents(web_contents.release())));
+    scoped_ptr<content::WebContents> old_web_contents(
+        tab_strip_model->ReplaceWebContentsAt(
+            tab_strip_model->active_index(), web_contents.release()));
     // Navigate with the new tab.
     ui_test_utils::NavigateToURL(browser(), url2_);
-    // old_tab_contents goes out of scope.
+    // old_web_contents goes out of scope.
   }
 
   // Check that the sessionStorage data is going to be persisted.
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 709debd..e66bcf1 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -1794,8 +1794,7 @@ void Browser::SwapTabContents(content::WebContents* old_contents,
                               content::WebContents* new_contents) {
   int index = tab_strip_model_->GetIndexOfWebContents(old_contents);
   DCHECK_NE(TabStripModel::kNoTab, index);
-  TabContents* new_tab_contents = TabContents::FromWebContents(new_contents);
-  tab_strip_model_->ReplaceTabContentsAt(index, new_tab_contents);
+  tab_strip_model_->ReplaceWebContentsAt(index, new_contents);
 }
 
 bool Browser::CanReloadContents(content::WebContents* web_contents) const {
diff --git a/chrome/browser/ui/browser_instant_controller.cc b/chrome/browser/ui/browser_instant_controller.cc
index 9b82049..de760fc 100644
--- a/chrome/browser/ui/browser_instant_controller.cc
+++ b/chrome/browser/ui/browser_instant_controller.cc
@@ -92,13 +92,12 @@ void BrowserInstantController::CommitInstant(content::WebContents* preview,
     browser_->tab_strip_model()->AddWebContents(preview, -1,
         instant_.last_transition_type(), TabStripModel::ADD_ACTIVE);
   } else {
-    content::WebContents* active_tab =
-        browser_->tab_strip_model()->GetActiveWebContents();
-    int index = browser_->tab_strip_model()->GetIndexOfWebContents(active_tab);
+    int index = browser_->tab_strip_model()->active_index();
     DCHECK_NE(TabStripModel::kNoTab, index);
+    content::WebContents* active_tab =
+        browser_->tab_strip_model()->GetWebContentsAt(index);
     // TabStripModel takes ownership of |preview|.
-    browser_->tab_strip_model()->ReplaceTabContentsAt(index,
-        TabContents::FromWebContents(preview));
+    browser_->tab_strip_model()->ReplaceWebContentsAt(index, preview);
     // InstantUnloadHandler takes ownership of |active_tab|.
     instant_unload_handler_.RunUnloadListenersOrDestroy(active_tab, index);
 
diff --git a/chrome/browser/ui/browser_tabrestore.cc b/chrome/browser/ui/browser_tabrestore.cc
index ac564a6..ff04683 100644
--- a/chrome/browser/ui/browser_tabrestore.cc
+++ b/chrome/browser/ui/browser_tabrestore.cc
@@ -140,7 +140,8 @@ void ReplaceRestoredTab(
                                                 session_storage_namespace,
                                                 user_agent_override);
 
-  // ReplaceTabContentsAt won't animate in the restoration, so do it manually.
+  // ReplaceWebContentsAt won't animate in the restoration, so manually do the
+  // equivalent of ReplaceWebContentsAt.
   int insertion_index = browser->active_index();
   browser->tab_strip_model()->InsertWebContentsAt(
       insertion_index + 1,
diff --git a/chrome/browser/ui/tab_contents/tab_contents.h b/chrome/browser/ui/tab_contents/tab_contents.h
index a031e63..86960b9 100644
--- a/chrome/browser/ui/tab_contents/tab_contents.h
+++ b/chrome/browser/ui/tab_contents/tab_contents.h
@@ -55,7 +55,6 @@ class TabContents : public content::WebContentsObserver {
     friend class TabAndroid;
     friend class TabStripModel;
     friend class TestTabStripModelDelegate;
-    FRIEND_TEST_ALL_PREFIXES(SessionRestoreTest, SessionStorageAfterTabReplace);
 
     static TabContents* CreateTabContents(content::WebContents* contents);
   };
diff --git a/chrome/browser/ui/tabs/tab_strip_model.cc b/chrome/browser/ui/tabs/tab_strip_model.cc
index cc51126..1c11ab6c 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model.cc
@@ -141,51 +141,48 @@ void TabStripModel::InsertWebContentsAt(int index,
   }
 }
 
-TabContents* TabStripModel::ReplaceTabContentsAt(int index,
-                                                 TabContents* new_contents) {
+WebContents* TabStripModel::ReplaceWebContentsAt(int index,
+                                                 WebContents* new_contents) {
+  delegate_->WillAddWebContents(new_contents);
+
   DCHECK(ContainsIndex(index));
-  TabContents* old_contents = GetTabContentsAtImpl(index);
+  WebContents* old_contents = GetWebContentsAtImpl(index);
 
-  ForgetOpenersAndGroupsReferencing(old_contents->web_contents());
+  ForgetOpenersAndGroupsReferencing(old_contents);
 
-  contents_data_[index]->contents = new_contents->web_contents();
+  contents_data_[index]->contents = new_contents;
 
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                    TabReplacedAt(this,
-                                  old_contents->web_contents(),
-                                  new_contents->web_contents(),
-                                  index));
+                    TabReplacedAt(this, old_contents, new_contents, index));
 
-  // When the active tab contents is replaced send out a selection notification
+  // When the active WebContents is replaced send out a selection notification
   // too. We do this as nearly all observers need to treat a replacement of the
   // selected contents as the selection changing.
   if (active_index() == index) {
     FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                      ActiveTabChanged(old_contents->web_contents(),
-                                       new_contents->web_contents(),
+                      ActiveTabChanged(old_contents, new_contents,
                                        active_index(), false));
   }
   return old_contents;
 }
 
-TabContents* TabStripModel::DiscardTabContentsAt(int index) {
+WebContents* TabStripModel::DiscardWebContentsAt(int index) {
   DCHECK(ContainsIndex(index));
   // Do not discard active tab.
   if (active_index() == index)
     return NULL;
 
-  TabContents* null_contents = TabContents::Factory::CreateTabContents(
+  WebContents* null_contents =
       WebContents::Create(profile(),
                           NULL /* site_instance */,
                           MSG_ROUTING_NONE,
-                          NULL /* base_tab_contents */));
-  TabContents* old_contents = GetTabContentsAtImpl(index);
+                          NULL /* base_web_contents */);
+  WebContents* old_contents = GetWebContentsAtImpl(index);
   // Copy over the state from the navigation controller so we preserve the
   // back/forward history and continue to display the correct title/favicon.
-  null_contents->web_contents()->GetController().CopyStateFrom(
-      old_contents->web_contents()->GetController());
+  null_contents->GetController().CopyStateFrom(old_contents->GetController());
   // Replace the tab we're discarding with the null version.
-  ReplaceTabContentsAt(index, null_contents);
+  ReplaceWebContentsAt(index, null_contents);
   // Mark the tab so it will reload when we click.
   contents_data_[index]->discarded = true;
   // Discard the old tab's renderer.
diff --git a/chrome/browser/ui/tabs/tab_strip_model.h b/chrome/browser/ui/tabs/tab_strip_model.h
index e50da63..94a9122 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.h
+++ b/chrome/browser/ui/tabs/tab_strip_model.h
@@ -188,17 +188,18 @@ class TabStripModel : public content::NotificationObserver {
   // user to confirm closure).
   bool CloseWebContentsAt(int index, uint32 close_types);
 
-  // Replaces the tab contents at |index| with |new_contents|. The
-  // TabContents that was at |index| is returned and ownership returns
+  // Replaces the WebContents at |index| with |new_contents|. The
+  // WebContents that was at |index| is returned and its ownership returns
   // to the caller.
-  TabContents* ReplaceTabContentsAt(int index,
-                                    TabContents* new_contents);
+  content::WebContents* ReplaceWebContentsAt(
+      int index,
+      content::WebContents* new_contents);
 
-  // Destroys the TabContents at the specified index, but keeps the tab
+  // Destroys the WebContents at the specified index, but keeps the tab
   // visible in the tab strip. Used to free memory in low-memory conditions,
   // especially on Chrome OS. The tab reloads if the user clicks on it.
-  // Returns an empty TabContents, used only for testing.
-  TabContents* DiscardTabContentsAt(int index);
+  // Returns the new empty WebContents, used only for testing.
+  content::WebContents* DiscardWebContentsAt(int index);
 
   // Detaches the WebContents at the specified index from this strip. The
   // WebContents is not destroyed, just removed from display. The caller
@@ -342,7 +343,7 @@ class TabStripModel : public content::NotificationObserver {
   bool IsTabBlocked(int index) const;
 
   // Returns true if the WebContents at |index| has been discarded to
-  // save memory.  See DiscardTabContentsAt() for details.
+  // save memory.  See DiscardWebContentsAt() for details.
   bool IsTabDiscarded(int index) const;
 
   // Returns the index of the first tab that is not a mini-tab. This returns
diff --git a/chrome/browser/ui/tabs/tab_strip_model_unittest.cc b/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
index cab4e37..aa4828b 100644
--- a/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model_unittest.cc
@@ -113,10 +113,6 @@ class TabStripModelTest : public ChromeRenderViewHostTestHarness {
   TabStripModelTest() : browser_thread_(BrowserThread::UI, &message_loop_) {
   }
 
-  TabContents* CreateTabContents() {
-    return chrome::TabContentsFactory(profile(), NULL, MSG_ROUTING_NONE, NULL);
-  }
-
   WebContents* CreateWebContents() {
     return WebContents::Create(profile(), NULL, MSG_ROUTING_NONE, NULL);
   }
@@ -2017,20 +2013,18 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
   MockTabStripModelObserver tabstrip_observer(&strip);
   strip.AddObserver(&tabstrip_observer);
 
-  TabContents* new_contents = CreateTabContents();
-  delete strip.ReplaceTabContentsAt(0, new_contents);
+  WebContents* new_contents = CreateWebContents();
+  delete strip.ReplaceWebContentsAt(0, new_contents);
 
   ASSERT_EQ(2, tabstrip_observer.GetStateCount());
 
   // First event should be for replaced.
-  State state(
-      new_contents->web_contents(), 0, MockTabStripModelObserver::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->web_contents(), 0, MockTabStripModelObserver::ACTIVATE);
+  state = State(new_contents, 0, MockTabStripModelObserver::ACTIVATE);
   state.src_contents = first_contents;
   EXPECT_TRUE(tabstrip_observer.StateEquals(1, state));
 
@@ -2043,13 +2037,12 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
   tabstrip_observer.ClearStates();
 
   // And replace it.
-  new_contents = CreateTabContents();
-  delete strip.ReplaceTabContentsAt(1, new_contents);
+  new_contents = CreateWebContents();
+  delete strip.ReplaceWebContentsAt(1, new_contents);
 
   ASSERT_EQ(1, tabstrip_observer.GetStateCount());
 
-  state = State(
-      new_contents->web_contents(), 1, MockTabStripModelObserver::REPLACED);
+  state = State(new_contents, 1, MockTabStripModelObserver::REPLACED);
   state.src_contents = third_contents;
   EXPECT_TRUE(tabstrip_observer.StateEquals(0, state));
 
@@ -2057,7 +2050,7 @@ TEST_F(TabStripModelTest, ReplaceSendsSelected) {
 }
 
 // Ensures discarding tabs leaves TabStripModel in a good state.
-TEST_F(TabStripModelTest, DiscardTabContentsAt) {
+TEST_F(TabStripModelTest, DiscardWebContentsAt) {
   typedef MockTabStripModelObserver::State State;
 
   TabStripDummyDelegate delegate;
@@ -2075,8 +2068,7 @@ TEST_F(TabStripModelTest, DiscardTabContentsAt) {
   tabstrip.AddObserver(&tabstrip_observer);
 
   // Discard one of the tabs.
-  TabContents* null_tab_contents1 = tabstrip.DiscardTabContentsAt(0);
-  WebContents* null_contents1 = null_tab_contents1->web_contents();
+  WebContents* null_contents1 = tabstrip.DiscardWebContentsAt(0);
   ASSERT_EQ(2, tabstrip.count());
   EXPECT_TRUE(tabstrip.IsTabDiscarded(0));
   EXPECT_FALSE(tabstrip.IsTabDiscarded(1));
@@ -2089,8 +2081,7 @@ TEST_F(TabStripModelTest, DiscardTabContentsAt) {
   tabstrip_observer.ClearStates();
 
   // Discard the same tab again.
-  TabContents* null_tab_contents2 = tabstrip.DiscardTabContentsAt(0);
-  WebContents* null_contents2 = null_tab_contents2->web_contents();
+  WebContents* null_contents2 = tabstrip.DiscardWebContentsAt(0);
   ASSERT_EQ(2, tabstrip.count());
   EXPECT_TRUE(tabstrip.IsTabDiscarded(0));
   EXPECT_FALSE(tabstrip.IsTabDiscarded(1));
@@ -2109,7 +2100,7 @@ TEST_F(TabStripModelTest, DiscardTabContentsAt) {
   EXPECT_FALSE(tabstrip.IsTabDiscarded(1));
 
   // Don't discard active tab.
-  tabstrip.DiscardTabContentsAt(0);
+  tabstrip.DiscardWebContentsAt(0);
   ASSERT_EQ(2, tabstrip.count());
   EXPECT_FALSE(tabstrip.IsTabDiscarded(0));
   EXPECT_FALSE(tabstrip.IsTabDiscarded(1));