Implement async touchmove dispatch during scroll

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 8662f4a293c8245bd13d60f79fbf0041b3850ad5..960d2300512a767771b155bc8764e2d403cb3623 100644
--- a/content/browser/renderer_host/input/touch_event_queue.cc
+++ b/content/browser/renderer_host/input/touch_event_queue.cc
@@ -21,6 +21,8 @@ using ui::LatencyInfo;
 namespace content {
 namespace {
 
+const double kAsyncTouchMoveIntervalS = .2;
+
 // Using a small epsilon when comparing slop distances allows pixel perfect
 // slop determination when using fractional DIP coordinates (assuming the slop
 // region and DPI scale are reasonably proportioned).
@@ -37,9 +39,10 @@ TouchEventWithLatencyInfo ObtainCancelEventForTouchEvent(
   return event;
 }
 
-bool ShouldTouchTypeTriggerTimeout(WebInputEvent::Type type) {
-  return type == WebInputEvent::TouchStart ||
-         type == WebInputEvent::TouchMove;
+bool ShouldTouchTriggerTimeout(const WebInputEvent& event) {
+  return (event.type == WebInputEvent::TouchStart ||
+          event.type == WebInputEvent::TouchMove) &&
+         !WebInputEventTraits::IgnoresAckDisposition(event);
 }
 
 }  // namespace
@@ -61,7 +64,7 @@ class TouchEventQueue::TouchTimeoutHandler {
 
   void Start(const TouchEventWithLatencyInfo& event) {
     DCHECK_EQ(pending_ack_state_, PENDING_ACK_NONE);
-    DCHECK(ShouldTouchTypeTriggerTimeout(event.event.type));
+    DCHECK(ShouldTouchTriggerTimeout(event.event));
     timeout_event_ = event;
     timeout_monitor_.Restart(timeout_delay_);
   }
@@ -230,14 +233,13 @@ class CoalescedWebTouchEvent {
                          bool ignore_ack)
       : coalesced_event_(event),
         ignore_ack_(ignore_ack) {
-    events_.push_back(event);
-    TRACE_EVENT_ASYNC_BEGIN0(
-        "input", "TouchEventQueue::QueueEvent", this);
+    if (!ignore_ack_)
+      events_.push_back(event);
+    TRACE_EVENT_ASYNC_BEGIN0("input", "TouchEventQueue::QueueEvent", this);
   }
 
   ~CoalescedWebTouchEvent() {
-    TRACE_EVENT_ASYNC_END0(
-        "input", "TouchEventQueue::QueueEvent", this);
+    TRACE_EVENT_ASYNC_END0("input", "TouchEventQueue::QueueEvent", this);
   }
 
   // Coalesces the event with the existing event if possible. Returns whether
@@ -253,7 +255,6 @@ class CoalescedWebTouchEvent {
     TRACE_EVENT_INSTANT0(
         "input", "TouchEventQueue::MoveCoalesced", TRACE_EVENT_SCOPE_THREAD);
     coalesced_event_.CoalesceWith(event_with_latency);
-    events_.push_back(event_with_latency);
     return true;
   }
 
@@ -262,14 +263,24 @@ class CoalescedWebTouchEvent {
   }
 
   WebTouchEventWithLatencyList::iterator begin() {
+    DCHECK(!ignore_ack_);

[aelias_OOO_until_Jul13, 2014/04/22 01:56:39]

This is a bit ugly.  Instead of returning these iterators, can
CoalescedWebTouchEvent have higher level methods like ApplyLatencyInfo that
apply on the whole set (and do nothing if !ignore_ack_)?

[jdduke (slow), 2014/04/23 19:57:41]

Done.

+    return events_.begin();
+  }
+
+  WebTouchEventWithLatencyList::const_iterator begin() const {
+    DCHECK(!ignore_ack_);
     return events_.begin();
   }
 
   WebTouchEventWithLatencyList::iterator end() {
+    DCHECK(!ignore_ack_);
     return events_.end();
   }
 
-  size_t size() const { return events_.size(); }
+  WebTouchEventWithLatencyList::const_iterator end() const {
+    DCHECK(!ignore_ack_);
+    return events_.end();
+  }
 
   bool ignore_ack() const { return ignore_ack_; }
 
@@ -298,6 +309,8 @@ TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client,
       touchmove_slop_suppressor_(new TouchMoveSlopSuppressor(
           touchmove_suppression_length_dips + kSlopEpsilon)),
       absorbing_touch_moves_(false),
+      async_touch_moves_(false),
+      last_sent_touch_timestamp_(0),
       touch_scrolling_mode_(mode) {
   DCHECK(client);
 }
@@ -310,9 +323,11 @@ TouchEventQueue::~TouchEventQueue() {
 void TouchEventQueue::QueueEvent(const TouchEventWithLatencyInfo& event) {
   TRACE_EVENT0("input", "TouchEventQueue::QueueEvent");
 
+  DCHECK(!dispatching_touch_ack_);
+
   // If the queueing of |event| was triggered by an ack dispatch, defer
   // processing the event until the dispatch has finished.
-  if (touch_queue_.empty() && !dispatching_touch_ack_) {
+  if (touch_queue_.empty()) {
     // Optimization of the case without touch handlers.  Removing this path
     // yields identical results, but this avoids unnecessary allocations.
     if (touch_filtering_state_ == DROP_ALL_TOUCHES ||
@@ -378,31 +393,29 @@ void TouchEventQueue::TryForwardNextEventToRenderer() {
   // If there are queued touch events, then try to forward them to the renderer
   // immediately, or ACK the events back to the client if appropriate.
   while (!touch_queue_.empty()) {
-    const TouchEventWithLatencyInfo& touch =
-        touch_queue_.front()->coalesced_event();
-    PreFilterResult result = FilterBeforeForwarding(touch.event);
+    PreFilterResult result =
+        FilterBeforeForwarding(touch_queue_.front()->coalesced_event().event);

[Rick Byers, 2014/04/22 14:52:59]

Thanks for the cleanup here - new names/signatures make things a little clearer
IMHO.

     switch (result) {
       case ACK_WITH_NO_CONSUMER_EXISTS:
-        PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS,
-                              LatencyInfo());
+        PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
         break;
       case ACK_WITH_NOT_CONSUMED:
-        PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NOT_CONSUMED,
-                              LatencyInfo());
+        PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
         break;
       case FORWARD_TO_RENDERER:
-        ForwardToRenderer(touch);
+        ForwardNextEventToRenderer();
         return;
     }
   }
 }
 
-void TouchEventQueue::ForwardToRenderer(
-    const TouchEventWithLatencyInfo& touch) {
+void TouchEventQueue::ForwardNextEventToRenderer() {
   TRACE_EVENT0("input", "TouchEventQueue::ForwardToRenderer");
 
+  DCHECK(!empty());
   DCHECK(!dispatching_touch_);
   DCHECK_NE(touch_filtering_state_, DROP_ALL_TOUCHES);
+  TouchEventWithLatencyInfo touch = touch_queue_.front()->coalesced_event();
 
   if (WebTouchEventTraits::IsTouchSequenceStart(touch.event)) {
     touch_filtering_state_ =
@@ -410,15 +423,57 @@ void TouchEventQueue::ForwardToRenderer(
                              : FORWARD_ALL_TOUCHES;
     touch_ack_states_.clear();
     absorbing_touch_moves_ = false;
+    async_touch_moves_ = false;
+  }
+
+  if (async_touch_moves_ && touch.event.type == WebInputEvent::TouchEnd)
+    touch.event.cancelable = false;
+
+  if (async_touch_moves_ && touch.event.type == WebInputEvent::TouchMove) {
+    // If there are any events following this TouchMove, or sufficient time has
+    // past since sending the last touch event, flush any pending touch moves.

[aelias_OOO_until_Jul13, 2014/04/22 01:56:39]

nit: "past" -> "passed"

[jdduke (slow), 2014/04/23 19:57:41]

Done.

+    // TODO(jdduke): Should the touchmove be dropped if there are other pending

[aelias_OOO_until_Jul13, 2014/04/22 01:56:39]

I don't think it should be dropped.  I think it's a good property that the app
eventually gets the full touchmove motion.

[Rick Byers, 2014/04/22 14:52:59]

Agreed.  Coalescing according to our rules is the way we deal with event
overload - I don't think we want to drop beyond that.  The way this is written,
throttling follows the exact same rules as coalescing, right?  I.e. whenever you
see a touchstart or touchend you know you've received the latest touchmove
state.

I'm thinking of scenarios like dragging one finger then adding a second.  I
think ideally sites should be able to differentiate that from placing one finger
and mostly holding steady then placing another (even when there's some jank on
the thread). 

+    // events?
+    const bool send_touch_move_now =
+        size() > 1 ||
+        touch.event.timeStampSeconds >
+            last_sent_touch_timestamp_ + kAsyncTouchMoveIntervalS;
+
+    // If there's a pending touchmove, coalescewith the new event. Otherwise

[aelias_OOO_until_Jul13, 2014/04/22 01:56:39]

nit: missing space after "coalesce"

[jdduke (slow), 2014/04/23 19:57:41]

Done.

+    // create it if necessary for deferral.
+    if (pending_async_touch_move_) {
+      DCHECK(pending_async_touch_move_->CanCoalesceWith(touch));

[Rick Byers, 2014/04/22 14:52:59]

Is this really guaranteed to be true?  Eg. what if the modifiers change during a
touch stream?

[jdduke (slow), 2014/04/23 19:57:41]

Do our touch events use any modifiers? Android doesn't, not sure about Aura, but
yeah, definitely need to make this robust.

+      pending_async_touch_move_->CoalesceWith(touch);
+    } else {
+      if (!send_touch_move_now)
+        pending_async_touch_move_.reset(new TouchEventWithLatencyInfo(touch));
+    }
+
+    // Ack the event immediately, allowing associated gestures to be dispatched
+    // immediately.
+    if (!send_touch_move_now) {
+      DCHECK(pending_async_touch_move_);
+      PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+      return;
+    }
+
+    if (pending_async_touch_move_)
+      touch = *pending_async_touch_move_.Pass();
+
+    touch.event.cancelable = false;
   }
 
+  // Sending an event should clear any pending async touchmoves.
+  pending_async_touch_move_.reset();
+  last_sent_touch_timestamp_ = touch.event.timeStampSeconds;
+
   // A synchronous ack will reset |dispatching_touch_|, in which case
   // the touch timeout should not be started.
   base::AutoReset<bool> dispatching_touch(&dispatching_touch_, true);
   client_->SendTouchEventImmediately(touch);
   if (dispatching_touch_ &&
       touch_filtering_state_ == FORWARD_TOUCHES_UNTIL_TIMEOUT &&
-      ShouldTouchTypeTriggerTimeout(touch.event.type)) {
+      ShouldTouchTriggerTimeout(touch.event)) {
     DCHECK(timeout_handler_);
     timeout_handler_->Start(touch);
   }
@@ -429,6 +484,11 @@ void TouchEventQueue::OnGestureScrollEvent(
   if (gesture_event.event.type != blink::WebInputEvent::GestureScrollBegin)
     return;
 
+  if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE) {
+    pending_async_touch_move_.reset();
+    async_touch_moves_ = true;
+  }
+
   if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ABSORB_TOUCHMOVE)
     absorbing_touch_moves_ = true;
 
@@ -461,18 +521,26 @@ void TouchEventQueue::OnGestureScrollEvent(
 void TouchEventQueue::OnGestureEventAck(
     const GestureEventWithLatencyInfo& event,
     InputEventAckState ack_result) {
-  if (touch_scrolling_mode_ != TOUCH_SCROLLING_MODE_ABSORB_TOUCHMOVE)
-    return;
-
   if (event.event.type != blink::WebInputEvent::GestureScrollUpdate)
     return;
 
+  bool event_consumed = (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED);
+
+  if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE) {
+    async_touch_moves_ = event_consumed;
+    if (!async_touch_moves_) {
+      // TODO(jdduke): Should this be flushed?

[aelias_OOO_until_Jul13, 2014/04/22 01:56:39]

I think it should.

+      pending_async_touch_move_.reset();
+    }
+  }
+
   // Suspend sending touchmove events as long as the scroll events are handled.
   // Note that there's no guarantee that this ACK is for the most recent
   // gesture event (or even part of the current sequence).  Worst case, the
   // delay in updating the absorption state should only result in minor UI
   // glitches.
-  absorbing_touch_moves_ = (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED);
+  if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ABSORB_TOUCHMOVE)
+    absorbing_touch_moves_ = event_consumed;
 }
 
 void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) {
@@ -490,6 +558,7 @@ void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) {
     // TODO(jdduke): Synthesize a TouchCancel if necessary to update Blink touch
     // state tracking (e.g., if the touch handler was removed mid-sequence).
     touch_filtering_state_ = DROP_ALL_TOUCHES;
+    pending_async_touch_move_.reset();
     if (timeout_handler_)
       timeout_handler_->Reset();
     if (!touch_queue_.empty())
@@ -542,39 +611,58 @@ TouchEventQueue::GetLatestEventForTesting() const {
 void TouchEventQueue::FlushQueue() {
   DCHECK(!dispatching_touch_ack_);
   DCHECK(!dispatching_touch_);
+  pending_async_touch_move_.reset();
   if (touch_filtering_state_ != DROP_ALL_TOUCHES)
     touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE;
-  while (!touch_queue_.empty()) {
-    PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS,
-                          LatencyInfo());
-  }
+  while (!touch_queue_.empty())
+    PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
+}
+
+void TouchEventQueue::PopTouchEventToClient(
+    InputEventAckState ack_result) {
+  AckTouchEventToClient(ack_result, *PopTouchEvent());
 }
 
 void TouchEventQueue::PopTouchEventToClient(
     InputEventAckState ack_result,
     const LatencyInfo& renderer_latency_info) {
-  DCHECK(!dispatching_touch_ack_);
-  if (touch_queue_.empty())
+  scoped_ptr<CoalescedWebTouchEvent> acked_event = PopTouchEvent();
+  if (acked_event->ignore_ack())
     return;
-  scoped_ptr<CoalescedWebTouchEvent> acked_event(touch_queue_.front());
-  touch_queue_.pop_front();
+  for (WebTouchEventWithLatencyList::iterator iter = acked_event->begin(),
+       end = acked_event->end();
+       iter != end; ++iter) {
+    iter->latency.AddNewLatencyFrom(renderer_latency_info);
+  }
+  AckTouchEventToClient(ack_result, *acked_event);
+}
 
-  if (acked_event->ignore_ack())
+void TouchEventQueue::AckTouchEventToClient(
+    InputEventAckState ack_result,
+    const CoalescedWebTouchEvent& acked_event) {
+  DCHECK(!dispatching_touch_ack_);
+  if (acked_event.ignore_ack())
     return;
 
   // Note that acking the touch-event may result in multiple gestures being sent
   // to the renderer, or touch-events being queued.
-  base::AutoReset<CoalescedWebTouchEvent*>
-      dispatching_touch_ack(&dispatching_touch_ack_, acked_event.get());
+  base::AutoReset<const CoalescedWebTouchEvent*>
+      dispatching_touch_ack(&dispatching_touch_ack_, &acked_event);
 
-  for (WebTouchEventWithLatencyList::iterator iter = acked_event->begin(),
-       end = acked_event->end();
+  for (WebTouchEventWithLatencyList::const_iterator iter = acked_event.begin(),
+       end = acked_event.end();
        iter != end; ++iter) {
-    iter->latency.AddNewLatencyFrom(renderer_latency_info);
     client_->OnTouchEventAck((*iter), ack_result);
   }
 }
 
+scoped_ptr<CoalescedWebTouchEvent> TouchEventQueue::PopTouchEvent() {
+  DCHECK(!touch_queue_.empty());
+  scoped_ptr<CoalescedWebTouchEvent> event(touch_queue_.front());
+  touch_queue_.pop_front();
+  return event.Pass();
+}
+
 TouchEventQueue::PreFilterResult
 TouchEventQueue::FilterBeforeForwarding(const WebTouchEvent& event) {
   if (timeout_handler_ && timeout_handler_->FilterEvent(event))