Delay instantiating InputDeviceCapabilities until accessed.

Delay instantiating InputDeviceCapabilities until accessed.

UIEvent.sourceCapabilities is an optional InputDeviceCapabilities object
( https://wicg.github.io/InputDeviceCapabilities/ ) which represents
the capabilities of the device that originated the event, if known.

Switch the internal representation of the event's device capabilities
to be a simple value, and only instantiate it as a full-fledged
InputDeviceCapabilities instance upon eventual access.

R=
BUG=688569

[sof, 2017-02-05 20:58:22]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-05 20:58:27]

Dry run: 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, 2017-02-05 22:18:56]

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

[commit-bot: I haz the power, 2017-02-05 22:18:57]

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

[sof, 2017-02-06 10:26:28]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-06 10:26:48]

Dry run: 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, 2017-02-06 11:57:35]

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

[commit-bot: I haz the power, 2017-02-06 11:57:36]

Dry run: This issue passed the CQ dry run.

[sof, 2017-02-10 09:56:26]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-10 09:56:44]

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

[sof, 2017-02-10 10:32:16]

Description was changed from

==========
Introduce InputDeviceCapabilitiesValue

R=
BUG=
==========

to

==========
Delay instantiating InputDeviceCapabilities until accessed.

UIEvent.sourceCapabilities is an optional InputDeviceCapabilities object
( https://wicg.github.io/InputDeviceCapabilities/ ) which represents
the capabilities of the device that originated the event, if known.

Switch the internal representation of the event's device capabilities
to be a simple value, and only instantiate it as a full-fledged
InputDeviceCapabilities instance upon eventual access.

R=
BUG=
==========

[sof, 2017-02-10 10:36:01]

Description was changed from

==========
Delay instantiating InputDeviceCapabilities until accessed.

UIEvent.sourceCapabilities is an optional InputDeviceCapabilities object
( https://wicg.github.io/InputDeviceCapabilities/ ) which represents
the capabilities of the device that originated the event, if known.

Switch the internal representation of the event's device capabilities
to be a simple value, and only instantiate it as a full-fledged
InputDeviceCapabilities instance upon eventual access.

R=
BUG=
==========

to

==========
Delay instantiating InputDeviceCapabilities until accessed.

UIEvent.sourceCapabilities is an optional InputDeviceCapabilities object
( https://wicg.github.io/InputDeviceCapabilities/ ) which represents
the capabilities of the device that originated the event, if known.

Switch the internal representation of the event's device capabilities
to be a simple value, and only instantiate it as a full-fledged
InputDeviceCapabilities instance upon eventual access.

R=
BUG=688569
==========

[sof, 2017-02-10 10:47:09]

sigbjornf@opera.com changed reviewers:
+ lanwei@chromium.org, rbyers@chromium.org

[sof, 2017-02-10 10:47:11]

please take a look.

I think this is a preferable internal representation for these capabilities, but
am open to input on that assessment.

[commit-bot: I haz the power, 2017-02-10 11:48:05]

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

[commit-bot: I haz the power, 2017-02-10 11:48:05]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Rick Byers, 2017-02-11 15:31:01]

Thanks for working on this!

Can you explain a little more why it's so valuable to delay creating
InputDeviceCapability instances until they're requested by script?  We don't do
that, for other DOM APIs like 'Document' or 'Node' instances, what's different
here?

Note that we expect to have non-boolean properties in the future (like the
granularity of the pointing device in pixels - see
https://docs.google.com/document/d/1WLadG2dn4vlCewOmUtUEoRsThiptC7Ox28CRmYUn8...),
so I'm not sure the concept of a Discriptor bit-flags is going to scale.

That said, I think this is still moving things in a direction to how I imagined
InputDeviceCapabilities evolving over time, just with a little different
abstraction focused on an (internal-only) concept of concrete input devices.

I.e. instead of InputDeviceCapabilityConstants, I imagined  a
InputDeviceRegistry on each DOMWindow which given some sort of device ID returns
an InputDeviceCapabilities representing that device.  So instead of 'Descriptor'
with flags like 'firesTouchEvents' we could (for now) just have an
InputDeviceType enum with values like:
 None (no / unknown input device)
 Keyboard
 Mouse
 Touchscreen

Keyboard and Mouse could supply the same or different (but equivalent)
InputDeviceCapabilities instances for now.

Longer term this InputDeviceType might evolve to be some sort of unique device
handle, so, for example, we could represent the difference between multiple
attached devices (USB mouse, touchpad, stylus and two different touchscreens,
etc.).  But I don't see needing that level of complexity anytime soon.

If we really want an abstraction between what the registry returns and the
InputDeviceCapabilities provided to script (i.e. your
InputDeviceCapabilitiesValue)) we could have a 'InputDevice' object which
represents the abstract device and creates it's InputDeviceCapabilities instance
on demand.  But again it's not clear to me what this extra level of indirection
really buys us.

WDYT?

[sof, 2017-02-13 12:57:32]

On 2017/02/11 15:31:01, Rick Byers wrote:
> Thanks for working on this!
> 
> Can you explain a little more why it's so valuable to delay creating
> InputDeviceCapability instances until they're requested by script?  We don't
do
> that, for other DOM APIs like 'Document' or 'Node' instances, what's different
> here?

I think it comes down two issues from here: this an object and property that's
new and rarely accessed (*), so delaying the cost of instantiating it, makes
some sense. More so if the "IDC" instances are created per UIEvent instance (if
that's the model you're moving to; not sure.) The other issue is that we're
relying on the event dispatch machinery to associate these ScriptWrappables with
the correct context, something that came up via
https://codereview.chromium.org/2675793005/#msg35 .

( * - not meant as a detraction in any way, the new functionality serves a clear
purpose. )

> 
> Note that we expect to have non-boolean properties in the future (like the
> granularity of the pointing device in pixels - see
>
https://docs.google.com/document/d/1WLadG2dn4vlCewOmUtUEoRsThiptC7Ox28CRmYUn8...),
> so I'm not sure the concept of a Discriptor bit-flags is going to scale.
> 

InputDeviceCapabilitesValue is the value rep, so non-boolean capabilities could
be accommodated via extra constructors for it. Along with some combinators for
composing such values together.

> That said, I think this is still moving things in a direction to how I
imagined
> InputDeviceCapabilities evolving over time, just with a little different
> abstraction focused on an (internal-only) concept of concrete input devices.
> 
> I.e. instead of InputDeviceCapabilityConstants, I imagined  a
> InputDeviceRegistry on each DOMWindow which given some sort of device ID
returns
> an InputDeviceCapabilities representing that device.  So instead of
'Descriptor'
> with flags like 'firesTouchEvents' we could (for now) just have an
> InputDeviceType enum with values like:
>  None (no / unknown input device)
>  Keyboard
>  Mouse
>  Touchscreen
> 
> Keyboard and Mouse could supply the same or different (but equivalent)
> InputDeviceCapabilities instances for now.
> 

That registration model along with passing abstract IDs around, would work just
as well (if not better) wrt the above concerns. I wouldn't mind going in that
direction instead.

[Rick Byers, 2017-02-13 20:25:51]

On 2017/02/13 12:57:32, sof wrote:
> On 2017/02/11 15:31:01, Rick Byers wrote:
> > Thanks for working on this!
> > 
> > Can you explain a little more why it's so valuable to delay creating
> > InputDeviceCapability instances until they're requested by script?  We don't
> do
> > that, for other DOM APIs like 'Document' or 'Node' instances, what's
different
> > here?
> 
> I think it comes down two issues from here: this an object and property that's
> new and rarely accessed (*), so delaying the cost of instantiating it, makes
> some sense. More so if the "IDC" instances are created per UIEvent instance
(if
> that's the model you're moving to; not sure.)

Ideally I think we'd ultimately have exactly one IDC instance per (web-exposed)
physical device that generates UIEvents.  So typically 2 or maybe as much as ~5
per window.  Delaying the cost of creating 2-5 C++ objects hardly seems worth
adding any complexity for, right?  I'm sure many pages have far more than 5 Node
instances which are never accessed by script, and we haven't worried about
optimizing away construction of never-accessed Node objects, right?

> The other issue is that we're
> relying on the event dispatch machinery to associate these ScriptWrappables
with
> the correct context, something that came up via
> https://codereview.chromium.org/2675793005/#msg35 .
> 
> ( * - not meant as a detraction in any way, the new functionality serves a
clear
> purpose. )

Ah, good point.  So if we instead created the ~3 InputDeviceCapabilities
instances during DOMWindow creation, that would address the concern there too,
right?

> > 
> > Note that we expect to have non-boolean properties in the future (like the
> > granularity of the pointing device in pixels - see
> >
>
https://docs.google.com/document/d/1WLadG2dn4vlCewOmUtUEoRsThiptC7Ox28CRmYUn8...),
> > so I'm not sure the concept of a Discriptor bit-flags is going to scale.
> > 
> 
> InputDeviceCapabilitesValue is the value rep, so non-boolean capabilities
could
> be accommodated via extra constructors for it. Along with some combinators for
> composing such values together.
> 
> > That said, I think this is still moving things in a direction to how I
> imagined
> > InputDeviceCapabilities evolving over time, just with a little different
> > abstraction focused on an (internal-only) concept of concrete input devices.
> > 
> > I.e. instead of InputDeviceCapabilityConstants, I imagined  a
> > InputDeviceRegistry on each DOMWindow which given some sort of device ID
> returns
> > an InputDeviceCapabilities representing that device.  So instead of
> 'Descriptor'
> > with flags like 'firesTouchEvents' we could (for now) just have an
> > InputDeviceType enum with values like:
> >  None (no / unknown input device)
> >  Keyboard
> >  Mouse
> >  Touchscreen
> > 
> > Keyboard and Mouse could supply the same or different (but equivalent)
> > InputDeviceCapabilities instances for now.
> > 
> 
> That registration model along with passing abstract IDs around, would work
just
> as well (if not better) wrt the above concerns. I wouldn't mind going in that
> direction instead.

That would be great!  That definitely seems easier to reason about to me, and is
more along the direction we expect to need to head anyway.

So concretely, that would mean each DOMWindow has an InputDeviceRegistry
initialized at DOMWindow creation time which maps some opaque DeviceID to
InputDeviceCapabilities instances (associated with the same ScriptWrapable as
the DOMWindow they were created for), right?  Would we just use some simple
constant DeviceID enum values (Keyboard, Mouse, Touchscreen) for now?

[sof, 2017-02-17 19:09:27]

On 2017/02/13 20:25:51, Rick Byers wrote:
> On 2017/02/13 12:57:32, sof wrote:
> > On 2017/02/11 15:31:01, Rick Byers wrote:
> > > Thanks for working on this!
> > > 
> > > Can you explain a little more why it's so valuable to delay creating
> > > InputDeviceCapability instances until they're requested by script?  We
don't
> > do
> > > that, for other DOM APIs like 'Document' or 'Node' instances, what's
> different
> > > here?
> > 
> > I think it comes down two issues from here: this an object and property
that's
> > new and rarely accessed (*), so delaying the cost of instantiating it, makes
> > some sense. More so if the "IDC" instances are created per UIEvent instance
> (if
> > that's the model you're moving to; not sure.)
> 
> Ideally I think we'd ultimately have exactly one IDC instance per
(web-exposed)
> physical device that generates UIEvents.  So typically 2 or maybe as much as
~5
> per window.  Delaying the cost of creating 2-5 C++ objects hardly seems worth
> adding any complexity for, right?  I'm sure many pages have far more than 5
Node
> instances which are never accessed by script, and we haven't worried about
> optimizing away construction of never-accessed Node objects, right?
> 

ok, I think that's one source of confusion here -- an earlier exchange suggested
that you wanted to move away from this stateful model and have these cap objects
be per-event values with the lifetime of an event dispatch. Eagerly creating
those seems..too eager, perhaps?

The above object lifetime model provides an info conduit where scripts can
attach whatever state they want for the lifetime of the renderer process, iiuc.

> > The other issue is that we're
> > relying on the event dispatch machinery to associate these ScriptWrappables
> with
> > the correct context, something that came up via
> > https://codereview.chromium.org/2675793005/#msg35 .
> > 
> > ( * - not meant as a detraction in any way, the new functionality serves a
> clear
> > purpose. )
> 
> Ah, good point.  So if we instead created the ~3 InputDeviceCapabilities
> instances during DOMWindow creation, that would address the concern there too,
> right?
> 

That's where they currently reside, but requires the code to associate the IDCs
with the correct window.

> > > 
> > > Note that we expect to have non-boolean properties in the future (like the
> > > granularity of the pointing device in pixels - see
> > >
> >
>
https://docs.google.com/document/d/1WLadG2dn4vlCewOmUtUEoRsThiptC7Ox28CRmYUn8...),
> > > so I'm not sure the concept of a Discriptor bit-flags is going to scale.
> > > 
> > 
> > InputDeviceCapabilitesValue is the value rep, so non-boolean capabilities
> could
> > be accommodated via extra constructors for it. Along with some combinators
for
> > composing such values together.
> > 
> > > That said, I think this is still moving things in a direction to how I
> > imagined
> > > InputDeviceCapabilities evolving over time, just with a little different
> > > abstraction focused on an (internal-only) concept of concrete input
devices.
> > > 
> > > I.e. instead of InputDeviceCapabilityConstants, I imagined  a
> > > InputDeviceRegistry on each DOMWindow which given some sort of device ID
> > returns
> > > an InputDeviceCapabilities representing that device.  So instead of
> > 'Descriptor'
> > > with flags like 'firesTouchEvents' we could (for now) just have an
> > > InputDeviceType enum with values like:
> > >  None (no / unknown input device)
> > >  Keyboard
> > >  Mouse
> > >  Touchscreen
> > > 
> > > Keyboard and Mouse could supply the same or different (but equivalent)
> > > InputDeviceCapabilities instances for now.
> > > 
> > 
> > That registration model along with passing abstract IDs around, would work
> just
> > as well (if not better) wrt the above concerns. I wouldn't mind going in
that
> > direction instead.
> 
> That would be great!  That definitely seems easier to reason about to me, and
is
> more along the direction we expect to need to head anyway.
> 

An (abstract) registry ID representing a capability value, which would then be
instantiated as an IDC on demand, is what I had in mind.

> So concretely, that would mean each DOMWindow has an InputDeviceRegistry
> initialized at DOMWindow creation time which maps some opaque DeviceID to
> InputDeviceCapabilities instances (associated with the same ScriptWrapable as
> the DOMWindow they were created for), right?  Would we just use some simple
> constant DeviceID enum values (Keyboard, Mouse, Touchscreen) for now?

[Rick Byers, 2017-02-25 15:18:51]

On 2017/02/17 19:09:27, sof wrote:
> On 2017/02/13 20:25:51, Rick Byers wrote:
> > On 2017/02/13 12:57:32, sof wrote:
> > > On 2017/02/11 15:31:01, Rick Byers wrote:
> > > > Thanks for working on this!
> > > > 
> > > > Can you explain a little more why it's so valuable to delay creating
> > > > InputDeviceCapability instances until they're requested by script?  We
> don't
> > > do
> > > > that, for other DOM APIs like 'Document' or 'Node' instances, what's
> > different
> > > > here?
> > > 
> > > I think it comes down two issues from here: this an object and property
> that's
> > > new and rarely accessed (*), so delaying the cost of instantiating it,
makes
> > > some sense. More so if the "IDC" instances are created per UIEvent
instance
> > (if
> > > that's the model you're moving to; not sure.)
> > 
> > Ideally I think we'd ultimately have exactly one IDC instance per
> (web-exposed)
> > physical device that generates UIEvents.  So typically 2 or maybe as much as
> ~5
> > per window.  Delaying the cost of creating 2-5 C++ objects hardly seems
worth
> > adding any complexity for, right?  I'm sure many pages have far more than 5
> Node
> > instances which are never accessed by script, and we haven't worried about
> > optimizing away construction of never-accessed Node objects, right?
> > 
> 
> ok, I think that's one source of confusion here -- an earlier exchange
suggested
> that you wanted to move away from this stateful model and have these cap
objects
> be per-event values with the lifetime of an event dispatch.

I did?  I think someone (Elliott?) suggested that maybe we should do that, and I
said I wasn't strongly opposed, but we had good reason for choosing a stronger
notion of identity.  I imagined developers sometimes wanting to maintain a map
where InputDevice objects were the key (eg. when have I last seen input from
each device).  But I'm not sure how valuable that still is after Mozilla folks
convinced us to move from 'InputDevice' to 'InputDeviceCapabilities'.  Still
seems strictly better than a new object per event though.  If we want to change
this it needs to start with a spec debate on the repo and AFAIK no-one has
proposed such a change there.

> Eagerly creating
> those seems..too eager, perhaps?

Oh definitely - if we're going to create one per Event then it shouldn't be
eager for sure.  I just didn't realize that was a design choice you were trying
to support.  Sorry for the miscommunication!

> The above object lifetime model provides an info conduit where scripts can
> attach whatever state they want for the lifetime of the renderer process,
iiuc.

When I said "per window" I meant "per DOMWindow".  I.e. lifetime coupled to the
Frame, not the renderer process.  Does that make more sense?

> 
> > > The other issue is that we're
> > > relying on the event dispatch machinery to associate these
ScriptWrappables
> > with
> > > the correct context, something that came up via
> > > https://codereview.chromium.org/2675793005/#msg35 .
> > > 
> > > ( * - not meant as a detraction in any way, the new functionality serves a
> > clear
> > > purpose. )
> > 
> > Ah, good point.  So if we instead created the ~3 InputDeviceCapabilities
> > instances during DOMWindow creation, that would address the concern there
too,
> > right?
> > 
> 
> That's where they currently reside, but requires the code to associate the
IDCs
> with the correct window.
> 
> > > > 
> > > > Note that we expect to have non-boolean properties in the future (like
the
> > > > granularity of the pointing device in pixels - see
> > > >
> > >
> >
>
https://docs.google.com/document/d/1WLadG2dn4vlCewOmUtUEoRsThiptC7Ox28CRmYUn8...),
> > > > so I'm not sure the concept of a Discriptor bit-flags is going to scale.
> > > > 
> > > 
> > > InputDeviceCapabilitesValue is the value rep, so non-boolean capabilities
> > could
> > > be accommodated via extra constructors for it. Along with some combinators
> for
> > > composing such values together.
> > > 
> > > > That said, I think this is still moving things in a direction to how I
> > > imagined
> > > > InputDeviceCapabilities evolving over time, just with a little different
> > > > abstraction focused on an (internal-only) concept of concrete input
> devices.
> > > > 
> > > > I.e. instead of InputDeviceCapabilityConstants, I imagined  a
> > > > InputDeviceRegistry on each DOMWindow which given some sort of device ID
> > > returns
> > > > an InputDeviceCapabilities representing that device.  So instead of
> > > 'Descriptor'
> > > > with flags like 'firesTouchEvents' we could (for now) just have an
> > > > InputDeviceType enum with values like:
> > > >  None (no / unknown input device)
> > > >  Keyboard
> > > >  Mouse
> > > >  Touchscreen
> > > > 
> > > > Keyboard and Mouse could supply the same or different (but equivalent)
> > > > InputDeviceCapabilities instances for now.
> > > > 
> > > 
> > > That registration model along with passing abstract IDs around, would work
> > just
> > > as well (if not better) wrt the above concerns. I wouldn't mind going in
> that
> > > direction instead.
> > 
> > That would be great!  That definitely seems easier to reason about to me,
and
> is
> > more along the direction we expect to need to head anyway.
> > 
> 
> An (abstract) registry ID representing a capability value, which would then be
> instantiated as an IDC on demand, is what I had in mind.
> 
> > So concretely, that would mean each DOMWindow has an InputDeviceRegistry
> > initialized at DOMWindow creation time which maps some opaque DeviceID to
> > InputDeviceCapabilities instances (associated with the same ScriptWrapable
as
> > the DOMWindow they were created for), right?  Would we just use some simple
> > constant DeviceID enum values (Keyboard, Mouse, Touchscreen) for now?

[sof, 2017-02-27 17:29:47]

Thanks for the helpful feedback and spec context, rbyers@. I'll park this CL for
now, but keep an eye on spec developments.