Bug 96821

Summary: Support constructor-type attribute in idls other than DOMWindow.
Product: WebKit Reporter: Chang Shu <cshu>
Component: WebCore JavaScriptAssignee: Chang Shu <cshu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, haraken, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 92413    
Attachments:
Description Flags
fix patch none

Chang Shu
Reported 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.
Attachments
fix patch (5.03 KB, patch)
2012-09-14 13:16 PDT, Chang Shu 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chang Shu
Comment 1 2012-09-14 13:16:46 PDT
Created attachment 164213 [details] fix patch
Kentaro Hara
Comment 2 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?
Adam Barth
Comment 3 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.
Kentaro Hara
Comment 4 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
Chang Shu
Comment 5 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.
Kentaro Hara
Comment 6 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!
WebKit Review Bot
Comment 7 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>
WebKit Review Bot
Comment 8 2012-09-14 14:43:21 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.