Consolidate all touch/gesture related constants in content

Reland "Consolidate all touch/gesture related constants in content"

InputRouter, TouchEventQueue and GestureEventQueue all use a variety of
configuration parameters that are littered through the code in a very ad-hoc
and hard-to-follow fashion.  Consolidate all such parameters into well-defined
configuration structs, with helper functions to generate the appropriate
parameters per platform.  This simplifies testing and eases the mental burden
of tracking down the origin of the various magic constants.

Note that this change disables scroll gesture debounce by default, enabling it
only for ChromeOS.

The original patch landed in r268051 but was reverted in r268052 due to
content_perftest compilation issues that have since been fixed.

BUG=343917, 353702
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268078

[jdduke (slow), 2014-04-11 21:05:52]

tdresser@: This isn't ready for review (by any means, very much WIP), but I'm
cc'ing you as an FYI as I clean things up.

[jdduke (slow), 2014-04-30 19:42:34]

tdreses@: PTAL.  If you think this reasonable I'll push for further OWNER
review, thanks.

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_config_helper.cc (right):

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/input_router_config_helper.cc:26: const int
kDebouncingIntervalTimeMs = 30;
I originally had these separated out into separate files, but I also find it
useful to have them all in one nice central location when comparing behavior...

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:234: process_, this,
this, routing_id_, GetInputRouterConfigForPlatform()));
I thought of making this a method on the View, but I'm still unsure if we can
get events outside of when |view_| is NULL...

[tdresser, 2014-05-01 13:51:55]

LGTM!

This is good stuff.

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_config_helper.cc (right):

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/input_router_config_helper.cc:26: const int
kDebouncingIntervalTimeMs = 30;
On 2014/04/30 19:42:34, jdduke wrote:
> I originally had these separated out into separate files, but I also find it
> useful to have them all in one nice central location when comparing
behavior...

A single config file is fine with me.

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/235003005/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:234: process_, this,
this, routing_id_, GetInputRouterConfigForPlatform()));
On 2014/04/30 19:42:34, jdduke wrote:
> I thought of making this a method on the View, but I'm still unsure if we can
> get events outside of when |view_| is NULL...

Sorry, which method are you referring to here?

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/gesture_event_queue.h:76: base::TimeDelta
debounce_interval;
A debounce_interval of 0 should be the same as disabling debounce, so we could
get rid of enable_debounce_during_scroll.

It may be clearer this way I suppose.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_config_helper.cc (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_config_helper.cc:32: //
TODO(jdduke): Disable and reove entirely when issues with Pixel are
reove -> remove

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_impl.cc:44:
GestureEventWithLatencyInfo MakeGestureEvent(WebInputEvent::Type type,
Is this ever used?

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/tap_suppression_controller.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/tap_suppression_controller.h:79: bool
enabled_;
enabled_ is unused.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/touch_event_queue.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue.h:61: // Determines
touchmove slop suppression within a region.
This comment should mention that it's determining the size of the region.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue.h:234: bool
needs_async_touchmove_for_outer_slop_region_;
Irrelevant thought:

At some point we should nail down the event naming conventions we want to use.

I've frequently use "touch_move" in code, and "touch-move" in comments, but then
I also use GestureScrollEnd in comments, which is completely inconsistent.

I think I prefer touch_move to touchmove, but the TouchEventQueue is currently
using the "touchmove" style, so I'm in favor of this rename, for the moment at
least.

[jdduke (slow), 2014-05-01 16:44:24]

PTAL for OWNER review, thanks.

sadrul@:content/browser/renderer_host/render_widget_host*
jam@: content/public/common/content_switches.*

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/gesture_event_queue.h:76: base::TimeDelta
debounce_interval;
On 2014/05/01 13:51:56, tdresser wrote:
> A debounce_interval of 0 should be the same as disabling debounce, so we could
> get rid of enable_debounce_during_scroll.
> 
> It may be clearer this way I suppose.

That's fine with me, I wasn't sure how explicit we wanted to be.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_config_helper.cc (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_config_helper.cc:32: //
TODO(jdduke): Disable and reove entirely when issues with Pixel are
On 2014/05/01 13:51:56, tdresser wrote:
> reove -> remove

Done.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_impl.cc:44:
GestureEventWithLatencyInfo MakeGestureEvent(WebInputEvent::Type type,
On 2014/05/01 13:51:56, tdresser wrote:
> Is this ever used?

Hmm, it appears not, I believe it was leftover from the synthetic pinch/scroll,
but that was just removed.  Good catch.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/tap_suppression_controller.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/tap_suppression_controller.h:79: bool
enabled_;
On 2014/05/01 13:51:56, tdresser wrote:
> enabled_ is unused.

Done.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/touch_event_queue.h (right):

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue.h:61: // Determines
touchmove slop suppression within a region.
On 2014/05/01 13:51:56, tdresser wrote:
> This comment should mention that it's determining the size of the region.

Done.

https://codereview.chromium.org/235003005/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue.h:234: bool
needs_async_touchmove_for_outer_slop_region_;
On 2014/05/01 13:51:56, tdresser wrote:
> Irrelevant thought:
> 
> At some point we should nail down the event naming conventions we want to use.
> 
> I've frequently use "touch_move" in code, and "touch-move" in comments, but
then
> I also use GestureScrollEnd in comments, which is completely inconsistent.
> 
> I think I prefer touch_move to touchmove, but the TouchEventQueue is currently
> using the "touchmove" style, so I'm in favor of this rename, for the moment at
> least.
> 

Yeah, I'm fairly indifferent here, I just noticed the inconsistencies in the
file and thought I'd switch to one. 'touchmove' is the name seen by Javascript
and it's one less underscore, but I honestly don't care.  I'll make this
'touchmove' for now and we can update in one big refactoring in the future if we
like.

[sadrul, 2014-05-02 01:49:16]

LGTM

https://codereview.chromium.org/235003005/diff/90001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_unittest.cc (right):

https://codereview.chromium.org/235003005/diff/90001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_unittest.cc:615:
host_->DisableGestureDebounce();
Just to double-check: the tests that expect the debounce to be active explicitly
call set_debounce_interval_time_ms()?

[jdduke (slow), 2014-05-02 01:50:37]

https://codereview.chromium.org/235003005/diff/90001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_unittest.cc (right):

https://codereview.chromium.org/235003005/diff/90001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_unittest.cc:615:
host_->DisableGestureDebounce();
On 2014/05/02 01:49:16, sadrul wrote:
> Just to double-check: the tests that expect the debounce to be active
explicitly
> call set_debounce_interval_time_ms()?

Yup, exactly.

[sadrul, 2014-05-02 01:50:43]

Actually +jam@ (from #msg4: "jam@: content/public/common/content_switches.*")

[jam, 2014-05-02 18:33:06]

On 2014/05/02 01:50:43, sadrul wrote:
> Actually +jam@ (from #msg4: "jam@: content/public/common/content_switches.*")

lgtm

[jdduke (slow), 2014-05-02 19:30:30]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 19:30:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/130001

[commit-bot: I haz the power, 2014-05-02 19:38:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 19:38:21]

Try jobs failed on following builders:
  linux_chromium_clang_dbg on tryserver.chromium

[jdduke (slow), 2014-05-02 20:22:57]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 20:23:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-02 20:36:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 20:36:12]

Try jobs failed on following builders:
  linux_chromium_rel on tryserver.chromium

[jdduke (slow), 2014-05-02 20:40:40]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 20:40:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-02 20:52:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 20:52:10]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium

[jdduke (slow), 2014-05-02 21:14:29]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 21:17:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-02 21:32:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 21:32:55]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium

[jdduke (slow), 2014-05-02 21:35:43]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 21:36:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-02 21:38:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 21:38:29]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium

[jdduke (slow), 2014-05-02 21:50:46]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-02 21:51:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-02 22:08:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-02 22:08:12]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium

[jdduke (slow), 2014-05-03 04:17:28]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-03 04:17:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/150001

[commit-bot: I haz the power, 2014-05-03 05:44:19]

Change committed as 268051

[jdduke (slow), 2014-05-03 05:56:01]

A revert of this CL has been created in
https://codereview.chromium.org/260923003/ by jdduke@chromium.org.

The reason for reverting is: Broke content perftest compile, 

http://build.chromium.org/p/chromium.linux/builders/Linux%20Clang%20%28dbg%29...

etc....

[jdduke (slow), 2014-05-03 18:40:47]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-05-03 18:40:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jdduke@chromium.org/235003005/170001

[commit-bot: I haz the power, 2014-05-03 21:39:45]

Change committed as 268078

[jdduke (slow), 2014-05-04 00:11:24]

On 2014/05/03 21:39:45, I haz the power (commit-bot) wrote:
> Change committed as 268078

FYI, if anybody's thinking to revert this for the
RepeatedQuickOverscrollGestures failure in content_browsertests, it looks like
that test was already flaky with failures starting here:
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...