Don't wait to close tabs waiting for JavaScript dialogs.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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 "content/browser/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 2370 matching lines @@
     // (if there was a cross-site "close" request pending when the user clicked
     // the close button). We want to keep the "for cross site" flag only if
     // both the old and the new ones are also for cross site.
     unload_ack_is_for_navigation_ =
         unload_ack_is_for_navigation_ && for_navigation;
   } else {
     // Start the hang monitor in case the renderer hangs in the beforeunload
     // handler.
     is_waiting_for_beforeunload_ack_ = true;
     unload_ack_is_for_navigation_ = for_navigation;
-    // Increment the in-flight event count, to ensure that input events won't
-    // cancel the timeout timer.
-    render_view_host_->GetWidget()->increment_in_flight_event_count();
-    render_view_host_->GetWidget()->StartHangMonitorTimeout(
-        TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS),
-        RenderWidgetHostDelegate::RENDERER_UNRESPONSIVE_BEFORE_UNLOAD);
-    send_before_unload_start_time_ = base::TimeTicks::Now();
-    Send(new FrameMsg_BeforeUnload(routing_id_, is_reload));
+    if (delegate_->IsJavaScriptDialogShowing()) {
+      // If there is a JavaScript dialog up, don't bother sending the renderer
+      // the unload event because it is known unresponsive, waiting for the
+      // reply from the dialog. Instead, force the page to be in "waiting for
+      // unload ack", and tell the widget's delegate that the renderer is
+      // unresponsive. That will let the page close immediately.
+      render_view_host_->is_waiting_for_close_ack_ = true;
+      render_view_host_->GetWidget()->delegate()->RendererUnresponsive(
+          render_view_host_->GetWidget(),
+          RenderWidgetHostDelegate::RENDERER_UNRESPONSIVE_DIALOG_SHOWING);

[Charlie Reis, 2016/10/07 22:44:23]

Could we call OnBeforeUnloadACK(true, ...) here instead?  

I'm not sure it's correct to call RendererUnresponsive.  I see that you would
end up hitting the Close() block in there (by setting is_waiting_for_close_ack_
to true), but that only seems to make sense in the close case.  We can get here
for navigations as well, right?

(I would also expect a similar check in RVH?)

[Avi (use Gerrit), 2016/10/08 00:13:19]

I was hoping to run past both issues with one call, but you're right that I
should go more slowly. BTW that call is titled
"RenderFrameHostImpl::SimulateBeforeUnloadAck()" and is called by the WC in
RendererUnresponsive.
Yes. D'oh. Sigh.
If we don't skip to always closing and mess up nav, yes.

+    } else {
+      // Increment the in-flight event count, to ensure that input events won't
+      // cancel the timeout timer.
+      render_view_host_->GetWidget()->increment_in_flight_event_count();
+      render_view_host_->GetWidget()->StartHangMonitorTimeout(
+          TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS),
+          RenderWidgetHostDelegate::RENDERER_UNRESPONSIVE_BEFORE_UNLOAD);
+      send_before_unload_start_time_ = base::TimeTicks::Now();
+      Send(new FrameMsg_BeforeUnload(routing_id_, is_reload));
+    }
   }
 }
 
 void RenderFrameHostImpl::SimulateBeforeUnloadAck() {
   DCHECK(is_waiting_for_beforeunload_ack_);
   base::TimeTicks approx_renderer_start_time = send_before_unload_start_time_;
   OnBeforeUnloadACK(true, approx_renderer_start_time, base::TimeTicks::Now());
 }
 
 bool RenderFrameHostImpl::ShouldDispatchBeforeUnload() {
@@ skipping 705 matching lines @@
   web_bluetooth_service_->SetClientConnectionErrorHandler(base::Bind(
       &RenderFrameHostImpl::DeleteWebBluetoothService, base::Unretained(this)));
   return web_bluetooth_service_.get();
 }
 
 void RenderFrameHostImpl::DeleteWebBluetoothService() {
   web_bluetooth_service_.reset();
 }
 
 }  // namespace content