Bug 67447 – Generate a Worker constructor of V8 using the IDL 'Constructor' extended attribute

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Kentaro Hara]

Currently, the Worker constructor is written manually as a custom constructor. We should replace this with automatically generated code by the IDL 'Constructor' extended attribute.

[Kentaro Hara]

Created attachment 106047 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 106047 [details]
Patch

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

> Source/WebCore/workers/Worker.idl:37
> +        ConstructorRaisesException,

shouldn't this be Constructor(in DOMString scriptUrl) raises (DOMException)?

Or is this still coming?

[WebKit Review Bot]

Comment on attachment 106047 [details]
Patch

Rejecting attachment 106047 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
" exit_code: 1

Parsed 3 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/bindings/v8/custom/V8WorkerCustom.cpp
Hunk #1 FAILED at 47.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/bindings/v8/custom/V8WorkerCustom.cpp.rej
patching file Source/WebCore/workers/Worker.idl

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Dimitri Glazkov', u'--..." exit_code: 1

Full output: http://queues.webkit.org/results/9578572

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 106047 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106047&action=review
> 
> > Source/WebCore/workers/Worker.idl:37
> > +        ConstructorRaisesException,
> 
> shouldn't this be Constructor(in DOMString scriptUrl) raises (DOMException)?

Dimitri: Thank you very much for the review. 

The spec of the 'Constructor' extended attribute does not allow the "Constructor(...) raises (...)" format (http://www.w3.org/TR/WebIDL/#Constructor). For example, the spec says that the IDL of a Worker constructor should be "Constructor(DOMString scriptURL)" (http://dev.w3.org/html5/workers/#dedicated-workers-and-the-worker-interface). On the other hand, the spec of the Worker constructor says that it can throw SYNTAX_ERR (http://dev.w3.org/html5/workers/#dom-worker).

Therefore, I think that the possible approaches for us are as follows:

Appraoch1: Change the spec of constructors, so that it can allow the "Constructor(...) raises (...)" format.

Approach2: We introduce a special extended attribute for WebKit (as we have done so far), 'ConstructorRaisesException'.

For now, this patch takes Approach2.

[Kentaro Hara]

Created attachment 106073 [details]
Patch

[Kentaro Hara]

Dimitri: I rebased the patch with the latest repository. Would you please try to commit it again? Thanks.

[WebKit Review Bot]

Comment on attachment 106073 [details]
Patch

Clearing flags on attachment: 106073

Committed r94381: <http://trac.webkit.org/changeset/94381>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.