Bug 58001 – Prevent custom type checking from undefining DOMStringList arguments [v8]

• Status: RESOLVED
• Resolution: LATER
• Component: New Bugs

[Tyler Close]

Prevent custom type checking from undefining DOMStringList arguments

[Tyler Close]

Created attachment 88546 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style']" exit_code: 1

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:248:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:144:  The parameter name "list" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 12 files


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

[Darin Adler]

Comment on attachment 88546 [details]
Patch

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

> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1043
> +    DOMStringList* list(toDOMStringList(exec->argument(0)));

Does the function toDOMStringList exist?

[Tyler Close]

I don't see a toDOMStringList() function, so it looks like this test class exposes a bug/missing feature of CodeGeneratorJS.pm. Does it cause a problem if some of the run-bindings-tests reference output files don't compile? AFAIK, the output files are only used for diff comparison. If this is a problem, I suppose we could remove the test code until all the code generators have been fixed. Obviously the CodeGeneratorV8.pm output compiles and runs, but I haven't looked into any of the other language bindings yet.

[Darin Adler]

(In reply to comment #4)
> Does it cause a problem if some of the run-bindings-tests reference output files don't compile?

Not directly, but we’ll immediately have a problem if we need a binding like this to work with JavaScript. We don’t want DOMStringList to be V8-only!

[Tyler Close]

DOMStringList needs to be able to go both in and out of DOM APIs. IDBDatabase depends on this and so does another API I'm working on. It looks like there are other APIs under development that would use this if it existed, instead of defining syntax for encoding a string list in a string. Hopefully this patch is just the first step towards a fully functional DOMStringList.

[Maciej Stachowiak]

Comment on attachment 88546 [details]
Patch

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

>> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1043
>> +    DOMStringList* list(toDOMStringList(exec->argument(0)));
> 
> Does the function toDOMStringList exist?

It does exist, in generated code (JSDOMStringList.cpp).

[Maciej Stachowiak]

Comment on attachment 88546 [details]
Patch

It seems like only the v8 code generator is changed. Does this mean the bug is v8-specific? If so, please add [v8] to the bug title for clarity.

[Adam Barth]

Comment on attachment 88546 [details]
Patch

It's probably better to address this issue the next time this issue actually occurs.