Fire attributeChangedCallback on style changes for Custom Elements V1

Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

CEReactions are added to CSSStyleDeclaration interface[2].

Ported from a canceled patch for Custom Elements V0[3].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://drafts.csswg.org/cssom/#cssstyledeclaration
[3] https://codereview.chromium.org/1446933002

BUG=594918
Committed: https://crrev.com/48bf4aae854f6b6eb1340b05dedf14d9c52e9701
Cr-Commit-Position: refs/heads/master@{#407112}

[kojii, 2016-07-21 06:12:22]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 06:12:37]

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

[kojii, 2016-07-21 06:12:54]

Description was changed from

==========
styleattr


styleattr

BUG=
==========

to

==========
Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

Ported from a canceled patch for Custom Elements V0[2].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://codereview.chromium.org/1446933002

BUG=594918
==========

[kojii, 2016-07-21 06:16:54]

The CQ bit was unchecked by kojii@chromium.org

[kojii, 2016-07-21 06:17:39]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 06:18:08]

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

[kojii, 2016-07-21 06:18:22]

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

[kojii, 2016-07-21 06:18:22]

PTAL. Thank you for your previous effort ;)

[commit-bot: I haz the power, 2016-07-21 06:26:10]

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

[commit-bot: I haz the power, 2016-07-21 06:26:11]

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

[kojii, 2016-07-21 08:36:46]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 08:36:56]

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, 2016-07-21 09:41:04]

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

[commit-bot: I haz the power, 2016-07-21 09:41:04]

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_...)

[kojii, 2016-07-21 16:01:56]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 16:02:10]

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, 2016-07-21 19:21:21]

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

[commit-bot: I haz the power, 2016-07-21 19:21:22]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-07-22 01:14:01]

I'm glad we're making style attribute fire changes. Thank you for working on
this. Comments inline.

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/custom-elements/spec/callback.html (right):

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/callback.html:120:
element.style.cssText = 'color: red';
Please add cssFloat tests.

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/callback.html:140: },
'style.color should enqueue attributeChangedCallback for style attribute');
This is really testing setProperty, removeProperty more than *color* per se. Use
a more accurate description.

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl (right):

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl:39:
[RaisesException=Setter, TreatNullAs=EmptyString] attribute DOMString cssFloat;
Does this need CEReactions too?

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/PropertySetCSSStyleDeclaration.cpp
(right):

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/PropertySetCSSStyleDeclaration.cpp:48: if
(!element || element->getCustomElementState() != CustomElementState::Custom)
I'm guessing if it is worth doing here, it is worth doing everywhere. Why not
make definitionForElement have an inlineable wrapper which checks
CustomElementState and does the early return.

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/PropertySetCSSStyleDeclaration.cpp:77: if
(m_mutationRecipients && m_mutationRecipients->isOldValueRequested())
I wonder if this is clearer by simply declaring

bool shouldReadOldValue = ...mutation observer part... || ...custom element
part...;

[kojii, 2016-07-22 04:22:19]

Description was changed from

==========
Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

Ported from a canceled patch for Custom Elements V0[2].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://codereview.chromium.org/1446933002

BUG=594918
==========

to

==========
Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

CEReactions are added to CSSStyleDeclaration interface[2].

Ported from a canceled patch for Custom Elements V0[3].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://drafts.csswg.org/cssom/#cssstyledeclaration
[3] https://codereview.chromium.org/1446933002

BUG=594918
==========

[kojii, 2016-07-22 04:43:10]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-22 04:43:32]

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

[kojii, 2016-07-22 04:43:53]

All done, thank you.

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl (right):

https://codereview.chromium.org/2166213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl:39:
[RaisesException=Setter, TreatNullAs=EmptyString] attribute DOMString cssFloat;
On 2016/07/22 at 01:14:01, dominicc wrote:
> Does this need CEReactions too?

Done, missed that, thank you. Added the spec link to the description too.

[commit-bot: I haz the power, 2016-07-22 05:58:53]

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

[commit-bot: I haz the power, 2016-07-22 05:58:54]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-07-22 07:35:55]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-07-22 07:35:56]

lgtm

Very slick.

[commit-bot: I haz the power, 2016-07-22 07:36:08]

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-07-22 07:41:38]

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

[commit-bot: I haz the power, 2016-07-22 07:41:40]

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

[kojii, 2016-07-22 08:19:48]

kojii@chromium.org changed reviewers:
+ haraken@chromium.org

[kojii, 2016-07-22 08:19:48]

haraken@, PTAL for binding

[haraken, 2016-07-22 08:28:03]

LGTM

[kojii, 2016-07-22 09:33:38]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2016-07-22 09:34:00]

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-07-22 09:37:22]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-22 09:38:43]

Description was changed from

==========
Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

CEReactions are added to CSSStyleDeclaration interface[2].

Ported from a canceled patch for Custom Elements V0[3].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://drafts.csswg.org/cssom/#cssstyledeclaration
[3] https://codereview.chromium.org/1446933002

BUG=594918
==========

to

==========
Fire attributeChangedCallback on style changes for Custom Elements V1

W3C discussion[1] resolved to allow observing the style attribute.

CEReactions are added to CSSStyleDeclaration interface[2].

Ported from a canceled patch for Custom Elements V0[3].

[1] https://github.com/w3c/webcomponents/issues/521
[2] https://drafts.csswg.org/cssom/#cssstyledeclaration
[3] https://codereview.chromium.org/1446933002

BUG=594918

Committed: https://crrev.com/48bf4aae854f6b6eb1340b05dedf14d9c52e9701
Cr-Commit-Position: refs/heads/master@{#407112}
==========

[commit-bot: I haz the power, 2016-07-22 09:38:44]

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