diff options
author | nasko@chromium.org <nasko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-16 15:58:27 +0000 |
---|---|---|
committer | nasko@chromium.org <nasko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-16 15:58:27 +0000 |
commit | 3cf9dee4caf9be8e385ee4d6e97041b297e57c09 (patch) | |
tree | e206709c1e685503bb0598dc857d887047eace1f /content/browser/renderer_host | |
parent | 770005b3ba678f0b4b4c4cdf3aa61df8e647b154 (diff) | |
download | chromium_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')
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() { |