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..ca003ebee81d2dd5e69629cad6ce9d8c3b5ba281 100644
--- a/content/browser/renderer_host/input/touch_event_queue.cc
+++ b/content/browser/renderer_host/input/touch_event_queue.cc
@@ -21,6 +21,15 @@ using ui::LatencyInfo;
 namespace content {
 namespace {
 
+// Time interval at which touchmove events will be forwarded to the client while
+// scrolling is active and possible.
+const double kAsyncTouchMoveIntervalSec = .2;
+
+// A slop region just larger than that used by many sites. When touchmove's are
+// being sent asynchronously, movement outside this region will trigger an
+// immediate async touchmove to cancel any potential tap/press-related logic.
+const double kOuterSlopRegionLengthDipsSqared = 15. * 15.;

[Rick Byers, 2014/04/23 21:56:17]

How about we call this 'application slop region' to make it clear it's not
really some special region for chromium behavior (having two of our own is kinda
nuts), but attempting to track typical app behavior?

[jdduke (slow), 2014/04/23 22:56:48]

Done.

+
 // 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 +46,16 @@ TouchEventWithLatencyInfo ObtainCancelEventForTouchEvent(
   return event;
 }
 
-bool ShouldTouchTypeTriggerTimeout(WebInputEvent::Type type) {
-  return type == WebInputEvent::TouchStart ||
-         type == WebInputEvent::TouchMove;
+bool ShouldTouchTriggerTimeout(const WebTouchEvent& event) {
+  return (event.type == WebInputEvent::TouchStart ||
+          event.type == WebInputEvent::TouchMove) &&
+         !WebInputEventTraits::IgnoresAckDisposition(event);
+}
+
+bool OutsideOuterSlopRegion(const WebTouchEvent& event,
+                            const gfx::PointF& anchor) {
+  return (gfx::PointF(event.touches[0].position) - anchor).LengthSquared() >
+         kOuterSlopRegionLengthDipsSqared;
 }
 
 }  // namespace
@@ -61,7 +77,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_);
   }
@@ -76,7 +92,7 @@ class TouchEventQueue::TouchTimeoutHandler {
           SetPendingAckState(PENDING_ACK_CANCEL_EVENT);
           TouchEventWithLatencyInfo cancel_event =
               ObtainCancelEventForTouchEvent(timeout_event_);
-          touch_queue_->client_->SendTouchEventImmediately(cancel_event);
+          touch_queue_->SendTouchEventImmediately(cancel_event);
         } else {
           SetPendingAckState(PENDING_ACK_NONE);
           touch_queue_->UpdateTouchAckStates(timeout_event_.event, ack_result);
@@ -226,25 +242,26 @@ class TouchEventQueue::TouchMoveSlopSuppressor {
 // the Client receives the event with their original timestamp.
 class CoalescedWebTouchEvent {
  public:
-  CoalescedWebTouchEvent(const TouchEventWithLatencyInfo& event,
-                         bool ignore_ack)
+  CoalescedWebTouchEvent(const TouchEventWithLatencyInfo& event, bool async)
       : coalesced_event_(event),
-        ignore_ack_(ignore_ack) {
-    events_.push_back(event);
-    TRACE_EVENT_ASYNC_BEGIN0(
-        "input", "TouchEventQueue::QueueEvent", this);
+        async_(async) {
+    if (async)
+      coalesced_event_.event.cancelable = false;
+    else
+      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
   // the event was coalesced.
   bool CoalesceEventIfPossible(
       const TouchEventWithLatencyInfo& event_with_latency) {
-    if (ignore_ack_)
+    if (async_)
       return false;
 
     if (!coalesced_event_.CanCoalesceWith(event_with_latency))
@@ -257,21 +274,35 @@ class CoalescedWebTouchEvent {
     return true;
   }
 
-  const TouchEventWithLatencyInfo& coalesced_event() const {
-    return coalesced_event_;
-  }
+  void UpdateLatencyInfo(const ui::LatencyInfo& renderer_latency_info) {

[Rick Byers, 2014/04/23 21:56:17]

Maybe 'updateLatencyInfoForAck'?  Otherwise it's not clear why we should skip
async_ events.

[jdduke (slow), 2014/04/23 22:56:48]

Done.

+    if (async_)
+      return;
 
-  WebTouchEventWithLatencyList::iterator begin() {
-    return events_.begin();
+    for (WebTouchEventWithLatencyList::iterator iter = events_.begin(),
+                                                end = events_.end();
+         iter != end;
+         ++iter) {
+      iter->latency.AddNewLatencyFrom(renderer_latency_info);
+    }
   }
 
-  WebTouchEventWithLatencyList::iterator end() {
-    return events_.end();
-  }
+  void DispatchAckToClient(InputEventAckState ack_result,
+                           TouchEventQueueClient* client) {
+    DCHECK(client);
+    if (async_)
+      return;
 
-  size_t size() const { return events_.size(); }
+    for (WebTouchEventWithLatencyList::const_iterator iter = events_.begin(),
+                                                      end = events_.end();
+         iter != end;
+         ++iter) {
+      client->OnTouchEventAck(*iter, ack_result);
+    }
+  }
 
-  bool ignore_ack() const { return ignore_ack_; }
+  const TouchEventWithLatencyInfo& coalesced_event() const {
+    return coalesced_event_;
+  }
 
  private:
   // This is the event that is forwarded to the renderer.
@@ -280,9 +311,8 @@ class CoalescedWebTouchEvent {
   // This is the list of the original events that were coalesced.
   WebTouchEventWithLatencyList events_;

[aelias_OOO_until_Jul13, 2014/04/23 21:42:22]

Could you make this a scoped_ptr<> and only create the object when async is
false?  Then, you won't need to hold the separate async_ bool but just check
this for existence.

[jdduke (slow), 2014/04/23 22:56:48]

Eh, I guess.  But then we're double heap allocating whenever the event is not
async.  I guess we could use an empty vector check instead, then we don't pay
for heap allocation or the bool... ?  I'll use a proper function name so it's
clear what's going on.

 
-  // If |ignore_ack_| is true, don't send this touch event to client
-  // when the event is acked.
-  bool ignore_ack_;
+  // If |async_| is true, the touch event will not be ack'ed to the client.
+  bool async_;
 
   DISALLOW_COPY_AND_ASSIGN(CoalescedWebTouchEvent);
 };
@@ -297,7 +327,8 @@ TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client,
       ack_timeout_enabled_(false),
       touchmove_slop_suppressor_(new TouchMoveSlopSuppressor(
           touchmove_suppression_length_dips + kSlopEpsilon)),
-      absorbing_touch_moves_(false),
+      async_touch_moves_(false),
+      last_sent_touch_timestamp_sec_(0),
       touch_scrolling_mode_(mode) {
   DCHECK(client);
 }
@@ -368,7 +399,6 @@ void TouchEventQueue::ProcessTouchAck(InputEventAckState ack_result,
     touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE;
   }
 
-  UpdateTouchAckStates(acked_event, ack_result);
   PopTouchEventToClient(ack_result, latency_info);
   TryForwardNextEventToRenderer();
 }
@@ -378,47 +408,94 @@ 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);
     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) {
-  TRACE_EVENT0("input", "TouchEventQueue::ForwardToRenderer");
+void TouchEventQueue::ForwardNextEventToRenderer() {
+  TRACE_EVENT0("input", "TouchEventQueue::ForwardNextEventToRenderer");
 
+  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_ =
         ack_timeout_enabled_ ? FORWARD_TOUCHES_UNTIL_TIMEOUT
                              : FORWARD_ALL_TOUCHES;
     touch_ack_states_.clear();
-    absorbing_touch_moves_ = false;
+    async_touch_moves_ = false;
+    touch_sequence_start_position_ =
+        gfx::PointF(touch.event.touches[0].position);
+  }
+
+  if (async_touch_moves_ && touch.event.type == WebInputEvent::TouchMove) {
+    // Only forward the new touhcmove if there are additional pending events,
+    // or sufficient time has passed since sending the last touch event.

[Rick Byers, 2014/04/23 21:56:17]

update comment to mention application slop region.  Or better: the code is
pretty self-explanatory as to the 'what', the comment should focus on why - i.e.
throttling to minimize cpu usage to minimize risk of scroll jank but also
trigger app tap-detection logic.

[jdduke (slow), 2014/04/23 22:56:48]

Done.

+    const bool send_touch_move_now =
+        size() > 1 ||
+        touch.event.timeStampSeconds >=
+            last_sent_touch_timestamp_sec_ + kAsyncTouchMoveIntervalSec ||
+        (needs_async_touch_move_for_outer_slop_region_ &&
+         OutsideOuterSlopRegion(touch.event, touch_sequence_start_position_));
+
+    if (!send_touch_move_now) {
+      // If the touchmoves cannot be coalesced (e.g., different modifiers), the
+      // new touchmove will simply replace the pending touchmove.

[aelias_OOO_until_Jul13, 2014/04/23 21:42:22]

This looks like it destroys information.  Should we flush the old touchmove
instead, even if it hasn't been 200ms (after all that's just a heuristic)?

[Rick Byers, 2014/04/23 21:58:13]

I like that idea.  Eg. if a modifier goes down and then quickly up during a
touch event sequence, the app should be able to see that (without watching the
key events themselves).

[jdduke (slow), 2014/04/23 22:56:48]

If the app relies on async touches for modifier updates, they've already lost
=/.  However, I have no problem forcing a flush here.

+      if (!pending_async_touch_move_ ||
+          !pending_async_touch_move_->CanCoalesceWith(touch)) {
+        pending_async_touch_move_.reset(new TouchEventWithLatencyInfo(touch));
+      } else {
+        pending_async_touch_move_->CoalesceWith(touch);
+      }
+      PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+      return;
+    }
+  }
+
+  last_sent_touch_timestamp_sec_ = touch.event.timeStampSeconds;
+
+  // Flush any pending async touch move. If it can be combined with the current
+  // (touchmove) event, great, otherwise send it immediately but separately. Its
+  // ack will trigger forwarding of the original |touch| event.
+  if (pending_async_touch_move_) {
+    if (pending_async_touch_move_->CanCoalesceWith(touch)) {
+      pending_async_touch_move_->CoalesceWith(touch);
+      pending_async_touch_move_->event.cancelable = !async_touch_moves_;
+      touch = *pending_async_touch_move_.Pass();
+    } else {
+      scoped_ptr<TouchEventWithLatencyInfo> async_move =
+          pending_async_touch_move_.Pass();
+      async_move->event.cancelable = false;
+      touch_queue_.push_front(new CoalescedWebTouchEvent(*async_move, true));
+      SendTouchEventImmediately(*async_move);
+      return;
+    }
   }
 
+  if (async_touch_moves_)
+    touch.event.cancelable = false;

[Rick Byers, 2014/04/23 21:56:17]

So you're intentionally making new touchstart events async too, right?  Probably
need a comment saying we may want to revisit this (but we definitely want to
send touchend async).  

Perhaps async_touch_moves_ should be renamed something like
send_touch_events_async?

[jdduke (slow), 2014/04/23 22:56:48]

Yeah, I think that makes the most sense at this point.

+
   // 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);
+  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,8 +506,12 @@ void TouchEventQueue::OnGestureScrollEvent(
   if (gesture_event.event.type != blink::WebInputEvent::GestureScrollBegin)
     return;
 
-  if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ABSORB_TOUCHMOVE)
-    absorbing_touch_moves_ = true;
+  if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE) {
+    pending_async_touch_move_.reset();
+    async_touch_moves_ = true;
+    needs_async_touch_move_for_outer_slop_region_ = true;
+    return;
+  }
 
   if (touch_scrolling_mode_ != TOUCH_SCROLLING_MODE_TOUCHCANCEL)
     return;
@@ -461,18 +542,21 @@ void TouchEventQueue::OnGestureScrollEvent(
 void TouchEventQueue::OnGestureEventAck(
     const GestureEventWithLatencyInfo& event,
     InputEventAckState ack_result) {
-  if (touch_scrolling_mode_ != TOUCH_SCROLLING_MODE_ABSORB_TOUCHMOVE)
+  if (touch_scrolling_mode_ != TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE)
     return;
 
   if (event.event.type != blink::WebInputEvent::GestureScrollUpdate)
     return;
 
-  // Suspend sending touchmove events as long as the scroll events are handled.
+  // Throttle 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);
+  // delay in updating the absorption state will result in minor UI glitches.
+  // A valid |pending_async_touch_move_| will be flushed when the next event is
+  // forwarded.
+  async_touch_moves_ = (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED);
+  if (!async_touch_moves_)
+    needs_async_touch_move_for_outer_slop_region_ = false;
 }
 
 void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) {
@@ -490,6 +574,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())
@@ -530,6 +615,10 @@ void TouchEventQueue::SetAckTimeoutEnabled(bool enabled,
     timeout_handler_->set_timeout_delay(ack_timeout_delay);
 }
 
+bool TouchEventQueue::HasPendingAsyncTouchMoveForTesting() const {
+  return pending_async_touch_move_;
+}
+
 bool TouchEventQueue::IsTimeoutRunningForTesting() const {
   return timeout_handler_ && timeout_handler_->IsTimeoutTimerRunning();
 }
@@ -542,37 +631,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())
-    return;
-  scoped_ptr<CoalescedWebTouchEvent> acked_event(touch_queue_.front());
-  touch_queue_.pop_front();
+ scoped_ptr<CoalescedWebTouchEvent> acked_event = PopTouchEvent();
+ acked_event->UpdateLatencyInfo(renderer_latency_info);
+ AckTouchEventToClient(ack_result, acked_event.Pass());
+}
 
-  if (acked_event->ignore_ack())
-    return;
+void TouchEventQueue::AckTouchEventToClient(
+    InputEventAckState ack_result,
+    scoped_ptr<CoalescedWebTouchEvent> acked_event) {
+  DCHECK(acked_event);
+  DCHECK(!dispatching_touch_ack_);
+  UpdateTouchAckStates(acked_event->coalesced_event().event, ack_result);
 
   // Note that acking the touch-event may result in multiple gestures being sent
   // to the renderer, or touch-events being queued.
-  base::AutoReset<CoalescedWebTouchEvent*>
+  base::AutoReset<const CoalescedWebTouchEvent*>
       dispatching_touch_ack(&dispatching_touch_ack_, acked_event.get());
+  acked_event->DispatchAckToClient(ack_result, client_);
+}
 
-  for (WebTouchEventWithLatencyList::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();
+}
+
+void TouchEventQueue::SendTouchEventImmediately(
+    const TouchEventWithLatencyInfo& touch) {
+  if (needs_async_touch_move_for_outer_slop_region_) {
+    // Any event other than a touchmove (e.g., touchcancel or secondary
+    // touchstart) after a scroll has started will interrupt the need to send a
+    // an outer slop-region exceeding touchmove.
+    if (touch.event.type != WebInputEvent::TouchMove ||
+        OutsideOuterSlopRegion(touch.event, touch_sequence_start_position_))
+      needs_async_touch_move_for_outer_slop_region_ = false;
   }
+
+  client_->SendTouchEventImmediately(touch);
 }
 
 TouchEventQueue::PreFilterResult
@@ -593,9 +703,6 @@ TouchEventQueue::FilterBeforeForwarding(const WebTouchEvent& event) {
     return ACK_WITH_NOT_CONSUMED;
   }
 
-  if (absorbing_touch_moves_ && event.type == WebInputEvent::TouchMove)
-    return ACK_WITH_NOT_CONSUMED;
-
   // Touch press events should always be forwarded to the renderer.
   if (event.type == WebInputEvent::TouchStart)
     return FORWARD_TO_RENDERER;