Support WebPointerProperties in MouseEvent

Support WebPointerProperties in MouseEvent

This allows pointer properties to be passed along with MouseEvents
to the PPAPI.

BUG=647045
Committed: https://crrev.com/96db64b4471ebc8aa7f01d9a38b65ff952fd6fe3
Cr-Commit-Position: refs/heads/master@{#419931}

[denniskempin, 2016-09-14 22:23:57]

denniskempin@google.com changed reviewers:
+ rbyers@chromium.org

[denniskempin, 2016-09-14 22:23:57]

Hi Rick!
Could you take a look at this change? I have noticed that pointer properties in
WebMouseEvents are being lost during translation when they reach the PPAPI, and
we need that information there.

I hope we can change this core/events API without any big consequences.

[Rick Byers, 2016-09-15 20:20:14]

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

[Rick Byers, 2016-09-15 20:20:14]

Sticking extra information into the dom MouseEvent object from the
WebPointerProperties makes sense.  But they way you've done it there's now a
bunch of redundant information, right?  This could be confusing - which should
be used, are they guaranteed to be in sync?

+Dave who is working on a plan to eliminate one of the layers of abstraction
here.  Perhaps we can just take one of the first steps as part of this CL (eg.
maybe it's removing a bunch of the fields in MouseEvent and pulling the data out
of the pointer properties as needed).  I'm about to be travelling so please work
with Dave on this so you're not blocked on me. I'll probably be able to give you
an owners rubber-stamp once Dave is happy.

The other option of course is to just extend the existing (sub-optimal) pattern
of adding a bunch more fields to MouseEvent and copying up/down from the
WebMouseEvent.

[Rick Byers, 2016-09-15 20:20:16]

Sticking extra information into the dom MouseEvent object from the
WebPointerProperties makes sense.  But they way you've done it there's now a
bunch of redundant information, right?  This could be confusing - which should
be used, are they guaranteed to be in sync?

+Dave who is working on a plan to eliminate one of the layers of abstraction
here.  Perhaps we can just take one of the first steps as part of this CL (eg.
maybe it's removing a bunch of the fields in MouseEvent and pulling the data out
of the pointer properties as needed).  I'm about to be travelling so please work
with Dave on this so you're not blocked on me. I'll probably be able to give you
an owners rubber-stamp once Dave is happy.

The other option of course is to just extend the existing (sub-optimal) pattern
of adding a bunch more fields to MouseEvent and copying up/down from the
WebMouseEvent.

[dtapuska, 2016-09-15 20:30:48]

On 2016/09/15 20:20:16, Rick Byers wrote:
> Sticking extra information into the dom MouseEvent object from the
> WebPointerProperties makes sense.  But they way you've done it there's now a
> bunch of redundant information, right?  This could be confusing - which should
> be used, are they guaranteed to be in sync?
> 
> +Dave who is working on a plan to eliminate one of the layers of abstraction
> here.  Perhaps we can just take one of the first steps as part of this CL (eg.
> maybe it's removing a bunch of the fields in MouseEvent and pulling the data
out
> of the pointer properties as needed).  I'm about to be travelling so please
work
> with Dave on this so you're not blocked on me. I'll probably be able to give
you
> an owners rubber-stamp once Dave is happy.
> 
> The other option of course is to just extend the existing (sub-optimal)
pattern
> of adding a bunch more fields to MouseEvent and copying up/down from the
> WebMouseEvent.

Would it be possible change this to store a copy of the PlatformMouseEvent on
the DOM MouseEvent ?

Take a look at the KeyboardEvent which does this. Although currently it has
changed to store an actual
WebKeyboardEvent but that is the model that I am going after.

[denniskempin, 2016-09-16 18:18:04]

On 2016/09/15 20:30:48, dtapuska wrote:
> On 2016/09/15 20:20:16, Rick Byers wrote:
> > Sticking extra information into the dom MouseEvent object from the
> > WebPointerProperties makes sense.  But they way you've done it there's now a
> > bunch of redundant information, right?  This could be confusing - which
should
> > be used, are they guaranteed to be in sync?
> > 
> > +Dave who is working on a plan to eliminate one of the layers of abstraction
> > here.  Perhaps we can just take one of the first steps as part of this CL
(eg.
> > maybe it's removing a bunch of the fields in MouseEvent and pulling the data
> out
> > of the pointer properties as needed).  I'm about to be travelling so please
> work
> > with Dave on this so you're not blocked on me. I'll probably be able to give
> you
> > an owners rubber-stamp once Dave is happy.
> > 
> > The other option of course is to just extend the existing (sub-optimal)
> pattern
> > of adding a bunch more fields to MouseEvent and copying up/down from the
> > WebMouseEvent.
> 
> Would it be possible change this to store a copy of the PlatformMouseEvent on
> the DOM MouseEvent ?
> 
> Take a look at the KeyboardEvent which does this. Although currently it has
> changed to store an actual
> WebKeyboardEvent but that is the model that I am going after.

Thanks Dave and Rick!

I am okay with either path:
- For our purposes we only need the pointer tool type. We can just store this as
a single value instead of the whole WebPointerProperties. That way we will not
have any duplicate information. 
- I am also ok with updating MouseEvent to keep a copy of WebMouseEvent and use
that in WebInputEventConversion.cpp. Thought his would duplicate a lot of
information, right?

[dtapuska, 2016-09-16 18:21:23]

On 2016/09/16 18:18:04, denniskempin wrote:
> On 2016/09/15 20:30:48, dtapuska wrote:
> > On 2016/09/15 20:20:16, Rick Byers wrote:
> > > Sticking extra information into the dom MouseEvent object from the
> > > WebPointerProperties makes sense.  But they way you've done it there's now
a
> > > bunch of redundant information, right?  This could be confusing - which
> should
> > > be used, are they guaranteed to be in sync?
> > > 
> > > +Dave who is working on a plan to eliminate one of the layers of
abstraction
> > > here.  Perhaps we can just take one of the first steps as part of this CL
> (eg.
> > > maybe it's removing a bunch of the fields in MouseEvent and pulling the
data
> > out
> > > of the pointer properties as needed).  I'm about to be travelling so
please
> > work
> > > with Dave on this so you're not blocked on me. I'll probably be able to
give
> > you
> > > an owners rubber-stamp once Dave is happy.
> > > 
> > > The other option of course is to just extend the existing (sub-optimal)
> > pattern
> > > of adding a bunch more fields to MouseEvent and copying up/down from the
> > > WebMouseEvent.
> > 
> > Would it be possible change this to store a copy of the PlatformMouseEvent
on
> > the DOM MouseEvent ?
> > 
> > Take a look at the KeyboardEvent which does this. Although currently it has
> > changed to store an actual
> > WebKeyboardEvent but that is the model that I am going after.
> 
> Thanks Dave and Rick!
> 
> I am okay with either path:
> - For our purposes we only need the pointer tool type. We can just store this
as
> a single value instead of the whole WebPointerProperties. That way we will not
> have any duplicate information. 
> - I am also ok with updating MouseEvent to keep a copy of WebMouseEvent and
use
> that in WebInputEventConversion.cpp. Thought his would duplicate a lot of
> information, right?

When going into and out of a plugin we re-constitute an event; and we don't
really want to do that. We just want a handle to the native event. For this case
you'll have to hold onto a PlatformMouseEvent for now. And then convert that
back into a WebMouseEvent. We want to really get rid of all the converters..

[denniskempin, 2016-09-16 18:26:32]

On 2016/09/16 18:21:23, dtapuska wrote:
> On 2016/09/16 18:18:04, denniskempin wrote:
> > On 2016/09/15 20:30:48, dtapuska wrote:
> > > On 2016/09/15 20:20:16, Rick Byers wrote:
> > > > Sticking extra information into the dom MouseEvent object from the
> > > > WebPointerProperties makes sense.  But they way you've done it there's
now
> a
> > > > bunch of redundant information, right?  This could be confusing - which
> > should
> > > > be used, are they guaranteed to be in sync?
> > > > 
> > > > +Dave who is working on a plan to eliminate one of the layers of
> abstraction
> > > > here.  Perhaps we can just take one of the first steps as part of this
CL
> > (eg.
> > > > maybe it's removing a bunch of the fields in MouseEvent and pulling the
> data
> > > out
> > > > of the pointer properties as needed).  I'm about to be travelling so
> please
> > > work
> > > > with Dave on this so you're not blocked on me. I'll probably be able to
> give
> > > you
> > > > an owners rubber-stamp once Dave is happy.
> > > > 
> > > > The other option of course is to just extend the existing (sub-optimal)
> > > pattern
> > > > of adding a bunch more fields to MouseEvent and copying up/down from the
> > > > WebMouseEvent.
> > > 
> > > Would it be possible change this to store a copy of the PlatformMouseEvent
> on
> > > the DOM MouseEvent ?
> > > 
> > > Take a look at the KeyboardEvent which does this. Although currently it
has
> > > changed to store an actual
> > > WebKeyboardEvent but that is the model that I am going after.
> > 
> > Thanks Dave and Rick!
> > 
> > I am okay with either path:
> > - For our purposes we only need the pointer tool type. We can just store
this
> as
> > a single value instead of the whole WebPointerProperties. That way we will
not
> > have any duplicate information. 
> > - I am also ok with updating MouseEvent to keep a copy of WebMouseEvent and
> use
> > that in WebInputEventConversion.cpp. Thought his would duplicate a lot of
> > information, right?
> 
> When going into and out of a plugin we re-constitute an event; and we don't
> really want to do that. We just want a handle to the native event. For this
case
> you'll have to hold onto a PlatformMouseEvent for now. And then convert that
> back into a WebMouseEvent. We want to really get rid of all the converters..

I totally approve of less converters. I'll try to put in the PlatformMouseEvent
then.

[denniskempin, 2016-09-19 23:37:06]

Dave, would this work?

Ideally we would do the init work in the constructor + an init method, so we do
not need to pass a raw pointer, similar to what KeyEvent is doing. But since I
am not too comfortable with this code and where it's being used I'd rather keep
the change small so we can continue with the NaCl work.

[dtapuska, 2016-09-20 14:55:16]

On 2016/09/19 23:37:06, denniskempin wrote:
> Dave, would this work?
> 
> Ideally we would do the init work in the constructor + an init method, so we
do
> not need to pass a raw pointer, similar to what KeyEvent is doing. But since I
> am not too comfortable with this code and where it's being used I'd rather
keep
> the change small so we can continue with the NaCl work.

Yup this seems good to me for now.

[Rick Byers, 2016-09-20 20:47:57]

RS LGTM (Dave is the expert)

[denniskempin, 2016-09-20 20:52:51]

The CQ bit was checked by denniskempin@google.com

[commit-bot: I haz the power, 2016-09-20 20:53:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2016-09-20 20:56:47]

On 2016/09/20 20:53:14, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

lgtm

[commit-bot: I haz the power, 2016-09-20 21:06:48]

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

[commit-bot: I haz the power, 2016-09-20 21:06:49]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[denniskempin, 2016-09-20 22:48:25]

The CQ bit was checked by denniskempin@google.com

[denniskempin, 2016-09-20 22:48:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/2344693002/#ps60001
(title: "fixed unit tests")

[commit-bot: I haz the power, 2016-09-20 22:49:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 23:20:18]

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

[commit-bot: I haz the power, 2016-09-20 23:20:19]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[denniskempin, 2016-09-21 00:05:19]

The CQ bit was checked by denniskempin@google.com

[denniskempin, 2016-09-21 00:05:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/2344693002/#ps80001
(title: "fixed osx build")

[commit-bot: I haz the power, 2016-09-21 00:05:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 01:33:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-21 01:35:38]

Description was changed from

==========
Support WebPointerProperties in MouseEvent

This allows pointer properties to be passed along with MouseEvents
to the PPAPI.

BUG=647045
==========

to

==========
Support WebPointerProperties in MouseEvent

This allows pointer properties to be passed along with MouseEvents
to the PPAPI.

BUG=647045

Committed: https://crrev.com/96db64b4471ebc8aa7f01d9a38b65ff952fd6fe3
Cr-Commit-Position: refs/heads/master@{#419931}
==========

[commit-bot: I haz the power, 2016-09-21 01:35:40]

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