Bug 67447 - Generate a Worker constructor of V8 using the IDL 'Constructor' extended attribute
Summary: Generate a Worker constructor of V8 using the IDL 'Constructor' extended attr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 65839
  Show dependency treegraph
 
Reported: 2011-09-01 15:20 PDT by Kentaro Hara
Modified: 2011-09-01 18:37 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.44 KB, patch)
2011-09-01 15:23 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (3.53 KB, patch)
2011-09-01 18:04 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-09-01 15:20:30 PDT
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.
Comment 1 Kentaro Hara 2011-09-01 15:23:20 PDT
Created attachment 106047 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2011-09-01 15:55:22 PDT
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?
Comment 3 WebKit Review Bot 2011-09-01 16:06:57 PDT
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
Comment 4 Kentaro Hara 2011-09-01 16:20:14 PDT
(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.
Comment 5 Kentaro Hara 2011-09-01 18:04:37 PDT
Created attachment 106073 [details]
Patch
Comment 6 Kentaro Hara 2011-09-01 18:07:16 PDT
Dimitri: I rebased the patch with the latest repository. Would you please try to commit it again? Thanks.
Comment 7 WebKit Review Bot 2011-09-01 18:37:00 PDT
Comment on attachment 106073 [details]
Patch

Clearing flags on attachment: 106073

Committed r94381: <http://trac.webkit.org/changeset/94381>
Comment 8 WebKit Review Bot 2011-09-01 18:37:05 PDT
All reviewed patches have been landed.  Closing bug.