Have the unload event execute in background on cross-site navigations

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 70db572f395d4ff160a8fb0e1789a41a9dc71061..6b2185e0fb1b45115f42d49c46aa8507396beb66 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -102,6 +102,12 @@ namespace {
 // fire.
 const int kUnloadTimeoutMS = 1000;
 
+// Delay to wait on closing the WebContents for an unload/handler to fire when
+// swapping renderers. The old renderer will allways be sent to the background

[Charlie Reis, 2013/11/26 19:35:00]

nit: always

As Ojan wondered, have you checked whether a long-running unload handler
actually finishes running in the background, or do we kill the process right
away?

+// since the unload handler cannot show dialogues.
+// http://crbug.com/323528.
+const int kUnloadSwapTimeoutMS = 0;
+
 // Translate a WebKit text direction into a base::i18n one.
 base::i18n::TextDirection WebTextDirectionToChromeTextDirection(
     blink::WebTextDirection dir) {
@@ -694,14 +700,15 @@ void RenderViewHostImpl::SwapOut() {
   // This will be set back to false in OnSwapOutACK, just before we replace
   // this RVH with the pending RVH.
   is_waiting_for_unload_ack_ = true;
-  // Start the hang monitor in case the renderer hangs in the unload handler.
-  // Increment the in-flight event count, to ensure that input events won't
-  // cancel the timeout timer.
-  increment_in_flight_event_count();
-  StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
 
   if (IsRenderViewLive()) {
     Send(new ViewMsg_SwapOut(GetRoutingID()));
+    // The hang monitor will immediately time out, which will cause the old
+    // renderer to go execute the unload event in the background. The swap out
+    // can then proceed without any delay.
+    // http://crbug.com/323528.
+    increment_in_flight_event_count();
+    StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadSwapTimeoutMS));

[Charlie Reis, 2013/11/26 19:35:00]

Moving these calls inside the if seems like a problem.  For example, if the
render view isn't live, we'll still call decrement_in_flight_event_count() in
OnSwappedOut.

Also, using a timeout of 0 ms seems like it's keeping a lot of complexity around
unnecessarily.  It's great for the measurements to confirm that this will help,
but if we know we're going down this path, can we remove the use of the timer
instead?

   } else {
     // This RenderViewHost doesn't have a live renderer, so just skip the unload
     // event.