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}

5 files changed, 61 insertions, 19 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 111e7a0..c1fffde 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -482,7 +482,7 @@ void BrowserPluginGuest::RenderViewReady() {
   Send(new InputMsg_SetFocus(routing_id(), focused_));
   UpdateVisibility();
 
-  RenderWidgetHostImpl::From(rvh)->set_hung_renderer_delay_ms(
+  RenderWidgetHostImpl::From(rvh)->set_hung_renderer_delay(
       base::TimeDelta::FromMilliseconds(kHungRendererDelayMs));
 }
 
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 7c5b4c8..440511d 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -1678,9 +1678,10 @@ void RenderFrameHostImpl::JavaScriptDialogClosed(
   // leave the current page. In this case, use the regular timeout value used
   // during the (before)unload handling.
   if (is_waiting) {
-    render_view_host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(
-        success ? RenderViewHostImpl::kUnloadTimeoutMS
-                : render_view_host_->hung_renderer_delay_ms_));
+    render_view_host_->StartHangMonitorTimeout(
+        success
+            ? TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS)
+            : render_view_host_->hung_renderer_delay_);
   }
 
   FrameHostMsg_RunJavaScriptMessage::WriteReplyParams(reply_msg,
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index f1cbc2a..29ec4b7 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -160,7 +160,8 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
                                            bool hidden)
     : view_(NULL),
       renderer_initialized_(false),
-      hung_renderer_delay_ms_(kHungRendererDelayMs),
+      hung_renderer_delay_(
+          base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)),
       delegate_(delegate),
       process_(process),
       routing_id_(routing_id),
@@ -537,6 +538,11 @@ void RenderWidgetHostImpl::WasShown(const ui::LatencyInfo& latency_info) {
 
   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.
   bool needs_repainting = true;
   needs_repainting_on_restore_ = false;
@@ -863,8 +869,7 @@ void RenderWidgetHostImpl::StartHangMonitorTimeout(base::TimeDelta delay) {
 
 void RenderWidgetHostImpl::RestartHangMonitorTimeout() {
   if (hang_monitor_timeout_)
-    hang_monitor_timeout_->Restart(
-        base::TimeDelta::FromMilliseconds(hung_renderer_delay_ms_));
+    hang_monitor_timeout_->Restart(hung_renderer_delay_);
 }
 
 void RenderWidgetHostImpl::StopHangMonitorTimeout() {
@@ -1210,6 +1215,7 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
 
   // Reset this to ensure the hung renderer mechanism is working properly.
   in_flight_event_count_ = 0;
+  StopHangMonitorTimeout();
 
   if (view_) {
     GpuSurfaceTracker::Get()->SetSurfaceHandle(surface_id_,
@@ -1793,9 +1799,9 @@ InputEventAckState RenderWidgetHostImpl::FilterInputEvent(
 }
 
 void RenderWidgetHostImpl::IncrementInFlightEventCount() {
-  StartHangMonitorTimeout(
-      TimeDelta::FromMilliseconds(hung_renderer_delay_ms_));
   increment_in_flight_event_count();
+  if (!is_hidden_)
+    StartHangMonitorTimeout(hung_renderer_delay_);
 }
 
 void RenderWidgetHostImpl::DecrementInFlightEventCount() {
@@ -1804,7 +1810,8 @@ void RenderWidgetHostImpl::DecrementInFlightEventCount() {
     StopHangMonitorTimeout();
   } else {
     // The renderer is responsive, but there are in-flight events to wait for.
-    RestartHangMonitorTimeout();
+    if (!is_hidden_)
+      RestartHangMonitorTimeout();
   }
 }
 
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index 99607fc..5e3ee8f 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -124,8 +124,8 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // uses RenderWidgetHost::AsRenderWidgetHostImpl().
   static RenderWidgetHostImpl* From(RenderWidgetHost* rwh);
 
-  void set_hung_renderer_delay_ms(const base::TimeDelta& timeout) {
-    hung_renderer_delay_ms_ = timeout.InMilliseconds();
+  void set_hung_renderer_delay(const base::TimeDelta& delay) {
+    hung_renderer_delay_ = delay;
   }
 
   // RenderWidgetHost implementation.
@@ -592,7 +592,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   bool renderer_initialized_;
 
   // This value indicates how long to wait before we consider a renderer hung.
-  int64 hung_renderer_delay_ms_;
+  base::TimeDelta hung_renderer_delay_;
 
  private:
   friend class MockRenderWidgetHost;
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index 74b40f07..cb7d64ef 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -158,10 +158,6 @@ class MockRenderWidgetHost : public RenderWidgetHostImpl {
     return unresponsive_timer_fired_;
   }
 
-  void set_hung_renderer_delay_ms(int64 delay_ms) {
-    hung_renderer_delay_ms_ = delay_ms;
-  }
-
   void DisableGestureDebounce() {
     input_router_.reset(new InputRouterImpl(
         process_, this, this, routing_id_, InputRouterImpl::Config()));
@@ -983,13 +979,51 @@ TEST_F(RenderWidgetHostTest, ShorterDelayHangMonitorTimeout) {
   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.
 // This can happen if the second input event causes the renderer to hang.
 // This test will catch a regression of crbug.com/111185.
 TEST_F(RenderWidgetHostTest, MultipleInputEvents) {
   // Configure the host to wait 10ms before considering
   // 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.
   SimulateKeyboardEvent(WebInputEvent::RawKeyDown);
@@ -1001,7 +1035,7 @@ TEST_F(RenderWidgetHostTest, MultipleInputEvents) {
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::MessageLoop::QuitClosure(),
-      TimeDelta::FromMilliseconds(40));
+      TimeDelta::FromMicroseconds(20));
   base::MessageLoop::current()->Run();
   EXPECT_TRUE(host_->unresponsive_timer_fired());
 }