Move modal dialogs from WebViewClient to WebFrameClient, part 1/3.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 868b9dd5c1379e1e80b0d388ebe08e6a309ee0d7..42d013351450a444cc5096e41693a953bf42c7c7 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -69,7 +69,8 @@ RenderFrameHostImpl::RenderFrameHostImpl(
       frame_tree_(frame_tree),
       frame_tree_node_(frame_tree_node),
       routing_id_(routing_id),
-      is_swapped_out_(is_swapped_out) {
+      is_swapped_out_(is_swapped_out),
+      are_javascript_messages_suppressed_(false) {
   frame_tree_->RegisterRenderFrameHost(this);
   GetProcess()->AddRoute(routing_id_, this);
   g_routing_id_frame_map.Get().insert(std::make_pair(
@@ -326,6 +327,10 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
     IPC_MESSAGE_HANDLER(FrameHostMsg_ContextMenu, OnContextMenu)
     IPC_MESSAGE_HANDLER(FrameHostMsg_JavaScriptExecuteResponse,
                         OnJavaScriptExecuteResponse)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(FrameHostMsg_RunJavaScriptMessage,
+                                    OnRunJavaScriptMessage)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(FrameHostMsg_RunBeforeUnloadConfirm,
+                                    OnRunBeforeUnloadConfirm)
   IPC_END_MESSAGE_MAP_EX()
 
   if (!msg_is_ok) {
@@ -647,6 +652,33 @@ void RenderFrameHostImpl::OnJavaScriptExecuteResponse(
   }
 }
 
+void RenderFrameHostImpl::OnRunJavaScriptMessage(
+    const base::string16& message,
+    const base::string16& default_prompt,
+    const GURL& frame_url,
+    JavaScriptMessageType type,
+    IPC::Message* reply_msg) {
+  // While a JS message dialog is showing, tabs in the same process shouldn't
+  // process input events.
+  GetProcess()->SetIgnoreInputEvents(true);
+  render_view_host_->StopHangMonitorTimeout();
+  delegate_->RunJavaScriptMessage(this, message, default_prompt,
+                                  frame_url, type, reply_msg,
+                                  AreJavaScriptMessagesSuppressed());
+}
+
+void RenderFrameHostImpl::OnRunBeforeUnloadConfirm(
+    const GURL& frame_url,
+    const base::string16& message,
+    bool is_reload,
+    IPC::Message* reply_msg) {
+  // While a JS before unload dialog is showing, tabs in the same process
+  // shouldn't process input events.
+  GetProcess()->SetIgnoreInputEvents(true);
+  render_view_host_->StopHangMonitorTimeout();
+  delegate_->RunBeforeUnloadConfirm(this, message, is_reload, reply_msg);
+}
+
 void RenderFrameHostImpl::SetPendingShutdown(const base::Closure& on_swap_out) {
   render_view_host_->SetPendingShutdown(on_swap_out);
 }
@@ -660,6 +692,11 @@ bool RenderFrameHostImpl::CanCommitURL(const GURL& url) {
   return GetContentClient()->browser()->CanCommitURL(GetProcess(), url);
 }
 
+bool* RenderFrameHostImpl::AreJavaScriptMessagesSuppressed() {
+  RenderFrameHostImpl* main_frame = frame_tree_->GetMainFrame();
+  return &main_frame->are_javascript_messages_suppressed_;

[jam, 2014/04/05 00:21:38]

why keep this in RFH at all instead of in the delegate then?

I ask because i worry that we will have per-page state on RFH if we
differentiate between mainframe and subframes, and it'll start being confusing
about whether we should put state/methods on WebContents or RFH as a result. why
not keep RFH dumb?

[Avi (use Gerrit), 2014/04/07 18:03:46]

This was added in https://codereview.chromium.org/9477 to save the user. The
scenario is:

1) Web page puts up a zillion alerts.
2) User suppresses them.
3) User navigates.
4) OnBeforeUnload fires "do you want to leave", but is suppressed.
5) Navigation is cancelled and the user is screwed.

RFH needs to know the suppression state so it can prevent this scenario.

[jam, 2014/04/07 18:11:36]

What I meant is why keep state on a main RFH, instead of the delegate (i.e.
WebContents). The question is for per-page state like this, do we want to keep
it on the main RFH, or in a central place? I'm worried about having data members
in RFH that are only used if the object is a main frame, and having RFHs special
case operations to their main RFH. It seems simpler if RFHs are dumb to the fact
that they're a main or sub frame, and instead ask their delegate for per-page
state.

[Avi (use Gerrit), 2014/04/07 19:53:55]

Gotcha.

+}
+
 void RenderFrameHostImpl::Navigate(const FrameMsg_Navigate_Params& params) {
   TRACE_EVENT0("frame_host", "RenderFrameHostImpl::Navigate");
   // Browser plugin guests are not allowed to navigate outside web-safe schemes,
@@ -731,4 +768,39 @@ void RenderFrameHostImpl::ExtendSelectionAndDelete(size_t before,
   Send(new FrameMsg_ExtendSelectionAndDelete(routing_id_, before, after));
 }
 
+void RenderFrameHostImpl::JavaScriptDialogClosed(
+    IPC::Message* reply_msg,
+    bool success,
+    const base::string16& user_input) {
+  GetProcess()->SetIgnoreInputEvents(false);
+  bool is_waiting = render_view_host_->is_waiting_for_beforeunload_ack() ||
+                    render_view_host_->IsWaitingForUnloadACK();
+
+  // If we are executing as part of (before)unload event handling, we don't
+  // want to use the regular hung_renderer_delay_ms_ if the user has agreed to
+  // leave the current page. In this case, use the regular timeout value used
+  // during the (before)unload handling.
+  if (is_waiting) {
+    render_view_host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(
+        success ? RenderViewHostImpl::kUnloadTimeoutMS
+                : render_view_host_->hung_renderer_delay_ms_));
+  }
+
+  FrameHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg,
+                                                      success, user_input);
+  Send(reply_msg);
+
+  // If we are waiting for an unload or beforeunload ack and the user has
+  // suppressed messages, kill the tab immediately; a page that's spamming
+  // alerts in onbeforeunload is presumably malicious, so there's no point in
+  // continuing to run its script and dragging out the process.
+  // This must be done after sending the reply since RenderView can't close
+  // correctly while waiting for a response.
+  if (is_waiting && *AreJavaScriptMessagesSuppressed())
+    render_view_host_->delegate_->RendererUnresponsive(
+        render_view_host_,
+        render_view_host_->is_waiting_for_beforeunload_ack(),
+        render_view_host_->IsWaitingForUnloadACK());
+}
+
 }  // namespace content