Forward secondary touch points even if the first had no consumer

content/browser/renderer_host/input/touch_event_queue.cc

Index: content/browser/renderer_host/input/touch_event_queue.cc
diff --git a/content/browser/renderer_host/input/touch_event_queue.cc b/content/browser/renderer_host/input/touch_event_queue.cc
index b0646ebdff63fe82ef2154076f3e3823eab4c65e..b8937156a191bd0d7f9fd6931e3b4dca5775356b 100644
--- a/content/browser/renderer_host/input/touch_event_queue.cc
+++ b/content/browser/renderer_host/input/touch_event_queue.cc
@@ -392,20 +392,11 @@ void TouchEventQueue::ProcessTouchAck(InputEventAckState ack_result,
   if (touch_queue_.empty())
     return;
 
-  const WebTouchEvent& acked_event =
-      touch_queue_.front()->coalesced_event().event;
-
   if (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED &&
       touch_filtering_state_ == FORWARD_TOUCHES_UNTIL_TIMEOUT) {
     touch_filtering_state_ = FORWARD_ALL_TOUCHES;
   }
 
-  if (ack_result == INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS &&
-      touch_filtering_state_ != DROP_ALL_TOUCHES &&
-      WebTouchEventTraits::IsTouchSequenceStart(acked_event)) {
-    touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE;
-  }
-
   PopTouchEventToClient(ack_result, latency_info);
   TryForwardNextEventToRenderer();
 }
@@ -473,7 +464,13 @@ void TouchEventQueue::ForwardNextEventToRenderer() {
         DCHECK(pending_async_touch_move_->CanCoalesceWith(touch));
         pending_async_touch_move_->CoalesceWith(touch);
       }
+      DCHECK_EQ(1U, size());
       PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+      // It's possible (though unlikely) that ack'ing the current touch will
+      // trigger the queueing of another touch event (e.g., a touchcancel). As
+      // forwarding of the queued event will be deferred while the ack is being
+      // dispatched (see |OnTouchEvent()|), try forwarding it now.
+      TryForwardNextEventToRenderer();
       return;
     }
   }
@@ -521,6 +518,26 @@ void TouchEventQueue::OnGestureScrollEvent(
   if (gesture_event.event.type != blink::WebInputEvent::GestureScrollBegin)
     return;
 
+  if (touch_filtering_state_ != DROP_ALL_TOUCHES &&
+      touch_filtering_state_ != DROP_TOUCHES_IN_SEQUENCE) {
+    // As an optimization, drop all remaining touch events in the sequence if
+    // scrolling begins while *none* of the touch points have a consumer.
+    bool no_consumers_exist = true;
+    for (TouchPointAckStates::const_iterator iter = touch_ack_states_.begin(),

[jdduke (slow), 2014/04/28 18:08:15]

Rick, do you think this is a worthwhile optimization? The case I'm thinking of
is when you start scrolling then pinching; if the secondary touchstart is async,
we'll never know if it has a consumer, so we'd start forwarding the pinch
touchmoves (even if the first touchstart had no consumer).

[Rick Byers, 2014/04/28 21:45:22]

Hmm - interesting scenario.  My gut instinct is that we should really send the
secondary touchstart synchronously.  Eg. should we allow people to build UIs
that allow you to, for example, scroll a list with one finger and grab something
from that list with another?  That doesn't seem completely insane to me and it
may be possible in Android browser, Firefox and Safari div scrolling (we should
test).  It's an edge case though, so I'm OK continuing to leave this scenario as
future follow-up.

In that case, I'm OK with this optimization but I don't think it buys us that
much really (a touchmove every 200ms should be nothing compared to the scroll
events that are being sent/processed every frame).  I think I'd prefer to err on
the side of simplicity and avoid it for now - we can always revisit if we ever
encounter a scenario where this shows up in a trace.  Your call though...

[jdduke (slow), 2014/04/28 22:04:59]

Interesting, that sounds like a reasonable use-case, but I'm curious how that
would interact with the gesture detector.  I guess their handling of the
secondary touchstart would suppress the GesturePinchBegin and all subsequent
pinch events? That would probably require some tweaking of the current model,
perhaps we revert to synchronous touchmoves in that case?  In that case it would
probably work.
Yeah.  I'm less worried about the 200ms touchmoves than I am the web app seeing
its first touchstart (as it never got the one with no consumer) but not being
able to preventDefault it.  Let's come up with a solution that handles the case
you mentioned (might be as simple as making touchstart synchronous), and we can
remove this (I'll update the comment, as it's less an optimization and more a
band-aid on the async touchstart case).

+                                             end = touch_ack_states_.end();
+         iter != end;
+         ++iter) {
+      if (iter->second != INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS) {
+        no_consumers_exist = false;
+        break;
+      }
+    }
+    if (no_consumers_exist) {
+      touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE;
+      return;
+    }
+  }
+
   if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE) {
     pending_async_touch_move_.reset();
     send_touch_events_async_ = true;