Bug 96821 - Support constructor-type attribute in idls other than DOMWindow.
Summary: Support constructor-type attribute in idls other than DOMWindow.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chang Shu
URL:
Keywords:
Depends on:
Blocks: 92413
  Show dependency treegraph
 
Reported: 2012-09-14 13:07 PDT by Chang Shu
Modified: 2012-09-14 14:43 PDT (History)
4 users (show)

See Also:


Attachments
fix patch (5.03 KB, patch)
2012-09-14 13:16 PDT, Chang Shu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chang Shu 2012-09-14 13:07:08 PDT
Currently, it is assumed that Constructor attribute is used only in DOMWindow.idl. We should release this condition.
Bug 92413 is supposed to depend on this bug.
Comment 1 Chang Shu 2012-09-14 13:16:46 PDT
Created attachment 164213 [details]
fix patch
Comment 2 Kentaro Hara 2012-09-14 13:30:33 PDT
Comment on attachment 164213 [details]
fix patch

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

> Source/WebCore/ChangeLog:8
> +        In CodeGeneratorJS.pm, we should not assume only DOMWindow uses Constructor

What about CodeGeneratorV8.pm?
Comment 3 Adam Barth 2012-09-14 13:39:33 PDT
Comment on attachment 164213 [details]
fix patch

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

>> Source/WebCore/ChangeLog:8
>> +        In CodeGeneratorJS.pm, we should not assume only DOMWindow uses Constructor
> 
> What about CodeGeneratorV8.pm?

In V8, we use info.Holder(), which probably works in either case.
Comment 4 Kentaro Hara 2012-09-14 13:45:50 PDT
Comment on attachment 164213 [details]
fix patch

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

>>> Source/WebCore/ChangeLog:8
>>> +        In CodeGeneratorJS.pm, we should not assume only DOMWindow uses Constructor
>> 
>> What about CodeGeneratorV8.pm?
> 
> In V8, we use info.Holder(), which probably works in either case.

CodeGeneratorV8.pm uses info.Holder(), which is fine, but it looks like that CodeGeneratorV8.pm returns v8Undefined() for non-DOMWindow cases. We might want to fix the code:

http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm&exact_package=chromium&q=codegeneratorv8.pm&type=cs&l=822
Comment 5 Chang Shu 2012-09-14 14:05:10 PDT
> > In V8, we use info.Holder(), which probably works in either case.
> 
> CodeGeneratorV8.pm uses info.Holder(), which is fine, but it looks like that CodeGeneratorV8.pm returns v8Undefined() for non-DOMWindow cases. We might want to fix the code:
> 
> http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm&exact_package=chromium&q=codegeneratorv8.pm&type=cs&l=822

The latest trunk looks like this:
http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm#L810

It's quite different from your link.
Comment 6 Kentaro Hara 2012-09-14 14:16:02 PDT
Comment on attachment 164213 [details]
fix patch

> The latest trunk looks like this:
> http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm#L810

Ah, the latest trunk looks good. Sorry for the noise!
Comment 7 WebKit Review Bot 2012-09-14 14:43:17 PDT
Comment on attachment 164213 [details]
fix patch

Clearing flags on attachment: 164213

Committed r128655: <http://trac.webkit.org/changeset/128655>
Comment 8 WebKit Review Bot 2012-09-14 14:43:21 PDT
All reviewed patches have been landed.  Closing bug.