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 a51fa198d876c3bb60fac511c9fd277f022276a7..8902988114763feedb1b1a9115c685fee5856dd9 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -182,6 +182,8 @@ RenderViewHostImpl::RenderViewHostImpl(
       run_modal_opener_id_(MSG_ROUTING_NONE),
       is_waiting_for_beforeunload_ack_(false),
       is_waiting_for_unload_ack_(false),
+      should_be_preserved_on_swap_out_(false),
+      need_to_perform_clean_up_on_swapped_out_(false),
       has_timed_out_on_unload_(false),
       unload_ack_is_for_cross_site_transition_(false),
       are_javascript_messages_suppressed_(false),
@@ -698,35 +700,60 @@ 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.
+  // Start a timer to kill the renderer if the unload event takes too long to
+  // execute in the background.
+  // http://crbug.com/323528.
   increment_in_flight_event_count();
   StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
 
+  // If the RenderViewHost doesn't have a live renderer, the unload event is
+  // skipped. Otherwise, the RenderViewHost will remain alive until the unload
+  // event has finished executing in the background (or timed out).
+  bool preserve_render_view_host = false;
+  bool timed_out = true;
   if (IsRenderViewLive()) {
     Send(new ViewMsg_SwapOut(GetRoutingID()));
-  } else {
-    // This RenderViewHost doesn't have a live renderer, so just skip the unload
-    // event.
-    OnSwappedOut(true);
+    preserve_render_view_host = true;
+    timed_out = false;
   }
+  OnSwappedOut(timed_out, preserve_render_view_host);
 }
 
 void RenderViewHostImpl::OnSwapOutACK() {
-  OnSwappedOut(false);
+  OnSwappedOut(false, false);
 }
 
-void RenderViewHostImpl::OnSwappedOut(bool timed_out) {
-  // Stop the hang monitor now that the unload handler has finished.
-  decrement_in_flight_event_count();
-  StopHangMonitorTimeout();
-  is_waiting_for_unload_ack_ = false;
-  has_timed_out_on_unload_ = timed_out;
+void RenderViewHostImpl::OnSwappedOut(bool timed_out,
+                                      bool preserve_render_view_host) {
+
+  should_be_preserved_on_swap_out_ = preserve_render_view_host;
+  if (!preserve_render_view_host) {
+    // Stop the hang monitor now that the unload handler has finished.
+    decrement_in_flight_event_count();
+    StopHangMonitorTimeout();
+    is_waiting_for_unload_ack_ = false;
+    has_timed_out_on_unload_ = timed_out;
+  }
   delegate_->SwappedOut(this);
 }
 
 void RenderViewHostImpl::WasSwappedOut() {
+  // Now that we're no longer the active RVH in the tab, start filtering out
+  // most IPC messages.  Usually the renderer will have stopped sending
+  // messages as of OnSwapOutACK.  However, we may have timed out waiting
+  // for that message, and additional IPC messages may keep streaming in.
+  // We filter them out, as long as that won't cause problems (e.g., we
+  // still allow synchronous messages through).
+  SetSwappedOut(true);
+
+  // The RenderView should not exit yet. It still has to execute the unload

[Charlie Reis, 2013/12/09 20:12:29]

This doesn't make sense to me.  The renderer won't try to process the
WasSwappedOut message until it finishes processing the SwapOut message anyway. 
There's no reason to avoid sending this.

+  // event.
+  if (should_be_preserved_on_swap_out_) {
+    need_to_perform_clean_up_on_swapped_out_ = true;
+    return;
+  }
+  need_to_perform_clean_up_on_swapped_out_ = false;

[pasko, 2013/12/10 19:59:29]

when is it the case that (should_be_preserved_on_swap_out_ !=
need_to_perform_clean_up_on_swapped_out_)?

+
   // Don't bother reporting hung state anymore.
   StopHangMonitorTimeout();
 
@@ -736,14 +763,6 @@ void RenderViewHostImpl::WasSwappedOut() {
   // unresponsive renderer timeout will catch it.
   bool hung = has_timed_out_on_unload_;
 
-  // Now that we're no longer the active RVH in the tab, start filtering out
-  // most IPC messages.  Usually the renderer will have stopped sending
-  // messages as of OnSwapOutACK.  However, we may have timed out waiting
-  // for that message, and additional IPC messages may keep streaming in.
-  // We filter them out, as long as that won't cause problems (e.g., we
-  // still allow synchronous messages through).
-  SetSwappedOut(true);
-
   // If we are not running the renderer in process and no other tab is using
   // the hung process, consider it eligible to be killed, assuming it is a real
   // process (unit tests don't have real processes).