Defer GestureTapDown events briefly.

GestureTapDown events are always handled by the WK thread. They are
frequently followed by a GestureScrollStart which can be handled on the
compositor thread. In this case, the GTD event is unnecessary and its only
side effect is to make the compositor thread block on the WK thread. This
makes touchscreen scrolling sluggish if the WK thread is busy.

This change defers sending the GTD briefly so that it can be suppressed
if the GTD event is part of a scroll gesture. This increases the likelyhood
that a scroll gesture can be successfully handled on the compositor thread.

BUG=137555
TESTS= by hand, by unit test.


Review URL: https://chromiumcodereview.appspot.com/10855200

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152230 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/renderer_host/gesture_event_filter.cc

@@ -12,6 +12,13 @@ using WebKit::WebInputEvent;
 
 namespace content {
 namespace {
+
+// Default maximum time between the GestureRecognizer generating a
+// GestureTapDown and when it is forwarded to the renderer.
+// TODO(rjkroege): Make this configurable.
+static const int kMaxiumTapGapTimeMs = 100;
+
+// TODO(rjkroege): Coalesce pinch updates.
 // Returns |true| if two gesture events should be coalesced.
 bool ShouldCoalesceGestureEvents(const WebKit::WebGestureEvent& last_event,
                                  const WebKit::WebGestureEvent& new_event) {
@@ -24,15 +31,14 @@ bool ShouldCoalesceGestureEvents(const WebKit::WebGestureEvent& last_event,
 GestureEventFilter::GestureEventFilter(RenderWidgetHostImpl* rwhv)
      : render_widget_host_(rwhv),
        fling_in_progress_(false),
-       gesture_event_pending_(false),
-       tap_suppression_controller_(new TapSuppressionController(rwhv)) {
+       tap_suppression_controller_(new TapSuppressionController(rwhv)),
+       maximum_tap_gap_time_ms_(kMaxiumTapGapTimeMs) {
 }
 
 GestureEventFilter::~GestureEventFilter() { }
 
 bool GestureEventFilter::ShouldDiscardFlingCancelEvent(
     const WebKit::WebGestureEvent& gesture_event) {
-  DCHECK(gesture_event.type == WebInputEvent::GestureFlingCancel);
   if (coalesced_gesture_events_.empty() && fling_in_progress_)
     return false;
   GestureEventQueue::reverse_iterator it =
@@ -47,59 +53,64 @@ bool GestureEventFilter::ShouldDiscardFlingCancelEvent(
   return true;
 }
 
+// TODO(rjkroege): separate touchpad and touchscreen events.
 bool GestureEventFilter::ShouldForward(const WebGestureEvent& gesture_event) {
-  if (gesture_event.type == WebInputEvent::GestureFlingCancel) {
-    if (ShouldDiscardFlingCancelEvent(gesture_event))
+  switch (gesture_event.type) {
+    case WebInputEvent::GestureFlingCancel:
+      if (!ShouldDiscardFlingCancelEvent(gesture_event)) {
+        coalesced_gesture_events_.push_back(gesture_event);
+        fling_in_progress_ = false;
+        return ShouldHandleEventNow();
+      }
       return false;
-    fling_in_progress_ = false;
-  }
-
-  if (gesture_event_pending_) {
-    if (coalesced_gesture_events_.empty() ||
-        !ShouldCoalesceGestureEvents(coalesced_gesture_events_.back(),
-                                     gesture_event)) {
-     coalesced_gesture_events_.push_back(gesture_event);
-    } else {
-      WebGestureEvent* last_gesture_event =
-          &coalesced_gesture_events_.back();
-      last_gesture_event->deltaX += gesture_event.deltaX;
-      last_gesture_event->deltaY += gesture_event.deltaY;
-      DCHECK_GE(gesture_event.timeStampSeconds,
-                last_gesture_event->timeStampSeconds);
-      last_gesture_event->timeStampSeconds = gesture_event.timeStampSeconds;
-    }
-    return false;
-  }
-  gesture_event_pending_ = true;
-
-  if (gesture_event.type == WebInputEvent::GestureFlingCancel) {
-    tap_suppression_controller_->GestureFlingCancel(
-        gesture_event.timeStampSeconds);
-  } else if (gesture_event.type == WebInputEvent::GestureFlingStart) {
-    fling_in_progress_ = true;
+    case WebInputEvent::GestureFlingStart:
+      fling_in_progress_ = true;
+      coalesced_gesture_events_.push_back(gesture_event);
+      return ShouldHandleEventNow();
+    case WebInputEvent::GestureTapDown:
+      deferred_tap_down_event_ = gesture_event;
+      send_gtd_timer_.Start(FROM_HERE,
+          base::TimeDelta::FromMilliseconds(maximum_tap_gap_time_ms_),
+          this,
+          &GestureEventFilter::SendGestureTapDownNow);
+      return false;
+    case WebInputEvent::GestureTap:
+      send_gtd_timer_.Stop();
+      coalesced_gesture_events_.push_back(deferred_tap_down_event_);
+      if (ShouldHandleEventNow()) {
+          render_widget_host_->ForwardGestureEventImmediately(
+              deferred_tap_down_event_);
+      }
+      coalesced_gesture_events_.push_back(gesture_event);
+      return false;
+    case WebInputEvent::GestureScrollBegin:
+    case WebInputEvent::GesturePinchBegin:
+      send_gtd_timer_.Stop();
+      coalesced_gesture_events_.push_back(gesture_event);
+      return ShouldHandleEventNow();
+    case WebInputEvent::GestureScrollUpdate:
+      MergeOrInsertScrollEvent(gesture_event);
+      return ShouldHandleEventNow();
+    default:
+      coalesced_gesture_events_.push_back(gesture_event);
+      return ShouldHandleEventNow();
   }
 
-  return true;
+  NOTREACHED();
+  return false;
 }
 
 void GestureEventFilter::Reset() {
   fling_in_progress_ = false;
   coalesced_gesture_events_.clear();
-  gesture_event_pending_ = false;
+  // TODO(rjkroege): Reset the tap suppression controller.
 }
 
 void GestureEventFilter::ProcessGestureAck(bool processed, int type) {
-  if (type == WebInputEvent::GestureFlingCancel)
-    tap_suppression_controller_->GestureFlingCancelAck(processed);
-
-  gesture_event_pending_ = false;
-
-  // Now send the next (coalesced) gesture event.
+  coalesced_gesture_events_.pop_front();
   if (!coalesced_gesture_events_.empty()) {
-    WebGestureEvent next_gesture_event =
-        coalesced_gesture_events_.front();
-    coalesced_gesture_events_.pop_front();
-    render_widget_host_->ForwardGestureEvent(next_gesture_event);
+    WebGestureEvent next_gesture_event = coalesced_gesture_events_.front();
+    render_widget_host_->ForwardGestureEventImmediately(next_gesture_event);
   }
 }
 
@@ -111,4 +122,36 @@ void GestureEventFilter::FlingHasBeenHalted() {
   fling_in_progress_ = false;
 }
 
+bool GestureEventFilter::ShouldHandleEventNow() {
+  return coalesced_gesture_events_.size() == 1;
+}
+
+void GestureEventFilter::SendGestureTapDownNow() {
+  coalesced_gesture_events_.push_back(deferred_tap_down_event_);
+  if (ShouldHandleEventNow()) {
+      render_widget_host_->ForwardGestureEventImmediately(
+          deferred_tap_down_event_);
+  }
+}
+
+void GestureEventFilter::MergeOrInsertScrollEvent(
+    const WebGestureEvent& gesture_event) {
+  WebGestureEvent* last_gesture_event = coalesced_gesture_events_.empty() ? 0 :
+      &coalesced_gesture_events_.back();
+  if (coalesced_gesture_events_.size() > 1 &&
+      last_gesture_event->type == gesture_event.type &&
+      last_gesture_event->modifiers == gesture_event.modifiers) {
+    last_gesture_event->deltaX += gesture_event.deltaX;
+    last_gesture_event->deltaY += gesture_event.deltaY;
+    DLOG_IF(WARNING,
+            gesture_event.timeStampSeconds >=
+            last_gesture_event->timeStampSeconds)
+            << "Event time not monotonic?\n";
+    DCHECK(last_gesture_event->type == WebInputEvent::GestureScrollUpdate);
+    last_gesture_event->timeStampSeconds = gesture_event.timeStampSeconds;
+  } else {
+    coalesced_gesture_events_.push_back(gesture_event);
+  }
+}
+
 } // namespace content

content/browser/renderer_host/gesture_event_filter.h

@@ -9,6 +9,8 @@
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/time.h"
+#include "base/timer.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 
 class MockRenderWidgetHost;
@@ -48,32 +50,50 @@ class GestureEventFilter {
  private:
   friend class ::MockRenderWidgetHost;
 
+  // Invoked on the expiration of the timer to release a deferred
+  // GestureTapDown to the renderer.
+  void SendGestureTapDownNow();
+
   // Returns |true| if the given GestureFlingCancel should be discarded
   // as unnecessary.
   bool ShouldDiscardFlingCancelEvent(
     const WebKit::WebGestureEvent& gesture_event);
 
+  // Return true if the only event in the queue is the current event and
+  // hence that event should be handled now.
+  bool ShouldHandleEventNow();
+
+  // Merge a GestureScrollUpdate into the queue if possible or append
+  // it to the queue.
+  void MergeOrInsertScrollEvent(
+       const WebKit::WebGestureEvent& gesture_event);
+
   // Only a RenderWidgetHostViewImpl can own an instance.
   RenderWidgetHostImpl* render_widget_host_;
 
-  // True if a GestureFlingStart is in progress on the renderer.
+  // True if a GestureFlingStart is in progress on the renderer or
+  // queued without a subsequent queued GestureFlingCancel event.
   bool fling_in_progress_;
 
-  // (Similar to |mouse_wheel_pending_|.). True if gesture event was sent and
-  // we are waiting for a corresponding ack.
-  bool gesture_event_pending_;
+  // Timer to release a previously deferred GestureTapDown event.
+  base::OneShotTimer<GestureEventFilter> send_gtd_timer_;
 
   // An object tracking the state of touchpad action on the delivery of mouse
-  // events to the renderer to filter mouse actiosn immediately after a touchpad
+  // events to the renderer to filter mouse  immediately after a touchpad
   // fling canceling tap.
   scoped_ptr<TapSuppressionController> tap_suppression_controller_;
 
   typedef std::deque<WebKit::WebGestureEvent> GestureEventQueue;
 
-  // (Similar to |coalesced_mouse_wheel_events_|.) GestureScrollUpdate events
-  // are coalesced by merging deltas in a similar fashion as wheel events.
+  // Queue of coalesced gesture events not yet sent to the renderer.
   GestureEventQueue coalesced_gesture_events_;
 
+  // Tap gesture event currently subject to deferral.
+  WebKit::WebGestureEvent deferred_tap_down_event_;
+
+  // Time window in which to defer a GestureTapDown.
+  int maximum_tap_gap_time_ms_;
+
   DISALLOW_COPY_AND_ASSIGN(GestureEventFilter);
 };
 

content/browser/renderer_host/render_widget_host_impl.cc

@@ -928,6 +928,13 @@ void RenderWidgetHostImpl::ForwardGestureEvent(
   ForwardInputEvent(gesture_event, sizeof(WebGestureEvent), false);
 }
 
+void RenderWidgetHostImpl::ForwardGestureEventImmediately(
+    const WebKit::WebGestureEvent& gesture_event) {
+  if (ignore_input_events_ || process_->IgnoreInputEvents())
+    return;
+  ForwardInputEvent(gesture_event, sizeof(WebGestureEvent), false);
+}
+
 void RenderWidgetHostImpl::ForwardKeyboardEvent(
     const NativeWebKeyboardEvent& key_event) {
   TRACE_EVENT0("renderer_host", "RenderWidgetHostImpl::ForwardKeyboardEvent");

content/browser/renderer_host/render_widget_host_impl.h

@@ -232,6 +232,10 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost,
   void ForwardGestureEvent(const WebKit::WebGestureEvent& gesture_event);
   virtual void ForwardTouchEvent(const WebKit::WebTouchEvent& touch_event);
 
+  // Forwards the given event immediately to the renderer.
+  void ForwardGestureEventImmediately(
+      const WebKit::WebGestureEvent& gesture_event);
+
 #if defined(TOOLKIT_GTK)
   // Give key press listeners a chance to handle this key press. This allow
   // widgets that don't have focus to still handle key presses.

content/browser/renderer_host/render_widget_host_unittest.cc

@@ -247,6 +247,10 @@ class MockRenderWidgetHost : public RenderWidgetHostImpl {
     return gesture_event_filter_->fling_in_progress_;
   }
 
+  void set_maximum_tap_gap_time_ms(int delay_ms) {
+    gesture_event_filter_->maximum_tap_gap_time_ms_ = delay_ms;
+  }
+
  protected:
   virtual void NotifyRendererUnresponsive() OVERRIDE {
     unresponsive_timer_fired_ = true;
@@ -809,6 +813,7 @@ TEST_F(RenderWidgetHostTest, CoalescesGesturesEvents) {
 
   // Make sure that the queue contains what we think it should.
   WebGestureEvent merged_event = host_->GestureEventLastQueueEvent();
+  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
   EXPECT_EQ(WebInputEvent::GestureScrollUpdate, merged_event.type);
 
   // Coalesced.
@@ -895,7 +900,7 @@ TEST_F(RenderWidgetHostTest, GestureFlingCancelsFiltered) {
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingCancel);
   EXPECT_FALSE(host_->FlingInProgress());
   EXPECT_EQ(1U, process_->sink().message_count());
-  EXPECT_EQ(1U, host_->GestureEventLastQueueEventSize());
+  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
 
   // Advance state realistically.
   SendInputEventACK(WebInputEvent::GestureFlingStart, true);
@@ -908,45 +913,111 @@ TEST_F(RenderWidgetHostTest, GestureFlingCancelsFiltered) {
   process_->sink().ClearMessages();
   SimulateGestureEvent(8, -7, 0, WebInputEvent::GestureScrollUpdate);
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingStart);
+  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
   EXPECT_EQ(1U, process_->sink().message_count());
   WebGestureEvent merged_event = host_->GestureEventLastQueueEvent();
   EXPECT_EQ(WebInputEvent::GestureFlingStart, merged_event.type);
-  EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(1U, host_->GestureEventLastQueueEventSize());
+  EXPECT_TRUE(host_->FlingInProgress());
+  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
 
   // GFS in queue means that a GFC is added to the queue
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingCancel);
   merged_event =host_->GestureEventLastQueueEvent();
   EXPECT_EQ(WebInputEvent::GestureFlingCancel, merged_event.type);
   EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
-
+  EXPECT_EQ(3U, host_->GestureEventLastQueueEventSize());
 
   // Adding a second GFC is dropped.
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingCancel);
   EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
+  EXPECT_EQ(3U, host_->GestureEventLastQueueEventSize());
 
   // Adding another GFS will add it to the queue.
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingStart);
   merged_event = host_->GestureEventLastQueueEvent();
   EXPECT_EQ(WebInputEvent::GestureFlingStart, merged_event.type);
-  EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(3U, host_->GestureEventLastQueueEventSize());
+  EXPECT_TRUE(host_->FlingInProgress());
+  EXPECT_EQ(4U, host_->GestureEventLastQueueEventSize());
 
   // GFS in queue means that a GFC is added to the queue
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingCancel);
   merged_event = host_->GestureEventLastQueueEvent();
   EXPECT_EQ(WebInputEvent::GestureFlingCancel, merged_event.type);
   EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(4U, host_->GestureEventLastQueueEventSize());
+  EXPECT_EQ(5U, host_->GestureEventLastQueueEventSize());
 
   // Adding another GFC with a GFC already there is dropped.
   SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureFlingCancel);
   merged_event = host_->GestureEventLastQueueEvent();
   EXPECT_EQ(WebInputEvent::GestureFlingCancel, merged_event.type);
   EXPECT_FALSE(host_->FlingInProgress());
-  EXPECT_EQ(4U, host_->GestureEventLastQueueEventSize());
+  EXPECT_EQ(5U, host_->GestureEventLastQueueEventSize());
+}
+
+// Test that GestureTapDown events are deferred.
+TEST_F(RenderWidgetHostTest, DeferredGestureTapDown) {
+  process_->sink().ClearMessages();
+
+  // Set some sort of short deferral timeout
+  host_->set_maximum_tap_gap_time_ms(5);
+
+  SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureTapDown);
+  EXPECT_EQ(0U, process_->sink().message_count());
+  EXPECT_EQ(0U, host_->GestureEventLastQueueEventSize());
+
+  // Wait long enough for first timeout and see if it fired.
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, MessageLoop::QuitClosure(), TimeDelta::FromMilliseconds(10));
+  MessageLoop::current()->Run();
+
+  EXPECT_EQ(1U, process_->sink().message_count());
+  EXPECT_EQ(1U, host_->GestureEventLastQueueEventSize());
+}
+
+// Test that GestureTapDown events are sent immediately on GestureTap.
+TEST_F(RenderWidgetHostTest, DeferredGestureTapDownSentOnTap) {
+  process_->sink().ClearMessages();
+
+  // Set some sort of short deferral timeout
+  host_->set_maximum_tap_gap_time_ms(5);
+
+  SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureTapDown);
+  EXPECT_EQ(0U, process_->sink().message_count());
+  EXPECT_EQ(0U, host_->GestureEventLastQueueEventSize());
+
+  SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureTap);
+  EXPECT_EQ(1U, process_->sink().message_count());
+  EXPECT_EQ(2U, host_->GestureEventLastQueueEventSize());
+
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, MessageLoop::QuitClosure(), TimeDelta::FromMilliseconds(10));
+  MessageLoop::current()->Run();
+
+  // If the deferral timer incorrectly fired, it sent an extra message.
+  EXPECT_EQ(1U, process_->sink().message_count());
+}
+
+// Test that scroll events during the deferral internal drop the GestureTapDown.
+TEST_F(RenderWidgetHostTest, DeferredGestureTapDownAnulledOnScroll) {
+  process_->sink().ClearMessages();
+
+  // Set some sort of short deferral timeout
+  host_->set_maximum_tap_gap_time_ms(5);
+
+  SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureTapDown);
+  EXPECT_EQ(0U, process_->sink().message_count());
+  EXPECT_EQ(0U, host_->GestureEventLastQueueEventSize());
+
+  SimulateGestureEvent(0, -10, 0, WebInputEvent::GestureScrollBegin);
+  EXPECT_EQ(1U, process_->sink().message_count());
+  EXPECT_EQ(1U, host_->GestureEventLastQueueEventSize());
+
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, MessageLoop::QuitClosure(), TimeDelta::FromMilliseconds(10));
+  MessageLoop::current()->Run();
+
+  // If the deferral timer incorrectly fired, it will send an extra message.
+  EXPECT_EQ(1U, process_->sink().message_count());
 }
 
 // Test that the hang monitor timer expires properly if a new timer is started