Send touch cancel to renderer when scrolling starts

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 9d0394ef162d034a0f43ebe90cb34969011867de..dfa4916d92ca88d0f54cdfe8f76b13c72e168eba 100644
--- a/content/browser/renderer_host/input/touch_event_queue.cc
+++ b/content/browser/renderer_host/input/touch_event_queue.cc
@@ -20,7 +20,9 @@ typedef std::vector<TouchEventWithLatencyInfo> WebTouchEventWithLatencyList;
 class CoalescedWebTouchEvent {
  public:
   explicit CoalescedWebTouchEvent(const TouchEventWithLatencyInfo& event)
-      : coalesced_event_(event) {
+      : coalesced_event_(event),
+        ignore_ack_(false),
+        has_been_sent_to_renderer_(false) {
     events_.push_back(event);
     TRACE_EVENT_ASYNC_BEGIN0(
         "input", "TouchEventQueue::QueueEvent", this);
@@ -79,6 +81,14 @@ class CoalescedWebTouchEvent {
 
   size_t size() const { return events_.size(); }
 
+  bool ignore_ack() const { return ignore_ack_; }
+  void set_ignore_ack(bool value) { ignore_ack_ = value; }
+
+  bool has_been_sent_to_renderer() const { return has_been_sent_to_renderer_; }
+  void set_has_been_sent_to_renderer(bool value) {
+    has_been_sent_to_renderer_ = value;
+  }
+
  private:
   // This is the event that is forwarded to the renderer.
   TouchEventWithLatencyInfo coalesced_event_;
@@ -86,13 +96,20 @@ class CoalescedWebTouchEvent {
   // This is the list of the original events that were coalesced.
   WebTouchEventWithLatencyList events_;
 
+  // If |ignore_ack_| is true, don't send this touch event to client
+  // when the event is acked.
+  bool ignore_ack_;
+
+  // Whether the touch event has been sent to renderer.
+  bool has_been_sent_to_renderer_;
+
   DISALLOW_COPY_AND_ASSIGN(CoalescedWebTouchEvent);
 };
 
 TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client)
     : client_(client),
-      dispatching_touch_ack_(false),
-      no_touch_move_to_renderer_(false) {
+      dispatching_touch_ack_(NULL),
+      no_touch_to_renderer_(false) {
   DCHECK(client);
 }
 
@@ -158,8 +175,11 @@ void TouchEventQueue::TryForwardNextEventToRenderer() {
   while (!touch_queue_.empty()) {
     const TouchEventWithLatencyInfo& touch =
         touch_queue_.front()->coalesced_event();
+    if (touch_queue_.front()->has_been_sent_to_renderer())

[jdduke (slow), 2013/09/30 19:21:51]

You can remove this flag (or make it a DCHECK(!...), by restricting when you
call TryForwardNextEventToRenderer(), as mentioned below.

[Yufeng Shen (Slow to review), 2013/09/30 20:43:14]

removed.

+      break;
     if (ShouldForwardToRenderer(touch.event)) {
       client_->SendTouchEventImmediately(touch);
+      touch_queue_.front()->set_has_been_sent_to_renderer(true);
       break;
     }
     PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS,
@@ -167,6 +187,50 @@ void TouchEventQueue::TryForwardNextEventToRenderer() {
   }
 }
 
+void TouchEventQueue::OnGestureScrollEvent(
+    const GestureEventWithLatencyInfo& gesture_event) {
+  WebKit::WebInputEvent::Type type = gesture_event.event.type;
+  if (type == WebKit::WebInputEvent::GestureScrollBegin) {
+    // We assume the scroll event are generated synchronously from
+    // dispatching a touch event ack, so that we can fake a cancel
+    // event that has the correct touch ids as the touch event that
+    // is being acked. If not, we don't do the touch-cancel optimization.
+    if (no_touch_to_renderer_ || !dispatching_touch_ack_)
+      return;
+    no_touch_to_renderer_ = true;
+    // Fake a TouchCancel to cancel the touch points of the touch event
+    // that is currently being acked.
+    TouchEventWithLatencyInfo cancel_event =
+        dispatching_touch_ack_->coalesced_event();
+    cancel_event.event.type = WebKit::WebInputEvent::TouchCancel;
+    for (size_t i = 0; i < cancel_event.event.touchesLength; i++)
+      cancel_event.event.touches[i].state =
+          WebKit::WebTouchPoint::StateCancelled;
+    CoalescedWebTouchEvent* coalesced_cancel_event =
+        new CoalescedWebTouchEvent(cancel_event);
+    // Ignore the ack of the touch cancel so when it is acked, it won't get
+    // sent to gesture recognizer.
+    coalesced_cancel_event->set_ignore_ack(true);
+    // If the touch event queue is empty, or the first touch event in the queue
+    // has not been sent to renderer (i.e. not waiting for ack), we insert the
+    // touch cancel at the beginning of the queue to be the first to send.
+    if (touch_queue_.empty() ||
+        !touch_queue_.front()->has_been_sent_to_renderer()) {
+      touch_queue_.push_front(coalesced_cancel_event);
+    } else {
+      // If there there is touch event already sent to the renderer and waiting
+      // for ack, we insert the touch cancel after that touch event.

[sadrul, 2013/09/27 15:13:51]

+ "This cancel event will be dispatched to the renderer in response to the ACK
for the earlier event."

[Yufeng Shen (Slow to review), 2013/09/30 20:43:14]

see the comments below.

+      TouchQueue::iterator it = touch_queue_.begin();
+      it++;
+      touch_queue_.insert(it, coalesced_cancel_event);
+    }
+    TryForwardNextEventToRenderer();

[jdduke (slow), 2013/09/30 19:21:51]

I believe you can remove this call.

|dispatching_touch_ack_| is true means that there is no touch-event in-flight. 
So, the head of the queue has yet to be sent, and you can |.push_front()|
regardless of whether or not the queue is empty.

This also means that we're withing the scope of |ProcessTouchAck()|, which will
make the TryForwardNextEventToRenderer() call for you at the end.

[sadrul, 2013/09/30 20:33:24]

Client::OnTouchEventAck can cause another touch-event to be added to the queue
on android, right (i.e. the ACK for a touch-event can trigger another
touch-event to be added to the queue), in which case the front isn't always the
right place for the touch-cancel event?

[Yufeng Shen (Slow to review), 2013/09/30 20:43:14]

Right. So all the complication comes from that I was afraid the gesture scroll
events
can come async for chrome on android. If in most cases the gesture scroll events
are
generated synchronously from acking a touch event, then the flow is much
simpler. As
jdduke@ said, we can always insert the touch cancel at the beginning of the TEQ.

A few exceptions of chrome on android that gesture scroll does not come from
acking a touch event is:
1. No touch handler on the page at all. So touch events are directly sent to GR
to generate
touch scroll. In this case, |dispatching_touch_ack| would be false, and we don't
fake the
touch cancel, which makes sense since no touch is sent to renderer anyway.
2. There is timeout for getting touch ack, and android touch queue sends a touch
cancel to TEQ and start sending its touch events directly to GR and generate
touch scroll events. In this  case, |dispatching_touch_ack_| is false, so we
don't fake the touch cancel. And chrome on android 's own touch cancel
optimization naturally kicks in.

[sadrul, 2013/10/01 11:32:45]

Thanks for the detailed explanation. This makes sense.

+  } else if (type == WebKit::WebInputEvent::GestureScrollEnd ||
+             type == WebKit::WebInputEvent::GestureFlingStart) {
+    no_touch_to_renderer_ = false;
+  }
+}
+
 void TouchEventQueue::FlushQueue() {
   DCHECK(!dispatching_touch_ack_);
   while (!touch_queue_.empty())
@@ -190,9 +254,13 @@ void TouchEventQueue::PopTouchEventToClient(
   scoped_ptr<CoalescedWebTouchEvent> acked_event(touch_queue_.front());
   touch_queue_.pop_front();
 
+  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<bool> dispatching_touch_ack(&dispatching_touch_ack_, true);
+  base::AutoReset<CoalescedWebTouchEvent*>
+      dispatching_touch_ack(&dispatching_touch_ack_, acked_event.get());

[sadrul, 2013/09/27 15:13:51]

Hm, this would only work if the acked touch-event generates the scroll-gesture
synchronously, which is the case on cros/win, but is it also always the case on
android? If it is, then this would work. If not, we have to do this differently.
jdduke@: Can you confirm the behaviour on android?

(Sorry, I hadn't considered this during our earlier discussion).

[jdduke (slow), 2013/09/30 19:21:51]

Yup, that's generally the case on Android, and otherwise we'll either send our
own cancel via a timeout, or we never send touches to the TEQ.

I'd like to move the timeout here (behind a flag or something like that) so we
don't have to reason about it in multiple places. 

 
   base::TimeTicks now = base::TimeTicks::HighResNow();
   for (WebTouchEventWithLatencyList::const_iterator iter = acked_event->begin(),
@@ -208,14 +276,14 @@ void TouchEventQueue::PopTouchEventToClient(
 
 bool TouchEventQueue::ShouldForwardToRenderer(
     const WebKit::WebTouchEvent& event) const {
+  if (no_touch_to_renderer_ &&
+      event.type != WebKit::WebInputEvent::TouchCancel)
+    return false;
+
   // Touch press events should always be forwarded to the renderer.
   if (event.type == WebKit::WebInputEvent::TouchStart)
     return true;
 
-  if (event.type == WebKit::WebInputEvent::TouchMove &&
-      no_touch_move_to_renderer_)
-    return false;
-
   for (unsigned int i = 0; i < event.touchesLength; ++i) {
     const WebKit::WebTouchPoint& point = event.touches[i];
     // If a point has been stationary, then don't take it into account.