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..42ed447c767dfe118553708ccbdc19998dca00f2 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,70 @@ 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 GetDispositionHandlingInfo(

[tdresser, 2014/02/13 18:14:09]

I think this was a lot more readable using the |Info| factory instead of the
constructor (just due to line length).

I don't really want to use a type name much less explicit than
DispositionHandlingInfo though.

Do you think this is reasonable?

[jdduke (slow), 2014/02/13 19:40:00]

I see, you're right, any reason you can't use a one-line factory function(s)
that simply delegate to the constructor?

[tdresser, 2014/02/13 20:34:31]

Done.
Why is this superior to just using the factory functions?

+    blink::WebInputEvent::Type type) {
+  switch (type) {
+    case WebInputEvent::GestureTapDown:
+      return DispositionHandlingInfo(RT_START);
+    case WebInputEvent::GestureTapCancel:
+      return DispositionHandlingInfo(RT_START);
+    case WebInputEvent::GestureShowPress:
+      return DispositionHandlingInfo(RT_START);
+    case WebInputEvent::GestureLongPress:
+      return DispositionHandlingInfo(RT_START);
+    case WebInputEvent::GestureLongTap:
+      return DispositionHandlingInfo(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureTap:
+      return DispositionHandlingInfo(RT_START | RT_CURRENT,
+                                     WebInputEvent::GestureTapUnconfirmed);
+    case WebInputEvent::GestureTwoFingerTap:
+      return DispositionHandlingInfo(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureTapUnconfirmed:
+      return DispositionHandlingInfo(RT_START | RT_CURRENT);
+    case WebInputEvent::GestureDoubleTap:
+      return DispositionHandlingInfo(RT_START | RT_CURRENT,
+                                     WebInputEvent::GestureTapUnconfirmed);
+    case WebInputEvent::GestureScrollBegin:
+      return DispositionHandlingInfo(RT_START | RT_MOVE);
+    case WebInputEvent::GestureScrollUpdate:
+      return DispositionHandlingInfo(RT_CURRENT,
+                                     WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureScrollEnd:
+      return DispositionHandlingInfo(RT_NONE,
+                                     WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureFlingStart:
+      return DispositionHandlingInfo(RT_NONE,
+                                     WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GestureFlingCancel:
+      return DispositionHandlingInfo(RT_NONE, WebInputEvent::GestureFlingStart);
+    case WebInputEvent::GesturePinchBegin:
+      return DispositionHandlingInfo(RT_START,
+                                     WebInputEvent::GestureScrollBegin);
+    case WebInputEvent::GesturePinchUpdate:
+      return DispositionHandlingInfo(RT_CURRENT,
+                                     WebInputEvent::GesturePinchBegin);
+    case WebInputEvent::GesturePinchEnd:
+      return DispositionHandlingInfo(RT_NONE, WebInputEvent::GesturePinchBegin);
+    default:
+      NOTREACHED();
+      return DispositionHandlingInfo(RT_NONE);
+  }
+}
+
 }  // namespace
 
 // TouchDispositionGestureFilter
@@ -41,23 +105,19 @@ TouchDispositionGestureFilter::~TouchDispositionGestureFilter() {}
 TouchDispositionGestureFilter::PacketResult
 TouchDispositionGestureFilter::OnGestureEventPacket(
     const GestureEventPacket& packet) {
- if (packet.gesture_source() == GestureEventPacket::INVALID)
+  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())
+  if (packet.gesture_source() == GestureEventPacket::OUT_OF_ORDER || 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 +137,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 +144,45 @@ 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_SEQUENCE_END ||
+        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);

[jdduke (slow), 2014/02/13 19:40:00]

Since we know we only have valid packets (we can always DCHECK) at this point,
we might shorten the conditional by instead checking the exceptional case, e.g.

if (packet.gesture_source() != GestureEventPacket::TOUCH_TIMEOUT) {
  
}

[tdresser, 2014/02/13 20:34:31]

Done.

       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();
 }
 
 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& sequence,

[jdduke (slow), 2014/02/13 19:40:00]

Hmm, I guess we might as well pass the sequence state here?

[tdresser, 2014/02/13 20:34:31]

Done.

+    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 +192,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 +234,13 @@ void TouchDispositionGestureFilter::CancelFlingIfNecessary() {
   DCHECK(!needs_fling_ending_event_);
 }
 
+TouchDispositionGestureFilter::GestureSequence::GestureHandlingState::
+    GestureHandlingState()
+    : seen_ack(false),
+      start_consumed(false),
+      move_consumed(false),
+      no_consumer(false) {}
+
 TouchDispositionGestureFilter::GestureSequence&
 TouchDispositionGestureFilter::Head() {
   DCHECK(!sequences_.empty());
@@ -177,11 +253,7 @@ TouchDispositionGestureFilter::Tail() {
   return sequences_.back();
 }
 
-
-// TouchDispositionGestureFilter::GestureSequence
-
-TouchDispositionGestureFilter::GestureSequence::GestureSequence()
-    : state_(PENDING) {}
+TouchDispositionGestureFilter::GestureSequence::GestureSequence() {}
 
 TouchDispositionGestureFilter::GestureSequence::~GestureSequence() {}
 
@@ -202,25 +274,49 @@ 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;
+
+  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_MOVE && state.move_consumed) ||
+      (required_touches & RT_CURRENT && current_consumed) ||
+      (last_event_of_type_dropped_.count(
+          disposition_handling_info.antecedent_event_type))) {
+    return true;
+  }
+  return false;
 }
 
 bool TouchDispositionGestureFilter::GestureSequence::IsEmpty() const {