Bug 69074 - Refactor IDL attributes about constructor
Summary: Refactor IDL attributes about constructor
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 tree / graph
 
Reported: 2011-09-29 01:53 PDT by Kentaro Hara
Modified: 2011-10-12 01:36 PDT (History)
9 users (show)

See Also:

Attachments
Patch (28.74 KB, patch)
2011-09-29 02:09 PDT, Kentaro Hara 		abarth: review+
Details | Formatted Diff | Diff
rebased patch for commit (28.74 KB, patch)
2011-10-05 21:47 PDT, Kentaro Hara 		no flags Details | Formatted Diff | Diff
rebased patch for commit (28.74 KB, patch)
2011-10-05 21:49 PDT, Kentaro Hara 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 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.
Comment 1 Kentaro Hara 2011-09-29 02:09:43 PDT 
Created attachment 109137 [details]
Patch
Comment 2 Hajime Morrita 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.
Comment 3 Dominic Cooney 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?
Comment 4 Kentaro Hara 2011-10-05 21:47:48 PDT 
Created attachment 109912 [details]
rebased patch for commit
Comment 5 Kentaro Hara 2011-10-05 21:49:21 PDT 
Created attachment 109913 [details]
rebased patch for commit
Comment 6 Kentaro Hara 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.
Comment 7 WebKit Review Bot 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>