Fix memory leak for extension uninstall dialog.

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc

Index: chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
diff --git a/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc b/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
index 6aed9daa19761504bf3c996fc6fcc8ded121dbbf..203704deb9f38cf30ee555a49c5d40489fc9678b 100644
--- a/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
+++ b/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
@@ -44,8 +44,8 @@ class ExtensionUninstallDialogViews
   ~ExtensionUninstallDialogViews() override;
 
   // Called when the ExtensionUninstallDialogDelegate has been destroyed to make
-  // sure we invalidate pointers.
-  void DialogDelegateDestroyed() { view_ = NULL; }
+  // sure we invalidate pointers. This object will also be freed.
+  void DialogDelegateDestroyed();
 
   // Forwards the accept and cancels to the delegate.
   void DialogAccepted(bool handle_report_abuse);
@@ -119,6 +119,7 @@ ExtensionUninstallDialogViews::ExtensionUninstallDialogViews(
       parent_(parent) {
   if (parent_)
     parent_window_tracker_ = NativeWindowTracker::Create(parent_);
+  LOG(ERROR) << this << " Dialog Created";
 }
 
 ExtensionUninstallDialogViews::~ExtensionUninstallDialogViews() {
@@ -127,6 +128,7 @@ ExtensionUninstallDialogViews::~ExtensionUninstallDialogViews() {
     view_->DialogDestroyed();
     view_->GetWidget()->CloseNow();
   }
+  LOG(ERROR) << this << " Dialog Died";
 }
 
 void ExtensionUninstallDialogViews::Show() {
@@ -140,6 +142,11 @@ void ExtensionUninstallDialogViews::Show() {
   constrained_window::CreateBrowserModalDialogViews(view_, parent_)->Show();
 }
 
+void ExtensionUninstallDialogViews::DialogDelegateDestroyed() {
+  view_ = NULL;
+  OnDialogClosed(CLOSE_ACTION_CANCELED);

[Devlin, 2016/11/05 06:26:18]

It seems like there's a potential that this could be called twice now, since the
dialog view is eventually destructed after e.g. hitting accept.  In the tests,
this won't necessarily fail, but it would be bad in production (where we have
lifetime issues and real consequences calling multiple times).  We should a)
guard this here and b) maybe put in a DCHECK
ExtensionUninstallDialog::OnDialogClosed to make sure we don't call twice.

[lgcheng, 2016/11/10 21:39:51]

Thanks for pointing out the potential issue. But after re-examine more on
details of implementation, I think it's safe since in the deconstructor of
ExtensionUninstallDialogDelegateView there is a check   
if (dialog_)
    dialog_->DialogDelegateDestroyed();

If the view is closed due to accept or cancel,
ExtensionUninstallDialogViews::DialogAccepted or
ExtensionUninstallDialogViews::DialogCanceled() will be called, in which
view_->DialogDestroyed() { dialog_ = NULL} is called and guarantees  
dialog_->DialogDelegateDestroyed() will not be called in this situation.

But I agree to add debug mode checks for this.

+}
+
 void ExtensionUninstallDialogViews::DialogAccepted(bool report_abuse_checked) {
   // The widget gets destroyed when the dialog is accepted.
   view_->DialogDestroyed();
@@ -176,6 +183,7 @@ ExtensionUninstallDialogDelegateView::ExtensionUninstallDialogDelegateView(
   heading_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   heading_->SetAllowCharacterBreak(true);
   AddChildView(heading_);
+  LOG(ERROR) << this << " Dialog View Created";
 }
 
 ExtensionUninstallDialogDelegateView::~ExtensionUninstallDialogDelegateView() {
@@ -187,6 +195,7 @@ ExtensionUninstallDialogDelegateView::~ExtensionUninstallDialogDelegateView() {
   // about to be freed by the Widget framework.
   if (dialog_)
     dialog_->DialogDelegateDestroyed();
+  LOG(ERROR) << this << " Dialog View Died";
 }
 
 views::View* ExtensionUninstallDialogDelegateView::CreateExtraView() {