Plumb gesture source value through Blink.

Plumb gesture source value through Blink.

The sourceDevice field of WebGestureEvent needs to be plumbed through
blink (PlatformGestureEvent, GestureEvent) since InputHandlerProxy (1)
uses sourceDevice, and (2) may receive events that have travelled through
Blink to a PluginContainer and been forwarded to another process.

BUG=445319
Committed: https://crrev.com/3bb814794b2509e95944cf36af0cc7538611ae03
Cr-Commit-Position: refs/heads/master@{#354946}

[wjmaclean, 2015-10-14 13:37:06]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-14 13:37:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/1

[wjmaclean, 2015-10-14 13:37:37]

wjmaclean@chromium.org changed reviewers:
+ rbyers@chromium.org, tdresser@chromium.org

[wjmaclean, 2015-10-14 13:38:15]

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

[wjmaclean, 2015-10-14 14:10:00]

Plumbing patch, please take a look?

[commit-bot: I haz the power, 2015-10-14 14:14:17]

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

[commit-bot: I haz the power, 2015-10-14 14:14:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[tdresser, 2015-10-14 14:25:59]

Failing DCHECKs look related, can you take a look?

[wjmaclean, 2015-10-14 14:31:36]

On 2015/10/14 14:25:59, tdresser wrote:
> Failing DCHECKs look related, can you take a look?

There's a unit test where it did not initialize sourceDevice, and was implicitly
relying on WebGestureDeviceTouchpad = 0. I'll have a fix up in a moment.

[jdduke (slow), 2015-10-14 15:08:06]

Ugg, yet another argument for getting rid of one of these intermediate event
types =/.

[wjmaclean, 2015-10-14 15:09:39]

On 2015/10/14 15:08:06, jdduke wrote:
> Ugg, yet another argument for getting rid of one of these intermediate event
> types =/.

jdduke@ - agreed, there's one-too-many event types right now :-(

[tdresser, 2015-10-14 15:14:20]

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:503: if
(gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad) {
This might read a bit better as a switch statement now, but I don't feel
strongly about it.

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:1019: break;
We generally put exceptional cases at the end of switch statements.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/GestureEvent.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/GestureEvent.cpp:51: source =
GestureSourceUninitialized;
Should this case ever happen?

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:1877: IntSize(), 0,
PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
Uninitialized doesn't feel quite right here. It might be better to keep track of
what device caused the fling.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:1615: 0,
PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
I'd like us to be able to assert throughout the codebase that this is never
uninitialized, so let's not use uninitialized in tests.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:1604: PlatformGestureEvent
gestureEvent(PlatformEvent::Type::GestureTap, hitPoint, hitPoint, IntSize(0, 0),
0, PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
Let's not use uninitialized.

[wjmaclean, 2015-10-14 15:55:10]

ptal ...

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:503: if
(gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad) {
On 2015/10/14 15:14:20, tdresser wrote:
> This might read a bit better as a switch statement now, but I don't feel
> strongly about it.

Done.

https://codereview.chromium.org/1403893003/diff/60001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:1019: break;
On 2015/10/14 15:14:20, tdresser wrote:
> We generally put exceptional cases at the end of switch statements.

Done.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/GestureEvent.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/GestureEvent.cpp:51: source =
GestureSourceUninitialized;
On 2015/10/14 15:14:20, tdresser wrote:
> Should this case ever happen?

Done.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:1877: IntSize(), 0,
PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
On 2015/10/14 15:14:20, tdresser wrote:
> Uninitialized doesn't feel quite right here. It might be better to keep track
of
> what device caused the fling.

Done.

Turns out WebViewImpl already had a var for this, though I had to convert it
from bool :-/

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:1615: 0,
PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
On 2015/10/14 15:14:20, tdresser wrote:
> I'd like us to be able to assert throughout the codebase that this is never
> uninitialized, so let's not use uninitialized in tests.

Done.

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:1604: PlatformGestureEvent
gestureEvent(PlatformEvent::Type::GestureTap, hitPoint, hitPoint, IntSize(0, 0),
0, PlatformEvent::NoModifiers, PlatformGestureSourceUninitialized);
On 2015/10/14 15:14:20, tdresser wrote:
> Let's not use uninitialized.

Done.

[tdresser, 2015-10-14 17:18:56]

Tests are still failing.

https://codereview.chromium.org/1403893003/diff/80001/components/test_runner/...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/1403893003/diff/80001/components/test_runner/...
components/test_runner/event_sender.cc:1660: event.sourceDevice =
blink::WebGestureDeviceTouchpad;
In an ideal world I think we'd have GestureFlingCancel take a parameter
indicating the source device in the same way as GestureFlingStart.

I suppose if it doesn't matter this is fine. Maybe add a note that it doesn't
matter?

https://codereview.chromium.org/1403893003/diff/80001/content/renderer/input/...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/1403893003/diff/80001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:1031: NOTREACHED();
Should we have an early return here?

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/GestureEvent.cpp (right):

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/GestureEvent.cpp:51: ASSERT_NOT_REACHED();
Should we have an early return here?

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:259: m_source =
PlatformGestureSourceUninitialized;
Should this ever happen?

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:720: sourceDevice =
WebGestureDeviceUninitialized;
Should this ever happen?

[wjmaclean, 2015-10-14 18:03:28]

> Tests are still failing.

Yes, fallout as we exclude uninitialized from more places ... working to stomp
these out as they turn up.

https://codereview.chromium.org/1403893003/diff/80001/components/test_runner/...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/1403893003/diff/80001/components/test_runner/...
components/test_runner/event_sender.cc:1660: event.sourceDevice =
blink::WebGestureDeviceTouchpad;
On 2015/10/14 17:18:56, tdresser wrote:
> In an ideal world I think we'd have GestureFlingCancel take a parameter
> indicating the source device in the same way as GestureFlingStart.
> 
> I suppose if it doesn't matter this is fine. Maybe add a note that it doesn't
> matter?

I'll add a not for now. It's a bunch of additional plumbing, and generally
GestureFlingCancel gets called all over the place, as a "just in case" sort of
thing, so I don't know if having to specify a source device mightn't get
cumbersom.

https://codereview.chromium.org/1403893003/diff/80001/content/renderer/input/...
File content/renderer/input/input_handler_proxy.cc (right):

https://codereview.chromium.org/1403893003/diff/80001/content/renderer/input/...
content/renderer/input/input_handler_proxy.cc:1031: NOTREACHED();
On 2015/10/14 17:18:56, tdresser wrote:
> Should we have an early return here?

Done.

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/GestureEvent.cpp (right):

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/GestureEvent.cpp:51: ASSERT_NOT_REACHED();
On 2015/10/14 17:18:56, tdresser wrote:
> Should we have an early return here?

Hmmm, not sure if callers will ever expect this to fail. Could we default to
using Touchpad here?

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:259: m_source =
PlatformGestureSourceUninitialized;
On 2015/10/14 17:18:56, tdresser wrote:
> Should this ever happen?

Done.

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:720: sourceDevice =
WebGestureDeviceUninitialized;
On 2015/10/14 17:18:56, tdresser wrote:
> Should this ever happen?

Done.

[wjmaclean, 2015-10-16 20:01:16]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[wjmaclean, 2015-10-16 20:01:48]

PTAL? (I think the WebGL conformance test failures are unrelated ...)

[commit-bot: I haz the power, 2015-10-16 20:03:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/140001

[commit-bot: I haz the power, 2015-10-16 20:19:45]

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

[commit-bot: I haz the power, 2015-10-16 20:19:46]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)

[wjmaclean, 2015-10-16 20:42:29]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-16 20:43:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/160001

[commit-bot: I haz the power, 2015-10-16 22:15:03]

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

[commit-bot: I haz the power, 2015-10-16 22:15:07]

Dry run: 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_...)

[tdresser, 2015-10-19 12:52:40]

LGTM with nits.

I'm thinking it probably would be worth it to DCHECK when reading the fields of
a gesture event that if the sourceDevice is touchpad, then the event is a
scroll/fling event.

Right now a bunch of these tests use type/source pairs that never occur in real
use.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LinkHighlightImplTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:75:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
Ideally only scroll gestures would ever have the touchpad source. It might be
worth asserting that.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:258: case
WebGestureDeviceUninitialized:
Let's put the exceptional case at the end.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:719: case
GestureSourceUninitialized:
ditto

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:4117: tapEvent.sourceDevice =
WebGestureDeviceTouchpad;
Let's make this touch, as we never dispatch tap events for touchpads.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp:80:
webGestureEvent.sourceDevice = WebGestureDeviceTouchpad;
Let's make this touchscreen.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
(right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:276:
webGestureEvent.sourceDevice = WebGestureDeviceTouchpad;
Let's use touchscreen throughout.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:321:
WebGestureEvent webGestureEvent;
If you're feeling industrious, I think a helper method for constructing these
events would make this test a lot easier to read.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp:342:
event.sourceDevice = WebGestureDeviceTouchpad;
touchscreen

[wjmaclean, 2015-10-19 15:23:01]

wjmaclean@chromium.org changed reviewers:
+ creis@chromium.org

[wjmaclean, 2015-10-19 15:26:55]

creis@ - please look at content/public/ changes
rbyers@ and/or jdduke@ - please look at remainder?

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LinkHighlightImplTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:75:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 12:52:39, tdresser wrote:
> Ideally only scroll gestures would ever have the touchpad source. It might be
> worth asserting that.

As we discussed offline, since WebGestureEvent is POD, we really have no way of
enforcing this, except for at places where we convert event types. Very few
places (outside of tests) actually ever look at the source device field.

We'll try at least to make sure that all tests use the Touchscreen device type
for anything that isn't scroll related. Filed a bug for that:
https://code.google.com/p/chromium/issues/detail?id=544985

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:258: case
WebGestureDeviceUninitialized:
On 2015/10/19 12:52:39, tdresser wrote:
> Let's put the exceptional case at the end.

Done.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:719: case
GestureSourceUninitialized:
On 2015/10/19 12:52:39, tdresser wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:4117: tapEvent.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 12:52:39, tdresser wrote:
> Let's make this touch, as we never dispatch tap events for touchpads.

Done.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/ImeOnFocusTest.cpp:80:
webGestureEvent.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 12:52:39, tdresser wrote:
> Let's make this touchscreen.

Done.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
(right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:276:
webGestureEvent.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 12:52:39, tdresser wrote:
> Let's use touchscreen throughout.

Done.

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:321:
WebGestureEvent webGestureEvent;
On 2015/10/19 12:52:39, tdresser wrote:
> If you're feeling industrious, I think a helper method for constructing these
> events would make this test a lot easier to read.

I'm good with that idea, but I suspect it would benefit from having different
builder methods (e.g. ScrollUpdate has a very different payload than
GestureTap). If we go ahead with it, I'd like to suggest doing it in a separate
CL :-)

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp:342:
event.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 12:52:39, tdresser wrote:
> touchscreen

Done.

[wjmaclean, 2015-10-19 15:44:12]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-19 15:44:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/180001

[jdduke (slow), 2015-10-19 16:24:44]

Can you add a simple source validation check in
GestureEventStreamValidator::Validate?

[wjmaclean, 2015-10-19 18:02:45]

jdduke@ - is this what you had in mind?

[wjmaclean, 2015-10-19 18:08:41]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-19 18:09:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/200001

[jdduke (slow), 2015-10-19 18:15:38]

https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
File content/common/input/gesture_event_stream_validator.cc (right):

https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
content/common/input/gesture_event_stream_validator.cc:108: error_msg->append(
Hmm, I was thinking something even more basic, like

if (event.sourceDevice == blink::WebGestureDeviceUninitialized)
  error_msg->append("Gesture event source is uninitialized.\n");

The touchpad validation can probably happen in a separate patch? Maybe Tim can
follow up on that as it's somewhat orthogonal to the issue at hand.

[Rick Byers, 2015-10-19 18:20:29]

third_party/WebKit LGTM with a bunch of nits (mainly that tests should really be
using only plausible source device values, even though it typically won't make
any difference today).

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebGestureDevice.h (right):

https://codereview.chromium.org/1403893003/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebGestureDevice.h:31: enum WebGestureDevice
{
nit: give this the value -1 for consistency with the pattern used by
WebInputEvent::Type

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LinkHighlightImplTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:75:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
Seems very weird to set touchpad here when it's clearly touchscreen.  I assume
it makes no different either way (since GestureShowPress is ALWAYS touchscreen,
we never check), can you just use touchscreen to avoid confusion?

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:146:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
ditto

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/TopControlsTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/TopControlsTest.cpp:99: event.sourceDevice =
WebGestureDeviceTouchpad;
top controls test should presumably be primarily about touchscreen also

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:1603: PlatformGestureEvent
gestureEvent(PlatformEvent::Type::GestureTap, hitPoint, hitPoint, IntSize(0, 0),
0, PlatformEvent::NoModifiers, PlatformGestureSourceTouchpad);
Tap should only ever be touchscreen

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:4761: event.sourceDevice =
WebGestureDeviceTouchpad;
ditto

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1459: event.sourceDevice =
WebGestureDeviceTouchpad;
tap should always be touchscreen

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1512: event.sourceDevice =
WebGestureDeviceTouchpad;
ditto

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1527: event.sourceDevice =
WebGestureDeviceTouchpad;
ditto

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1553: event.sourceDevice =
WebGestureDeviceTouchpad;
ditto

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1647: event.sourceDevice =
WebGestureDeviceTouchpad;
ditto

[Charlie Reis, 2015-10-19 18:37:47]

content/public LGTM.  (Didn't look at the rest.)

[wjmaclean, 2015-10-19 18:58:49]

https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
File content/common/input/gesture_event_stream_validator.cc (right):

https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
content/common/input/gesture_event_stream_validator.cc:108: error_msg->append(
On 2015/10/19 18:15:38, jdduke wrote:
> Hmm, I was thinking something even more basic, like
> 
> if (event.sourceDevice == blink::WebGestureDeviceUninitialized)
>   error_msg->append("Gesture event source is uninitialized.\n");
> 
> The touchpad validation can probably happen in a separate patch? Maybe Tim can
> follow up on that as it's somewhat orthogonal to the issue at hand.

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LinkHighlightImplTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:75:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> Seems very weird to set touchpad here when it's clearly touchscreen.  I assume
> it makes no different either way (since GestureShowPress is ALWAYS
touchscreen,
> we never check), can you just use touchscreen to avoid confusion?

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:146:
touchEvent.sourceDevice = WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/TopControlsTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/TopControlsTest.cpp:99: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> top controls test should presumably be primarily about touchscreen also

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:1603: PlatformGestureEvent
gestureEvent(PlatformEvent::Type::GestureTap, hitPoint, hitPoint, IntSize(0, 0),
0, PlatformEvent::NoModifiers, PlatformGestureSourceTouchpad);
On 2015/10/19 18:20:29, Rick Byers wrote:
> Tap should only ever be touchscreen

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:4761: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1459: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> tap should always be touchscreen

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1512: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1527: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1553: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:1647: event.sourceDevice =
WebGestureDeviceTouchpad;
On 2015/10/19 18:20:29, Rick Byers wrote:
> ditto

Done.

[wjmaclean, 2015-10-19 18:58:57]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-19 19:00:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1403893003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1403893003/220001

[Rick Byers, 2015-10-19 19:07:22]

On 2015/10/19 18:58:49, wjmaclean wrote:
>
https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
> File content/common/input/gesture_event_stream_validator.cc (right):
> 
>
https://codereview.chromium.org/1403893003/diff/200001/content/common/input/g...
> content/common/input/gesture_event_stream_validator.cc:108: error_msg->append(
> On 2015/10/19 18:15:38, jdduke wrote:
> > Hmm, I was thinking something even more basic, like
> > 
> > if (event.sourceDevice == blink::WebGestureDeviceUninitialized)
> >   error_msg->append("Gesture event source is uninitialized.\n");
> > 
> > The touchpad validation can probably happen in a separate patch? Maybe Tim
can
> > follow up on that as it's somewhat orthogonal to the issue at hand.
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/web/LinkHighlightImplTest.cpp (right):
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:75:
> touchEvent.sourceDevice = WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > Seems very weird to set touchpad here when it's clearly touchscreen.  I
assume
> > it makes no different either way (since GestureShowPress is ALWAYS
> touchscreen,
> > we never check), can you just use touchscreen to avoid confusion?
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/LinkHighlightImplTest.cpp:146:
> touchEvent.sourceDevice = WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/web/tests/TopControlsTest.cpp (right):
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/TopControlsTest.cpp:99: event.sourceDevice
=
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > top controls test should presumably be primarily about touchscreen also
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebFrameTest.cpp:1603:
PlatformGestureEvent
> gestureEvent(PlatformEvent::Type::GestureTap, hitPoint, hitPoint, IntSize(0,
0),
> 0, PlatformEvent::NoModifiers, PlatformGestureSourceTouchpad);
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > Tap should only ever be touchscreen
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebFrameTest.cpp:4761: event.sourceDevice
=
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:1459: event.sourceDevice =
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > tap should always be touchscreen
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:1512: event.sourceDevice =
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:1527: event.sourceDevice =
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:1553: event.sourceDevice =
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/1403893003/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:1647: event.sourceDevice =
> WebGestureDeviceTouchpad;
> On 2015/10/19 18:20:29, Rick Byers wrote:
> > ditto
> 
> Done.

Thanks.  WebKit still LGTM

[jdduke (slow), 2015-10-19 19:58:53]

content/*/input lgtm

[commit-bot: I haz the power, 2015-10-19 20:57:03]

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

[commit-bot: I haz the power, 2015-10-19 20:57:04]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2015-10-20 01:06:46]

The CQ bit was checked by wjmaclean@chromium.org

[wjmaclean, 2015-10-20 01:06:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1403893003/#ps220001
(title: "Convert more tests (related to CL) to use Touchscreen.")

[commit-bot: I haz the power, 2015-10-20 01:07:35]

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

[commit-bot: I haz the power, 2015-10-20 01:13:40]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-10-20 01:14:18]

Patchset 12 (id:??) landed as
https://crrev.com/3bb814794b2509e95944cf36af0cc7538611ae03
Cr-Commit-Position: refs/heads/master@{#354946}