Consuming a touch move prevents only the next scroll update.

content/browser/renderer_host/input/touch_disposition_gesture_filter.cc

Index: content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
diff --git a/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc b/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
index 35db76779b68b34b26912e864c09b3983abf38ee..d9e6c43121da37be0f646f1227e84f9b3e8a786f 100644
--- a/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
+++ b/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
@@ -34,6 +34,7 @@ TouchDispositionGestureFilter::TouchDispositionGestureFilter(
       needs_tap_ending_event_(false),
       needs_fling_ending_event_(false) {
   DCHECK(client_);
+  SetupDispositionHandlingTable();
 }
 
 TouchDispositionGestureFilter::~TouchDispositionGestureFilter() {}
@@ -44,20 +45,16 @@ TouchDispositionGestureFilter::OnGestureEventPacket(
  if (packet.gesture_source() == GestureEventPacket::INVALID)
     return INVALID_PACKET_TYPE;
 
-  if (packet.gesture_source() == GestureEventPacket::TOUCH_BEGIN)
+ if (packet.gesture_source() == GestureEventPacket::TOUCH_SEQUENCE_BEGIN)
     sequences_.push(GestureSequence());
 
   if (IsEmpty())
     return INVALID_PACKET_ORDER;
 
-  if (packet.gesture_source() == GestureEventPacket::TOUCH_TIMEOUT) {
-    // Handle the timeout packet immediately if the packet preceding the
-    // timeout has already been dispatched.
-    if (Tail().IsEmpty()) {
-      if (!Tail().IsGesturePrevented())
-        SendPacket(packet);
-      return SUCCESS;
-    }
+  if (packet.gesture_source() == GestureEventPacket::TOUCH_TIMEOUT &&
+      Tail().IsEmpty()) {
+    FilterAndSendPacket(packet, Tail(), INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+    return SUCCESS;
   }
 
   Tail().Push(packet);
@@ -77,7 +74,6 @@ void TouchDispositionGestureFilter::OnTouchEventAck(
   }
 
   GestureSequence& sequence = Head();
-  sequence.UpdateState(ack_state);
 
   // Dispatch the packet corresponding to the ack'ed touch, as well as any
   // additional timeout-based packets queued before the ack was received.
@@ -85,36 +81,105 @@ void TouchDispositionGestureFilter::OnTouchEventAck(
   while (!sequence.IsEmpty()) {
     const GestureEventPacket& packet = sequence.Front();
 
-    if (packet.gesture_source() == GestureEventPacket::TOUCH_BEGIN ||
-        packet.gesture_source() == GestureEventPacket::TOUCH) {
+    if (packet.gesture_source() == GestureEventPacket::TOUCH_SEQUENCE_BEGIN ||
+        packet.gesture_source() == GestureEventPacket::TOUCH_BEGIN ||
+        packet.gesture_source() == GestureEventPacket::TOUCH_MOVE ||
+        packet.gesture_source() == GestureEventPacket::TOUCH_END) {
       // We should handle at most one touch-based packet corresponding to a
       // given ack.
       if (touch_packet_for_current_ack_handled)
         break;
+      sequence.UpdateState(packet.gesture_source(), ack_state);
       touch_packet_for_current_ack_handled = true;
     }
-
-    if (!sequence.IsGesturePrevented())
-      SendPacket(packet);
+    FilterAndSendPacket(packet, sequence, ack_state);
 
     sequence.Pop();
   }
   DCHECK(touch_packet_for_current_ack_handled);
+}
 
-  // Immediately cancel a TapDown if TouchStart went unconsumed, but a
-  // subsequent TouchMove is consumed.
-  if (sequence.IsGesturePrevented())
-    CancelTapIfNecessary();
+void TouchDispositionGestureFilter::SetupDispositionHandlingTable() {
+  AddDispositionHandlingInfo(WebInputEvent::GestureTapDown, RT_START);

[jdduke (slow), 2014/02/11 16:02:25]

These are such lightweight structs, why not simply have a function local to this
file that instead does something like:

GetDispositionHandlingInfo(type) {
 switch (type) {
  case FOO: return Info(a, b, c);
}

I'd prefer that over duplicating the map for each TouchDispositionGestureFilter
instance.

The alternative would be building either a static map or lookup table
(re-indexed from the WebInputEvent::Type to something 0-based).  I think either
of these approaches would make the code a bit easier to follow as well.

[tdresser, 2014/02/11 21:04:33]

Done.

+  AddDispositionHandlingInfo(WebInputEvent::GestureTapCancel, RT_START);
+  AddDispositionHandlingInfo(WebInputEvent::GestureShowPress, RT_START);
+  AddDispositionHandlingInfo(WebInputEvent::GestureLongPress, RT_START);
+  AddDispositionHandlingInfo(WebInputEvent::GestureLongTap,
+                             RT_START | RT_CURRENT);
+  AddDispositionHandlingInfo(WebInputEvent::GestureTap,
+                             RT_START | RT_CURRENT,
+                             WebInputEvent::GestureTapUnconfirmed);
+  AddDispositionHandlingInfo(WebInputEvent::GestureTwoFingerTap,
+                             RT_START | RT_CURRENT);
+  AddDispositionHandlingInfo(WebInputEvent::GestureTapUnconfirmed,
+                             RT_START | RT_CURRENT);
+  AddDispositionHandlingInfo(WebInputEvent::GestureDoubleTap,
+                             RT_START | RT_CURRENT,
+                             WebInputEvent::GestureTapUnconfirmed);
+  AddDispositionHandlingInfo(WebInputEvent::GestureScrollBegin,
+                             RT_START | RT_MOVE);
+  AddDispositionHandlingInfo(WebInputEvent::GestureScrollUpdate,
+                             RT_CURRENT,
+                             WebInputEvent::GestureScrollBegin);
+  AddDispositionHandlingInfo(WebInputEvent::GestureScrollEnd,
+                             RT_NONE,
+                             WebInputEvent::GestureScrollBegin);
+  AddDispositionHandlingInfo(WebInputEvent::GestureFlingStart,
+                             RT_NONE,
+                             WebInputEvent::GestureScrollBegin);
+  AddDispositionHandlingInfo(WebInputEvent::GestureFlingCancel,
+                             RT_NONE,
+                             WebInputEvent::GestureFlingStart);
+  AddDispositionHandlingInfo(WebInputEvent::GesturePinchBegin,
+                             RT_START,
+                             WebInputEvent::GestureScrollBegin);
+  AddDispositionHandlingInfo(WebInputEvent::GesturePinchUpdate,
+                             RT_CURRENT,
+                             WebInputEvent::GesturePinchBegin);
+  AddDispositionHandlingInfo(WebInputEvent::GesturePinchEnd,
+                             RT_NONE,
+                             WebInputEvent::GesturePinchBegin);
 }
 
 bool TouchDispositionGestureFilter::IsEmpty() const {
   return sequences_.empty();
 }
 
-void TouchDispositionGestureFilter::SendPacket(
-    const GestureEventPacket& packet) {
-  for (size_t i = 0; i < packet.gesture_count(); ++i)
-    SendGesture(packet.gesture(i));
+void TouchDispositionGestureFilter::AddDispositionHandlingInfo(
+    WebInputEvent::Type type,
+    int required_touches) {
+  DispositionHandlingInfo disposition_handling_info;
+  disposition_handling_info.required_touches = required_touches;
+  disposition_handling_info.antecedent_event_type = WebInputEvent::Undefined;
+  disposition_handling_table_[type] = disposition_handling_info;
+}
+
+void TouchDispositionGestureFilter::AddDispositionHandlingInfo(
+    WebInputEvent::Type type,
+    int required_touches,
+    WebInputEvent::Type antecedent_event_type) {
+  AddDispositionHandlingInfo(type, required_touches);
+  disposition_handling_table_[type].antecedent_event_type =
+      antecedent_event_type;
+}
+
+void TouchDispositionGestureFilter::FilterAndSendPacket(
+    const GestureEventPacket& packet,
+    const GestureSequence& sequence,
+    InputEventAckState ack_state) {
+  for (size_t i = 0; i < packet.gesture_count(); ++i) {
+    const blink::WebGestureEvent& gesture = packet.gesture(i);
+    bool dropped = IsGesturePrevented(
+        gesture.type, ack_state, sequence.state());
+    if (!dropped)
+      SendGesture(gesture);
+    else
+      CancelTapIfNecessary();
+    if (dropped)
+      last_event_of_type_dropped_.insert(gesture.type);
+    else
+      last_event_of_type_dropped_.erase(gesture.type);
+  }
 }
 
 void TouchDispositionGestureFilter::SendGesture(const WebGestureEvent& event) {
@@ -124,6 +189,7 @@ void TouchDispositionGestureFilter::SendGesture(const WebGestureEvent& event) {
       CancelFlingIfNecessary();
       break;
     case WebInputEvent::GestureTapDown:
+      DCHECK(!needs_tap_ending_event_);
       needs_tap_ending_event_ = true;
       break;
     case WebInputEvent::GestureTapCancel:
@@ -177,11 +243,7 @@ TouchDispositionGestureFilter::Tail() {
   return sequences_.back();
 }
 
-
-// TouchDispositionGestureFilter::GestureSequence
-
-TouchDispositionGestureFilter::GestureSequence::GestureSequence()
-    : state_(PENDING) {}
+TouchDispositionGestureFilter::GestureSequence::GestureSequence() {}
 
 TouchDispositionGestureFilter::GestureSequence::~GestureSequence() {}
 
@@ -202,25 +264,51 @@ TouchDispositionGestureFilter::GestureSequence::Front() const {
 }
 
 void TouchDispositionGestureFilter::GestureSequence::UpdateState(
+    GestureEventPacket::GestureSource gesture_source,
     InputEventAckState ack_state) {
   DCHECK_NE(INPUT_EVENT_ACK_STATE_UNKNOWN, ack_state);
   // Permanent states will not be affected by subsequent ack's.
-  if (state_ != PENDING && state_ != ALLOWED_UNTIL_PREVENTED)
+  if (state_.no_consumer || (state_.start_consumed && state_.move_consumed))
     return;
 
   // |NO_CONSUMER| should only be effective when the *first* touch is ack'ed.
-  if (state_ == PENDING &&
-      ack_state == INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS)
-    state_ = ALWAYS_ALLOWED;
-  else if (ack_state == INPUT_EVENT_ACK_STATE_CONSUMED)
-    state_ = ALWAYS_PREVENTED;
-  else
-    state_ = ALLOWED_UNTIL_PREVENTED;
-}
-
-bool TouchDispositionGestureFilter::GestureSequence::IsGesturePrevented()
-    const {
-  return state_ == ALWAYS_PREVENTED;
+  if (!state_.seen_ack &&
+      ack_state == INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS) {
+    state_.no_consumer = true;
+  } else if (ack_state == INPUT_EVENT_ACK_STATE_CONSUMED) {
+    if (gesture_source == GestureEventPacket::TOUCH_SEQUENCE_BEGIN ||
+        gesture_source == GestureEventPacket::TOUCH_BEGIN) {
+      state_.start_consumed = true;
+    } else if (gesture_source == GestureEventPacket::TOUCH_MOVE) {
+      state_.move_consumed = true;
+    }
+  }
+  state_.seen_ack = true;
+}
+
+bool TouchDispositionGestureFilter::IsGesturePrevented(
+    WebInputEvent::Type gesture_type,
+    InputEventAckState current,
+    const GestureSequence::GestureHandlingState& state) const {
+
+  if (state.no_consumer)
+    return false;
+
+  DispositionHandlingTable::const_iterator it =
+      disposition_handling_table_.find(gesture_type);
+  DCHECK(it != disposition_handling_table_.end());
+  DispositionHandlingInfo disposition_handling_info = it->second;
+
+  int required_touches = disposition_handling_info.required_touches;
+  if ((required_touches & RT_MOVE && state.move_consumed) ||
+      (required_touches & RT_START && state.start_consumed) ||
+      (required_touches & RT_CURRENT &&
+       current == INPUT_EVENT_ACK_STATE_CONSUMED) ||
+      (last_event_of_type_dropped_.count(

[jdduke (slow), 2014/02/11 16:02:25]

Nit: Indent "current ==" by 4

[tdresser, 2014/02/11 21:04:33]

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Boolea...

The example of a boolean expression in the style guide doesn't indent by 4 in
this case, nor does clang-format.

[jdduke (slow), 2014/02/11 21:18:16]

Of course, sorry, you're completely right... I just finished a Java code review
=/  Hmm, but I was also thinking about readability, what if we had something
like:

bool current_consumed = current == INPUT_EVENT_ACK_STATE_CONSUMED
...
if ((required_touches & RT_MOVE && state.move_consumed) ||
    (required_touches & RT_START && state.start_consumed) ||
    (required_touches & RT_CURRENT && current_consumed) ||
...

It's pretty nitty, but then you get a nice lineup of the checks and it's a
little easier to follow.  Not a big deal.

[tdresser, 2014/02/11 21:50:44]

Definitely a little nicer.
Done.

+          disposition_handling_info.antecedent_event_type))) {
+    return true;
+  }
+  return false;
 }
 
 bool TouchDispositionGestureFilter::GestureSequence::IsEmpty() const {