If we use DOMString[] for - an argument of a non-overload function - an return value of a function, our binding generators produce wrong code.
Created attachment 147510 [details] Patch
Created attachment 147512 [details] Patch 2 Remove useless comment
Looks reasonable, but I'd like haraken to take a look.
Comment on attachment 147512 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review Looks reasonable to me too. > Source/WebCore/bindings/js/JSDOMBinding.cpp:153 > + if (!stringList) > + return jsUndefined(); Just a confirmation: You are intending to return (not [] but) undefined for a null stringList, right?
Comment on attachment 147512 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-3075 > - if (!$codeGenerator->SkipIncludeHeader($arrayType)) { tkent: can we add "DOMString[]" to SkipIncludeHeader() ..? because I think this will fail for numerictypes like float[]. I mean will include float.h to cpp files.
Comment on attachment 147512 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review >> Source/WebCore/bindings/js/JSDOMBinding.cpp:153 >> + return jsUndefined(); > > Just a confirmation: You are intending to return (not [] but) undefined for a null stringList, right? Oh, this should be null to match V8 binding behavior. I'll change it so. Thanks.
Comment on attachment 147512 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-3075 >> - if (!$codeGenerator->SkipIncludeHeader($arrayType)) { > > tkent: can we add "DOMString[]" to SkipIncludeHeader() ..? because I think this will fail for numerictypes like float[]. I mean will include float.h to cpp files. ok, will do.
(In reply to comment #7) > (From update of attachment 147512 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review > > >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-3075 > >> - if (!$codeGenerator->SkipIncludeHeader($arrayType)) { > > > > tkent: can we add "DOMString[]" to SkipIncludeHeader() ..? because I think this will fail for numerictypes like float[]. I mean will include float.h to cpp files. > > ok, will do. Ahh sorry tkent previous comment you are handling that in elsif.
Comment on attachment 147512 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review >>>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-3075 >>>> - if (!$codeGenerator->SkipIncludeHeader($arrayType)) { >>> >>> tkent: can we add "DOMString[]" to SkipIncludeHeader() ..? because I think this will fail for numerictypes like float[]. I mean will include float.h to cpp files. >> >> ok, will do. > > Ahh sorry tkent previous comment you are handling that in elsif. ok, no problem. Anyway, we might not need to include JSDOMStringList.h and DOMStringList.h.
(In reply to comment #9) > Anyway, we might not need to include JSDOMStringList.h and DOMStringList.h. I'll investigate this later.
Created attachment 147517 [details] Patch for landing undefined -> null
Committed r120304: <http://trac.webkit.org/changeset/120304>
(In reply to comment #6) > (From update of attachment 147512 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147512&action=review > > >> Source/WebCore/bindings/js/JSDOMBinding.cpp:153 > >> + return jsUndefined(); > > > > Just a confirmation: You are intending to return (not [] but) undefined for a null stringList, right? > > Oh, this should be null to match V8 binding behavior. I'll change it so. I was incorrect. We doesn't have V8 binding code to convert a DOMStringList to v8 array. I looked a wrong code. Probably we should have different behavior for "DOMString[]" and "DOMString[]?".
(In reply to comment #13) > I was incorrect. We doesn't have V8 binding code to convert a DOMStringList to v8 array. I looked a wrong code. > Probably we should have different behavior for "DOMString[]" and "DOMString[]?". Yes. DOMString[] is not nullable, but DOMString[]? is nullable: http://www.w3.org/TR/WebIDL/#idl-nullable-type
Ok, code generators should produce v8ArrayOrNull() / jsArrayOrNull() for "DOMString[]?". So, jsArray() / v8Array() should not return null/undefined. I'll make a patch.