Events ignoring ack disposition receive synthetic acks.

content/browser/renderer_host/input/gesture_event_filter_unittest.cc

Index: content/browser/renderer_host/input/gesture_event_filter_unittest.cc
diff --git a/content/browser/renderer_host/input/gesture_event_filter_unittest.cc b/content/browser/renderer_host/input/gesture_event_filter_unittest.cc
index c852ef4ae328372eff0c385fd8a61671783d264e..b9cfc3a43e41d2f54a5fc0a7bb9830828ec32519 100644
--- a/content/browser/renderer_host/input/gesture_event_filter_unittest.cc
+++ b/content/browser/renderer_host/input/gesture_event_filter_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/time/time.h"
 #include "content/browser/renderer_host/input/gesture_event_filter.h"
@@ -47,8 +48,10 @@ class GestureEventFilterTest : public testing::Test,
   virtual void SendGestureEventImmediately(
       const GestureEventWithLatencyInfo& event) OVERRIDE {
     ++sent_gesture_event_count_;
-    if (sync_ack_result_) {
-      scoped_ptr<InputEventAckState> ack_result = sync_ack_result_.Pass();
+    while (!sync_ack_results_.empty()) {
+      scoped_ptr<InputEventAckState> ack_result;
+      ack_result.reset(sync_ack_results_.front());
+      sync_ack_results_.weak_erase(sync_ack_results_.begin());
       SendInputEventACK(event.event.type, *ack_result);

[jdduke (slow), 2013/11/07 15:57:42]

This is a bit odd, why would sending a gesture trigger multiple synchronous
acks? To make this simpler, just keep the scoped_ptr |sync_ack_result_|, but
don't reset it when you |SendInputEventAck()|.  So the sync_ack stays until it's
reset manually by the test.  That should cover the cases we care about here.

[tdresser, 2013/11/07 22:12:01]

I think it's clearer to explicitly set |sync_ack_result_| each time.
I've fixed the tests to only deal with one sync ack at a time.

     }
   }
@@ -72,7 +75,7 @@ class GestureEventFilterTest : public testing::Test,
     GestureEventWithLatencyInfo gesture_with_latency(gesture,
                                                      ui::LatencyInfo());
     if (filter()->ShouldForward(gesture_with_latency)) {
-      ++sent_gesture_event_count_;
+      SendGestureEventImmediately(gesture_with_latency);
       return true;
     }
     return false;
@@ -139,8 +142,8 @@ class GestureEventFilterTest : public testing::Test,
     filter()->debounce_interval_time_ms_ = ms;
   }
 
-  void set_synchronous_ack(InputEventAckState ack_result) {
-    sync_ack_result_.reset(new InputEventAckState(ack_result));
+  void append_synchronous_ack(InputEventAckState ack_result) {
+    sync_ack_results_.push_back(new InputEventAckState(ack_result));
   }
 
   unsigned GestureEventQueueSize() {
@@ -185,7 +188,7 @@ class GestureEventFilterTest : public testing::Test,
   size_t acked_gesture_event_count_;
   size_t sent_gesture_event_count_;
   WebGestureEvent last_acked_event_;
-  scoped_ptr<InputEventAckState> sync_ack_result_;
+  ScopedVector<InputEventAckState> sync_ack_results_;
   base::MessageLoopForUI message_loop_;
 };
 
@@ -581,6 +584,70 @@ TEST_F(GestureEventFilterTest, CoalescesMultiplePinchEventSequences) {
   EXPECT_EQ(1, merged_event.modifiers);
 }
 
+// Tests a single event with an synchronous ack.
+TEST_F(GestureEventFilterTest, SimpleSyncAck) {
+  append_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
+  SimulateGestureEvent(WebInputEvent::GestureTapDown,
+                       WebGestureEvent::Touchscreen);
+
+  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(0U, GestureEventQueueSize());

[jdduke (slow), 2013/11/07 15:57:42]

Please make sure we get the ack, "GetAndResetAckedGestureEventCount()", also in
tests below.

[tdresser, 2013/11/07 22:12:01]

Done.

+}

[jdduke (slow), 2013/11/07 15:57:42]

Could we have a test similar to "SimpleSyncAck" where the synchronous ack
triggers an additionally queued event in the call to |OnGestureEventAck()|? So
you'd have a scoped_ptr<WebGestureEvent> sync_followup_event_, and you'll need
to Pass()/reset() it before queueing to avoid an infinite loop.

[tdresser, 2013/11/07 22:12:01]

Can you clarify what you mean by "triggers an additionally queued event"?
Do you mean that the sync ack queues an additional event, which will then need
to be acked?

That's what I've implemented, but I'm not sure I've interpreted you correctly.

+
+// Tests an event with an async ack followed by an event with a sync ack.
+TEST_F(GestureEventFilterTest, AsyncThenSyncAck) {
+  // Turn off debounce handling for test isolation.
+  set_debounce_interval_time_ms(0);
+
+  SimulateGestureEvent(WebInputEvent::GestureTapDown,
+                       WebGestureEvent::Touchscreen);
+
+  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(1U, GestureEventQueueSize());
+
+  SimulateGestureEvent(WebInputEvent::GestureScrollBegin,
+                       WebGestureEvent::Touchscreen);
+  append_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
+
+  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(2U, GestureEventQueueSize());
+
+  SendInputEventACK(WebInputEvent::GestureTapDown,
+                    INPUT_EVENT_ACK_STATE_CONSUMED);
+  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(0U, GestureEventQueueSize());
+}
+
+// Tests an event with an async ack followed by two events with sync acks.
+TEST_F(GestureEventFilterTest, AsyncThenSyncThenSyncAck) {
+  // Turn off debounce handling for test isolation.
+  set_debounce_interval_time_ms(0);
+
+  SimulateGestureEvent(WebInputEvent::GestureTapDown,
+                       WebGestureEvent::Touchscreen);
+
+  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(1U, GestureEventQueueSize());
+
+  SimulateGestureEvent(WebInputEvent::GestureScrollBegin,
+                       WebGestureEvent::Touchscreen);
+  append_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
+
+  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(2U, GestureEventQueueSize());
+
+  SimulateGestureEvent(WebInputEvent::GestureScrollUpdate,
+                       WebGestureEvent::Touchscreen);
+  append_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
+
+  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(3U, GestureEventQueueSize());
+
+  SendInputEventACK(WebInputEvent::GestureTapDown,
+                    INPUT_EVENT_ACK_STATE_CONSUMED);
+  EXPECT_EQ(2U, GetAndResetSentGestureEventCount());
+  EXPECT_EQ(0U, GestureEventQueueSize());
+}
 
 TEST_F(GestureEventFilterTest, CoalescesScrollAndPinchEventWithSyncAck) {
   // Turn off debounce handling for test isolation.
@@ -611,7 +678,7 @@ TEST_F(GestureEventFilterTest, CoalescesScrollAndPinchEventWithSyncAck) {
   EXPECT_EQ(3U, GestureEventQueueSize());
 
   // Ack the PinchBegin, and schedule a synchronous ack for GestureScrollUpdate.
-  set_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
+  append_synchronous_ack(INPUT_EVENT_ACK_STATE_CONSUMED);
   SendInputEventACK(WebInputEvent::GesturePinchBegin,
                     INPUT_EVENT_ACK_STATE_CONSUMED);
 
@@ -725,127 +792,6 @@ INSTANTIATE_TEST_CASE_P(AllSources,
                                         WebGestureEvent::Touchpad));
 #endif  // GTEST_HAS_PARAM_TEST
 
-// Test that GestureShowPress, GestureTapDown and GestureTapCancel events don't
-// wait for ACKs.
-TEST_F(GestureEventFilterTest, GestureTypesIgnoringAck) {
-  set_debounce_interval_time_ms(0);
-
-  // The show press, tap down and tap cancel events will escape the queue
-  // immediately when they reach the queue head, since they ignore acks.
-  SimulateGestureEvent(WebInputEvent::GestureShowPress,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureShowPress,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureTapCancel,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-
-  // Interleave a few events that do and do not ignore acks, ensuring that
-  // ack-ignoring events remain queued until they reach the queue head.
-  SimulateGestureEvent(WebInputEvent::GesturePinchBegin,
-                       WebGestureEvent::Touchpad);
-  ASSERT_EQ(1U, GetAndResetSentGestureEventCount());
-  ASSERT_EQ(1U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureTapDown,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(2U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GesturePinchUpdate,
-                       WebGestureEvent::Touchpad);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(3U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureShowPress,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(4U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GesturePinchEnd,
-                       WebGestureEvent::Touchpad);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(5U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureTapCancel,
-                       WebGestureEvent::Touchscreen);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(6U, GestureEventQueueSize());
-
-  // Now ack each event. Ack-ignoring events should remain queued until they
-  // reach the head of the queue, at which point they should be sent immediately
-  // and removed from the queue, unblocking subsequent events.
-  SendInputEventACK(WebInputEvent::GesturePinchBegin,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(2U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(4U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GestureTapDown,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(4U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GesturePinchUpdate,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(2U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(2U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GestureShowPress,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(2U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GesturePinchEnd,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GestureTapCancel,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-}
-
-// Test that GestureShowPress events don't get out of order due to
-// ignoring their acks.
-TEST_F(GestureEventFilterTest, GestureShowPressIsInOrder) {
-  SimulateGestureEvent(WebInputEvent::GestureTap,
-                       WebGestureEvent::Touchscreen);
-
-  EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(1U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureShowPress,
-                       WebGestureEvent::Touchscreen);
-
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  // The ShowPress, though it ignores ack, is still stuck in the queue
-  // behind the Tap which requires an ack.
-  EXPECT_EQ(2U, GestureEventQueueSize());
-
-  SimulateGestureEvent(WebInputEvent::GestureShowPress,
-                       WebGestureEvent::Touchscreen);
-
-  EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
-  // ShowPress has entered the queue.
-  EXPECT_EQ(3U, GestureEventQueueSize());
-
-  SendInputEventACK(WebInputEvent::GestureTap,
-                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
-
-  // Now that the Tap has been ACKed, the ShowPress events should fire
-  // immediately.
-  EXPECT_EQ(2U, GetAndResetSentGestureEventCount());
-  EXPECT_EQ(0U, GestureEventQueueSize());
-}
-
 // Test that a GestureScrollEnd | GestureFlingStart are deferred during the
 // debounce interval, that Scrolls are not and that the deferred events are
 // sent after that timer fires.