Don't leave renderer process frozen when canceling JavaScript dialogs.

chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc

Index: chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc
diff --git a/chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc b/chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc
index c65a732ceba0cf96993196f6b6f54fb912404664..016bef90d3e069231cb6b9407b27626c5de5e72c 100644
--- a/chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc
+++ b/chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.cc
@@ -69,6 +69,8 @@ JavaScriptAppModalDialog::JavaScriptAppModalDialog(
     bool is_reload,
     const JavaScriptDialogManager::DialogClosedCallback& callback)
     : AppModalDialog(web_contents, title),
+      WebContentsObserver(web_contents),
+      render_view_host_(web_contents->GetRenderViewHost()),
       extra_data_map_(extra_data_map),
       javascript_message_type_(javascript_message_type),
       display_suppress_checkbox_(display_suppress_checkbox),
@@ -85,7 +87,7 @@ JavaScriptAppModalDialog::~JavaScriptAppModalDialog() {
 
 NativeAppModalDialog* JavaScriptAppModalDialog::CreateNativeDialog() {
   gfx::NativeWindow parent_window =
-      web_contents()->GetView()->GetTopLevelNativeWindow();
+      AppModalDialog::web_contents()->GetView()->GetTopLevelNativeWindow();
 
 #if defined(USE_AURA)
   if (!parent_window->GetRootWindow()) {
@@ -108,11 +110,23 @@ void JavaScriptAppModalDialog::Invalidate() {
     return;
 
   AppModalDialog::Invalidate();
-  callback_.Reset();
+  if (!callback_.is_null()) {
+    callback_.Run(false, string16());
+    callback_.Reset();
+  }
   if (native_dialog())
     CloseModalDialog();
 }
 
+void JavaScriptAppModalDialog::RenderViewDeleted(

[Avi (use Gerrit), 2013/10/01 03:20:18]

This really, really makes me sad. The old JavaScript dialog code knew all about
its callers and had a million hooks into them, and this smells of those days.

The WebContentsImpl knows when things are shifting around. Can't it do a better
job of cancelling the dialog if the RVH changes?

[Charlie Reis, 2013/10/01 16:39:29]

That part was optional.  The new WebContents method I added handles all the ways
of the RVH disappearing that I know of, so I went ahead and deleted the
WebContentsObserver stuff here.

+    content::RenderViewHost* render_view_host) {
+  // Don't try to run the callback after the RenderViewHost has been deleted.
+  if (render_view_host_ == render_view_host) {
+    render_view_host_ = NULL;
+    callback_.Reset();
+  }
+}
+
 void JavaScriptAppModalDialog::OnCancel(bool suppress_js_messages) {
   // If we are shutting down and this is an onbeforeunload dialog, cancel the
   // shutdown.
@@ -161,12 +175,16 @@ void JavaScriptAppModalDialog::NotifyDelegate(bool success,
   if (!IsValid())
     return;
 
-  callback_.Run(success, user_input);
+  if (!callback_.is_null()) {
+    callback_.Run(success, user_input);
+    callback_.Reset();
+  }
 
   // The callback_ above may delete web_contents_, thus removing the extra
   // data from the map owned by ChromeJavaScriptDialogManager. Make sure
   // to only use the data if still present. http://crbug.com/236476
-  ExtraDataMap::iterator extra_data = extra_data_map_->find(web_contents());
+  ExtraDataMap::iterator extra_data = extra_data_map_->find(
+      AppModalDialog::web_contents());
   if (extra_data != extra_data_map_->end()) {
     extra_data->second.last_javascript_message_dismissal_ =
         base::TimeTicks::Now();