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

content/browser/renderer_host/render_view_host.cc

 // Copyright (c) 2012 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/renderer_host/render_view_host.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 23 matching lines @@
 #include "content/common/view_messages.h"
 #include "content/port/browser/render_widget_host_view_port.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_message_filter.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/dom_operation_notification_details.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #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"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "net/base/net_util.h"
@@ skipping 319 matching lines @@
   // Stop the hang monitor now that the unload handler has finished.
   StopHangMonitorTimeout();
   is_waiting_for_unload_ack_ = false;
   delegate_->SwappedOut(this);
 }
 
 void RenderViewHost::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
   // 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, kill it, assuming it is a real process (unit tests don't
+  // have real processes).
+  if (hung) {
+    base::ProcessHandle process_handle = process_->GetHandle();
+    if (!content::RenderProcessHost::run_renderer_in_process() &&
+        process_->AllowTerminateOnUnresponsive() && process_handle) {
+      // We expect the delegate for this RVH to be TabContents, as it is the
+      // only one aware of swapping render views on navigation.

[Charlie Reis, 2012/03/01 21:08:17]

More specifically, it is the only class that swaps out RenderViewHosts.  (I
believe prerendering and instant will swap them in, but this doesn't affect
that.)

[nasko, 2012/03/01 22:50:15]

Done.

+      DCHECK(delegate_->GetRenderViewType() == content::VIEW_TYPE_TAB_CONTENTS);
+
+      // If we load data URLs, such as about:blank, there will be no network
+      // request and we arrive here without timeout. Since the TabContents
+      // sets the sudden termination allowed on real timeout, respect it and
+      // don't kill the process. 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 is hung. At this time, the
+      // complexity to solve this edge case is not worthwhile.

[Charlie Reis, 2012/03/01 21:08:17]

I'm having trouble following this comment...  Not sure I have suggestions yet
for how to improve it, though.

+      if (SuddenTerminationAllowed()) {

[Charlie Reis, 2012/03/01 21:08:17]

nit: No braces on one-line if.

[nasko, 2012/03/01 22:50:15]

Done.

+        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(routing_id()));
 }
 
 void RenderViewHost::ClosePage() {
   // Start the hang monitor in case the renderer hangs in the unload handler.
   is_waiting_for_unload_ack_ = true;
   StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
 
   if (IsRenderViewLive()) {
@@ skipping 1199 matching lines @@
   // Whenever we change swap out state, we should not be waiting for
   // beforeunload or unload acks.  We clear them here to be safe, since they
   // can cause navigations to be ignored in OnMsgNavigate.
   is_waiting_for_beforeunload_ack_ = false;
   is_waiting_for_unload_ack_ = false;
 }
 
 void RenderViewHost::ClearPowerSaveBlockers() {
   STLDeleteValues(&power_save_blockers_);
 }