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..ef283eb763f6914d46655c5256f14535a19b85a8 100644
--- a/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
+++ b/content/browser/renderer_host/input/touch_disposition_gesture_filter.cc
@@ -24,6 +24,73 @@ WebGestureEvent CreateGesture(WebInputEvent::Type type) {
   return event;
 }
 
+struct DispositionHandlingInfo {
+  // A bitwise-OR of |RequiredTouches|.
+  int required_touches;
+  blink::WebInputEvent::Type antecedent_event_type;
+
+  DispositionHandlingInfo(int required_touches)
+      : required_touches(required_touches) {}
+
+  DispositionHandlingInfo(int required_touches,
+                          blink::WebInputEvent::Type antecedent_event_type)
+      : required_touches(required_touches),
+        antecedent_event_type(antecedent_event_type) {}
+};
+
+DispositionHandlingInfo Info(int required_touches) {
+  return DispositionHandlingInfo(required_touches);
+}
+
+DispositionHandlingInfo Info(int required_touches,
+                             blink::WebInputEvent::Type antecedent_event_type) {
+  return DispositionHandlingInfo(required_touches, antecedent_event_type);
+}
+
+// This approach to disposition handling is described at http://goo.gl/5G8PWJ.
+DispositionHandlingInfo GetDispositionHandlingInfo(
+    blink::WebInputEvent::Type type) {
+  switch (type) {
+    case WebInputEvent::GestureTapDown:
+      return Info(RT_START);
+    case WebInputEvent::GestureTapCancel:
+      return Info(RT_START);
+    case WebInputEvent::GestureShowPress:
+      return Info(RT_START);
+    case WebInputEvent::GestureLongPress:
+      return Info(RT_START);
+    case WebInputEvent::GestureLongTap:
+      return Info(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureTap:
+      return Info(RT_START | RT_CURRENT, WebInputEvent::GestureTapUnconfirmed);
+    case WebInputEvent::GestureTwoFingerTap:
+      return Info(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureTapUnconfirmed:
+      return Info(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureDoubleTap:
+      return Info(RT_START | RT_CURRENT, WebInputEvent::GestureTapUnconfirmed);
+    case WebInputEvent::GestureScrollBegin:
+      return Info(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureScrollUpdate:
+      return Info(RT_CURRENT, WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureScrollEnd:
+      return Info(RT_NONE, WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureFlingStart:
+      return Info(RT_NONE, WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureFlingCancel:
+      return Info(RT_NONE, WebInputEvent::GestureFlingStart);
+    case WebInputEvent::GesturePinchBegin:
+      return Info(RT_START, WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GesturePinchUpdate:
+      return Info(RT_CURRENT, WebInputEvent::GesturePinchBegin);
+    case WebInputEvent::GesturePinchEnd:
+      return Info(RT_NONE, WebInputEvent::GesturePinchBegin);
+    default:
+      NOTREACHED();
+      return Info(RT_NONE);
+  }
+}
+
 }  // namespace
 
 // TouchDispositionGestureFilter
@@ -41,23 +108,22 @@ TouchDispositionGestureFilter::~TouchDispositionGestureFilter() {}
 TouchDispositionGestureFilter::PacketResult
 TouchDispositionGestureFilter::OnGestureEventPacket(
     const GestureEventPacket& packet) {
- if (packet.gesture_source() == GestureEventPacket::INVALID)
+  if (packet.gesture_source() == GestureEventPacket::UNDEFINED)
     return INVALID_PACKET_TYPE;
 
-  if (packet.gesture_source() == GestureEventPacket::TOUCH_BEGIN)
+  if (packet.gesture_source() == GestureEventPacket::TOUCH_SEQUENCE_BEGIN)
     sequences_.push(GestureSequence());
 
-  if (IsEmpty())
+  if (packet.gesture_source() == GestureEventPacket::INVALID || IsEmpty())

[jdduke (slow), 2014/02/14 16:59:52]

Please move this |GestureEventPacket::INVALID| check into the same check as
|UNDEFINED|.

[tdresser, 2014/02/14 17:56:11]

Done.

     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()) {
+    // Handle the timeout packet immediately if the packet preceding the timeout
+    // has already been dispatched.
+    FilterAndSendPacket(
+        packet, Tail().state(), INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+    return SUCCESS;
   }
 
   Tail().Push(packet);
@@ -73,48 +139,52 @@ void TouchDispositionGestureFilter::OnTouchEventAck(
   if (Head().IsEmpty()) {
     CancelTapIfNecessary();
     CancelFlingIfNecessary();
+    last_event_of_type_dropped_.clear();

[jdduke (slow), 2014/02/14 16:59:52]

Does std::set typically deallocate memory hear (unlike, say, std::vector)? If
so, this is a shame, as set/map are notoriously awful at memory allocation. 
Perhaps this is premature optimization, but I may revisit this down the road
with a bitset or SmallMap (sorry, this past week I've discovered that
underpowered Android devices really get hammered by things like this). 

[tdresser, 2014/02/14 17:56:11]

Yeah, this does deallocate memory. I was initially thinking we could just never
clear it, but this does feel safer.

I'd be in favour of cleaning this up later. 

     sequences_.pop();
   }
 
   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.
   bool touch_packet_for_current_ack_handled = false;
   while (!sequence.IsEmpty()) {
     const GestureEventPacket& packet = sequence.Front();
+    DCHECK(packet.gesture_source() != GestureEventPacket::UNDEFINED);

[jdduke (slow), 2014/02/14 16:59:52]

DCHECK_NE

[tdresser, 2014/02/14 17:56:11]

Done.

+    DCHECK(packet.gesture_source() != GestureEventPacket::INVALID);
 
-    if (packet.gesture_source() == GestureEventPacket::TOUCH_BEGIN ||
-        packet.gesture_source() == GestureEventPacket::TOUCH) {
+    if (packet.gesture_source() != GestureEventPacket::TOUCH_TIMEOUT) {
       // We should handle at most one touch-based packet corresponding to a

[jdduke (slow), 2014/02/14 16:59:52]

Hmm, I realize this was my comment, but maybe change it to say "at most one
non-timeout based packet".

[tdresser, 2014/02/14 17:56:11]

Done.

       // 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.state(), 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();
 }
 
 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::FilterAndSendPacket(
+    const GestureEventPacket& packet,
+    const GestureSequence::GestureHandlingState& sequence_state,
+    InputEventAckState ack_state) {
+  for (size_t i = 0; i < packet.gesture_count(); ++i) {
+    const blink::WebGestureEvent& gesture = packet.gesture(i);
+    if (IsGesturePrevented(gesture.type, ack_state, sequence_state)) {
+      last_event_of_type_dropped_.insert(gesture.type);
+      CancelTapIfNecessary();
+      continue;
+    }
+    last_event_of_type_dropped_.erase(gesture.type);
+    SendGesture(gesture);
+  }
 }
 
 void TouchDispositionGestureFilter::SendGesture(const WebGestureEvent& event) {
@@ -124,6 +194,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:
@@ -165,6 +236,12 @@ void TouchDispositionGestureFilter::CancelFlingIfNecessary() {
   DCHECK(!needs_fling_ending_event_);
 }
 
+TouchDispositionGestureFilter::GestureSequence::GestureHandlingState::
+    GestureHandlingState()
+    : seen_ack(false),
+      start_consumed(false),
+      no_consumer(false) {}
+
 TouchDispositionGestureFilter::GestureSequence&
 TouchDispositionGestureFilter::Head() {
   DCHECK(!sequences_.empty());
@@ -177,11 +254,7 @@ TouchDispositionGestureFilter::Tail() {
   return sequences_.back();
 }
 
-
-// TouchDispositionGestureFilter::GestureSequence

[jdduke (slow), 2014/02/14 16:59:52]

Hmm, did we still want this comment? 

[tdresser, 2014/02/14 17:56:11]

Readded.

-
-TouchDispositionGestureFilter::GestureSequence::GestureSequence()
-    : state_(PENDING) {}
+TouchDispositionGestureFilter::GestureSequence::GestureSequence() {}
 
 TouchDispositionGestureFilter::GestureSequence::~GestureSequence() {}
 
@@ -202,25 +275,46 @@ TouchDispositionGestureFilter::GestureSequence::Front() const {
 }
 
 void TouchDispositionGestureFilter::GestureSequence::UpdateState(
+    GestureEventPacket::GestureSource gesture_source,

[jdduke (slow), 2014/02/14 16:59:52]

Can't tell if this is aligned.

[tdresser, 2014/02/14 17:56:11]

It is.

     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)
     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;
+    }
+  }
+  state_.seen_ack = true;
+}
+
+bool TouchDispositionGestureFilter::IsGesturePrevented(
+    WebInputEvent::Type gesture_type,
+    InputEventAckState current,
+    const GestureSequence::GestureHandlingState& state) const {
+
+  if (state.no_consumer)
+    return false;
+
+  DispositionHandlingInfo disposition_handling_info =
+      GetDispositionHandlingInfo(gesture_type);
+
+  int required_touches = disposition_handling_info.required_touches;
+  bool current_consumed = current == INPUT_EVENT_ACK_STATE_CONSUMED;
+  if ((required_touches & RT_START && state.start_consumed) ||
+      (required_touches & RT_CURRENT && current_consumed) ||
+      (last_event_of_type_dropped_.count(
+           disposition_handling_info.antecedent_event_type) > 0)) {
+    return true;
+  }
+  return false;
 }
 
 bool TouchDispositionGestureFilter::GestureSequence::IsEmpty() const {