Fix the ownership model of TabContents and NavigationController. Previously the

NavigationController owned the TabContents, and there were extra steps required
at creation and destruction to clean everything up properly.

NavigationController is now a member of TabContents, and there is no setup or
tear down necessary other than the constructor and destructor. I could remove
the tab contents creation in the NavigationController, as well as all the
weird destruction code in WebContents which got moved to the destructor.

I made the controller getter return a reference since the ownership is clear
and there is no possibility of NULL. This required changing a lot of tiles, but
many of them were simplified since they no longer have to NULL check.
Review URL: http://codereview.chromium.org/69043

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

4 files changed, 25 insertions, 25 deletions

diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 47fbad9..f05a444 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -342,10 +342,10 @@ class SessionRestoreImpl : public NotificationObserver {
           std::min(selected_index,
                    static_cast<int>(tab.navigations.size() - 1)));
       tab_loader_->AddTab(
-          browser->AddRestoredTab(tab.navigations,
-                                  static_cast<int>(i - window.tabs.begin()),
-                                  selected_index,
-                                  false));
+          &browser->AddRestoredTab(tab.navigations,
+                                   static_cast<int>(i - window.tabs.begin()),
+                                   selected_index,
+                                   false)->controller());
     }
   }
 
@@ -383,7 +383,7 @@ class SessionRestoreImpl : public NotificationObserver {
   void NotifySessionServiceOfRestoredTabs(Browser* browser, int initial_count) {
     SessionService* session_service = profile_->GetSessionService();
     for (int i = initial_count; i < browser->tab_count(); ++i)
-      session_service->TabRestored(browser->GetTabContentsAt(i)->controller());
+      session_service->TabRestored(&browser->GetTabContentsAt(i)->controller());
   }
 
   // The profile to create the sessions for.
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index ba473c9..c0b8158 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -938,7 +938,7 @@ void SessionService::BuildCommandsForBrowser(
     TabContents* tab = browser->GetTabContentsAt(i);
     DCHECK(tab);
     if (tab->profile() == profile()) {
-      BuildCommandsForTab(browser->session_id(), tab->controller(), i,
+      BuildCommandsForTab(browser->session_id(), &tab->controller(), i,
                           commands, tab_to_available_range);
       if (windows_to_track && !added_to_windows_to_track) {
         windows_to_track->insert(browser->session_id().id());
diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index 340bf34..5892013 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -146,7 +146,7 @@ void TabRestoreService::BrowserClosing(Browser* browser) {
   size_t entry_index = 0;
   for (int tab_index = 0; tab_index < browser->tab_count(); ++tab_index) {
     PopulateTabFromController(
-        browser->GetTabContentsAt(tab_index)->controller(),
+        &browser->GetTabContentsAt(tab_index)->controller(),
         &(window->tabs[entry_index]));
     if (window->tabs[entry_index].navigations.empty())
       window->tabs.erase(window->tabs.begin() + entry_index);
@@ -229,13 +229,13 @@ void TabRestoreService::RestoreEntryById(Browser* browser,
       Browser* browser = Browser::Create(profile());
       for (size_t tab_i = 0; tab_i < window->tabs.size(); ++tab_i) {
         const Tab& tab = window->tabs[tab_i];
-        NavigationController* restored_controller =
+        TabContents* restored_tab =
             browser->AddRestoredTab(tab.navigations, browser->tab_count(),
                                     tab.current_navigation_index,
                                     (static_cast<int>(tab_i) ==
                                      window->selected_tab_index));
-        if (restored_controller)
-          restored_controller->LoadIfNecessary();
+        if (restored_tab)
+          restored_tab->controller().LoadIfNecessary();
       }
       browser->window()->Show();
     } else {
diff --git a/chrome/browser/sessions/tab_restore_service_unittest.cc b/chrome/browser/sessions/tab_restore_service_unittest.cc
index 9be6429..7a24a91 100644
--- a/chrome/browser/sessions/tab_restore_service_unittest.cc
+++ b/chrome/browser/sessions/tab_restore_service_unittest.cc
@@ -41,9 +41,9 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
   void NavigateToIndex(int index) {
     // Navigate back. We have to do this song and dance as NavigationController
     // isn't happy if you navigate immediately while going back.
-    controller()->GoToIndex(index);
-    rvh()->SendNavigate(controller()->pending_entry()->page_id(),
-                        controller()->pending_entry()->url());
+    controller().GoToIndex(index);
+    rvh()->SendNavigate(controller().pending_entry()->page_id(),
+                        controller().pending_entry()->url());
   }
 
   void RecreateService() {
@@ -92,7 +92,7 @@ TEST_F(TabRestoreServiceTest, Basic) {
   AddThreeNavigations();
 
   // Have the service record the tab.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
 
   // Make sure an entry was created.
   ASSERT_EQ(1U, service_->entries().size());
@@ -110,7 +110,7 @@ TEST_F(TabRestoreServiceTest, Basic) {
   NavigateToIndex(1);
 
   // And check again.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
 
   // There should be two entries now.
   ASSERT_EQ(2U, service_->entries().size());
@@ -129,7 +129,7 @@ TEST_F(TabRestoreServiceTest, Basic) {
 // Make sure TabRestoreService doesn't create an entry for a tab with no
 // navigations.
 TEST_F(TabRestoreServiceTest, DontCreateEmptyTab) {
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
   EXPECT_TRUE(service_->entries().empty());
 }
 
@@ -138,7 +138,7 @@ TEST_F(TabRestoreServiceTest, Restore) {
   AddThreeNavigations();
 
   // Have the service record the tab.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
 
   // Recreate the service and have it load the tabs.
   RecreateService();
@@ -162,7 +162,7 @@ TEST_F(TabRestoreServiceTest, DontLoadRestoredTab) {
   AddThreeNavigations();
 
   // Have the service record the tab.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
   ASSERT_EQ(1U, service_->entries().size());
 
   // Restore the tab.
@@ -179,12 +179,12 @@ TEST_F(TabRestoreServiceTest, DontLoadRestoredTab) {
 // Make sure we persist entries to disk that have post data.
 TEST_F(TabRestoreServiceTest, DontPersistPostData) {
   AddThreeNavigations();
-  controller()->GetEntryAtIndex(0)->set_has_post_data(true);
-  controller()->GetEntryAtIndex(1)->set_has_post_data(true);
-  controller()->GetEntryAtIndex(2)->set_has_post_data(true);
+  controller().GetEntryAtIndex(0)->set_has_post_data(true);
+  controller().GetEntryAtIndex(1)->set_has_post_data(true);
+  controller().GetEntryAtIndex(2)->set_has_post_data(true);
 
   // Have the service record the tab.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
   ASSERT_EQ(1U, service_->entries().size());
 
   // Recreate the service and have it load the tabs.
@@ -209,7 +209,7 @@ TEST_F(TabRestoreServiceTest, DontLoadTwice) {
   AddThreeNavigations();
 
   // Have the service record the tab.
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
   ASSERT_EQ(1U, service_->entries().size());
 
   // Recreate the service and have it load the tabs.
@@ -276,7 +276,7 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) {
 
   AddThreeNavigations();
 
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
 
   RecreateService();
 
@@ -317,7 +317,7 @@ TEST_F(TabRestoreServiceTest, ManyWindowsInSessionService) {
 
   AddThreeNavigations();
 
-  service_->CreateHistoricalTab(controller());
+  service_->CreateHistoricalTab(&controller());
 
   RecreateService();