Fixing a problem, where a hung renderer process is not killed when navigating away

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 3edbcc9fb6ad251ba86e33ad69c477e26dd10334..8ef3c4283265f6f027d9216e4acc8ecd7e5969ee 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -41,6 +41,7 @@
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host_delegate.h"
 #include "content/public/browser/render_view_host_observer.h"
 #include "content/public/browser/user_metrics.h"
@@ -419,14 +420,17 @@ void RenderViewHostImpl::SwapOut(int new_render_process_host_id,
 
 void RenderViewHostImpl::OnSwapOutACK() {
   // Stop the hang monitor now that the unload handler has finished.
-  StopHangMonitorTimeout();
   is_waiting_for_unload_ack_ = false;
+  StopHangMonitorTimeout();
   delegate_->SwappedOut(this);
 }
 
 void RenderViewHostImpl::WasSwappedOut() {
-  // Don't bother reporting hung state anymore.
-  StopHangMonitorTimeout();
+  // If we are still waiting on the unload handler to be run, we consider
+  // the process hung and we should terminate it if there are no other tabs
+  // using the process. If there are other views using this process, the
+  // unresponsive renderer timeout will catch it.
+  bool hung = is_waiting_for_unload_ack_;
 
   // 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
@@ -436,6 +440,43 @@ void RenderViewHostImpl::WasSwappedOut() {
   // still allow synchronous messages through).
   SetSwappedOut(true);
 
+  // Don't bother reporting hung state anymore.
+  StopHangMonitorTimeout();
+
+  // If we are not running the renderer in process and no other tab is using
+  // the hung process, kill it, assuming it is a real process (unit tests don't
+  // have real processes).
+  if (hung) {
+    base::ProcessHandle process_handle = GetProcess()->GetHandle();
+    int views = 0;
+
+    // Count the number of listeners for the process, which is equivalent to
+    // views using the process as of this writing.
+    content::RenderProcessHost::RenderWidgetHostsIterator iter(
+        GetProcess()->GetRenderWidgetHostsIterator());
+    for (; !iter.IsAtEnd(); iter.Advance())
+      ++views;
+
+    if (!content::RenderProcessHost::run_renderer_in_process() &&
+        process_handle && views <= 1) {
+      // We expect the delegate for this RVH to be TabContents, as it is the
+      // only class that swaps out render view hosts on navigation.
+      DCHECK(delegate_->GetRenderViewType() == content::VIEW_TYPE_TAB_CONTENTS);
+
+      // Kill the process only if TabContents sets SuddenTerminationAllowed,
+      // which indicates that the timer has expired.
+      // This is not the case if we load data URLs or about:blank. The reason
+      // is that there is no network requests and this code is hit without
+      // setting the unresponsiveness timer. This allows a corner case where a
+      // navigation to a data URL will leave a process running, if the
+      // beforeunload handler completes fine, but the unload handler hangs.
+      // At this time, the complexity to solve this edge case is not worthwhile.
+      if (SuddenTerminationAllowed()) {
+        base::KillProcess(process_handle, content::RESULT_CODE_HUNG, false);
+      }
+    }
+  }
+
   // Inform the renderer that it can exit if no one else is using it.
   Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
 }
@@ -462,9 +503,9 @@ void RenderViewHostImpl::ClosePage() {
 }
 
 void RenderViewHostImpl::ClosePageIgnoringUnloadEvents() {
-  StopHangMonitorTimeout();
   is_waiting_for_beforeunload_ack_ = false;
   is_waiting_for_unload_ack_ = false;
+  StopHangMonitorTimeout();
 
   sudden_termination_allowed_ = true;
   delegate_->Close(this);
@@ -1329,14 +1370,16 @@ void RenderViewHostImpl::OnMsgShouldCloseACK(
     bool proceed,
     const base::TimeTicks& renderer_before_unload_start_time,
     const base::TimeTicks& renderer_before_unload_end_time) {
-  StopHangMonitorTimeout();
   // If this renderer navigated while the beforeunload request was in flight, we
   // may have cleared this state in OnMsgNavigate, in which case we can ignore
   // this message.
-  if (!is_waiting_for_beforeunload_ack_ || is_swapped_out_)
+  if (!is_waiting_for_beforeunload_ack_ || is_swapped_out_) {
+    StopHangMonitorTimeout();
     return;
+  }
 
   is_waiting_for_beforeunload_ack_ = false;
+  StopHangMonitorTimeout();
 
   RenderViewHostDelegate::RendererManagement* management_delegate =
       delegate_->GetRendererManagementDelegate();
@@ -1738,4 +1781,17 @@ void RenderViewHostImpl::ClearPowerSaveBlockers() {
   STLDeleteValues(&power_save_blockers_);
 }
 
+// During cross-site navigation, we have timeouts on beforeunload and unload
+// handler processing. If we are asked to stop that timeout monitoring while
+// we have an outstanding handler processing, we must not stop it. If we do,
+// then slow or unresponsive renderer process will hang the navigation as there
+// will be no way for us to detect it. Stopping of the timer is most frequently
+// caused by input events, which restart the hang detection timeout.
+void RenderViewHostImpl::StopHangMonitorTimeout() {
+  if (is_waiting_for_beforeunload_ack_ || is_waiting_for_unload_ack_)
+    return;
+
+  RenderWidgetHostImpl::StopHangMonitorTimeout();
+}
+
 }  // namespace content