Re-land my change to clean up TabContents/WebContents ownership. This

is the same except in tab_strip_model_unittest I fixed a leak by making a
WebContents on the stack, I added a factory to the SiteInstance unittest to
prevent another leak, and I re-added a NULL set to the external_tab_container.

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.
Previous review URL: http://codereview.chromium.org/69043

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

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

3 files changed, 19 insertions, 21 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 0c1e95e..fd07b9d6 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -85,7 +85,7 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
       unsafe_resources_(unsafe_resources) {
   if (!is_main_frame_) {
     navigation_entry_index_to_remove_ =
-        tab()->controller()->last_committed_entry_index();
+        tab()->controller().last_committed_entry_index();
   } else {
     navigation_entry_index_to_remove_ = -1;
   }
@@ -402,8 +402,8 @@ void SafeBrowsingBlockingPage::DontProceed() {
   // would trigger a navigation that would cause trouble as the render view host
   // for the tab has by then already been destroyed.
   if (navigation_entry_index_to_remove_ != -1 && !tab()->is_being_destroyed()) {
-    tab()->controller()->RemoveEntryAtIndex(navigation_entry_index_to_remove_,
-                                            GURL(chrome::kChromeUINewTabURL));
+    tab()->controller().RemoveEntryAtIndex(navigation_entry_index_to_remove_,
+                                           GURL(chrome::kChromeUINewTabURL));
     navigation_entry_index_to_remove_ = -1;
   }
   InterstitialPage::DontProceed();
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index e8311db..32f3ab6 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -154,7 +154,7 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness,
 // Tests showing a blocking page for a malware page and not proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) {
   // Start a load.
-  controller()->LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
+  controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
 
   // Simulate the load causing a safe browsing interstitial to be shown.
   ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
@@ -169,13 +169,13 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) {
   EXPECT_FALSE(GetSafeBrowsingBlockingPage());
 
   // We did not proceed, the pending entry should be gone.
-  EXPECT_FALSE(controller()->pending_entry());
+  EXPECT_FALSE(controller().pending_entry());
 }
 
 // Tests showing a blocking page for a malware page and then proceeding.
 TEST_F(SafeBrowsingBlockingPageTest, MalwarePageProceed) {
   // Start a load.
-  controller()->LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
+  controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);
 
   // Simulate the load causing a safe browsing interstitial to be shown.
   ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
@@ -215,8 +215,8 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceDontProceed) {
 
   // We did not proceed, we should be back to the first page, the 2nd one should
   // have been removed from the navigation controller.
-  ASSERT_EQ(1, controller()->entry_count());
-  EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec());
+  ASSERT_EQ(1, controller().entry_count());
+  EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
 }
 
 // Tests showing a blocking page for a page that contains malware subresources
@@ -237,8 +237,8 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceProceed) {
   EXPECT_FALSE(GetSafeBrowsingBlockingPage());
 
   // We did proceed, we should be back to showing the page.
-  ASSERT_EQ(1, controller()->entry_count());
-  EXPECT_EQ(kGoodURL, controller()->GetActiveEntry()->url().spec());
+  ASSERT_EQ(1, controller().entry_count());
+  EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
@@ -270,8 +270,8 @@ TEST_F(SafeBrowsingBlockingPageTest,
 
   // We did not proceed, we should be back to the first page, the 2nd one should
   // have been removed from the navigation controller.
-  ASSERT_EQ(1, controller()->entry_count());
-  EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec());
+  ASSERT_EQ(1, controller().entry_count());
+  EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
@@ -313,8 +313,8 @@ TEST_F(SafeBrowsingBlockingPageTest,
 
   // We did not proceed, we should be back to the first page, the 2nd one should
   // have been removed from the navigation controller.
-  ASSERT_EQ(1, controller()->entry_count());
-  EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec());
+  ASSERT_EQ(1, controller().entry_count());
+  EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec());
 }
 
 // Tests showing a blocking page for a page that contains multiple malware
@@ -350,6 +350,6 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) {
   EXPECT_EQ(OK, user_response());
 
   // We did proceed, we should be back to the initial page.
-  ASSERT_EQ(1, controller()->entry_count());
-  EXPECT_EQ(kGoodURL, controller()->GetActiveEntry()->url().spec());
+  ASSERT_EQ(1, controller().entry_count());
+  EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec());
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 1a1c88a3..6369b3d 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -285,11 +285,9 @@ void SafeBrowsingService::DoDisplayBlockingPage(
       resource.threat_type == SafeBrowsingService::URL_MALWARE) {
     GURL page_url = wc->GetURL();
     GURL referrer_url;
-    if (wc->controller()) {
-      NavigationEntry* entry = wc->controller()->GetActiveEntry();
-      if (entry)
-        referrer_url = entry->referrer();
-    }
+    NavigationEntry* entry = wc->controller().GetActiveEntry();
+    if (entry)
+      referrer_url = entry->referrer();
     io_loop_->PostTask(FROM_HERE,
         NewRunnableMethod(this,
                           &SafeBrowsingService::ReportMalware,