Make sure all paths out of the dialog properly unregister observers.

chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc

 // Copyright 2016 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/javascript_dialogs/javascript_dialog_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 20 matching lines @@
 }
 
 bool IsWebContentsForemost(content::WebContents* web_contents) {
   Browser* browser = BrowserList::GetInstance()->GetLastActive();
   DCHECK(browser);
   return browser->tab_strip_model()->GetActiveWebContents() == web_contents;
 }
 
 }  // namespace
 
+// A note on the interaction between the JavaScriptDialogTabHelper and the
+// JavaScriptDialog classes.
+//
+// Either side can start the process of closing a dialog, and we need to ensure
+// that everything is properly torn down no matter which side initiates.
+//
+// If closing is initiated by the JavaScriptDialogTabHelper, then it will call
+// CloseDialog(), which calls JavaScriptDialog::CloseDialogWithoutCallback().
+// That will clear the callback inside of JavaScriptDialog, and start the
+// JavaScriptDialog on its own path of destruction. CloseDialog() then calls
+// ClearDialogInfo() which removes observers.
+//
+// If closing is initiated by the JavaScriptDialog, which is a self-deleting
+// object, then it will make its callback. The callback will have been wrapped
+// within JavaScriptDialogTabHelper::RunJavaScriptDialog() to be a call to
+// JavaScriptDialogTabHelper::OnDialogClosed(), which, after doing the callback,
+// again calls ClearDialogInfo() to remove observers.
+
 JavaScriptDialogTabHelper::JavaScriptDialogTabHelper(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {
 }
 
 JavaScriptDialogTabHelper::~JavaScriptDialogTabHelper() {
+  if (dialog_)
+    CloseDialog(true /*suppress_callback*/, false, base::string16());
 }
 
 void JavaScriptDialogTabHelper::SetDialogShownCallbackForTesting(
     base::Closure callback) {
   dialog_shown_ = callback;
 }
 
 void JavaScriptDialogTabHelper::RunJavaScriptDialog(
     content::WebContents* alerting_web_contents,
     const GURL& origin_url,
@@ skipping 55 matching lines @@
     if (dialog_) {
       // There's already a dialog up; clear it out.
       CloseDialog(false, false, base::string16());
     }
 
     base::string16 title =
         AppModalDialogManager()->GetTitle(alerting_web_contents, origin_url);
     dialog_callback_ = callback;
     dialog_ = JavaScriptDialog::Create(
         parent_web_contents, alerting_web_contents, title, message_type,
-        message_text, default_prompt_text, callback);
+        message_text, default_prompt_text,
+        base::Bind(&JavaScriptDialogTabHelper::OnDialogClosed,
+                   base::Unretained(this), callback));
 
     BrowserList::AddObserver(this);
 
     // Message suppression is something that we don't give the user a checkbox
     // for any more. It was useful back in the day when dialogs were app-modal
     // and clicking the checkbox was the only way to escape a loop that the page
     // was doing, but now the user can just close the page.
     *did_suppress_message = false;
 
     if (!dialog_shown_.is_null()) {
@@ skipping 82 matching lines @@
 void JavaScriptDialogTabHelper::WasHidden() {
   if (dialog_)
     CloseDialog(false, false, base::string16());
 }
 
 void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) {
   if (dialog_ && !IsWebContentsForemost(web_contents()))
     CloseDialog(false, false, base::string16());
 }
 
+void JavaScriptDialogTabHelper::OnDialogClosed(
+    DialogClosedCallback callback,
+    bool success,
+    const base::string16& user_input) {
+  callback.Run(success, user_input);
+
+  ClearDialogInfo();
+}
+
 void JavaScriptDialogTabHelper::CloseDialog(bool suppress_callback,
                                             bool success,
                                             const base::string16& user_input) {
   DCHECK(dialog_);
 
   dialog_->CloseDialogWithoutCallback();
   if (!suppress_callback)
     dialog_callback_.Run(success, user_input);
 
+  ClearDialogInfo();
+}
+
+void JavaScriptDialogTabHelper::ClearDialogInfo() {
   dialog_.reset();
   dialog_callback_.Reset();
   BrowserList::RemoveObserver(this);
 }