Add [CEReactions] IDL attributes for Custom Elements V1

Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
https://html.spec.whatwg.org/multipage/scripting.html#cereactions
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

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

[kojii, 2016-06-05 09:48:23]

Description was changed from

==========
cereactions

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

to

==========
Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

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

[kojii, 2016-06-05 09:49:01]

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

[kojii, 2016-06-05 09:49:02]

PTAL.

dominicc: core
haraken: binding

[bashi, 2016-06-05 23:17:37]

bashi@chromium.org changed reviewers:
+ bashi@chromium.org

[bashi, 2016-06-05 23:17:38]

I'd be great to add the link to the spec in the description.
https://html.spec.whatwg.org/multipage/scripting.html#cereactions

Also could you update IDLExtendedAttributes.md?

[dominicc (has gone to gerrit), 2016-06-06 01:43:34]

core/dom LGTM

[kojii, 2016-06-06 05:09:53]

Description was changed from

==========
Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

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

to

==========
Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
https://html.spec.whatwg.org/multipage/scripting.html#cereactions
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

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

[Yuki, 2016-06-06 06:13:55]

yukishiino@chromium.org changed reviewers:
+ yukishiino@chromium.org

[Yuki, 2016-06-06 06:13:57]

Do you have any plan to support setters and deleters in addition to operations
and attributes?

https://html.spec.whatwg.org/multipage/scripting.html#cereactions
seems saying that we have to support setters and deleters, too.

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:335:
CEReactionsScope reactions;
nit: I'd prefer cereactionsScope or ceReactionsScope or somethingScope because
it's a scope, not reactions.

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp:25: CEReactionsScope
reactions;
Ditto.

[haraken, 2016-06-07 07:43:54]

LGTM

I think it's better to make an announcement to blink-dev, since this CL adds a
lot of unspeced [CEReactions].

[kojii, 2016-06-07 07:49:16]

On 2016/06/07 at 07:43:54, haraken wrote:
> I think it's better to make an announcement to blink-dev, since this CL adds a
lot of unspeced [CEReactions].

Thanks, will do so. It is spec'ed in HTML
https://html.spec.whatwg.org/multipage/scripting.html#cereactions
but not in webidl.

[kojii, 2016-06-07 07:51:17]

...and forgot to publish drafts, but bashi@ and yukishiino@ review results are
done yesterday in PS3. Perf check was done using this.

It'd be great if bashi@ and yukishiino@ could have another look.

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:335:
CEReactionsScope reactions;
On 2016/06/06 at 06:13:56, Yuki wrote:
> nit: I'd prefer cereactionsScope or ceReactionsScope or somethingScope because
it's a scope, not reactions.

Done.

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/2043503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp:25: CEReactionsScope
reactions;
On 2016/06/06 at 06:13:56, Yuki wrote:
> Ditto.

Done too.

[bashi, 2016-06-07 08:02:55]

lgtm

[Yuki, 2016-06-07 08:16:57]

You may want some tests to confirm the behavior of [CEReactions], especially for
{indexed,named} {setter,deleter}, but not limited to them.

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:394: {% if
attribute.is_custom_element_callbacks or attribute.is_reflect %}
[CEReactions] is only supported for V1?

If so, line 393 and 394 can be combined to {% elif ...V0... %} ?

This seems it's possible that both of CEReactionsScope and CallbackDeliveryScope
are effective.

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/interface.cpp:277: static void
namedPropertySetterCallback(v8::Local<v8::Name> name, v8::Local<v8::Value>
v8Value, const v8::PropertyCallbackInfo<v8::Value>& info)
Why don't you add CEReactionsScope to setters?
Not only to named_property_setter_callback but also
indexed_property_setter_callback.

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp:495: static void
{{method.name}}MethodCallback{{world_suffix}}(const
v8::FunctionCallbackInfo<v8::Value>& info)
Why don't you add CEReactionsScope to xxxMethodCallback instead of xxxMethod?

Seems not consistent with attributes, named properties, and indexed properties.

[kojii, 2016-06-07 09:56:36]

Thank you for your prompt review, all done.

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/attributes.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/attributes.cpp:394: {% if
attribute.is_custom_element_callbacks or attribute.is_reflect %}
On 2016/06/07 at 08:16:57, Yuki wrote:
> [CEReactions] is only supported for V1?
> 
> If so, line 393 and 394 can be combined to {% elif ...V0... %} ?
> 
> This seems it's possible that both of CEReactionsScope and
CallbackDeliveryScope are effective.

[CEReactions] supports only V1, but since we support both V0 and V1 at this
moment and thus an element could be either V0 or V1, we need both scopes until
we deprecate V0.

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/interface.cpp:277: static void
namedPropertySetterCallback(v8::Local<v8::Name> name, v8::Local<v8::Value>
v8Value, const v8::PropertyCallbackInfo<v8::Value>& info)
On 2016/06/07 at 08:16:57, Yuki wrote:
> Why don't you add CEReactionsScope to setters?
> Not only to named_property_setter_callback but also
indexed_property_setter_callback.

That was missed, thank you for catching!

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/2043503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/methods.cpp:495: static void
{{method.name}}MethodCallback{{world_suffix}}(const
v8::FunctionCallbackInfo<v8::Value>& info)
On 2016/06/07 at 08:16:57, Yuki wrote:
> Why don't you add CEReactionsScope to xxxMethodCallback instead of xxxMethod?
> 
> Seems not consistent with attributes, named properties, and indexed
properties.

Done, agree we should, thank you for catching this too.

[Yuki, 2016-06-07 10:29:26]

lgtm

[kojii, 2016-06-07 17:02:01]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-06-07 17:02:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org, haraken@chromium.org, bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/2043503002/#ps60001
(title: "yukishiino review")

[commit-bot: I haz the power, 2016-06-07 17:02:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2043503002/60001

[commit-bot: I haz the power, 2016-06-07 18:41:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-07 18:42:32]

Description was changed from

==========
Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
https://html.spec.whatwg.org/multipage/scripting.html#cereactions
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

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

to

==========
Add [CEReactions] IDL attributes for Custom Elements V1

This patch adds:
* [CEReactions] support in IDL parser.
https://html.spec.whatwg.org/multipage/scripting.html#cereactions
* [CEReactions] attributes to IDL from the following specs:
https://dom.spec.whatwg.org/
https://html.spec.whatwg.org/
https://w3c.github.io/DOM-Parsing/
https://w3c.github.io/selection-api/

BUG=594918

Committed: https://crrev.com/a509fa4b0fa1a34fe113a10ad9502fa58f6c7fef
Cr-Commit-Position: refs/heads/master@{#398352}
==========

[commit-bot: I haz the power, 2016-06-07 18:42:34]

Patchset 4 (id:??) landed as
https://crrev.com/a509fa4b0fa1a34fe113a10ad9502fa58f6c7fef
Cr-Commit-Position: refs/heads/master@{#398352}