Prevent custom type checking from undefining DOMStringList arguments
Created attachment 88546 [details] Patch
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.
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?
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.
(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!
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.
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).
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.
Comment on attachment 88546 [details] Patch It's probably better to address this issue the next time this issue actually occurs.