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

9 files changed, 19 insertions, 21 deletions

diff --git a/chrome/browser/views/blocked_popup_container.cc b/chrome/browser/views/blocked_popup_container.cc
index ca5b996..3906d5b 100644
--- a/chrome/browser/views/blocked_popup_container.cc
+++ b/chrome/browser/views/blocked_popup_container.cc
@@ -254,12 +254,12 @@ void BlockedPopupContainer::AddTabContents(TabContents* blocked_contents,
                                            const gfx::Rect& bounds) {
   if (has_been_dismissed_) {
     // We simply bounce this popup without notice.
-    blocked_contents->CloseContents();
+    delete blocked_contents;
     return;
   }
 
   if (blocked_popups_.size() > kImpossibleNumberOfPopups) {
-    blocked_contents->CloseContents();
+    delete blocked_contents;
     LOG(INFO) << "Warning: Renderer is sending more popups to us then should be"
         " possible. Renderer compromised?";
     return;
@@ -477,7 +477,7 @@ void BlockedPopupContainer::SetPosition() {
 void BlockedPopupContainer::CloseEachTabContents() {
   while (!blocked_popups_.empty()) {
     blocked_popups_.back().first->set_delegate(NULL);
-    blocked_popups_.back().first->CloseContents();
+    delete blocked_popups_.back().first;
     blocked_popups_.pop_back();
   }
 
diff --git a/chrome/browser/views/blocked_popup_container.h b/chrome/browser/views/blocked_popup_container.h
index e469da6..c634308 100644
--- a/chrome/browser/views/blocked_popup_container.h
+++ b/chrome/browser/views/blocked_popup_container.h
@@ -220,7 +220,7 @@ class BlockedPopupContainer : public ConstrainedWindow,
   // change.
   void SetPosition();
 
-  // Sends a CloseContents() to each message in |blocked_popups_|.
+  // Deletes each contents in |blocked_popups_|.
   void CloseEachTabContents();
 
   // The TabContents that owns and constrains this BlockedPopupContainer.
diff --git a/chrome/browser/views/bug_report_view.cc b/chrome/browser/views/bug_report_view.cc
index 1144065..647dfc1 100644
--- a/chrome/browser/views/bug_report_view.cc
+++ b/chrome/browser/views/bug_report_view.cc
@@ -482,7 +482,7 @@ void BugReportView::SendReport() {
 }
 
 void BugReportView::ReportPhishing() {
-  tab_->controller()->LoadURL(
+  tab_->controller().LoadURL(
       safe_browsing_util::GeneratePhishingReportUrl(
           kReportPhishingUrl, WideToUTF8(page_url_text_->GetText())),
       GURL(),
diff --git a/chrome/browser/views/dom_view.cc b/chrome/browser/views/dom_view.cc
index 3e1c777..ffa9c1d 100644
--- a/chrome/browser/views/dom_view.cc
+++ b/chrome/browser/views/dom_view.cc
@@ -11,11 +11,8 @@ DOMView::DOMView() : initialized_(false), web_contents_(NULL) {
 }
 
 DOMView::~DOMView() {
-  if (web_contents_) {
+  if (web_contents_.get())
     Detach();
-    web_contents_->Destroy();
-    web_contents_ = NULL;
-  }
 }
 
 bool DOMView::Init(Profile* profile, SiteInstance* instance) {
@@ -23,13 +20,13 @@ bool DOMView::Init(Profile* profile, SiteInstance* instance) {
     return true;
 
   initialized_ = true;
-  web_contents_ = new WebContents(profile, instance, MSG_ROUTING_NONE, NULL);
+  web_contents_.reset(new WebContents(profile, instance,
+                                      MSG_ROUTING_NONE, NULL));
   views::HWNDView::Attach(web_contents_->GetNativeView());
-  web_contents_->SetupController(profile);
   return true;
 }
 
 void DOMView::LoadURL(const GURL& url) {
   DCHECK(initialized_);
-  web_contents_->controller()->LoadURL(url, GURL(), PageTransition::START_PAGE);
+  web_contents_->controller().LoadURL(url, GURL(), PageTransition::START_PAGE);
 }
diff --git a/chrome/browser/views/dom_view.h b/chrome/browser/views/dom_view.h
index bfa6d13..132a444 100644
--- a/chrome/browser/views/dom_view.h
+++ b/chrome/browser/views/dom_view.h
@@ -8,6 +8,7 @@
 #ifndef CHROME_BROWSER_VIEWS_DOM_VIEW_H_
 #define CHROME_BROWSER_VIEWS_DOM_VIEW_H_
 
+#include "base/scoped_ptr.h"
 #include "chrome/views/controls/hwnd_view.h"
 #include "googleurl/src/gurl.h"
 
@@ -34,7 +35,7 @@ class DOMView : public views::HWNDView {
  protected:
   virtual bool CanProcessTabKeyEvents() { return true; }
 
-  WebContents* web_contents_;
+  scoped_ptr<WebContents> web_contents_;
 
  private:
   bool initialized_;
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 2505ff8..2cd0823 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -769,10 +769,10 @@ void BrowserView::ShowReportBugDialog() {
   BugReportView* bug_report_view = new BugReportView(browser_->profile(),
                                                      current_tab);
 
-  if (current_tab->controller()->GetLastCommittedEntry()) {
+  if (current_tab->controller().GetLastCommittedEntry()) {
     // URL for the current page
     bug_report_view->SetUrl(
-        current_tab->controller()->GetActiveEntry()->url());
+        current_tab->controller().GetActiveEntry()->url());
   }
 
   // retrieve the application version info
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index c1b96b9..59bce3b 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -989,7 +989,7 @@ bool LocationBarView::SecurityImageView::OnMousePressed(
     const views::MouseEvent& event) {
   NavigationEntry* nav_entry =
       BrowserList::GetLastActive()->GetSelectedTabContents()->
-          controller()->GetActiveEntry();
+          controller().GetActiveEntry();
   if (!nav_entry) {
     NOTREACHED();
     return true;
@@ -1031,7 +1031,7 @@ bool LocationBarView::RssImageView::OnMousePressed(
     const views::MouseEvent& event) {
   NavigationEntry* entry =
       BrowserList::GetLastActive()->GetSelectedTabContents()->
-      controller()->GetActiveEntry();
+      controller().GetActiveEntry();
   if (!entry) {
     NOTREACHED();
     return true;
diff --git a/chrome/browser/views/tab_contents_container_view.cc b/chrome/browser/views/tab_contents_container_view.cc
index 4b721da..04c9354 100644
--- a/chrome/browser/views/tab_contents_container_view.cc
+++ b/chrome/browser/views/tab_contents_container_view.cc
@@ -181,7 +181,7 @@ void TabContentsContainerView::AddObservers() {
     // the focus subclass on the shown HWND so we intercept focus change events.
     NotificationService::current()->AddObserver(
         this, NotificationType::RENDER_VIEW_HOST_CHANGED,
-        Source<NavigationController>(tab_contents_->controller()));
+        Source<NavigationController>(&tab_contents_->controller()));
   }
   NotificationService::current()->AddObserver(
       this,
@@ -195,7 +195,7 @@ void TabContentsContainerView::RemoveObservers() {
     NotificationService::current()->RemoveObserver(
         this,
         NotificationType::RENDER_VIEW_HOST_CHANGED,
-        Source<NavigationController>(tab_contents_->controller()));
+        Source<NavigationController>(&tab_contents_->controller()));
   }
   NotificationService::current()->RemoveObserver(
       this,
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index 51baf95..5328927 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -703,8 +703,8 @@ int TabStrip::OnPerformDrop(const DropTargetEvent& event) {
   } else {
     UserMetrics::RecordAction(L"Tab_DropURLOnTab", model_->profile());
 
-    model_->GetTabContentsAt(drop_index)->controller()->
-        LoadURL(url, GURL(), PageTransition::GENERATED);
+    model_->GetTabContentsAt(drop_index)->controller().LoadURL(
+        url, GURL(), PageTransition::GENERATED);
     model_->SelectTabContentsAt(drop_index, true);
   }