WebUI: prevent WebContent to hold invalid pointer.

chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc

Index: chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
diff --git a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
index fc5275f224f37bb50e7f8be00abf2afa5c3ac54d..b65986ea0e8048c92289bf94f923e7e02b90c685 100644
--- a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
+++ b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
@@ -82,6 +82,9 @@ bool ConstrainedWebDialogDelegateBase::closed_via_webui() const {
 }
 
 void ConstrainedWebDialogDelegateBase::ReleaseWebContentsOnDialogClose() {
+  // 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());

[Avi (use Gerrit), 2017/04/04 14:49:46]

This doesn't look right.

  // If called, on dialog closure, the dialog will release its WebContents
  // instead of destroying it. After which point, the caller will own the
  // released WebContents.
  virtual void ReleaseWebContentsOnDialogClose() = 0;

This is a note that *in the future* the close behavior should change. When this
function is called, the close isn't happening right then.

Can you move the clearing of the delegate to the actual close?

[arthursonzogni, 2017/04/04 15:17:52]

I can't. [See the answer above].
Clearing the pointer to the ConstrainedDelegate early will cause problem if
someone needs it between
* ConstrainedWebDialogDelegateBase::ReleaseWebContentsOnDialogClose() and
* ConstrainedWebDialogDelegateBase::~ConstrainedWebDialogDelegateBase()

It looks like there is no call to
ConstrainedWebDialogUI::GetConstrainedDelegate() that make the assumption that
it will return something. I hope @thestig would knows what problems may arise.

[Lei Zhang, 2017/04/06 09:12:54]

Please double check my work:

Starting from PrintPreviewUI::OnHidePreviewDialog() we have:

delegate->ReleaseWebContentsOnDialogClose();
background_printing_manager->OwnPrintPreviewDialog(preview_dialog);
OnClosePrintPreviewDialog();

and PrintPreviewUI::OnClosePrintPreviewDialog() tries to call
GetConstrainedDelegate() too.

With this patch set, that GetConstrainedDelegate() call will return nullptr, and
the following code doesn't run:

delegate->GetWebDialogDelegate()->OnDialogClosed(std::string());
delegate->OnDialogCloseFromWebUI();

OnDialogClosed() is overridden by PrintPreviewDialogDelegate::OnDialogClosed()
which is a no-op.

OnDialogCloseFromWebUI() goes to
ConstrainedWebDialogDelegateBase::OnDialogCloseFromWebUI() which tries to call
CloseContents(). CloseContents() is overridden by
ConstrainedWebDialogDelegateViews::CloseContents(). If that does not run, then
does that mean the print preview dialog stays open?

   release_contents_on_close_ = true;
 }