Disable tap suppression logic for WebView

Disable tap suppression logic for WebView

WebView embedders can suppress fling animation updates, potentially
confusing the tap suppression controller. As the feature isn't critical,
simply disable it for WebView.

Also refactor the input router settings helper to use
ui::GestureConfiguration directly for configuring tap suppression. This
makes it simpler for different platforms to customize behavior.

BUG=514783
Committed: https://crrev.com/36fee7bc622102a5a616a48262d54bdbe1d30903
Cr-Commit-Position: refs/heads/master@{#342019}

[jdduke (slow), 2015-07-31 16:45:08]

jdduke@chromium.org changed reviewers:
+ tdresser@chromium.org

[jdduke (slow), 2015-07-31 16:45:09]

tdresser@: PTAL. See https://codereview.chromium.org/1264963003/ for more
context.

hush@: Will this work for your needs? As long as there's a WebView hook to
disable tap suppression on ui::GestureConfiguration before the web contents is
created, we should be all set.

[hush (inactive), 2015-07-31 20:41:54]

On 2015/07/31 16:45:09, jdduke wrote:
> tdresser@: PTAL. See https://codereview.chromium.org/1264963003/ for more
> context.
> 
> hush@: Will this work for your needs? As long as there's a WebView hook to
> disable tap suppression on ui::GestureConfiguration before the web contents is
> created, we should be all set.

This will suit my need. I am working on a WebView part CL to make use of this
setter.

[hush (inactive), 2015-07-31 21:00:40]

On 2015/07/31 20:41:54, hush wrote:
> On 2015/07/31 16:45:09, jdduke wrote:
> > tdresser@: PTAL. See https://codereview.chromium.org/1264963003/ for more
> > context.
> > 
> > hush@: Will this work for your needs? As long as there's a WebView hook to
> > disable tap suppression on ui::GestureConfiguration before the web contents
is
> > created, we should be all set.
> 
> This will suit my need. I am working on a WebView part CL to make use of this
> setter.

Actually could you just combine https://codereview.chromium.org/1262403005 with
this CL?
We gotta cherry pick this to m45, so I figure one CL would be less churn?

[jdduke (slow), 2015-07-31 21:10:06]

On 2015/07/31 21:00:40, hush wrote:
> On 2015/07/31 20:41:54, hush wrote:
> > On 2015/07/31 16:45:09, jdduke wrote:
> > > tdresser@: PTAL. See https://codereview.chromium.org/1264963003/ for more
> > > context.
> > > 
> > > hush@: Will this work for your needs? As long as there's a WebView hook to
> > > disable tap suppression on ui::GestureConfiguration before the web
contents
> is
> > > created, we should be all set.
> > 
> > This will suit my need. I am working on a WebView part CL to make use of
this
> > setter.
> 
> Actually could you just combine https://codereview.chromium.org/1262403005
with
> this CL?
> We gotta cherry pick this to m45, so I figure one CL would be less churn?

I can if you prefer, maybe let's get some feedback from android_webview owners
on that change first?

[jdduke (slow), 2015-07-31 23:23:40]

jdduke@chromium.org changed reviewers:
+ boliu@chromium.org

[jdduke (slow), 2015-07-31 23:23:40]

+boliu for android_webview/

[boliu, 2015-07-31 23:27:09]

On 2015/07/31 23:23:40, jdduke wrote:
> +boliu for android_webview/

rs lgtm

[tdresser, 2015-08-04 12:20:50]

LGTM with nits.

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_configuration.h (right):

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration.h:214: bool
fling_touchscreen_tap_suppression_enabled_;
I have a mild preference for the name:
touch.*_fling_tap_suppression_enabled_.

Feel free to leave it this way if you prefer this.

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_configuration_android.cc (right):

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration_android.cc:56:
set_fling_touchpad_tap_suppression_enabled(false);
If you plug in an external touchpad on Android, do you get fling behavior?

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration_android.cc:61: // There is no
"semi" longpress on Android, just use the longpress value.
I think it might be clearer to set this to 0.

[jdduke (slow), 2015-08-04 17:19:25]

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_configuration.h (right):

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration.h:214: bool
fling_touchscreen_tap_suppression_enabled_;
On 2015/08/04 12:20:50, tdresser wrote:
> I have a mild preference for the name:
> touch.*_fling_tap_suppression_enabled_.
> 
> Feel free to leave it this way if you prefer this.

Yeah, I actually had that originally, and went back and forth. The reason I
somewhat prefer this version is because the source (touchpad/touchscreen) refers
to the tap source, rather than the fling source. I considered dropping the
"fling_" prefix altogether, but there's some precedent in the other variables.
Let me know what you'd like either way.

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_configuration_android.cc (right):

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration_android.cc:56:
set_fling_touchpad_tap_suppression_enabled(false);
On 2015/08/04 12:20:50, tdresser wrote:
> If you plug in an external touchpad on Android, do you get fling behavior?

I don't actually know =/, I was about to order an Apple magic trackpad. We would
never get a fling event I don't think, but I wonder if the OS handles such
touchpad flings, or maybe it's even lower level like the driver/firmware (like
on Windows)? We can always enable it when we find the right scenario (though, I
don't think we ever send mouedown/mouseup events yet on Android, so this
probably wouldn't do anything yet).

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration_android.cc:61: // There is no
"semi" longpress on Android, just use the longpress value.
On 2015/08/04 12:20:50, tdresser wrote:
> I think it might be clearer to set this to 0.

Yeah, I went ahead and removed this line, and changed the tap configuration
logic so it uses long_press_time_in_ms instead.

[jdduke (slow), 2015-08-04 20:07:44]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2015-08-04 20:07:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1267693004/#ps60001
(title: "Code review")

[commit-bot: I haz the power, 2015-08-04 20:09:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1267693004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1267693004/60001

[commit-bot: I haz the power, 2015-08-04 20:55:15]

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

[commit-bot: I haz the power, 2015-08-04 20:55:16]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tdresser, 2015-08-04 21:00:15]

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_configuration.h (right):

https://codereview.chromium.org/1267693004/diff/40001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_configuration.h:214: bool
fling_touchscreen_tap_suppression_enabled_;
On 2015/08/04 17:19:25, jdduke wrote:
> On 2015/08/04 12:20:50, tdresser wrote:
> > I have a mild preference for the name:
> > touch.*_fling_tap_suppression_enabled_.
> > 
> > Feel free to leave it this way if you prefer this.
> 
> Yeah, I actually had that originally, and went back and forth. The reason I
> somewhat prefer this version is because the source (touchpad/touchscreen)
refers
> to the tap source, rather than the fling source. I considered dropping the
> "fling_" prefix altogether, but there's some precedent in the other variables.
> Let me know what you'd like either way.

LGTM.

[jdduke (slow), 2015-08-05 19:18:26]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2015-08-05 19:18:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1267693004/#ps80001
(title: "Fix component build")

[commit-bot: I haz the power, 2015-08-05 19:18:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1267693004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1267693004/80001

[commit-bot: I haz the power, 2015-08-05 22:01:58]

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

[commit-bot: I haz the power, 2015-08-05 22:01:59]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jdduke (slow), 2015-08-05 23:11:38]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2015-08-05 23:12:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1267693004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1267693004/80001

[commit-bot: I haz the power, 2015-08-06 00:16:58]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-08-06 00:17:44]

Patchset 5 (id:??) landed as
https://crrev.com/36fee7bc622102a5a616a48262d54bdbe1d30903
Cr-Commit-Position: refs/heads/master@{#342019}