EventSender::GestureFlingStart takes a source_device.

EventSender::GestureFlingStart takes a source_device.

This is the first of three patches.

The second patch (in blink) will make all existing uses of
window.eventSender.gestureFlingStart pass a source device.
(https://codereview.chromium.org/901323004)

The third patch (in chrome) will change
EventSenderBindings::GestureFlingStart to require a source device.

BUG=456136
TEST=LayoutTests/fast/events/touch/

Committed: https://crrev.com/3a1c97231767dfea7d9c079d77ac9889ed9fe208
Cr-Commit-Position: refs/heads/master@{#316221}

[tdresser, 2015-02-06 16:18:28]

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

[tdresser, 2015-02-06 16:18:29]

I think that the long term correct approach is to require passing the source
device. I could just make it default to touchpad, but this should make it less
likely for people to use the wrong device.

[Rick Byers, 2015-02-07 11:20:23]

lgtm with nit - thanks for fixing this.

Note that until very recently (ccameron's pinch patches) event_sender wasn't
initializing the source device for gesture events at all!

https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/event_sender.cc (right):

https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1635: event.sourceDevice =
blink::WebGestureDeviceTouchscreen;
This should probably have logic similar to GestureEvent below where unrecognized
strings throw an error, right?

[tdresser, 2015-02-09 14:48:08]

https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/event_sender.cc (right):

https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1635: event.sourceDevice =
blink::WebGestureDeviceTouchscreen;
On 2015/02/07 11:20:23, Rick Byers wrote:
> This should probably have logic similar to GestureEvent below where
unrecognized
> strings throw an error, right?

My plan was to switch to use |std::string| instead of |gin::Arguments| once I've
updated blink to always pass a device_string. Without a |gin::Arguments|, you
can't throw an error.

This approach has precedent, for example, EventSender::MouseDown defaults to
assuming a middle click.
https://code.google.com/p/chromium/codesearch#chromium/src/content/shell/rend...

If you'd prefer, I can switch to always passing a |gin::Arguments|.

[Rick Byers, 2015-02-11 07:12:17]

On 2015/02/09 14:48:08, tdresser wrote:
>
https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
> File content/shell/renderer/test_runner/event_sender.cc (right):
> 
>
https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
> content/shell/renderer/test_runner/event_sender.cc:1635: event.sourceDevice =
> blink::WebGestureDeviceTouchscreen;
> On 2015/02/07 11:20:23, Rick Byers wrote:
> > This should probably have logic similar to GestureEvent below where
> unrecognized
> > strings throw an error, right?
> 
> My plan was to switch to use |std::string| instead of |gin::Arguments| once
I've
> updated blink to always pass a device_string. Without a |gin::Arguments|, you
> can't throw an error.
> 
> This approach has precedent, for example, EventSender::MouseDown defaults to
> assuming a middle click.
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/shell/rend...
> 
> If you'd prefer, I can switch to always passing a |gin::Arguments|.

It just seems so easy to get a typo in the string and not realize we're testing
something subtly different than we think (eg. I always forget whether it's
'touchpad' or 'trackpad').  We could just ASSERT, but then we'd get ClusterFuzz
failures.  So I guess I'd prefer you retain the ability to throw on unexpected
value...

[tdresser, 2015-02-11 14:54:37]

New patchsets have been uploaded after l-g-t-m from rbyers@chromium.org

[tdresser, 2015-02-11 14:56:10]

On 2015/02/11 07:12:17, Rick Byers wrote:
> On 2015/02/09 14:48:08, tdresser wrote:
> >
>
https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
> > File content/shell/renderer/test_runner/event_sender.cc (right):
> > 
> >
>
https://codereview.chromium.org/902173003/diff/1/content/shell/renderer/test_...
> > content/shell/renderer/test_runner/event_sender.cc:1635: event.sourceDevice
=
> > blink::WebGestureDeviceTouchscreen;
> > On 2015/02/07 11:20:23, Rick Byers wrote:
> > > This should probably have logic similar to GestureEvent below where
> > unrecognized
> > > strings throw an error, right?
> > 
> > My plan was to switch to use |std::string| instead of |gin::Arguments| once
> I've
> > updated blink to always pass a device_string. Without a |gin::Arguments|,
you
> > can't throw an error.
> > 
> > This approach has precedent, for example, EventSender::MouseDown defaults to
> > assuming a middle click.
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/shell/rend...
> > 
> > If you'd prefer, I can switch to always passing a |gin::Arguments|.
> 
> It just seems so easy to get a typo in the string and not realize we're
testing
> something subtly different than we think (eg. I always forget whether it's
> 'touchpad' or 'trackpad').  We could just ASSERT, but then we'd get
ClusterFuzz
> failures.  So I guess I'd prefer you retain the ability to throw on unexpected
> value...

Sounds good, done.

Can you take another look when you get the chance?

[Rick Byers, 2015-02-13 08:28:57]

Thanks!   LGTM

[tdresser, 2015-02-13 15:19:30]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2015-02-13 15:19:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902173003/20001

[commit-bot: I haz the power, 2015-02-13 15:45:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-13 15:46:02]

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