Bug 152171 – WebIDL generator should support the possibility for C++ classes to have a JS Builtin constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[youenn fablet]

It might be handy to use JS builtin to process the parameters passed to create a DOM object.
One solution is to create the C++ object using standard XX::create() and then call a JS builtin function similarly to JSBuiltin classes.
This might need the creation of a JSBuiltinConstructor

[youenn fablet]

Created attachment 267168 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/bindings/scripts/test/ObjC/DOMTestClassWithJSBuiltinConstructor.mm:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestClassWithJSBuiltinConstructor.cpp:153:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 11 files


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

[youenn fablet]

Created attachment 267295 [details]
Minor refactoring

[youenn fablet]

Created attachment 267297 [details]
Rebasing

[youenn fablet]

Created attachment 267298 [details]
Rebasing

[WebKit Commit Bot]

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


ERROR: Source/WebCore/bindings/scripts/test/ObjC/DOMTestClassWithJSBuiltinConstructor.mm:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestClassWithJSBuiltinConstructor.cpp:158:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 11 files


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

[youenn fablet]

Created attachment 267299 [details]
Patch

[youenn fablet]

Created attachment 267300 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/bindings/scripts/test/ObjC/DOMTestClassWithJSBuiltinConstructor.mm:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestClassWithJSBuiltinConstructor.cpp:158:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 11 files


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

[youenn fablet]

(In reply to comment #8)
> Created attachment 267300 [details]
> Patch

@adambe, patch is currently not supporting ConstructorCallWith for JSBuiltinConstructor. Is that ok with your needs?

[Darin Adler]

Comment on attachment 267300 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=267300&action=review

> Source/WebCore/ChangeLog:23
> +        (GenerateConstructorDefinition):
> +        (IsConstructable):
> +        (IsJSBuiltinConstructor):
> +        (AddJSBuiltinIncludesIfNeeded):
> +        (GenerateHeader): Deleted.
> +        (GetJSBuiltinScopeName): Deleted.

Is this list of functions accurate? Did we actually delete two functions? If not, please fix this before landing.

> Source/WebCore/ChangeLog:31
> +        (WebKit::kit):
> +        (WebKit::core):
> +        (WebKit::wrapTestClassWithJSBuiltinConstructor):
> +        (webkit_dom_test_class_with_js_builtin_constructor_finalize):
> +        (webkit_dom_test_class_with_js_builtin_constructor_constructor):
> +        (webkit_dom_test_class_with_js_builtin_constructor_class_init):
> +        (webkit_dom_test_class_with_js_builtin_constructor_init):

These lists of functions are pointless in test expectations files like this. I suggest omitting them.

> Source/WebCore/ChangeLog:53
> +        (WebCore::JSTestClassWithJSBuiltinConstructorPrototype::create):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorPrototype::createStructure):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorPrototype::JSTestClassWithJSBuiltinConstructorPrototype):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorConstructor::createJSObject):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorConstructor::initializeProperties):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorConstructor::initializeExecutable):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorPrototype::finishCreation):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::JSTestClassWithJSBuiltinConstructor):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::createPrototype):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::getPrototype):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::destroy):
> +        (WebCore::jsTestClassWithJSBuiltinConstructorConstructor):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::getConstructor):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::visitChildren):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorOwner::isReachableFromOpaqueRoots):
> +        (WebCore::JSTestClassWithJSBuiltinConstructorOwner::finalize):
> +        (WebCore::toJSNewlyCreated):
> +        (WebCore::toJS):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::toWrapped):

These lists of functions are pointless in test expectations files like this. I suggest omitting them.

> Source/WebCore/ChangeLog:59
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::create):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::createStructure):
> +        (WebCore::JSTestClassWithJSBuiltinConstructor::finishCreation):
> +        (WebCore::wrapperOwner):
> +        (WebCore::toJS):

These lists of functions are pointless in test expectations files like this. I suggest omitting them.

> Source/WebCore/ChangeLog:64
> +        (-[DOMTestClassWithJSBuiltinConstructor dealloc]):
> +        (core):
> +        (kit):

These lists of functions are pointless in test expectations files like this. I suggest omitting them.

[youenn fablet]

> > Source/WebCore/ChangeLog:23
> > +        (GenerateConstructorDefinition):
> > +        (IsConstructable):
> > +        (IsJSBuiltinConstructor):
> > +        (AddJSBuiltinIncludesIfNeeded):
> > +        (GenerateHeader): Deleted.
> > +        (GetJSBuiltinScopeName): Deleted.
> 
> Is this list of functions accurate? Did we actually delete two functions? If
> not, please fix this before landing.

OK

> 
> > Source/WebCore/ChangeLog:31
> > +        (WebKit::kit):
> > +        (WebKit::core):
> > +        (WebKit::wrapTestClassWithJSBuiltinConstructor):
> > +        (webkit_dom_test_class_with_js_builtin_constructor_finalize):
> > +        (webkit_dom_test_class_with_js_builtin_constructor_constructor):
> > +        (webkit_dom_test_class_with_js_builtin_constructor_class_init):
> > +        (webkit_dom_test_class_with_js_builtin_constructor_init):
> 
> These lists of functions are pointless in test expectations files like this.
> I suggest omitting them.

Are you referring to the changelog or the test expectation files?
If the latter, how can that be done?
Also functions related to WebCore::JSTestClassWithJSBuiltinConstructor are important to keep.

[youenn fablet]

Created attachment 267350 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 267350 [details]
Patch for landing

Clearing flags on attachment: 267350

Committed r194100: <http://trac.webkit.org/changeset/194100>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.