[Android] Properly disable the touch ack timeout during a touch sequence

content/browser/renderer_host/input/touch_event_queue_unittest.cc

Index: content/browser/renderer_host/input/touch_event_queue_unittest.cc
diff --git a/content/browser/renderer_host/input/touch_event_queue_unittest.cc b/content/browser/renderer_host/input/touch_event_queue_unittest.cc
index 330d6cdf8e0c96441856131ab8e782f965536f32..dad2a433bbfb428636fc50abd8a516a5faa3626a 100644
--- a/content/browser/renderer_host/input/touch_event_queue_unittest.cc
+++ b/content/browser/renderer_host/input/touch_event_queue_unittest.cc
@@ -169,6 +169,14 @@ class TouchEventQueueTest : public testing::Test,
     queue_->OnHasTouchEventHandlers(has_handlers);
   }
 
+  void SetAckTimeoutDisabled() {
+    queue_->SetAckTimeoutEnabled(false, 0);
+  }
+
+  bool IsTimeoutEnabled() {
+    return queue_->ack_timeout_enabled();
+  }
+
   bool IsTimeoutRunning() {
     return queue_->IsTimeoutRunningForTesting();
   }
@@ -193,6 +201,12 @@ class TouchEventQueueTest : public testing::Test,
     return last_acked_event_state_;
   }
 
+  static void Wait(base::TimeDelta delay) {
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE, base::MessageLoop::QuitClosure(), delay);
+    base::MessageLoop::current()->Run();
+  }
+
  private:
   void SendTouchEvent() {
     SendTouchEvent(touch_event_);
@@ -1015,11 +1029,7 @@ TEST_F(TouchEventQueueTest, TouchTimeoutBasic) {
   EXPECT_TRUE(IsTimeoutRunning());
 
   // Delay the ack.
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::MessageLoop::QuitClosure(),
-      base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
-  base::MessageLoop::current()->Run();
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));

[Rick Byers, 2014/03/06 22:46:11]

To what extent do you believe this is immune from timing-related flakiness? 
This should guarantee that the timeout fires, but what about the inverse - is
the timeout guaranteed not to fire before we expect it too (eg. on a bot under
heavy load)?

I guess since we're not pumping the message loop it should be fine (even if the
timer expires early, we won't see any effect until we're ready for it and pump
the loop by calling wait).  If that's true, then shouldn't we be able to use a
0ms (or 1ms if 0 is handled synchronously) timeout which will both be faster and
more clearly non-flaky than using an apparently magic value of 10ms?

[jdduke (slow), 2014/03/06 22:52:20]

This isn't just spinning.  We post a delayed quit task which is guaranteed to be
ordered properly w.r.t. the already posted timeout task.  Then we run the
message loop, which will block until it dispatches the posted quit task. As far
as I'm aware, these tests have never flaked, and we could probably shorten the
timeout without any loss.

 
   // The timeout should have fired, synthetically ack'ing the timed-out event.
   // TouchEvent forwarding is disabled until the ack is received for the
@@ -1089,6 +1099,35 @@ TEST_F(TouchEventQueueTest, NoTouchTimeoutIfRendererIsConsumingGesture) {
   EXPECT_FALSE(IsTimeoutRunning());
 }
 
+// Tests that the timeout is never started if the renderer consumes
+// a TouchEvent from the current touch sequence.
+TEST_F(TouchEventQueueTest, NoTouchTimeoutIfDisabledAfterTouchStart) {
+  SetUpForTimeoutTesting(kDefaultTouchTimeoutDelayMs);
+
+  // Queue a TouchStart.
+  PressTouchPoint(0, 1);
+  ASSERT_TRUE(IsTimeoutRunning());
+
+  // Send the ack immediately. The timeout should not have fired.
+  SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_FALSE(IsTimeoutRunning());
+  EXPECT_TRUE(IsTimeoutEnabled());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+
+  // Now explicitly disable the timeout.
+  SetAckTimeoutDisabled();
+  EXPECT_FALSE(IsTimeoutRunning());
+  EXPECT_FALSE(IsTimeoutEnabled());
+
+  // A TouchMove should not start or trigger the timeout.
+  MoveTouchPoint(0, 5, 5);
+  EXPECT_FALSE(IsTimeoutRunning());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+}
+
 // Tests that the timeout is never started if the ack is synchronous.
 TEST_F(TouchEventQueueTest, NoTouchTimeoutIfAckIsSynchronous) {
   SetUpForTimeoutTesting(kDefaultTouchTimeoutDelayMs);
@@ -1101,7 +1140,7 @@ TEST_F(TouchEventQueueTest, NoTouchTimeoutIfAckIsSynchronous) {
 }
 
 // Tests that the timeout is disabled if the touch handler disappears.
-TEST_F(TouchEventQueueTest, TouchTimeoutStoppedIfTouchHandlerRemoved) {
+TEST_F(TouchEventQueueTest, NoTouchTimeoutIfTouchHandlerRemoved) {
   SetUpForTimeoutTesting(kDefaultTouchTimeoutDelayMs);
 
   // Queue a TouchStart.
@@ -1113,6 +1152,22 @@ TEST_F(TouchEventQueueTest, TouchTimeoutStoppedIfTouchHandlerRemoved) {
   EXPECT_FALSE(IsTimeoutRunning());
 }
 
+// Tests that the timeout does not fire if explicitly disabled while an event
+// is in-flight.
+TEST_F(TouchEventQueueTest, NoTouchTimeoutIfDisabledWhileTimerIsActive) {
+  SetUpForTimeoutTesting(kDefaultTouchTimeoutDelayMs);
+
+  // Queue a TouchStart.
+  PressTouchPoint(0, 1);
+  ASSERT_TRUE(IsTimeoutRunning());
+
+  // Verify that disabling the timeout also turns off the timer.
+  SetAckTimeoutDisabled();
+  EXPECT_FALSE(IsTimeoutRunning());
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+}
+
 // Tests that a TouchCancel timeout plays nice when the timed out touch stream
 // turns into a scroll gesture sequence.
 TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGesture) {
@@ -1129,11 +1184,7 @@ TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGesture) {
   SetFollowupEvent(followup_scroll);
 
   // Delay the ack.
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::MessageLoop::QuitClosure(),
-      base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
-  base::MessageLoop::current()->Run();
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
 
   // The timeout should have fired, disabling touch forwarding until both acks
   // are received, acking the timed out event.
@@ -1188,11 +1239,7 @@ TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGestureAndDelayedAck) {
   SetFollowupEvent(followup_scroll);
 
   // Delay the ack.
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::MessageLoop::QuitClosure(),
-      base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
-  base::MessageLoop::current()->Run();
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
 
   // The timeout should have fired, disabling touch forwarding until both acks
   // are received and acking the timed out event.
@@ -1244,11 +1291,7 @@ TEST_F(TouchEventQueueTest, NoCancelOnTouchTimeoutWithoutConsumer) {
   EXPECT_TRUE(IsTimeoutRunning());
 
   // Delay the ack.
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::MessageLoop::QuitClosure(),
-      base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
-  base::MessageLoop::current()->Run();
+  Wait(base::TimeDelta::FromMilliseconds(kDefaultTouchTimeoutDelayMs * 2));
 
   // The timeout should have fired, synthetically ack'ing the timed out event.
   // TouchEvent forwarding is disabled until the original ack is received.