If a WebContents is deleted, delete any associated TabContents.

BUG=107201
TEST=no visible change


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168583

Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=168842

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

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

4 files changed, 22 insertions, 26 deletions

diff --git a/chrome/browser/ui/blocked_content/blocked_content_container.cc b/chrome/browser/ui/blocked_content/blocked_content_container.cc
index 3ff2708..9780f08 100644
--- a/chrome/browser/ui/blocked_content/blocked_content_container.cc
+++ b/chrome/browser/ui/blocked_content/blocked_content_container.cc
@@ -6,25 +6,12 @@
 
 #include "base/logging.h"
 #include "chrome/browser/ui/blocked_content/blocked_content_tab_helper.h"
-#include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/gfx/rect.h"
 
 using content::OpenURLParams;
 using content::WebContents;
 
-namespace {
-
-void DestroyBlockedContents(WebContents* web_contents) {
-  TabContents* tab_contents = TabContents::FromWebContents(web_contents);
-  if (tab_contents)
-    delete tab_contents;
-  else
-    delete web_contents;
-}
-
-}  // namespace
-
 // static
 const size_t BlockedContentContainer::kImpossibleNumberOfPopups = 30;
 
@@ -121,7 +108,7 @@ void BlockedContentContainer::Clear() {
     WebContents* web_contents = i->web_contents;
     web_contents->SetDelegate(NULL);
     BlockedContentTabHelper::FromWebContents(web_contents)->set_delegate(NULL);
-    DestroyBlockedContents(web_contents);
+    delete web_contents;
   }
   blocked_contents_.clear();
 }
@@ -156,7 +143,7 @@ void BlockedContentContainer::CloseContents(WebContents* source) {
       BlockedContentTabHelper::FromWebContents(web_contents)->
           set_delegate(NULL);
       blocked_contents_.erase(i);
-      DestroyBlockedContents(web_contents);
+      delete web_contents;
       break;
     }
   }
diff --git a/chrome/browser/ui/tab_contents/tab_contents.cc b/chrome/browser/ui/tab_contents/tab_contents.cc
index 8fe6bda..d49d306 100644
--- a/chrome/browser/ui/tab_contents/tab_contents.cc
+++ b/chrome/browser/ui/tab_contents/tab_contents.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 
+#include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "chrome/browser/autofill/autofill_external_delegate.h"
@@ -78,8 +79,11 @@ class TabContentsUserData : public base::SupportsUserData::Data {
   virtual ~TabContentsUserData() {}
   TabContents* tab_contents() { return tab_contents_; }
 
+  void MakeContentsOwned() { owned_tab_contents_.reset(tab_contents_); }
+
  private:
   TabContents* tab_contents_;  // unowned
+  scoped_ptr<TabContents> owned_tab_contents_;
 };
 
 }  // namespace
@@ -100,7 +104,9 @@ TabContents* TabContents::Factory::CloneTabContents(TabContents* contents) {
 TabContents::TabContents(WebContents* contents)
     : content::WebContentsObserver(contents),
       in_destructor_(false),
-      web_contents_(contents) {
+      web_contents_(contents),
+      profile_(Profile::FromBrowserContext(contents->GetBrowserContext())),
+      owned_web_contents_(contents) {
   DCHECK(contents);
   DCHECK(!FromWebContents(contents));
 
@@ -223,19 +229,22 @@ const TabContents* TabContents::FromWebContents(const WebContents* contents) {
 }
 
 WebContents* TabContents::web_contents() const {
-  return web_contents_.get();
+  return web_contents_;
 }
 
 Profile* TabContents::profile() const {
-  return Profile::FromBrowserContext(web_contents()->GetBrowserContext());
+  return profile_;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // WebContentsObserver overrides
 
 void TabContents::WebContentsDestroyed(WebContents* tab) {
-  // Destruction of the WebContents should only be done by us from our
-  // destructor. Otherwise it's very likely we (or one of the helpers we own)
-  // will attempt to access the WebContents and we'll crash.
-  DCHECK(in_destructor_);
+  if (!in_destructor_) {
+    // The owned WebContents is being destroyed independently, so delete this.
+    ignore_result(owned_web_contents_.release());
+    TabContentsUserData* user_data = static_cast<TabContentsUserData*>(
+        tab->GetUserData(&kTabContentsUserDataKey));
+    user_data->MakeContentsOwned();
+  }
 }
diff --git a/chrome/browser/ui/tab_contents/tab_contents.h b/chrome/browser/ui/tab_contents/tab_contents.h
index 8ba2856..f3fd3ae 100644
--- a/chrome/browser/ui/tab_contents/tab_contents.h
+++ b/chrome/browser/ui/tab_contents/tab_contents.h
@@ -116,10 +116,12 @@ class TabContents : public content::WebContentsObserver {
   // If true, we're running the destructor.
   bool in_destructor_;
 
+  content::WebContents* web_contents_;
+  Profile* profile_;
   // The supporting objects need to outlive the WebContents dtor (as they may
   // be called upon during its execution). As a result, this must come last
   // in the list.
-  scoped_ptr<content::WebContents> web_contents_;
+  scoped_ptr<content::WebContents> owned_web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContents);
 };
diff --git a/chrome/browser/ui/tabs/tab_strip_model.cc b/chrome/browser/ui/tabs/tab_strip_model.cc
index 61e072a..1252aa8 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model.cc
@@ -1130,9 +1130,7 @@ void TabStripModel::InternalCloseTab(WebContents* contents,
 
   // Deleting the WebContents will call back to us via
   // NotificationObserver and detach it.
-  TabContents* tab_contents = TabContents::FromWebContents(contents);
-  DCHECK(tab_contents);
-  delete tab_contents;
+  delete contents;
 }
 
 TabContents* TabStripModel::GetTabContentsAtImpl(int index) const {