Prepare for Unified Gesture Recognizer landing in Aura

ui/events/gestures/gesture_recognizer_impl.cc

Index: ui/events/gestures/gesture_recognizer_impl.cc
diff --git a/ui/events/gestures/gesture_recognizer_impl.cc b/ui/events/gestures/gesture_recognizer_impl.cc
index c27af4aaafdce5247c72a2ed3b375c43fbf4ddb9..6ec5ccf96f1932ea09af8cada291058428dcb528 100644
--- a/ui/events/gestures/gesture_recognizer_impl.cc
+++ b/ui/events/gestures/gesture_recognizer_impl.cc
@@ -308,7 +308,7 @@ void GestureRecognizerImpl::DispatchGestureEvent(GestureEvent* event) {
   }
 }
 
-GestureSequence::Gestures* GestureRecognizerImpl::ProcessTouchEventForGesture(
+ScopedVector<GestureEvent>* GestureRecognizerImpl::ProcessTouchEventForGesture(

[jdduke (slow), 2014/05/28 16:26:00]

I'm trying to understand how this signature changed without changing the header?
In any case, must the ui::GestureEvent objects persist? It seems a little odd to
both heap allocate each gesture AND heap allocate the container.  If the objects
must persist, you can just rely on ScopedVector being a "move-only" type (you
can call .Pass() like a scoped_ptr), and return an empty ScopedVector in the
NULL case.

[tdresser, 2014/05/29 14:26:21]

GestureSequence::Gestures is typedef'ed to ScopedVector<GestureEvent>*. I think
it probably makes sense to keep the same signature here as we had before, we can
clean this up in a followup patch.

[jdduke (slow), 2014/05/29 16:25:40]

I see, that's really unfortunate.  At the least, could you file a bug? Seems to
me the gestures don't need to be heap allocated, and neither does the vector,
i.e., we could just return std::vector<ui::GestureEvent> and as long as we're
using swap and RVO properly, there should be much less overhead (we'll have just
1 heap allocation, instead of 1 for vector storage, 1 for the vector itself, and
1 for every member event =/).  It's a shame we don't have a SmallVector
construct as we do in Blink (hmm.... tempting...).

Keeping as is seems fine for now, as we're just matching existing behavior.

[tdresser, 2014/05/30 14:05:37]

Bug filed: crbug.com/378817.

     const TouchEvent& event,
     ui::EventResult result,
     GestureConsumer* target) {
@@ -322,8 +322,10 @@ GestureSequence::Gestures* GestureRecognizerImpl::ProcessTouchEventForGesture(
         GetGestureProviderForConsumer(target);

[jdduke (slow), 2014/05/28 16:26:00]

I'm a little worried about not doing eager gesture recognition here, I guess as
long as it works...

     // TODO(tdresser) - detect gestures eagerly.
     if (!(result & ER_CONSUMED)) {
-      if (gesture_provider->OnTouchEvent(event))
+      if (gesture_provider->OnTouchEvent(event)) {
         gesture_provider->OnTouchEventAck(result != ER_UNHANDLED);
+        return gesture_provider->GetAndResetPendingGestures();
+      }
     }
     return NULL;
   }
@@ -342,10 +344,7 @@ bool GestureRecognizerImpl::CleanupStateForConsumer(
   } else {
     if (consumer_gesture_provider_.count(consumer)) {
       state_cleaned_up = true;
-      // Don't immediately delete the GestureProvider, as we could be in the
-      // middle of dispatching a set of gestures.
-      base::MessageLoop::current()->DeleteSoon(
-          FROM_HERE, consumer_gesture_provider_[consumer]);
+      delete consumer_gesture_provider_[consumer];
       consumer_gesture_provider_.erase(consumer);
     }
   }