Queue Custom Element attribute changes arising from style mutations.

Queue Custom Element attribute changes arising from style mutations.

Elements' inline style attributes are synchronized lazily with the CSS
property set. Custom Elements were not hooked into this mechanism, and
did not fire attributeChangedCallbacks when properties were modified
through the element's 'style' property.

BUG=492176

[dominicc (has gone to gerrit), 2015-11-16 06:57:53]

dominicc@chromium.org changed reviewers:
+ esprehn@chromium.org

[dominicc (has gone to gerrit), 2015-11-16 06:58:19]

PTAL

[esprehn, 2015-11-16 07:36:14]

Hmm, I think we want to get the spec changed so we can batch this and notify
maybe on micro task, it's super expensive to serialize the style attr on every
single mutation, that means doing:

e.style.color = "...";
e.style.display = "...";
e.style.border = "...";
e.style.flexBasis = "...";

is 4 attributeChangedCallback invocations and we serialized the style 4 times.
Indeed we don't even do this internally because it's too expensive so we
serialize lazily. Doing this puts custom  elements at a big disadvantage.

[dglazkov, 2015-11-16 15:55:30]

On 2015/11/16 at 07:36:14, esprehn wrote:
> Hmm, I think we want to get the spec changed so we can batch this and notify
maybe on micro task, it's super expensive to serialize the style attr on every
single mutation, that means doing:
> 
> e.style.color = "...";
> e.style.display = "...";
> e.style.border = "...";
> e.style.flexBasis = "...";
> 
> is 4 attributeChangedCallback invocations and we serialized the style 4 times.
Indeed we don't even do this internally because it's too expensive so we
serialize lazily. Doing this puts custom  elements at a big disadvantage.

/me scratches head. Yes, applying the spec systematically here really does
create a perf footgun. Changing all attributes on a microtask won't work,
because some of these changes are observably sync (input.type).

OTOH, maybe if we had a way to flush the callbacks (without forcing the
microtask), we could get away that? WDYT?

[dominicc (has gone to gerrit), 2015-11-17 01:27:33]

On 2015/11/16 at 15:55:30, dglazkov wrote:
> On 2015/11/16 at 07:36:14, esprehn wrote:
> > Hmm, I think we want to get the spec changed so we can batch this and notify
maybe on micro task, it's super expensive to serialize the style attr on every
single mutation, that means doing:
> > 
> > e.style.color = "...";
> > e.style.display = "...";
> > e.style.border = "...";
> > e.style.flexBasis = "...";
> > 
> > is 4 attributeChangedCallback invocations and we serialized the style 4
times. Indeed we don't even do this internally because it's too expensive so we
serialize lazily. Doing this puts custom  elements at a big disadvantage.
> 
> /me scratches head. Yes, applying the spec systematically here really does
create a perf footgun. Changing all attributes on a microtask won't work,
because some of these changes are observably sync (input.type).
> 
> OTOH, maybe if we had a way to flush the callbacks (without forcing the
microtask), we could get away that? WDYT?

Yes, we could have expensive properties flip callbacks into a delayed mode,
collapse successive expensive properties, and read them lazily. This is
definitely a design question.

FWIW, in investigating this, I found 84 other properties where we weren't
delivering attributeChangedCallbacks at the right time (although style is
special because those were *never* being delivered.) Evidently attribute
callback timing is not enough completely kill Custom Elements adoption [1] so we
have room to reconsider the design.

In the short term, shall I leave the Issue open and file a spec issue?

As an aside, those 84 properties lead me to think that the current system of
marking up DOM mutators with CustomElementCallbacks metadata is impractical and
we should do this differently, perhaps automatically. In addition, it is not
expressive enough for interfaces like URLUtils which are used by Workers and
pages. The current hook for running callbacks is cheap but unsafe for workers.
I'm looking into this.

[1] https://www.chromestatus.com/metrics/feature/timeline/popularity/457

[esprehn, 2015-11-17 01:30:20]

Yeah I think we should just leave this broken for now and file a spec issue, I
don't think conforming to the spec and making existing apps slower is worth it
yet.

I wonder if we should make you list what attributes you care about in
registerElement, you could do "*" to get everything and be slow, or instead be
explicit and we'd filter for you. We should talk to Polymer and see if they'd
use the more restrictive and fast list, or if they'd just end up doing "*".

[dominicc (has gone to gerrit), 2015-11-17 04:10:49]

On 2015/11/17 at 01:30:20, esprehn wrote:
> Yeah I think we should just leave this broken for now and file a spec issue, I
don't think conforming to the spec and making existing apps slower is worth it
yet.
> 
> I wonder if we should make you list what attributes you care about in
registerElement, you could do "*" to get everything and be slow, or instead be
explicit and we'd filter for you. We should talk to Polymer and see if they'd
use the more restrictive and fast list, or if they'd just end up doing "*".

I have filed https://github.com/w3c/webcomponents/issues/350