Summary: | Binding generator should generate safe accessors for constructors used in JS builtin code | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, adam.bergkvist, cdumez, commit-queue, eric.carlson | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
youenn fablet
2016-06-24 04:51:38 PDT
Created attachment 281969 [details]
Patch
With this patch, it also becomes a bit easier to do type checks in JS-builtins like: if(arg1 instanceof @MyClass) @throw... Could you add a bindings test that shows the new behavior of the bindings generator? This feature would be very useful. Thanks Youenn. Comment on attachment 281969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281969&action=review > Source/WebCore/Modules/mediastream/MediaStream.idl:32 > + PublicIdentifier Is it obvious that these extended attributes are referring to the constructor? Would it be more descriptive to use 'PrivateConstructorIdentifier' and 'PublicConstructorIdentifier'? Created attachment 282072 [details]
Patch
(In reply to comment #3) > Could you add a bindings test that shows the new behavior of the bindings > generator? I added a binding test for global object attribute. This does not fully test these attributes on constructors though. Maybe run-binding-tests should be beefed up and support that as well. > > Source/WebCore/Modules/mediastream/MediaStream.idl:32 > > + PublicIdentifier > > Is it obvious that these extended attributes are referring to the > constructor? Would it be more descriptive to use > 'PrivateConstructorIdentifier' and 'PublicConstructorIdentifier'? I am not opposed to that although I did not do it in the uploaded patch. Doing so would require to convert PrivateConstructorIdentifier into PrivateIdentifier when adding attributes to the global object for each defined constructor in Source/WebCore/bindings/scripts/preprocess-idls.pl script. (In reply to comment #7) > > Is it obvious that these extended attributes are referring to the > > constructor? Would it be more descriptive to use > > 'PrivateConstructorIdentifier' and 'PublicConstructorIdentifier'? > > I am not opposed to that although I did not do it in the uploaded patch. > Doing so would require to convert PrivateConstructorIdentifier into > PrivateIdentifier when adding attributes to the global object for each > defined constructor in Source/WebCore/bindings/scripts/preprocess-idls.pl > script. I think it works as is as well. Comment on attachment 282072 [details]
Patch
Sure. We can change the names later if we're unhappy with them.
Comment on attachment 282072 [details] Patch Clearing flags on attachment: 282072 Committed r202551: <http://trac.webkit.org/changeset/202551> All reviewed patches have been landed. Closing bug. |