summaryrefslogtreecommitdiffstats
path: root/content/browser/renderer_host
diff options
context:
space:
mode:
authornasko@chromium.org <nasko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-16 15:58:27 +0000
committernasko@chromium.org <nasko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-16 15:58:27 +0000
commit3cf9dee4caf9be8e385ee4d6e97041b297e57c09 (patch)
treee206709c1e685503bb0598dc857d887047eace1f /content/browser/renderer_host
parent770005b3ba678f0b4b4c4cdf3aa61df8e647b154 (diff)
downloadchromium_src-3cf9dee4caf9be8e385ee4d6e97041b297e57c09.zip
chromium_src-3cf9dee4caf9be8e385ee4d6e97041b297e57c09.tar.gz
chromium_src-3cf9dee4caf9be8e385ee4d6e97041b297e57c09.tar.bz2
Fixing a problem, where a hung renderer process is not killed when navigating away
BUG=104346 TEST=Steps to reproduce listed in the bug. Review URL: http://codereview.chromium.org/10065028 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132407 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content/browser/renderer_host')
-rw-r--r--content/browser/renderer_host/render_process_host_impl.cc4
-rw-r--r--content/browser/renderer_host/render_process_host_impl.h4
-rw-r--r--content/browser/renderer_host/render_view_host_browsertest.cc141
-rw-r--r--content/browser/renderer_host/render_view_host_impl.cc76
-rw-r--r--content/browser/renderer_host/render_view_host_impl.h8
-rw-r--r--content/browser/renderer_host/render_widget_helper.cc8
-rw-r--r--content/browser/renderer_host/render_widget_helper.h7
-rw-r--r--content/browser/renderer_host/render_widget_host_impl.cc12
-rw-r--r--content/browser/renderer_host/render_widget_host_impl.h5
-rw-r--r--content/browser/renderer_host/render_widget_host_unittest.cc17
-rw-r--r--content/browser/renderer_host/resource_dispatcher_host_impl.cc21
-rw-r--r--content/browser/renderer_host/resource_dispatcher_host_impl.h9
-rw-r--r--content/browser/renderer_host/test_render_view_host.cc2
13 files changed, 290 insertions, 24 deletions
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 29724ec..fa61be4 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -553,9 +553,9 @@ void RenderProcessHostImpl::CancelResourceRequests(int render_widget_id) {
widget_helper_->CancelResourceRequests(render_widget_id);
}
-void RenderProcessHostImpl::CrossSiteSwapOutACK(
+void RenderProcessHostImpl::SimulateSwapOutACK(
const ViewMsg_SwapOut_Params& params) {
- widget_helper_->CrossSiteSwapOutACK(params);
+ widget_helper_->SimulateSwapOutACK(params);
}
bool RenderProcessHostImpl::WaitForUpdateMsg(
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index b1986e7..09e861d 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -61,8 +61,8 @@ class CONTENT_EXPORT RenderProcessHostImpl
virtual bool Init() OVERRIDE;
virtual int GetNextRoutingID() OVERRIDE;
virtual void CancelResourceRequests(int render_widget_id) OVERRIDE;
- virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params)
- OVERRIDE;
+ virtual void SimulateSwapOutACK(
+ const ViewMsg_SwapOut_Params& params) OVERRIDE;
virtual bool WaitForUpdateMsg(int render_widget_id,
const base::TimeDelta& max_delay,
IPC::Message* msg) OVERRIDE;
diff --git a/content/browser/renderer_host/render_view_host_browsertest.cc b/content/browser/renderer_host/render_view_host_browsertest.cc
index 71846f9..d516f43 100644
--- a/content/browser/renderer_host/render_view_host_browsertest.cc
+++ b/content/browser/renderer_host/render_view_host_browsertest.cc
@@ -11,6 +11,7 @@
#include "content/browser/renderer_host/render_view_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/view_messages.h"
+#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents_observer.h"
#include "net/base/host_port_pair.h"
@@ -228,3 +229,143 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostTest, BaseURLParam) {
ui_test_utils::NavigateToURL(browser(), test_url);
EXPECT_EQ("http://www.google.com/", observer.base_url().spec());
}
+
+// Test that a hung renderer is killed after navigating away during cross-site
+// navigation.
+IN_PROC_BROWSER_TEST_F(RenderViewHostTest, UnresponsiveCrossSiteNavigation) {
+ WebContents* tab = NULL;
+ WebContents* tab2 = NULL;
+ content::RenderProcessHost* rph = NULL;
+ base::ProcessHandle process;
+ FilePath doc_root;
+
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("content"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("test"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("data"));
+
+ // Start two servers to enable cross-site navigations.
+ net::TestServer server(net::TestServer::TYPE_HTTP,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(server.Start());
+ net::TestServer https_server(net::TestServer::TYPE_HTTPS,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(https_server.Start());
+
+ GURL infinite_beforeunload_url(
+ server.GetURL("files/infinite_beforeunload.html"));
+ GURL infinite_unload_url(server.GetURL("files/infinite_unload.html"));
+ GURL same_process_url(server.GetURL("files/english_page.html"));
+ GURL new_process_url(https_server.GetURL("files/english_page.html"));
+
+ // Navigate the tab to the page which will lock up the process when we
+ // navigate away from it.
+ ui_test_utils::NavigateToURL(browser(), infinite_beforeunload_url);
+ tab = browser()->GetWebContentsAt(0);
+ rph = tab->GetRenderProcessHost();
+ EXPECT_EQ(tab->GetURL(), infinite_beforeunload_url);
+
+ // Remember the process prior to navigation, as we expect it to get killed.
+ process = rph->GetHandle();
+ ASSERT_TRUE(process);
+
+ {
+ ui_test_utils::WindowedNotificationObserver process_exit_observer(
+ content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+ content::NotificationService::AllSources());
+ ui_test_utils::WindowedNotificationObserver process_hang_observer(
+ content::NOTIFICATION_RENDERER_PROCESS_HANG,
+ content::NotificationService::AllSources());
+ ui_test_utils::NavigateToURL(browser(), new_process_url);
+ process_hang_observer.Wait();
+ process_exit_observer.Wait();
+
+ // Since the process is killed during the navigation, we don't expect the
+ // renderer host to have a connection to it.
+ EXPECT_FALSE(rph->HasConnection());
+ }
+
+ ui_test_utils::NavigateToURL(browser(), same_process_url);
+ tab = browser()->GetWebContentsAt(0);
+ rph = tab->GetRenderProcessHost();
+ ASSERT_TRUE(tab != NULL);
+ EXPECT_EQ(tab->GetURL(), same_process_url);
+
+ // Now, let's open another tab with the unresponsive page, which will cause
+ // the previous page and the unresponsive one to use the same process.
+ ui_test_utils::NavigateToURLWithDisposition(browser(),
+ infinite_unload_url, NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+ EXPECT_EQ(browser()->tab_count(), 2);
+ tab2 = browser()->GetWebContentsAt(1);
+ ASSERT_TRUE(tab2 != NULL);
+ EXPECT_EQ(tab2->GetURL(), infinite_unload_url);
+ EXPECT_EQ(rph, tab2->GetRenderProcessHost());
+
+ process = rph->GetHandle();
+ ASSERT_TRUE(process);
+
+ // Navigating to the cross site URL will not kill the process, since it will
+ // have more than one tab using it. Kill it to confirm that it is still there,
+ // as well as finish the test faster.
+ {
+ ui_test_utils::WindowedNotificationObserver process_exit_observer(
+ content::NOTIFICATION_RENDERER_PROCESS_HANG,
+ content::NotificationService::AllSources());
+ ui_test_utils::NavigateToURL(browser(), new_process_url);
+ process_exit_observer.Wait();
+ }
+
+ EXPECT_TRUE(base::KillProcess(process, 2, false));
+}
+
+// Test that a hung renderer is killed when we are closing the page.
+IN_PROC_BROWSER_TEST_F(RenderViewHostTest, UnresponsiveClosePage) {
+ WebContents* tab = NULL;
+ FilePath doc_root;
+
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("content"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("test"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("data"));
+
+ net::TestServer server(net::TestServer::TYPE_HTTP,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(server.Start());
+ net::TestServer https_server(net::TestServer::TYPE_HTTPS,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(https_server.Start());
+
+ GURL infinite_beforeunload_url(
+ server.GetURL("files/infinite_beforeunload.html"));
+ GURL infinite_unload_url(server.GetURL("files/infinite_unload.html"));
+ GURL new_process_url(https_server.GetURL("files/english_page.html"));
+
+ ui_test_utils::NavigateToURL(browser(), new_process_url);
+
+ // Navigate a tab to a page which will spin into an infinite loop in the
+ // beforeunload handler, tying up the process when we close the tab.
+ ui_test_utils::NavigateToURLWithDisposition(browser(),
+ infinite_beforeunload_url, NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+ tab = browser()->GetWebContentsAt(1);
+ {
+ ui_test_utils::WindowedNotificationObserver process_exit_observer(
+ content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+ content::NotificationService::AllSources());
+ browser()->CloseTabContents(tab);
+ process_exit_observer.Wait();
+ }
+
+ // Navigate a tab to a page which will spin into an infinite loop in the
+ // unload handler, tying up the process when we close the tab.
+ ui_test_utils::NavigateToURLWithDisposition(browser(),
+ infinite_unload_url, NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+ tab = browser()->GetWebContentsAt(1);
+ {
+ ui_test_utils::WindowedNotificationObserver process_exit_observer(
+ content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+ content::NotificationService::AllSources());
+ browser()->CloseTabContents(tab);
+ process_exit_observer.Wait();
+ }
+}
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 04de175..8c2d7db 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -13,6 +13,7 @@
#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"
@@ -147,6 +148,7 @@ RenderViewHostImpl::RenderViewHostImpl(SiteInstance* instance,
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),
@@ -402,6 +404,9 @@ void RenderViewHostImpl::FirePageBeforeUnload(bool for_cross_site_transition) {
// 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()));
@@ -414,6 +419,9 @@ void RenderViewHostImpl::SwapOut(int new_render_process_host_id,
// 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;
@@ -427,14 +435,16 @@ void RenderViewHostImpl::SwapOut(int new_render_process_host_id,
// 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()->CrossSiteSwapOutACK(params);
+ GetProcess()->SimulateSwapOutACK(params);
}
}
-void RenderViewHostImpl::OnSwapOutACK() {
+void RenderViewHostImpl::OnSwapOutACK(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;
delegate_->SwappedOut(this);
}
@@ -442,6 +452,12 @@ 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
@@ -450,6 +466,46 @@ void RenderViewHostImpl::WasSwappedOut() {
// 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 TabContents, as it is the
+ // only class that swaps out render view hosts on navigation.
+ DCHECK_EQ(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);
+ // 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()));
}
@@ -460,6 +516,11 @@ void RenderViewHostImpl::ClosePage() {
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(
@@ -639,8 +700,14 @@ void RenderViewHostImpl::JavaScriptDialogClosed(IPC::Message* reply_msg,
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(kUnloadTimeoutMS));
+ StartHangMonitorTimeout(TimeDelta::FromMilliseconds(
+ success ? kUnloadTimeoutMS : hung_renderer_delay_ms_));
ViewHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg,
success, user_input);
@@ -1328,6 +1395,7 @@ 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
@@ -1369,6 +1437,7 @@ void RenderViewHostImpl::OnMsgShouldCloseACK(
}
void RenderViewHostImpl::OnMsgClosePageACK() {
+ decrement_in_flight_event_count();
ClosePageIgnoringUnloadEvents();
}
@@ -1744,6 +1813,7 @@ void RenderViewHostImpl::SetSwappedOut(bool is_swapped_out) {
// 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() {
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index 05cd36c..9c8c279 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -284,8 +284,9 @@ class CONTENT_EXPORT RenderViewHostImpl
// of the parameters.
void SwapOut(int new_render_process_host_id, int new_request_id);
- // Called by ResourceDispatcherHost after the SwapOutACK is received.
- void OnSwapOutACK();
+ // Called by ResourceDispatcherHost after the SwapOutACK is received or the
+ // response times out.
+ void OnSwapOutACK(bool timed_out);
// Called to notify the renderer that it has been visibly swapped out and
// replaced by another RenderViewHost, after an earlier call to SwapOut.
@@ -583,6 +584,9 @@ class CONTENT_EXPORT RenderViewHostImpl
// is_waiting_for_beforeunload_ack_, unload_ack_is_for_cross_site_transition_.
bool is_waiting_for_unload_ack_;
+ // Set to true when waiting for ViewHostMsg_SwapOut_ACK has timed out.
+ bool has_timed_out_on_unload_;
+
// Valid only when is_waiting_for_beforeunload_ack_ or
// is_waiting_for_unload_ack_ is true. This tells us if the unload request
// is for closing the entire tab ( = false), or only this RenderViewHost in
diff --git a/content/browser/renderer_host/render_widget_helper.cc b/content/browser/renderer_host/render_widget_helper.cc
index a29babb..8639565 100644
--- a/content/browser/renderer_host/render_widget_helper.cc
+++ b/content/browser/renderer_host/render_widget_helper.cc
@@ -101,11 +101,11 @@ void RenderWidgetHelper::CancelResourceRequests(int render_widget_id) {
render_widget_id));
}
-void RenderWidgetHelper::CrossSiteSwapOutACK(
+void RenderWidgetHelper::SimulateSwapOutACK(
const ViewMsg_SwapOut_Params& params) {
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
- base::Bind(&RenderWidgetHelper::OnCrossSiteSwapOutACK,
+ base::Bind(&RenderWidgetHelper::OnSimulateSwapOutACK,
this,
params));
}
@@ -207,9 +207,9 @@ void RenderWidgetHelper::OnCancelResourceRequests(
render_process_id_, render_widget_id);
}
-void RenderWidgetHelper::OnCrossSiteSwapOutACK(
+void RenderWidgetHelper::OnSimulateSwapOutACK(
const ViewMsg_SwapOut_Params& params) {
- resource_dispatcher_host_->OnSwapOutACK(params);
+ resource_dispatcher_host_->OnSimulateSwapOutACK(params);
}
void RenderWidgetHelper::CreateNewWindow(
diff --git a/content/browser/renderer_host/render_widget_helper.h b/content/browser/renderer_host/render_widget_helper.h
index a830b62..a57f0e6 100644
--- a/content/browser/renderer_host/render_widget_helper.h
+++ b/content/browser/renderer_host/render_widget_helper.h
@@ -113,7 +113,7 @@ class RenderWidgetHelper
// corresponding functions in RenderProcessHost. See those declarations
// for documentation.
void CancelResourceRequests(int render_widget_id);
- void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params);
+ void SimulateSwapOutACK(const ViewMsg_SwapOut_Params& params);
bool WaitForUpdateMsg(int render_widget_id,
const base::TimeDelta& max_delay,
IPC::Message* msg);
@@ -190,8 +190,9 @@ class RenderWidgetHelper
// Called on the IO thread to cancel resource requests for the render widget.
void OnCancelResourceRequests(int render_widget_id);
- // Called on the IO thread to resume a cross-site response.
- void OnCrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params);
+ // Called on the IO thread to resume a cross-site response, if the ack is
+ // not received as expected.
+ void OnSimulateSwapOutACK(const ViewMsg_SwapOut_Params& params);
#if defined(OS_MACOSX)
// Called on destruction to release all allocated transport DIBs
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 3a526a3..d4779ae 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -650,9 +650,13 @@ void RenderWidgetHostImpl::StartHangMonitorTimeout(TimeDelta delay) {
return;
}
- // Set time_when_considered_hung_ if it's null.
+ // Set time_when_considered_hung_ if it's null. Otherwise, update
+ // time_when_considered_hung_ if the caller's request is sooner than the
+ // existing one. This will have the side effect that the existing timeout will
+ // be forgotten.
Time requested_end_time = Time::Now() + delay;
- if (time_when_considered_hung_.is_null())
+ if (time_when_considered_hung_.is_null() ||
+ time_when_considered_hung_ > requested_end_time)
time_when_considered_hung_ = requested_end_time;
// If we already have a timer with the same or shorter duration, then we can
@@ -871,7 +875,7 @@ void RenderWidgetHostImpl::ForwardInputEvent(const WebInputEvent& input_event,
// after this line.
next_mouse_move_.reset();
- in_flight_event_count_++;
+ increment_in_flight_event_count();
StartHangMonitorTimeout(
TimeDelta::FromMilliseconds(hung_renderer_delay_ms_));
}
@@ -1313,7 +1317,7 @@ void RenderWidgetHostImpl::OnMsgInputEventAck(WebInputEvent::Type event_type,
UMA_HISTOGRAM_TIMES("MPArch.RWH_InputEventDelta", delta);
// Cancel pending hung renderer checks since the renderer is responsive.
- if (--in_flight_event_count_ == 0)
+ if (decrement_in_flight_event_count() == 0)
StopHangMonitorTimeout();
int type = static_cast<int>(event_type);
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index adb71f5..2089b3c 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -422,6 +422,11 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost,
void SetShouldAutoResize(bool enable);
protected:
+ // Expose increment/decrement of the in-flight event count, so
+ // RenderViewHostImpl can account for in-flight beforeunload/unload events.
+ int increment_in_flight_event_count() { return ++in_flight_event_count_; }
+ int decrement_in_flight_event_count() { return --in_flight_event_count_; }
+
// The View associated with the RenderViewHost. The lifetime of this object
// is associated with the lifetime of the Render process. If the Renderer
// crashes, its View is destroyed and this pointer becomes NULL, even though
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index d4d7fdd..8f5c465 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -770,6 +770,23 @@ TEST_F(RenderWidgetHostTest, StopAndStartHangMonitorTimeout) {
EXPECT_TRUE(host_->unresponsive_timer_fired());
}
+// Test that the hang monitor timer expires properly if it is started, then
+// updated to a shorter duration.
+TEST_F(RenderWidgetHostTest, ShorterDelayHangMonitorTimeout) {
+ // Start with a timeout.
+ host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(100));
+
+ // Start it again with shorter delay.
+ EXPECT_FALSE(host_->unresponsive_timer_fired());
+ host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(20));
+
+ // Wait long enough for first timeout and see if it fired.
+ MessageLoop::current()->PostDelayedTask(
+ FROM_HERE, MessageLoop::QuitClosure(), TimeDelta::FromMilliseconds(25));
+ MessageLoop::current()->Run();
+ EXPECT_TRUE(host_->unresponsive_timer_fired());
+}
+
// Test that the hang monitor catches two input events but only one ack.
// This can happen if the second input event causes the renderer to hang.
// This test will catch a regression of crbug.com/111185.
diff --git a/content/browser/renderer_host/resource_dispatcher_host_impl.cc b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
index 4ddcbb6..fcb5893 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_impl.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
@@ -288,11 +288,13 @@ net::RequestPriority DetermineRequestPriority(ResourceType::Type type) {
}
}
-void OnSwapOutACKHelper(int render_process_id, int render_view_id) {
+void OnSwapOutACKHelper(int render_process_id,
+ int render_view_id,
+ bool timed_out) {
RenderViewHostImpl* rvh = RenderViewHostImpl::FromID(render_process_id,
render_view_id);
if (rvh)
- rvh->OnSwapOutACK();
+ rvh->OnSwapOutACK(timed_out);
}
net::Error CallbackAndReturn(
@@ -1083,8 +1085,20 @@ ResourceRequestInfoImpl* ResourceDispatcherHostImpl::CreateRequestInfo(
context);
}
+
void ResourceDispatcherHostImpl::OnSwapOutACK(
+ const ViewMsg_SwapOut_Params& params) {
+ HandleSwapOutACK(params, false);
+}
+
+void ResourceDispatcherHostImpl::OnSimulateSwapOutACK(
const ViewMsg_SwapOut_Params& params) {
+ // Call the real implementation with true, which means that we timed out.
+ HandleSwapOutACK(params, true);
+}
+
+void ResourceDispatcherHostImpl::HandleSwapOutACK(
+ const ViewMsg_SwapOut_Params& params, bool timed_out) {
// Closes for cross-site transitions are handled such that the cross-site
// transition continues.
GlobalRequestID global_id(params.new_render_process_host_id,
@@ -1103,7 +1117,8 @@ void ResourceDispatcherHostImpl::OnSwapOutACK(
FROM_HERE,
base::Bind(&OnSwapOutACKHelper,
params.closing_process_id,
- params.closing_route_id));
+ params.closing_route_id,
+ timed_out));
}
void ResourceDispatcherHostImpl::OnDidLoadResourceFromMemoryCache(
diff --git a/content/browser/renderer_host/resource_dispatcher_host_impl.h b/content/browser/renderer_host/resource_dispatcher_host_impl.h
index d4b1c3c..39620ea 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_impl.h
+++ b/content/browser/renderer_host/resource_dispatcher_host_impl.h
@@ -164,6 +164,11 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
// Called when the unload handler for a cross-site request has finished.
void OnSwapOutACK(const ViewMsg_SwapOut_Params& params);
+ // Called when we want to simulate the renderer process sending
+ // ViewHostMsg_SwapOut_ACK in cases where the renderer has died or is
+ // unresponsive.
+ void OnSimulateSwapOutACK(const ViewMsg_SwapOut_Params& params);
+
// Called when the renderer loads a resource from its internal cache.
void OnDidLoadResourceFromMemoryCache(const GURL& url,
const std::string& security_info,
@@ -276,6 +281,10 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
// A shutdown helper that runs on the IO thread.
void OnShutdown();
+ // The real implementation of the OnSwapOutACK logic. Public methods call
+ // this method with the proper value for the timed_out parameter.
+ void HandleSwapOutACK(const ViewMsg_SwapOut_Params& params, bool timed_out);
+
void StartRequest(net::URLRequest* request);
// Returns true if the request is paused.
diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc
index d4ccbb3..735ec82 100644
--- a/content/browser/renderer_host/test_render_view_host.cc
+++ b/content/browser/renderer_host/test_render_view_host.cc
@@ -281,7 +281,7 @@ void TestRenderViewHost::SetContentsMimeType(const std::string& mime_type) {
}
void TestRenderViewHost::SimulateSwapOutACK() {
- OnSwapOutACK();
+ OnSwapOutACK(false);
}
void TestRenderViewHost::SimulateWasHidden() {