Pass real pointer type to be passed to PointerEvents.

Pass real pointer type to be passed to PointerEvents.

This extends platform touch pointer and pointer event handlers to pass
pointer type to pointer events.

This CL is a part of a patch series:
 1. https://codereview.chromium.org/1253183005/
    for new WebPointerProperties fields
 2. https://codereview.chromium.org/1260693003/
    for eventSender web pointer property support
 3. https://codereview.chromium.org/1192463008/ (this)
 4. https://codereview.chromium.org/1190013002/
    for handling pointer type in Chromium event handlers

BUG=514360
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200159

[USE eero AT chromium.org, 2015-06-17 14:07:28]

e.hakkinen@samsung.com changed reviewers:
+ mustaq@chromium.org, rbyers@chromium.org

[USE eero AT chromium.org, 2015-06-17 14:07:29]

PTAL.

A subsequent patch to set the pointerType and the isPrimary fields of a PE could
be squashed into this one after https://codereview.chromium.org/1144313003/ has
landed.

[Rick Byers, 2015-06-18 21:20:50]

On 2015/06/17 14:07:29, e_hakkinen wrote:
> PTAL.
> 
> A subsequent patch to set the pointerType and the isPrimary fields of a PE
could
> be squashed into this one after https://codereview.chromium.org/1144313003/
has
> landed.

On 2015/06/17 14:07:29, e_hakkinen wrote:
> PTAL.
> 
> A subsequent patch to set the pointerType and the isPrimary fields of a PE
could
> be squashed into this one after https://codereview.chromium.org/1144313003/
has
> landed.

As discussed at https://codereview.chromium.org/1192563002/#msg5 I was thinking
it would be WebMouseEvent that we'd extend to become the new pointer event API
(since pointer events are a strict superset of mouse events).  I'm just trying
to think through the tradeoffs of doing it on WebTouchPoint, let me know if you
see others:

Advantages of extending WebTouchPoint:
- can start sending pointer events for stylus right away using Mustaq's support
for pointer events for WebTouchEvent
- gives you existing force and radius properties for stylus
- immediately gives you behavior compatible with the current behavior of firing
touch events on android for stylus input

Advantages of extending WebMouseEvent:
- closer to the final goal where WebMouseEvent is replaced entirely by
WebPointerEvent (and WebTouchEvent can probably eventually be eliminated)
- gives you button & buttons properties that you're going to want for stylus
(adding those to WebTouchPoint would be weird)

Bigger picture, it's not entirely clear to me what the right behavior is for
stylus on.  Should it fire touch events on Android (as it does today) for pages
that don't handle pointer events?  You do want touch/drag with the stylus to
scroll, right?  I'm guessing this will be different for stylus devices on
desktop - no reason to fire touch events or trigger scrolling by dragging them. 
So maybe there's really two different types of stylus input here (one that's
more mouse like and one that's more touch like)?

[Rick Byers, 2015-06-18 21:20:50]

On 2015/06/17 14:07:29, e_hakkinen wrote:
> PTAL.
> 
> A subsequent patch to set the pointerType and the isPrimary fields of a PE
could
> be squashed into this one after https://codereview.chromium.org/1144313003/
has
> landed.

On 2015/06/17 14:07:29, e_hakkinen wrote:
> PTAL.
> 
> A subsequent patch to set the pointerType and the isPrimary fields of a PE
could
> be squashed into this one after https://codereview.chromium.org/1144313003/
has
> landed.

As discussed at https://codereview.chromium.org/1192563002/#msg5 I was thinking
it would be WebMouseEvent that we'd extend to become the new pointer event API
(since pointer events are a strict superset of mouse events).  I'm just trying
to think through the tradeoffs of doing it on WebTouchPoint, let me know if you
see others:

Advantages of extending WebTouchPoint:
- can start sending pointer events for stylus right away using Mustaq's support
for pointer events for WebTouchEvent
- gives you existing force and radius properties for stylus
- immediately gives you behavior compatible with the current behavior of firing
touch events on android for stylus input

Advantages of extending WebMouseEvent:
- closer to the final goal where WebMouseEvent is replaced entirely by
WebPointerEvent (and WebTouchEvent can probably eventually be eliminated)
- gives you button & buttons properties that you're going to want for stylus
(adding those to WebTouchPoint would be weird)

Bigger picture, it's not entirely clear to me what the right behavior is for
stylus on.  Should it fire touch events on Android (as it does today) for pages
that don't handle pointer events?  You do want touch/drag with the stylus to
scroll, right?  I'm guessing this will be different for stylus devices on
desktop - no reason to fire touch events or trigger scrolling by dragging them. 
So maybe there's really two different types of stylus input here (one that's
more mouse like and one that's more touch like)?

[USE eero AT chromium.org, 2015-06-30 17:19:33]

On 2015/06/18 21:20:50, Rick Byers wrote:
> I'm just trying to think through the tradeoffs of doing it on WebTouchPoint,
> let me know if you see others:

I think summarised the tradeoffs very well, so I have nothing to add.

> Advantages of extending WebTouchPoint:
> - can start sending pointer events for stylus right away using Mustaq's
support
> for pointer events for WebTouchEvent
> - gives you existing force and radius properties for stylus
> - immediately gives you behavior compatible with the current behavior of
firing
> touch events on android for stylus input
> 
> Advantages of extending WebMouseEvent:
> - closer to the final goal where WebMouseEvent is replaced entirely by
> WebPointerEvent (and WebTouchEvent can probably eventually be eliminated)
> - gives you button & buttons properties that you're going to want for stylus
> (adding those to WebTouchPoint would be weird)

> Bigger picture, it's not entirely clear to me what the right behavior is for
> stylus on.  Should it fire touch events on Android (as it does today) for
pages
> that don't handle pointer events?  You do want touch/drag with the stylus to
> scroll, right? I'm guessing this will be different for stylus devices on
> desktop - no reason to fire touch events or trigger scrolling by dragging
them. 
> So maybe there's really two different types of stylus input here (one that's
> more mouse like and one that's more touch like)?

IMO, it partially depends on UI. It should be possible to scroll with a stylus
somehow. On an Android phone, I think that a stylus should fire touch events
as it does today so that touch/drag with the stylus scrolls.

On desktop/big tablets, there are more options.

It seems that Surface tablet does not allow scrolling with a stylus in the same
way as with a finger. Instead of that, it shows thick scrollbars (instead of
the normal thin ones) when it detects a stylus. Those thick scrollbars can then
be used with the stylus (using the stylus like a mouse).

On the other hand, the Windows UI on Surface table does allow dragging of
objects with a styles, and that feels very natural thing to do.

So in my opinion, a stylus
- should fire touch events on all platforms (to support dragging)
- should cause scrolling on Android phones if events are not cancelled
  (so causesScrollingIfUncanceled in WebTouchEvent should be true when
  appropriate)
- could be not causing scrolling on desktop/big tablets even if events are not
  cancelled provided that the browser UI provides some other way (like thick
  scrollbars) to scroll with the stylus (causesScrollingIfUncanceled in
  WebTouchEvent could be always false).

[Rick Byers, 2015-07-02 14:27:04]

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

[Rick Byers, 2015-07-02 14:30:02]

On 2015/06/30 17:19:33, e_hakkinen wrote:
> On 2015/06/18 21:20:50, Rick Byers wrote:
> > I'm just trying to think through the tradeoffs of doing it on WebTouchPoint,
> > let me know if you see others:
> 
> I think summarised the tradeoffs very well, so I have nothing to add.
> 
> > Advantages of extending WebTouchPoint:
> > - can start sending pointer events for stylus right away using Mustaq's
> support
> > for pointer events for WebTouchEvent
> > - gives you existing force and radius properties for stylus
> > - immediately gives you behavior compatible with the current behavior of
> firing
> > touch events on android for stylus input
> > 
> > Advantages of extending WebMouseEvent:
> > - closer to the final goal where WebMouseEvent is replaced entirely by
> > WebPointerEvent (and WebTouchEvent can probably eventually be eliminated)
> > - gives you button & buttons properties that you're going to want for stylus
> > (adding those to WebTouchPoint would be weird)
> 
> > Bigger picture, it's not entirely clear to me what the right behavior is for
> > stylus on.  Should it fire touch events on Android (as it does today) for
> pages
> > that don't handle pointer events?  You do want touch/drag with the stylus to
> > scroll, right? I'm guessing this will be different for stylus devices on
> > desktop - no reason to fire touch events or trigger scrolling by dragging
> them. 
> > So maybe there's really two different types of stylus input here (one that's
> > more mouse like and one that's more touch like)?
> 
> IMO, it partially depends on UI. It should be possible to scroll with a stylus
> somehow. On an Android phone, I think that a stylus should fire touch events
> as it does today so that touch/drag with the stylus scrolls.
> 
> On desktop/big tablets, there are more options.
> 
> It seems that Surface tablet does not allow scrolling with a stylus in the
same
> way as with a finger. Instead of that, it shows thick scrollbars (instead of
> the normal thin ones) when it detects a stylus. Those thick scrollbars can
then
> be used with the stylus (using the stylus like a mouse).
> 
> On the other hand, the Windows UI on Surface table does allow dragging of
> objects with a styles, and that feels very natural thing to do.
> 
> So in my opinion, a stylus
> - should cause scrolling on Android phones if events are not cancelled
>   (so causesScrollingIfUncanceled in WebTouchEvent should be true when
>   appropriate)

Agreed

> - could be not causing scrolling on desktop/big tablets even if events are not
>   cancelled provided that the browser UI provides some other way (like thick
>   scrollbars) to scroll with the stylus (causesScrollingIfUncanceled in
>   WebTouchEvent could be always false).

Agreed

> - should fire touch events on all platforms (to support dragging)

I'm not so sure about this one.  If they won't cause scrolling then I think it's
better to fire mouse events than touch events.  Basically a stylus either
emulates a mouse or touch depending on platform conventions - not some third
compatibility model.  WDYT?

This is sounding to me like the right way to support stylus is by unifying touch
and mouse into a single event type from chromium (WebPointerEvent) with a
'device emulation mode' property that's either touch or mouse (up to chromium to
choose).  I see two paths for getting there incrementally:

The hardest part of getting there incrementally I think is all the chromium
InputRouter / GestureRecognizer code that works in terms of WebTouchEvent.  If
you want drag-scrolling then you really do want to be sending WebTouchEvent
instances to blink, at least for now.

Given that I'm OK with an incremental path which expands BOTH WebTouchEvent and
WebMouseEvent with pointer event properties, with a TODO that says that these
two event types should eventually get unified into a WebPointerEventType.  I.e.
our "device emulation mode' property is really just the choice of WebMouseEvent
vs. WebTouchEvent.  It means we'll have some redundancy for a little while, but
that's true as long as all the chromium code is operating in terms of
WebTouchEvent.  We can keep an eye out for opportunities to share types / code
(eg. maybe we have a WebPointerProperties that's contained in both WebMouseEvent
and WebTouchPoint?).

+tdresser and jdduke for their opinion.

If they don't have any objection then I will add some notes to the PE design doc
and we can proceed with reviewing / landing these patches as your originally
designed (sorry it's taking awhile to get here - I'm struggling a bit to wrap my
head around all the changes we'll ultimately need to make).

[USE eero AT chromium.org, 2015-07-02 15:29:30]

On 2015/07/02 14:30:02, Rick Byers wrote:
> On 2015/06/30 17:19:33, e_hakkinen wrote:
> > So in my opinion, a stylus
> > - should cause scrolling on Android phones if events are not cancelled
> >   (so causesScrollingIfUncanceled in WebTouchEvent should be true when
> >   appropriate)
> 
> Agreed
> 
> > - could be not causing scrolling on desktop/big tablets even if events are
not
> >   cancelled provided that the browser UI provides some other way (like thick
> >   scrollbars) to scroll with the stylus (causesScrollingIfUncanceled in
> >   WebTouchEvent could be always false).
> 
> Agreed
> 
> > - should fire touch events on all platforms (to support dragging)
> 
> I'm not so sure about this one.  If they won't cause scrolling then I think
it's
> better to fire mouse events than touch events.  Basically a stylus either
> emulates a mouse or touch depending on platform conventions - not some third
> compatibility model.  WDYT?

You are right. I was thinking in too mobile centric way. If a stylus generates
mouse events even when in contact with touch screen, it should work fine
(provided that websites handle mouse events which they should as otherwise they
do not work on desktop at all).

> If they don't have any objection then I will add some notes to the PE design
doc
> and we can proceed with reviewing / landing these patches as your originally
> designed (sorry it's taking awhile to get here - I'm struggling a bit to wrap
my
> head around all the changes we'll ultimately need to make).

Sounds good to me.

[tdresser, 2015-07-02 15:52:36]

This SGTM.

> (eg. maybe we have a WebPointerProperties that's contained in both
WebMouseEvent
> and WebTouchPoint?).
+1

[USE eero AT chromium.org, 2015-07-31 12:17:34]

PTAL. Now this has been rebased enough, I think.

[Rick Byers, 2015-07-31 13:13:36]

Looks good, but don't you need to update expected output files?

https://codereview.chromium.org/1192463008/diff/80001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1192463008/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:3492: PointerIdManager::PointerType
pointerTypeForWebPointPointerType(WebPointerProperties::PointerType type)
You could probably simplify this by changing PointerIdManager to use your new
enum rather than defining its own.  I don't have a strong preference though - up
to you and Mustaq.

[Rick Byers, 2015-07-31 13:14:31]

rbyers@chromium.org changed reviewers:
- jdduke@chromium.org, tdresser@chromium.org

[Rick Byers, 2015-07-31 13:16:40]

Moving jdduke and tdresser to cc.  Blink changes like this only require review
from Mustaq and myself (chromium side is usually jdduke with the rest of us on
cc).

[USE eero AT chromium.org, 2015-07-31 13:39:53]

On 2015/07/31 13:13:36, Rick Byers (Out until Aug 4th) wrote:
> Looks good, but don't you need to update expected output files?

No, there is no need to. In https://codereview.chromium.org/1192563002/ I did
not test pointerType field (so there was no related failures) and PASSes in
expected output files are reported on event level (and not on event property
level) thus the passes do not change either even though there are new asserts.

On 2015/07/31 13:16:40, Rick Byers (Out until Aug 4th) wrote:
> Moving jdduke and tdresser to cc.  Blink changes like this only require review
> from Mustaq and myself (chromium side is usually jdduke with the rest of us on
> cc).

Thanks for info and for doing that.

[Rick Byers, 2015-08-06 14:03:18]

Ping Mustaq - PTAL (I didn't want to approve before you had a chance to take a
look - sorry if that wasn't clear).

If Mustaq is happy, then I'm happy - proactive LGTM but please wait for Mustaq's
LGTM before landing.

[mustaq, 2015-08-06 20:37:28]

Just discovered that I never sent my draft comment below. LGTM but at least add
a TODO for the enum cleanup.

https://codereview.chromium.org/1192463008/diff/80001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1192463008/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:3492: PointerIdManager::PointerType
pointerTypeForWebPointPointerType(WebPointerProperties::PointerType type)
On 2015/07/31 13:13:35, Rick Byers (Out until Aug 4th) wrote:
> You could probably simplify this by changing PointerIdManager to use your new
> enum rather than defining its own.  I don't have a strong preference though -
up
> to you and Mustaq.

Yes, the enums defined only in WebPointerProperties would be a cleaner solution.

[Rick Byers, 2015-08-06 20:43:14]

Thanks Mustaq.  LGTM also (assuming a TODO added for enum cleanup which can
hopefully come later).

[USE eero AT chromium.org, 2015-08-07 10:14:03]

The CQ bit was checked by e.hakkinen@samsung.com

[USE eero AT chromium.org, 2015-08-07 10:14:04]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1192463008/#ps100001
(title: "TODO")

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

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

[commit-bot: I haz the power, 2015-08-07 11:35:50]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200159