Bug 117183 – Automatically generate WorkerContext constructor attributes

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

The bindings generator are currently able to generate constructor attributes on the Window object but not yet on the WorkerContext. This should be addressed.

[Chris Dumez]

Created attachment 203667 [details]
Patch

[Kentaro Hara]

(I know I shouldn't say this after you implemented the patch but) another idea might be:

- Keep [NoInterfaceObject] as is, since it's a speced IDL attribute.
- Add [InterfaceObjectOnWorker], which indicates that the interface should be exposed on WorkerContext.

[Build Bot]

Comment on attachment 203667 [details]
Patch

Attachment 203667 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/775053

[Chris Dumez]

(In reply to comment #2)
> (I know I shouldn't say this after you implemented the patch but) another idea might be:
> 
> - Keep [NoInterfaceObject] as is, since it's a speced IDL attribute.
> - Add [InterfaceObjectOnWorker], which indicates that the interface should be exposed on WorkerContext.

I don't fully understand your comment. I kept [NoInterfaceObject] as is already. I merely introduced a new global extended attribute called [GlobalContext].

The approach you propose is fairly similar despite the extended attribute name being different. The issue is how do you indicate that a constructor should be generated the WindowOnly, or on the WorkerContext only?

Note, for example, that according to specification FileReaderSync does not have [NoInterfaceObject]:
http://dev.w3.org/2006/webapi/FileAPI/#FileReaderSync

However, the spec clearly states:
"In environments where the global object is represented by a WorkerGlobalScope object, the FileReaderSync constructor must be available."
-> http://dev.w3.org/2006/webapi/FileAPI/#filereadersyncConstrctr

The Web IDL spec (http://dev.w3.org/2006/webapi/WebIDL/#es-interfaces) says:
"
For every interface that:
- is a callback interface that has constants declared on it, or
- is a non-callback interface that is not declared with the [NoInterfaceObject] 
  extended attribute,
a corresponding property must exist on the ECMAScript global object. The name of the property is the identifier of the interface, and its value is an object called the interface object.
"

The global object may be a Window or a WorkerContext depending on the environment. Currently, this is only defined in prose in the specs. This is the reason why I proposed to introduce [GlobalContext] extended attribute to indicate on which global context the constructor should be exposed.

[Kentaro Hara]

ah, sorry! I was misunderstanding the patch. You approach completely looks reasonable.

[Chris Dumez]

Created attachment 203674 [details]
Patch

Should fix win-ews.

[Kentaro Hara]

Comment on attachment 203674 [details]
Patch

Looks reasonable.

[Chris Dumez]

FYI, I am porting this to Blink as well (but in Python).

[Kentaro Hara]

(In reply to comment #8)
> FYI, I am porting this to Blink as well (but in Python).

Two engineers are working on the rewrite, but it will take time to switch to Python. Please feel free to make changes to the current Perl scripts.

[Chris Dumez]

(In reply to comment #9)
> (In reply to comment #8)
> > FYI, I am porting this to Blink as well (but in Python).
> 
> Two engineers are working on the rewrite, but it will take time to switch to Python. Please feel free to make changes to the current Perl scripts.

The preprocess-idls script was already rewritten in Python in Blink. This is the only script I edited.

[Chris Dumez]

Comment on attachment 203674 [details]
Patch

Seems I broke the bindings tests.

[Chris Dumez]

Created attachment 203691 [details]
Patch

Fixed bindings tests by passing new --workerContextConstructorsFile argument to preprocess-idls.pl script. Kentaro, would you mind taking another look please?

[Kentaro Hara]

Comment on attachment 203691 [details]
Patch

LGTM

[WebKit Commit Bot]

Comment on attachment 203691 [details]
Patch

Clearing flags on attachment: 203691

Committed r151169: <http://trac.webkit.org/changeset/151169>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

I updated the WebKit IDL wiki accordingly:
https://trac.webkit.org/wiki/WebKitIDL?action=diff&version=120&old_version=119