Ensure that modal dialogs from subframes can be cleaned up correctly.

BUG=366510
TEST=See bug comment 7 for repro steps.

Review URL: https://codereview.chromium.org/255543009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266638 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/browser_browsertest.cc

@@ -574,6 +574,33 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, SadTabCancelsDialogs) {
   ui_test_utils::NavigateToURL(browser(), url2);
 }
 
+// Make sure that dialogs opened by subframes are closed when the process dies.
+// See http://crbug.com/366510.
+IN_PROC_BROWSER_TEST_F(BrowserTest, SadTabCancelsSubframeDialogs) {
+  // Navigate to an iframe that opens an alert dialog.
+  WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  contents->GetMainFrame()->ExecuteJavaScript(
+      ASCIIToUTF16("window.location.href = 'data:text/html,"
+                   "<iframe srcdoc=\"<script>alert(1)</script>\">'"));
+  AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog();
+  EXPECT_TRUE(alert->IsValid());
+  AppModalDialogQueue* dialog_queue = AppModalDialogQueue::GetInstance();
+  EXPECT_TRUE(dialog_queue->HasActiveDialog());
+
+  // Crash the renderer process and ensure the dialog is gone.
+  content::RenderProcessHost* child_process = contents->GetRenderProcessHost();
+  content::RenderProcessHostWatcher crash_observer(
+      child_process,
+      content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  base::KillProcess(child_process->GetHandle(), 0, false);
+  crash_observer.Wait();
+  EXPECT_FALSE(dialog_queue->HasActiveDialog());
+
+  // Make sure subsequent navigations work.
+  GURL url2("data:text/html,foo");
+  ui_test_utils::NavigateToURL(browser(), url2);
+}
+
 // Test for crbug.com/22004.  Reloading a page with a before unload handler and
 // then canceling the dialog should not leave the throbber spinning.
 IN_PROC_BROWSER_TEST_F(BrowserTest, ReloadThenCancelBeforeUnload) {

content/browser/renderer_host/render_view_host_impl.cc

@@ -1166,7 +1166,9 @@ void RenderViewHostImpl::OnRenderProcessGone(int status, int exit_code) {
   render_view_termination_status_ =
       static_cast<base::TerminationStatus>(status);
 
-  // Reset frame tree state associated with this process.
+  // Reset frame tree state associated with this process.  This must happen
+  // before RenderViewTerminated because observers expect the subframes of any
+  // affected frames to be cleared first.
   delegate_->GetFrameTree()->RenderProcessGone(this);
 
   // Our base class RenderWidgetHost needs to reset some stuff.

content/browser/web_contents/web_contents_impl.cc

@@ -3089,7 +3089,8 @@ void WebContentsImpl::RunJavaScriptMessage(
         default_prompt,
         base::Bind(&WebContentsImpl::OnDialogClosed,
                    base::Unretained(this),
-                   rfh,
+                   rfh->GetProcess()->GetID(),
+                   rfh->GetRoutingID(),
                    reply_msg,
                    false),
         &suppress_this_message);
@@ -3098,7 +3099,8 @@ void WebContentsImpl::RunJavaScriptMessage(
   if (suppress_this_message) {
     // If we are suppressing messages, just reply as if the user immediately
     // pressed "Cancel", passing true to |dialog_was_suppressed|.
-    OnDialogClosed(rfh, reply_msg, true, false, base::string16());
+    OnDialogClosed(rfh->GetProcess()->GetID(), rfh->GetRoutingID(), reply_msg,
+                   true, false, base::string16());
   }
 
   // OnDialogClosed (two lines up) may have caused deletion of this object (see
@@ -3131,7 +3133,8 @@ void WebContentsImpl::RunBeforeUnloadConfirm(
   dialog_manager_->RunBeforeUnloadDialog(
       this, message, is_reload,
       base::Bind(&WebContentsImpl::OnDialogClosed, base::Unretained(this),
-                 rfh, reply_msg, false));
+                 rfh->GetProcess()->GetID(), rfh->GetRoutingID(), reply_msg,
+                 false));
 }
 
 WebContents* WebContentsImpl::GetAsWebContents() {
@@ -3807,17 +3810,21 @@ bool WebContentsImpl::CreateRenderViewForInitialEmptyDocument() {
 }
 #endif
 
-void WebContentsImpl::OnDialogClosed(RenderFrameHost* rfh,
+void WebContentsImpl::OnDialogClosed(int render_process_id,
+                                     int render_frame_id,
                                      IPC::Message* reply_msg,
                                      bool dialog_was_suppressed,
                                      bool success,
                                      const base::string16& user_input) {
+  RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(render_process_id,
+                                                         render_frame_id);
   last_dialog_suppressed_ = dialog_was_suppressed;
 
   if (is_showing_before_unload_dialog_ && !success) {
     // If a beforeunload dialog is canceled, we need to stop the throbber from
     // spinning, since we forced it to start spinning in Navigate.
-    DidStopLoading(rfh);
+    if (rfh)
+      DidStopLoading(rfh);
     controller_.DiscardNonCommittedEntries();
 
     FOR_EACH_OBSERVER(WebContentsObserver, observers_,
@@ -3825,8 +3832,10 @@ void WebContentsImpl::OnDialogClosed(RenderFrameHost* rfh,
   }
 
   is_showing_before_unload_dialog_ = false;
-  static_cast<RenderFrameHostImpl*>(rfh)->JavaScriptDialogClosed(
-      reply_msg, success, user_input, dialog_was_suppressed);
+  if (rfh) {
+    rfh->JavaScriptDialogClosed(reply_msg, success, user_input,
+                                dialog_was_suppressed);
+  }
 }
 
 void WebContentsImpl::SetEncoding(const std::string& encoding) {

content/browser/web_contents/web_contents_impl.h

@@ -670,8 +670,11 @@ class CONTENT_EXPORT WebContentsImpl
   // watching |web_contents|. No-op if there is no such observer.
   void RemoveDestructionObserver(WebContentsImpl* web_contents);
 
-  // Callback function when showing JavaScript dialogs.
-  void OnDialogClosed(RenderFrameHost* rfh,
+  // Callback function when showing JavaScript dialogs.  Takes in a routing ID
+  // pair to identify the RenderFrameHost that opened the dialog, because it's
+  // possible for the RenderFrameHost to be deleted by the time this is called.
+  void OnDialogClosed(int render_process_id,
+                      int render_frame_id,
                       IPC::Message* reply_msg,
                       bool dialog_was_suppressed,
                       bool success,