Bug 117403

Summary: Support latest Web IDL named property getters
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: BindingsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, darin, haraken, laszlo.gombos
Priority: P2 Keywords: BlinkMergeCandidate
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.w3.org/TR/WebIDL/#idl-named-properties
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2013-06-10 03:52:37 PDT
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
Comment 1 Chris Dumez 2013-06-11 03:27:52 PDT
Created attachment 204333 [details]
Patch
Comment 2 Chris Dumez 2013-06-11 03:31:14 PDT
Comment on attachment 204333 [details]
Patch

Sorry, changelog issue.
Comment 3 Chris Dumez 2013-06-11 03:31:58 PDT
Created attachment 204334 [details]
Patch
Comment 4 Kentaro Hara 2013-06-11 03:36:16 PDT
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] ?
Comment 5 Chris Dumez 2013-06-11 05:06:41 PDT
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.
Comment 6 Kentaro Hara 2013-06-11 05:12:19 PDT
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'.
Comment 7 WebKit Commit Bot 2013-06-11 06:04:17 PDT
Comment on attachment 204334 [details]
Patch

Clearing flags on attachment: 204334

Committed r151434: <http://trac.webkit.org/changeset/151434>
Comment 8 WebKit Commit Bot 2013-06-11 06:04:21 PDT
All reviewed patches have been landed.  Closing bug.