Bug 71093 – [meta] CodeGenerator*.pm should support [NamedConstructor] IDL

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Kentaro Hara]

CodeGeneratorJS.pm and CodeGeneratorV8.pm should support [NamedConstructor] IDL.
The spec: http://www.w3.org/TR/WebIDL/#NamedConstructor

Currently, HTMLAudioElement, HTMLImageElement and HTMLOptionElement have [NamedConstructor] and implement the constructors as custom constructors. We should generate the constructors automatically by the [NamedConstructor] IDL.

First I will make a patch for V8, and then for JSC.

[Kentaro Hara]

Created attachment 113745 [details]
WIP patch for V8

[WebKit Review Bot]

Attachment 113745 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestNamedConstructorPrivate_h  [build/header_guard] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:21:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:22:  Found WebCore config.h after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:24:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:29:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:32:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:120:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:145:  Extra space between return and WEBKIT_DOM_TEST_NAMED_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/ObjC/DOMTestNamedConstructorInternal.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 13 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kentaro Hara]

I uploaded a big WIP patch that implements the [NamedConstructor] IDL for V8. I am planning to land it in the following steps:

[1] Refactor CodeGeneratorV8.pm.
[2] Fix the wrong test results of fast/js/custom-constructors.html.
[3] Remove [CustomGetter] from HTMLAudioElementConstructor.
[4] Remove [CustomGetter] from HTMLOptionElementConstructor.
[5] Implement the [NamedConstructor] IDL in IDLParser.pm and CodeGeneratorV8.pm.
[6] Replace the custom constructor of HTMLAudioElement with the [NamedConstructor] IDL.
[7] Replace the custom constructor of HTMLOptionElement with the [NamedConstructor] IDL.

Any thoughts?

[Adam Barth]

Sounds like a reasonable approach.  I can look at the work in progress patch, or I'm also happy to review the changes in the order you propose them (which sounds quite a bit easier to review).

[Early Warning System Bot]

Comment on attachment 113745 [details]
WIP patch for V8

Attachment 113745 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10334204

[Gyuyoung Kim]

Comment on attachment 113745 [details]
WIP patch for V8

Attachment 113745 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10332171

[Gustavo Noronha (kov)]

Comment on attachment 113745 [details]
WIP patch for V8

Attachment 113745 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10334209

[Kentaro Hara]

(In reply to comment #4)
> Sounds like a reasonable approach.  I can look at the work in progress patch, or I'm also happy to review the changes in the order you propose them (which sounds quite a bit easier to review).

Thank you very much, Adam!

After the WIP patch passes all builds and tests, I'll upload sub-patches with more explanations. I would be happy if you take a detailed look at those sub-patches. (Please use the WIP patch just for capturing the overall image of those patches.)

[Kentaro Hara]

Created attachment 113793 [details]
WIP patch for V8 to see if a build passes

[WebKit Review Bot]

Attachment 113793 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestNamedConstructorPrivate_h  [build/header_guard] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructorPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:21:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:22:  Found WebCore config.h after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:24:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:29:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:32:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:120:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:145:  Extra space between return and WEBKIT_DOM_TEST_NAMED_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/ObjC/DOMTestNamedConstructorInternal.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 13 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kentaro Hara]

OK. All V8-related patches were landed.

I will write a patch for JSC in a few weeks.

[Kentaro Hara]

Created attachment 116943 [details]
Patch

[Kentaro Hara]

I uploaded a WIP patch that implements the [NamedConstructor] IDL for JSC. I am planning to land it in the following steps:

[1] Implement the [NamedConstructor] IDL in CodeGeneratorV8.pm.
[2] Replace the custom constructor of HTMLAudioElement with the [NamedConstructor] IDL.
[3] Replace the custom constructor of HTMLOptionElement with the [NamedConstructor] IDL.

[Kentaro Hara]

(In reply to comment #13)
> I uploaded a WIP patch that implements the [NamedConstructor] IDL for JSC. I am planning to land it in the following steps:
> 
> [1] Implement the [NamedConstructor] IDL in CodeGeneratorV8.pm.

Correction: [1] Implement the [NamedConstructor] IDL in CodeGeneratorJS.pm.

[Kentaro Hara]

All related patches were landed. Now both JSC and V8 have the [NamedConstructor] IDL. Closing bug.