Defer GestureTapDown events briefly.

content/browser/renderer_host/gesture_event_filter.cc

Index: content/browser/renderer_host/gesture_event_filter.cc
diff --git a/content/browser/renderer_host/gesture_event_filter.cc b/content/browser/renderer_host/gesture_event_filter.cc
index 3c61cd412758e8c06fcadbc3d8dd98fc86479167..ab9eeee55d1d7f9255d0f1bc081cf6f916f7f554 100644
--- a/content/browser/renderer_host/gesture_event_filter.cc
+++ b/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,63 @@ 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))
-      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;
+  switch (gesture_event.type) {
+  case WebInputEvent::GestureFlingCancel:

[sadrul, 2012/08/16 16:08:15]

indent!

[rjkroege, 2012/08/16 17:12:26]

Done.

+    if (!ShouldDiscardFlingCancelEvent(gesture_event)) {
+      coalesced_gesture_events_.push_back(gesture_event);
+      fling_in_progress_ = false;
+      return ShouldHandleEventNow();
     }
     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) {
+  case WebInputEvent::GestureFlingStart:
     fling_in_progress_ = true;
+    coalesced_gesture_events_.push_back(gesture_event);
+    return ShouldHandleEventNow();
+  case WebInputEvent::GestureTapDown:
+    deferred_tap_gesture_event_ = gesture_event;

[sadrul, 2012/08/16 16:08:15]

Let's call it deferred_tap_down_event_

[rjkroege, 2012/08/16 17:12:26]

Done.

+    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_gesture_event_);
+    if (ShouldHandleEventNow())
+        render_widget_host_->ForwardGestureEventImmediately(
+            deferred_tap_gesture_event_);

[sadrul, 2012/08/16 16:08:15]

Should the tap-event be also forwarded immediately?

and braces!

[rjkroege, 2012/08/16 17:12:26]

Probably not. Doing so would deviate from the pattern of one event at a time.
Done.

+    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;
+  DLOG(FATAL) << "Unhandled case.\n";

[sadrul, 2012/08/16 16:08:15]

You can use NOTREACHED here.

[rjkroege, 2012/08/16 17:12:26]

Done.

+  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 +121,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_gesture_event_);
+  if (ShouldHandleEventNow())

[sadrul, 2012/08/16 16:08:15]

braces!

[rjkroege, 2012/08/16 17:12:26]

Done.

+      render_widget_host_->ForwardGestureEventImmediately(
+          deferred_tap_gesture_event_);
+}
+
+void GestureEventFilter::MergeOrInsertScrollEvent(
+    const WebGestureEvent& gesture_event) {
+  WebGestureEvent* last_gesture_event = &coalesced_gesture_events_.back();
+  if (coalesced_gesture_events_.size() > 1 &&

[sadrul, 2012/08/16 16:08:15]

Could it be == 1 as well?

[rjkroege, 2012/08/16 17:12:26]

No. The 0th event has already been sent to the rendererer. Coalescing with it
would be wrong.

+      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";
+    DLOG_IF(FATAL,
+            last_gesture_event->type != WebInputEvent::GestureScrollUpdate)
+            << "Bad gesture type after coalescing?\n";

[sadrul, 2012/08/16 16:08:15]

Perhaps just DCHECK at the beginning that gesture_event.type ==
GestureScrollUpdate

[rjkroege, 2012/08/16 17:12:26]

Done.

+    last_gesture_event->timeStampSeconds = gesture_event.timeStampSeconds;
+  } else {
+    coalesced_gesture_events_.push_back(gesture_event);
+  }
+}
+
 } // namespace content