Support GestureBegin and GestureEnd in ui::GestureProvider

ui/events/gesture_detection/gesture_provider.cc

Index: ui/events/gesture_detection/gesture_provider.cc
diff --git a/ui/events/gesture_detection/gesture_provider.cc b/ui/events/gesture_detection/gesture_provider.cc
index 4deeba2580f3a7a72928cd3cec30f4a401aa2f18..19ebd59bb496cee29c5abd9638abf6a0fc14a395 100644
--- a/ui/events/gesture_detection/gesture_provider.cc
+++ b/ui/events/gesture_detection/gesture_provider.cc
@@ -76,7 +76,8 @@ GestureEventDetails CreateTapGestureDetails(EventType type,
 
 // GestureProvider:::Config
 
-GestureProvider::Config::Config() : disable_click_delay(false) {}
+GestureProvider::Config::Config()
+    : disable_click_delay(false), gesture_begin_end_types_enabled(false) {}
 
 GestureProvider::Config::~Config() {}
 
@@ -604,7 +605,8 @@ GestureProvider::GestureProvider(const Config& config,
       needs_show_press_event_(false),
       needs_tap_ending_event_(false),
       touch_scroll_in_progress_(false),
-      pinch_in_progress_(false) {
+      pinch_in_progress_(false),
+      gesture_begin_end_types_enabled_(false) {

[jdduke (slow), 2014/04/03 22:03:28]

gesture_begin_end_types_enabled_(config.gesture_begin_end_types_enabled) {

The double-tap/multi-touch toggles can all occur at different points in the
provider lifetime, so we need explicit setters, but this flag should be set only
once.

[tdresser, 2014/04/04 16:39:31]

Done.

   DCHECK(client);
   InitGestureDetectors(config);
 }
@@ -617,35 +619,17 @@ bool GestureProvider::OnTouchEvent(const MotionEvent& event) {
   if (!CanHandle(event))
     return false;
 
-  const bool was_touch_scrolling_ = touch_scroll_in_progress_;
+  OnTouchEventHandlingBegin(event);
+
+  const bool was_touch_scrolling = touch_scroll_in_progress_;
   const bool in_scale_gesture =
       scale_gesture_listener_->IsScaleGestureDetectionInProgress();
 
-  if (event.GetAction() == MotionEvent::ACTION_DOWN) {
-    current_down_event_ = event.Clone();
-    touch_scroll_in_progress_ = false;
-    needs_show_press_event_ = true;
-    current_longpress_time_ = base::TimeTicks();
-    SendTapCancelIfNecessary(event);
-  }
 
   bool handled = gesture_listener_->OnTouchEvent(event, in_scale_gesture);

[jdduke (slow), 2014/04/03 22:03:28]

I guess we'll no longer need the |handled| variable (see below).

[tdresser, 2014/04/04 16:39:31]

Done.

   handled |= scale_gesture_listener_->OnTouchEvent(event);
 
-  if (event.GetAction() == MotionEvent::ACTION_UP ||
-      event.GetAction() == MotionEvent::ACTION_CANCEL) {
-    // "Last finger raised" could be an end to movement, but it should
-    // only terminate scrolling if the event did not cause a fling.
-    if (was_touch_scrolling_ && !handled)
-      EndTouchScrollIfNecessary(event.GetEventTime(), true);
-
-    // We shouldn't necessarily cancel a tap on ACTION_UP, as the double-tap
-    // timeout may yet trigger a SINGLE_TAP.
-    if (event.GetAction() == MotionEvent::ACTION_CANCEL)
-      SendTapCancelIfNecessary(event);
-
-    current_down_event_.reset();
-  }
+  OnTouchEventHandlingEnd(event, handled, was_touch_scrolling);
 
   return true;
 }
@@ -670,6 +654,10 @@ void GestureProvider::SetDoubleTapSupportForPageEnabled(bool enabled) {
   gesture_listener_->SetDoubleTapSupportForPageEnabled(enabled);
 }
 
+void GestureProvider::SetBeginEndTypesEnabled(bool enabled) {
+  gesture_begin_end_types_enabled_ = enabled;
+}
+
 bool GestureProvider::IsScrollInProgress() const {
   // TODO(wangxianzhu): Also return true when fling is active once the UI knows
   // exactly when the fling ends.
@@ -696,6 +684,7 @@ void GestureProvider::InitGestureDetectors(const Config& config) {
 
   scale_gesture_listener_.reset(
       new ScaleGestureListenerImpl(config.scale_gesture_detector_config, this));
+  gesture_begin_end_types_enabled_ = config.gesture_begin_end_types_enabled;
 }
 
 bool GestureProvider::CanHandle(const MotionEvent& event) const {
@@ -734,7 +723,8 @@ void GestureProvider::Send(const GestureEventData& gesture) {
   // are SHOW_PRESS and TAP, potentially triggered by the double-tap
   // delay timing out.
   DCHECK(current_down_event_ || gesture.type == ET_GESTURE_TAP ||
-         gesture.type == ET_GESTURE_SHOW_PRESS);
+         gesture.type == ET_GESTURE_SHOW_PRESS ||
+         gesture.type == ET_GESTURE_BEGIN || gesture.type == ET_GESTURE_END);

[jdduke (slow), 2014/04/03 22:03:28]

If we send GESTURE_END before resetting the down event in
OnTouchEventHandlingEnd, we shouldn't need these extra exceptions.

[tdresser, 2014/04/04 16:39:31]

Done.

 
   switch (gesture.type) {
     case ET_GESTURE_TAP_DOWN:
@@ -818,4 +808,57 @@ void GestureProvider::EndTouchScrollIfNecessary(base::TimeTicks time,
     Send(CreateGesture(ET_GESTURE_SCROLL_END, time, 0, 0));
 }
 
+void GestureProvider::OnTouchEventHandlingBegin(const MotionEvent& event) {
+  switch (event.GetAction()) {
+    case MotionEvent::ACTION_DOWN:
+      current_down_event_ = event.Clone();
+      touch_scroll_in_progress_ = false;
+      needs_show_press_event_ = true;

[jdduke (slow), 2014/04/03 22:03:28]

Hmm, this show press assignment looks funky (pretty sure that's my bad).  Could
you change this to |needs_show_press_event_ = false;|, and then when we send a
TAP_DOWN set it to true?

[tdresser, 2014/04/04 16:39:31]

Done.

+      current_longpress_time_ = base::TimeTicks();
+      SendTapCancelIfNecessary(event);
+      if (gesture_begin_end_types_enabled_)
+        Send(CreateGesture(ET_GESTURE_BEGIN, event));
+      break;
+    case MotionEvent::ACTION_POINTER_DOWN:
+      if (gesture_begin_end_types_enabled_)
+        Send(CreateGesture(ET_GESTURE_BEGIN, event));
+      break;
+    case MotionEvent::ACTION_POINTER_UP:
+    case MotionEvent::ACTION_UP:
+    case MotionEvent::ACTION_CANCEL:
+    case MotionEvent::ACTION_MOVE:
+      break;
+  }
+}
+
+void GestureProvider::OnTouchEventHandlingEnd(
+    const MotionEvent& event, bool handled, bool was_touch_scrolling) {
+  switch (event.GetAction()) {
+    case MotionEvent::ACTION_UP:
+    case MotionEvent::ACTION_CANCEL:
+      // "Last finger raised" could be an end to movement, but it should
+      // only terminate scrolling if the event did not cause a fling.
+      if (was_touch_scrolling && !handled)
+        EndTouchScrollIfNecessary(event.GetEventTime(), true);

[jdduke (slow), 2014/04/03 22:03:28]

This will probably fail during rebase after
https://codereview.chromium.org/200623003/.  In short, you shouldn't need the
|handled| or |was_touch_scrolling| arguments.

[tdresser, 2014/04/04 16:39:31]

Done.

+
+      // We shouldn't necessarily cancel a tap on ACTION_UP, as the double-tap
+      // timeout may yet trigger a SINGLE_TAP.
+      if (event.GetAction() == MotionEvent::ACTION_CANCEL)
+        SendTapCancelIfNecessary(event);
+
+      current_down_event_.reset();
+      if (gesture_begin_end_types_enabled_)

[jdduke (slow), 2014/04/03 22:03:28]

This should come before resetting the down event (see comment above).

[tdresser, 2014/04/04 16:39:31]

Done.

+        Send(CreateGesture(ET_GESTURE_END, event));
+      break;
+    case MotionEvent::ACTION_POINTER_UP:
+      if (gesture_begin_end_types_enabled_)
+        Send(CreateGesture(ET_GESTURE_END, event));
+      break;
+    case MotionEvent::ACTION_DOWN:
+    case MotionEvent::ACTION_POINTER_DOWN:
+    case MotionEvent::ACTION_MOVE:
+      break;
+  }
+}
+
 }  //  namespace ui