Bug 69074

Summary: Refactor IDL attributes about constructor
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, dominicc, donggwan.kim, haraken, japhet, morrita, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 65839    
Attachments:
Description Flags
Patch
abarth: review+
rebased patch for commit
none
rebased patch for commit none

Kentaro Hara
Reported 2011-09-29 01:53:54 PDT
Current situation: - [CustomConstructFunction] means that there is a custom constructor for JSC. - [V8CustomConstructor] means that there is a custom constructor for V8. - [CustomConstructor] exists in CodeGenerator*.pm but is not used in any IDL files. I don't figure out what [CustomConstructor] means. As far as I see CodeGeneratorV8.pm, [CustomConstructor] works equivalent to [V8CustomConstructor]. On the other hand, as far as I see CodeGeneratorJS.pm, [CustomConstructor] works equivalent to [OmitConstructor]. Anyway, no IDL files are using [CustomConstructor]. - For almost all IDL files, [CustomConstructFunction] and [V8CustomConstructor] are used at the same time. - ObjC, CPP and GObject bindings do not support custom constructors. I am planning to refactor the above situation as follows: - Remove [CustomConstructFunction] and add [JSCustomConstructor]. - [JSCustomConstructor] means that there is a custom constructor for JSC. - [V8CustomConstructor] means that there is a custom constructor for V8. - [CustomConstructor] means that there is a custom constructor for both JSC and V8.
Attachments
Patch (28.74 KB, patch)
2011-09-29 02:09 PDT, Kentaro Hara 		abarth: review+
DetailsFormatted DiffDiff
rebased patch for commit (28.74 KB, patch)
2011-10-05 21:47 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
rebased patch for commit (28.74 KB, patch)
2011-10-05 21:49 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2011-09-29 02:09:43 PDT
Created attachment 109137 [details] Patch
Hajime Morrita
Comment 2 2011-10-05 19:33:09 PDT
The change looks trivial and making sense. - A simple renaming from [CustomConstructFunction] to [JSCustomConstructor]. - Addition of [CustomConstructor] for a shorthand for JSCustomConstructor + V8CustomConstructor. I'd like to r+ this later but a quick glance from an expert would be appreciated.
Dominic Cooney
Comment 3 2011-10-05 21:18:48 PDT
Comment on attachment 109137 [details] Patch Looks like good clean-up. Possible to split into a few patches but makes sense as-is. No change to test IDL files/output?
Kentaro Hara
Comment 4 2011-10-05 21:47:48 PDT
Created attachment 109912 [details] rebased patch for commit
Kentaro Hara
Comment 5 2011-10-05 21:49:21 PDT
Created attachment 109913 [details] rebased patch for commit
Kentaro Hara
Comment 6 2011-10-05 21:54:05 PDT
(In reply to comment #3) > (From update of attachment 109137 [details]) > Looks like good clean-up. Possible to split into a few patches but makes sense as-is. No change to test IDL files/output? Dominic: No change to test IDL files, since they have an [Constructor] IDL and thus do not have an IDL for custom constructor.
WebKit Review Bot
Comment 7 2011-10-05 22:59:50 PDT
Comment on attachment 109913 [details] rebased patch for commit Clearing flags on attachment: 109913 Committed r96788: <http://trac.webkit.org/changeset/96788>
Note You need to log in before you can comment on or make changes to this bug.