WebUI: prevent WebContent to hold invalid pointer.

chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h"
 
 #include <string>
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
@@ skipping 16 matching lines @@
     WebDialogDelegate* delegate,
     WebDialogWebContentsDelegate* tab_delegate)
     : WebDialogWebContentsDelegate(browser_context,
                                    new ChromeWebContentsHandler),
       web_dialog_delegate_(delegate),
       closed_via_webui_(false),
       release_contents_on_close_(false) {
   CHECK(delegate);
   web_contents_.reset(
       WebContents::Create(WebContents::CreateParams(browser_context)));
+  WebContentsObserver::Observe(web_contents_.get());
   zoom::ZoomController::CreateForWebContents(web_contents_.get());
   if (tab_delegate) {
     override_tab_delegate_.reset(tab_delegate);
     web_contents_->SetDelegate(tab_delegate);
   } else {
     web_contents_->SetDelegate(this);
   }
   content::RendererPreferences* prefs =
       web_contents_->GetMutableRendererPrefs();
   renderer_preferences_util::UpdateFromSystemSettings(
       prefs, Profile::FromBrowserContext(browser_context), web_contents_.get());
 
   web_contents_->GetRenderViewHost()->SyncRendererPrefs();
 
   // Set |this| as a delegate so the ConstrainedWebDialogUI can retrieve it.
   ConstrainedWebDialogUI::SetConstrainedDelegate(web_contents_.get(), this);
 
   web_contents_->GetController().LoadURL(delegate->GetDialogContentURL(),
                                          content::Referrer(),
                                          ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
                                          std::string());
 }
 
 ConstrainedWebDialogDelegateBase::~ConstrainedWebDialogDelegateBase() {
-  if (release_contents_on_close_)
+  if (release_contents_on_close_ && web_contents_) {
+    // Remove reference to |this| in the WebContent since the lifetime of the
+    // WebContent may exceed the one of this object.
+    ConstrainedWebDialogUI::ClearConstrainedDelegate(web_contents_.get());
     ignore_result(web_contents_.release());
+  }
 }
 
 const WebDialogDelegate*
     ConstrainedWebDialogDelegateBase::GetWebDialogDelegate() const {
   return web_dialog_delegate_.get();
 }
 
 WebDialogDelegate*
     ConstrainedWebDialogDelegateBase::GetWebDialogDelegate() {
   return web_dialog_delegate_.get();
@@ skipping 34 matching lines @@
 gfx::Size ConstrainedWebDialogDelegateBase::GetMaximumSize() const {
   NOTREACHED();
   return gfx::Size();
 }
 
 gfx::Size ConstrainedWebDialogDelegateBase::GetPreferredSize() const {
   NOTREACHED();
   return gfx::Size();
 }
 
+void ConstrainedWebDialogDelegateBase::WebContentsDestroyed() {
+  web_contents_.release();

[Lei Zhang, 2017/04/07 00:55:22]

This release() makes the ownership of |web_contents_| questionable. I'll try to
write a CL to make it cleaner.

The failing test in patch set 1 tries to simulate what's happening in the
browser, and the WebContents should outlive the ConstrainedWebDialog. However,
some implementation detail in ConstrainedWebDialogDelegateViewViews means even
though the test called OnDialogCloseFromWebUI(), the dialog does not actually
get destroyed until later.

+}
+
 void ConstrainedWebDialogDelegateBase::ResizeToGivenSize(
     const gfx::Size size) {
   NOTREACHED();
 }