Bug 117403 – Support latest Web IDL named property getters

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Chris Dumez]

We should support the latest Web IDL named property getters and stop using the outdated [NamedGetter] IDL extended attribute.

This change was already done in Blink:
https://chromiumcodereview.appspot.com/14044026

[Chris Dumez]

Created attachment 204333 [details]
Patch

[Chris Dumez]

Comment on attachment 204333 [details]
Patch

Sorry, changelog issue.

[Chris Dumez]

Created attachment 204334 [details]
Patch

[Kentaro Hara]

Comment on attachment 204333 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204333&action=review

> Source/WebCore/dom/DOMStringMap.idl:33
> +    getter DOMString (DOMString name);

Just to confirm: Is it OK to omit the method name? I'm not sure of how it is supported in JSC.

> Source/WebCore/dom/NodeList.idl:28
> +    getter (Node or unsigned long) (DOMString name);

How does this work? I thought IDL union is not yet supported in JSC.

> Source/WebCore/html/HTMLAllCollection.idl:34
> +    [Custom] getter Node namedItem(DOMString name);

What's the difference between '[Custom] getter' and [CustomNamedGetter] ?

[Chris Dumez]

Comment on attachment 204333 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204333&action=review

>> Source/WebCore/dom/DOMStringMap.idl:33
>> +    getter DOMString (DOMString name);
> 
> Just to confirm: Is it OK to omit the method name? I'm not sure of how it is supported in JSC.

I store anonymous functions such as this one in Interface->anonymousFunctions instead of Interface->functions so that no code is generated for those. interface->anonymousFunctions is currently used by GetSpecialAccessorFunctionForType() only to find specials. Therefore, the generated code is identical, this is covered by TestEventTarget.idl bindings test above.

>> Source/WebCore/dom/NodeList.idl:28
>> +    getter (Node or unsigned long) (DOMString name);
> 
> How does this work? I thought IDL union is not yet supported in JSC.

This is purely informative at this point. The JSC bindings generator does not currently generate everything.

The following methods are (need to be) defined in JSNodeListCustom.cpp:
bool JSNodeList::canGetItemsForName(ExecState*, NodeList* impl, PropertyName propertyName);
JSValue JSNodeList::nameGetter(ExecState* exec, JSValue slotBase, PropertyName propertyName);

Having [NamedGetter] only makes sure this custom code gets called.

>> Source/WebCore/html/HTMLAllCollection.idl:34
>> +    [Custom] getter Node namedItem(DOMString name);
> 
> What's the difference between '[Custom] getter' and [CustomNamedGetter] ?

There is currently a subtle difference:

1. If you use [CustomNamedGetter] and don't define the namedItem operation.
  - No "namedItem" method will be exposed on the object

2. If you use [CustomNamedGetter] and define the namedItem operation.
  - The namedItem operation will be exposed on the object but it will call the implementation code directly (no custom code)

3. If you use [Custom] getter namedItem(DOMString name)
  - the namedItem operation will be exposed on the object and will call its custom implementation.

[Kentaro Hara]

Comment on attachment 204334 [details]
Patch

Thanks for the clarification. The confusions comes from the fact that JSC's support for named getters isn't mature, but in summary, this patch improves the situation of current IDL files. LGTM.

Finally, we want to remove [CustomNamedGetter] and leave 'getter' and '[Custom] getter'.

[WebKit Commit Bot]

Comment on attachment 204334 [details]
Patch

Clearing flags on attachment: 204334

Committed r151434: <http://trac.webkit.org/changeset/151434>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.