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

content/browser/renderer_host/render_view_host_impl.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_impl.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/i18n/rtl.h"
 #include "base/json/json_reader.h"
 #include "base/message_loop.h"
-#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #endif
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/cross_site_request_manager.h"
@@ skipping 115 matching lines @@
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
       guest_(false),
       pending_request_id_(-1),
       navigations_suspended_(false),
       suspended_nav_message_(NULL),
       is_swapped_out_(swapped_out),
       run_modal_reply_msg_(NULL),
       is_waiting_for_beforeunload_ack_(false),
       is_waiting_for_unload_ack_(false),
-      has_timed_out_on_unload_(false),
       unload_ack_is_for_cross_site_transition_(false),
       are_javascript_messages_suppressed_(false),
       sudden_termination_allowed_(false),
       session_storage_namespace_(
           static_cast<SessionStorageNamespaceImpl*>(session_storage)),
       save_accessibility_tree_for_testing_(false),
       send_accessibility_updated_notifications_(false),
       render_view_termination_status_(base::TERMINATION_STATUS_STILL_RUNNING) {
   if (!session_storage_namespace_) {
     DOMStorageContext* dom_storage_context =
@@ skipping 237 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_cross_site_transition_ =
         unload_ack_is_for_cross_site_transition_ && for_cross_site_transition;
   } else {
     // Start the hang monitor in case the renderer hangs in the beforeunload
     // handler.
     is_waiting_for_beforeunload_ack_ = true;
     unload_ack_is_for_cross_site_transition_ = for_cross_site_transition;
-    // 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));
     send_should_close_start_time_ = base::TimeTicks::Now();
     Send(new ViewMsg_ShouldClose(GetRoutingID()));
   }
 }
 
 void RenderViewHostImpl::SwapOut(int new_render_process_host_id,
                                  int new_request_id) {
   // 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));
 
   ViewMsg_SwapOut_Params params;
   params.closing_process_id = GetProcess()->GetID();
   params.closing_route_id = GetRoutingID();
   params.new_render_process_host_id = new_render_process_host_id;
   params.new_request_id = new_request_id;
   if (IsRenderViewLive()) {
     Send(new ViewMsg_SwapOut(GetRoutingID(), params));
   } else {
     // This RenderViewHost doesn't have a live renderer, so just skip the unload
     // event.  We must notify the ResourceDispatcherHost on the IO thread,
     // which we will do through the RenderProcessHost's widget helper.
-    GetProcess()->SimulateSwapOutACK(params);
+    GetProcess()->CrossSiteSwapOutACK(params);
   }
 }
 
-void RenderViewHostImpl::OnSwapOutACK(bool timed_out) {
+void RenderViewHostImpl::OnSwapOutACK() {
   // 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() {
   // Don't bother reporting hung state anymore.
   StopHangMonitorTimeout();
 
-  // If we have timed out on running the unload handler, 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 = 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, 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 widget hosts 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 WebContents, as it is the
-      // only class that swaps out render view hosts on navigation.
-      DCHECK_EQ(delegate_->GetRenderViewType(),
-                content::VIEW_TYPE_WEB_CONTENTS);
-
-      // Kill the process only if WebContents 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);
-        // Log a histogram point to help us diagnose how many of those kills
-        // we have performed. 1 is the enum value for RendererType Normal for
-        // the histogram.
-        UMA_HISTOGRAM_PERCENTAGE(
-            "BrowserRenderProcessHost.ChildKillsUnresponsive", 1);
-      }
-    }
-  }
-
   // Inform the renderer that it can exit if no one else is using it.
   Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
 }
 
 void RenderViewHostImpl::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()) {
-    // Since we are sending an IPC message to the renderer, increase the event
-    // count to prevent the hang monitor timeout from being stopped by input
-    // event acknowledgements.
-    increment_in_flight_event_count();
-
     // TODO(creis): Should this be moved to Shutdown?  It may not be called for
     // RenderViewHosts that have been swapped out.
     content::NotificationService::current()->Notify(
         content::NOTIFICATION_RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW,
         content::Source<RenderViewHost>(this),
         content::NotificationService::NoDetails());
 
     Send(new ViewMsg_ClosePage(GetRoutingID()));
   } else {
     // This RenderViewHost doesn't have a live renderer, so just skip the unload
@@ skipping 167 matching lines @@
   loop->Run();
   return observer.value()->DeepCopy();
 }
 
 void RenderViewHostImpl::JavaScriptDialogClosed(IPC::Message* reply_msg,
                                                 bool success,
                                                 const string16& user_input) {
   GetProcess()->SetIgnoreInputEvents(false);
   bool is_waiting =
       is_waiting_for_beforeunload_ack_ || is_waiting_for_unload_ack_;
-
-  // If we are executing as part of (before)unload event handling, we don't
-  // want to use the regular hung_renderer_delay_ms_ if the user has agreed to
-  // leave the current page. In this case, use the regular timeout value used
-  // during the (before)unload handling.
   if (is_waiting)
-    StartHangMonitorTimeout(TimeDelta::FromMilliseconds(
-        success ? kUnloadTimeoutMS : hung_renderer_delay_ms_));
+    StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
 
   ViewHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg,
                                                      success, user_input);
   Send(reply_msg);
 
   // If we are waiting for an unload or beforeunload ack and the user has
   // suppressed messages, kill the tab immediately; a page that's spamming
   // alerts in onbeforeunload is presumably malicious, so there's no point in
   // continuing to run its script and dragging out the process.
   // This must be done after sending the reply since RenderView can't close
@@ skipping 688 matching lines @@
 }
 
 void RenderViewHostImpl::OnUserGesture() {
   delegate_->OnUserGesture();
 }
 
 void RenderViewHostImpl::OnMsgShouldCloseACK(
     bool proceed,
     const base::TimeTicks& renderer_before_unload_start_time,
     const base::TimeTicks& renderer_before_unload_end_time) {
-  decrement_in_flight_event_count();
   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_)
     return;
 
   is_waiting_for_beforeunload_ack_ = false;
 
   RenderViewHostDelegate::RendererManagement* management_delegate =
@@ skipping 21 matching lines @@
         unload_ack_is_for_cross_site_transition_, proceed,
         before_unload_end_time);
   }
 
   // If canceled, notify the delegate to cancel its pending navigation entry.
   if (!proceed)
     delegate_->DidCancelLoading();
 }
 
 void RenderViewHostImpl::OnMsgClosePageACK() {
-  decrement_in_flight_event_count();
   ClosePageIgnoringUnloadEvents();
 }
 
 void RenderViewHostImpl::NotifyRendererUnresponsive() {
   delegate_->RendererUnresponsive(
       this, is_waiting_for_beforeunload_ack_ || is_waiting_for_unload_ack_);
 }
 
 void RenderViewHostImpl::NotifyRendererResponsive() {
   delegate_->RendererResponsive(this);
@@ skipping 351 matching lines @@
 }
 
 void RenderViewHostImpl::SetSwappedOut(bool is_swapped_out) {
   is_swapped_out_ = is_swapped_out;
 
   // 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;
-  has_timed_out_on_unload_ = false;
 }
 
 void RenderViewHostImpl::ClearPowerSaveBlockers() {
   STLDeleteValues(&power_save_blockers_);
 }
 
 }  // namespace content