gesture recognizer: Remove the touch-event queue.

touch: Coalesce touch-events, and remove the touch-event queue from the gesture recognizer.

The major changes in this CL:
 * Remove the touch-event queue and the relevant code from the gesture
   recognizer. This is no longer necessary since there is a new queue maintained
   by the RenderWidgetHost.
 * Touch events are now coalesced in the queue when appropriate.
 * Create a list of ui::TouchEvents out of the processed/coalesced
   WebTouchEvents and feed those to the gesture recognizer.
 * Tests.
 * Remove asynchronous event-processing in ui::Events.

BUG=138153, 110231
TBR=avi@chromium.org (content_browser.gypi)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=162518

[sadrul, 2012-10-16 22:56:39]

Hi! This is the follow CL for http://codereview.chromium.org/11148008/. I tried
to split it out into smaller pieces, but it would've been tricky. So I kept it
all in one. Please review. Thanks!

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
File content/browser/renderer_host/ui_events_helper.h (right):

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
content/browser/renderer_host/ui_events_helper.h:6: #define
CONTENT_BROWSER_RENDERER_HOST_UI_EVENTS_HELPER_H_
In a subsequent CL, I plan on moving the function for creating a WebGestureEvent
from a ui::GestureEvent into this file so that it can be shared between the aura
and windows implementations. So I gave this file a generic 'ui_events_helper'
instead of some touch-specific name.

[aelias_OOO_until_Jul13, 2012-10-17 01:52:57]

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_impl.cc:1964: return
has_touch_handler_ || !touch_event_queue_->empty();
Should the queue class hold the has_touch_handler_ value and this new method?  I
think we should centralize more logic there.

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
File content/browser/renderer_host/touch_event_queue.cc (right):

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
content/browser/renderer_host/touch_event_queue.cc:30:
touch_queue_.push_back(event);
Can we just move the coalescing logic into QueueEvent?  It seems weird to have
two push_back calls.

[sadrul, 2012-10-17 02:17:59]

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_impl.cc:1964: return
has_touch_handler_ || !touch_event_queue_->empty();
On 2012/10/17 01:52:57, aelias wrote:
> Should the queue class hold the has_touch_handler_ value and this new method? 
I
> think we should centralize more logic there.

I kind of feel like has_touch_handler_ would be out of place in TouchEventQueue.
But I don't have a strong opinion on this.

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
File content/browser/renderer_host/touch_event_queue.cc (right):

http://codereview.chromium.org/11188012/diff/10024/content/browser/renderer_h...
content/browser/renderer_host/touch_event_queue.cc:30:
touch_queue_.push_back(event);
On 2012/10/17 01:52:57, aelias wrote:
> Can we just move the coalescing logic into QueueEvent?  It seems weird to have
> two push_back calls.

Good idea. Done.

[rjkroege, 2012-10-17 15:57:35]

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_win.cc (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_win.cc:936:
ScopedVector<ui::TouchEvent> events;
this code looks really similar with the _aura code. Is there an additional
refactoring opportunity here?

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/touch_event_queue.cc (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/touch_event_queue.cc:29: // If the last queued
touch-event was a touch-move, and the current event is
don't you need to queue moves separately for different ids?

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/ui_events_helper.h (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/ui_events_helper.h:19: bool
MakeUITouchEventsFromWebTouchEvents(const WebKit::WebTouchEvent& touch,
Perhaps you could a why comment?

http://codereview.chromium.org/11188012/diff/5047/ui/base/gestures/gesture_re...
File ui/base/gestures/gesture_recognizer.h (right):

http://codereview.chromium.org/11188012/diff/5047/ui/base/gestures/gesture_re...
ui/base/gestures/gesture_recognizer.h:35: virtual void
CleanupStateForConsumer(GestureConsumer* consumer) = 0;
needs a comment?

[sadrul, 2012-10-17 16:12:44]

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_win.cc (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_win.cc:936:
ScopedVector<ui::TouchEvent> events;
On 2012/10/17 15:57:35, rjkroege wrote:
> this code looks really similar with the _aura code. Is there an additional
> refactoring opportunity here?

Unfortunately, the way the ui::TouchEvents are dispatched in RWHVA and here in
RWHVW are different (in RWHVA, the events go to the RootWindow, in RWHVW, the
events go directly to the GestureRecognizer). So we are sharing the code to
create the ui::TouchEvents here, but the dispatch code needs to be different.

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/touch_event_queue.cc (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/touch_event_queue.cc:29: // If the last queued
touch-event was a touch-move, and the current event is
On 2012/10/17 15:57:35, rjkroege wrote:
> don't you need to queue moves separately for different ids?

The current WebTouchEvent (i.e. |event|) will still include the correct location
for all the touch-points. So checking the touch-id isn't necessary here.

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
File content/browser/renderer_host/ui_events_helper.h (right):

http://codereview.chromium.org/11188012/diff/5047/content/browser/renderer_ho...
content/browser/renderer_host/ui_events_helper.h:19: bool
MakeUITouchEventsFromWebTouchEvents(const WebKit::WebTouchEvent& touch,
On 2012/10/17 15:57:35, rjkroege wrote:
> Perhaps you could a why comment?

Done.

http://codereview.chromium.org/11188012/diff/5047/ui/base/gestures/gesture_re...
File ui/base/gestures/gesture_recognizer.h (right):

http://codereview.chromium.org/11188012/diff/5047/ui/base/gestures/gesture_re...
ui/base/gestures/gesture_recognizer.h:35: virtual void
CleanupStateForConsumer(GestureConsumer* consumer) = 0;
On 2012/10/17 15:57:35, rjkroege wrote:
> needs a comment?

Done.

[rjkroege, 2012-10-17 17:55:53]

thanks for the explanation.

lgtm

[aelias_OOO_until_Jul13, 2012-10-17 18:18:39]

lgtm

[sadrul, 2012-10-17 18:30:14]

+ben for ui/ changes

[Ben Goodger (Google), 2012-10-17 19:28:08]

lgtm

[sadrul, 2012-10-17 20:41:06]

+avi@ for the change in content/content_browser.gypi [going to TBR this change]