Commit d20d7b3d authored by jdduke's avatar jdduke Committed by Commit bot

Prevent hang monitor restarts when hidden

When the render widget is hidden, the hang monitor timeout is explicitly
stopped. However, tasks executed *after* the widget hide signal may
trigger a restart of the hang monitor timeout, e.g., queued input
events. Prevent such restarts when the widget is hidden while
also ensuring the hang monitor restarts when the widget is shown again.

BUG=458594

Review URL: https://codereview.chromium.org/938123003

Cr-Commit-Position: refs/heads/master@{#319778}
parent 2bdff99f
...@@ -482,7 +482,7 @@ void BrowserPluginGuest::RenderViewReady() { ...@@ -482,7 +482,7 @@ void BrowserPluginGuest::RenderViewReady() {
Send(new InputMsg_SetFocus(routing_id(), focused_)); Send(new InputMsg_SetFocus(routing_id(), focused_));
UpdateVisibility(); UpdateVisibility();
RenderWidgetHostImpl::From(rvh)->set_hung_renderer_delay_ms( RenderWidgetHostImpl::From(rvh)->set_hung_renderer_delay(
base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)); base::TimeDelta::FromMilliseconds(kHungRendererDelayMs));
} }
......
...@@ -1678,9 +1678,10 @@ void RenderFrameHostImpl::JavaScriptDialogClosed( ...@@ -1678,9 +1678,10 @@ void RenderFrameHostImpl::JavaScriptDialogClosed(
// leave the current page. In this case, use the regular timeout value used // leave the current page. In this case, use the regular timeout value used
// during the (before)unload handling. // during the (before)unload handling.
if (is_waiting) { if (is_waiting) {
render_view_host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds( render_view_host_->StartHangMonitorTimeout(
success ? RenderViewHostImpl::kUnloadTimeoutMS success
: render_view_host_->hung_renderer_delay_ms_)); ? TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS)
: render_view_host_->hung_renderer_delay_);
} }
FrameHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg, FrameHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg,
......
...@@ -160,7 +160,8 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate, ...@@ -160,7 +160,8 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
bool hidden) bool hidden)
: view_(NULL), : view_(NULL),
renderer_initialized_(false), renderer_initialized_(false),
hung_renderer_delay_ms_(kHungRendererDelayMs), hung_renderer_delay_(
base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)),
delegate_(delegate), delegate_(delegate),
process_(process), process_(process),
routing_id_(routing_id), routing_id_(routing_id),
...@@ -537,6 +538,11 @@ void RenderWidgetHostImpl::WasShown(const ui::LatencyInfo& latency_info) { ...@@ -537,6 +538,11 @@ void RenderWidgetHostImpl::WasShown(const ui::LatencyInfo& latency_info) {
SendScreenRects(); SendScreenRects();
// When hidden, timeout monitoring for input events is disabled. Restore it
// now to ensure consistent hang detection.
if (in_flight_event_count_)
RestartHangMonitorTimeout();
// Always repaint on restore. // Always repaint on restore.
bool needs_repainting = true; bool needs_repainting = true;
needs_repainting_on_restore_ = false; needs_repainting_on_restore_ = false;
...@@ -863,8 +869,7 @@ void RenderWidgetHostImpl::StartHangMonitorTimeout(base::TimeDelta delay) { ...@@ -863,8 +869,7 @@ void RenderWidgetHostImpl::StartHangMonitorTimeout(base::TimeDelta delay) {
void RenderWidgetHostImpl::RestartHangMonitorTimeout() { void RenderWidgetHostImpl::RestartHangMonitorTimeout() {
if (hang_monitor_timeout_) if (hang_monitor_timeout_)
hang_monitor_timeout_->Restart( hang_monitor_timeout_->Restart(hung_renderer_delay_);
base::TimeDelta::FromMilliseconds(hung_renderer_delay_ms_));
} }
void RenderWidgetHostImpl::StopHangMonitorTimeout() { void RenderWidgetHostImpl::StopHangMonitorTimeout() {
...@@ -1210,6 +1215,7 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status, ...@@ -1210,6 +1215,7 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
// Reset this to ensure the hung renderer mechanism is working properly. // Reset this to ensure the hung renderer mechanism is working properly.
in_flight_event_count_ = 0; in_flight_event_count_ = 0;
StopHangMonitorTimeout();
if (view_) { if (view_) {
GpuSurfaceTracker::Get()->SetSurfaceHandle(surface_id_, GpuSurfaceTracker::Get()->SetSurfaceHandle(surface_id_,
...@@ -1793,9 +1799,9 @@ InputEventAckState RenderWidgetHostImpl::FilterInputEvent( ...@@ -1793,9 +1799,9 @@ InputEventAckState RenderWidgetHostImpl::FilterInputEvent(
} }
void RenderWidgetHostImpl::IncrementInFlightEventCount() { void RenderWidgetHostImpl::IncrementInFlightEventCount() {
StartHangMonitorTimeout(
TimeDelta::FromMilliseconds(hung_renderer_delay_ms_));
increment_in_flight_event_count(); increment_in_flight_event_count();
if (!is_hidden_)
StartHangMonitorTimeout(hung_renderer_delay_);
} }
void RenderWidgetHostImpl::DecrementInFlightEventCount() { void RenderWidgetHostImpl::DecrementInFlightEventCount() {
...@@ -1804,7 +1810,8 @@ void RenderWidgetHostImpl::DecrementInFlightEventCount() { ...@@ -1804,7 +1810,8 @@ void RenderWidgetHostImpl::DecrementInFlightEventCount() {
StopHangMonitorTimeout(); StopHangMonitorTimeout();
} else { } else {
// The renderer is responsive, but there are in-flight events to wait for. // The renderer is responsive, but there are in-flight events to wait for.
RestartHangMonitorTimeout(); if (!is_hidden_)
RestartHangMonitorTimeout();
} }
} }
......
...@@ -124,8 +124,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -124,8 +124,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl
// uses RenderWidgetHost::AsRenderWidgetHostImpl(). // uses RenderWidgetHost::AsRenderWidgetHostImpl().
static RenderWidgetHostImpl* From(RenderWidgetHost* rwh); static RenderWidgetHostImpl* From(RenderWidgetHost* rwh);
void set_hung_renderer_delay_ms(const base::TimeDelta& timeout) { void set_hung_renderer_delay(const base::TimeDelta& delay) {
hung_renderer_delay_ms_ = timeout.InMilliseconds(); hung_renderer_delay_ = delay;
} }
// RenderWidgetHost implementation. // RenderWidgetHost implementation.
...@@ -592,7 +592,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl ...@@ -592,7 +592,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl
bool renderer_initialized_; bool renderer_initialized_;
// This value indicates how long to wait before we consider a renderer hung. // This value indicates how long to wait before we consider a renderer hung.
int64 hung_renderer_delay_ms_; base::TimeDelta hung_renderer_delay_;
private: private:
friend class MockRenderWidgetHost; friend class MockRenderWidgetHost;
......
...@@ -158,10 +158,6 @@ class MockRenderWidgetHost : public RenderWidgetHostImpl { ...@@ -158,10 +158,6 @@ class MockRenderWidgetHost : public RenderWidgetHostImpl {
return unresponsive_timer_fired_; return unresponsive_timer_fired_;
} }
void set_hung_renderer_delay_ms(int64 delay_ms) {
hung_renderer_delay_ms_ = delay_ms;
}
void DisableGestureDebounce() { void DisableGestureDebounce() {
input_router_.reset(new InputRouterImpl( input_router_.reset(new InputRouterImpl(
process_, this, this, routing_id_, InputRouterImpl::Config())); process_, this, this, routing_id_, InputRouterImpl::Config()));
...@@ -983,13 +979,51 @@ TEST_F(RenderWidgetHostTest, ShorterDelayHangMonitorTimeout) { ...@@ -983,13 +979,51 @@ TEST_F(RenderWidgetHostTest, ShorterDelayHangMonitorTimeout) {
EXPECT_TRUE(host_->unresponsive_timer_fired()); EXPECT_TRUE(host_->unresponsive_timer_fired());
} }
// Test that the hang monitor timer is effectively disabled when the widget is
// hidden.
TEST_F(RenderWidgetHostTest, HangMonitorTimeoutDisabledForInputWhenHidden) {
host_->set_hung_renderer_delay(base::TimeDelta::FromMicroseconds(1));
SimulateMouseEvent(WebInputEvent::MouseMove, 10, 10, 0, false);
// Hiding the widget should deactivate the timeout.
host_->WasHidden();
// The timeout should not fire.
EXPECT_FALSE(host_->unresponsive_timer_fired());
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::MessageLoop::QuitClosure(),
TimeDelta::FromMicroseconds(2));
base::MessageLoop::current()->Run();
EXPECT_FALSE(host_->unresponsive_timer_fired());
// The timeout should never reactivate while hidden.
SimulateMouseEvent(WebInputEvent::MouseMove, 10, 10, 0, false);
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::MessageLoop::QuitClosure(),
TimeDelta::FromMicroseconds(2));
base::MessageLoop::current()->Run();
EXPECT_FALSE(host_->unresponsive_timer_fired());
// Showing the widget should restore the timeout, as the events have
// not yet been ack'ed.
host_->WasShown(ui::LatencyInfo());
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::MessageLoop::QuitClosure(),
TimeDelta::FromMicroseconds(2));
base::MessageLoop::current()->Run();
EXPECT_TRUE(host_->unresponsive_timer_fired());
}
// Test that the hang monitor catches two input events but only one ack. // 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 can happen if the second input event causes the renderer to hang.
// This test will catch a regression of crbug.com/111185. // This test will catch a regression of crbug.com/111185.
TEST_F(RenderWidgetHostTest, MultipleInputEvents) { TEST_F(RenderWidgetHostTest, MultipleInputEvents) {
// Configure the host to wait 10ms before considering // Configure the host to wait 10ms before considering
// the renderer hung. // the renderer hung.
host_->set_hung_renderer_delay_ms(10); host_->set_hung_renderer_delay(base::TimeDelta::FromMicroseconds(10));
// Send two events but only one ack. // Send two events but only one ack.
SimulateKeyboardEvent(WebInputEvent::RawKeyDown); SimulateKeyboardEvent(WebInputEvent::RawKeyDown);
...@@ -1001,7 +1035,7 @@ TEST_F(RenderWidgetHostTest, MultipleInputEvents) { ...@@ -1001,7 +1035,7 @@ TEST_F(RenderWidgetHostTest, MultipleInputEvents) {
base::MessageLoop::current()->PostDelayedTask( base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, FROM_HERE,
base::MessageLoop::QuitClosure(), base::MessageLoop::QuitClosure(),
TimeDelta::FromMilliseconds(40)); TimeDelta::FromMicroseconds(20));
base::MessageLoop::current()->Run(); base::MessageLoop::current()->Run();
EXPECT_TRUE(host_->unresponsive_timer_fired()); EXPECT_TRUE(host_->unresponsive_timer_fired());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment