[webkit-reviews] review denied: [Bug 84540] JS binding code generator doesn't handle "attribute unsigned long[]" well : [Attachment 138297] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 84540: JS binding code generator doesn't handle "attribute unsigned long[]"
well
https://bugs.webkit.org/show_bug.cgi?id=84540

Attachment 138297: Patch
https://bugs.webkit.org/attachment.cgi?id=138297&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138297&action=review


The approach looks good.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3028
> +	   if ($arrayType ne "String" && $arrayType ne "unsigned long") {

We do not want to add '$arrayType ne ...' in both CodeGeneratorJS.pm and
CodeGeneratorV8.pm every time we support sequence<some_primitive_type>. Shall
we make it a common subroutine in CodeGenerator.pm?

sub IsPrimitiveType
{
    my $type = shift;
    return 1 if $type eq "String";
    return 1 if $type eq "unsigned long";
    ...;
    return 0;
}

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3805
> +	   if ($arrayType ne "String" && $arrayType ne "unsigned long") {

Ditto.

> Source/WebCore/bindings/scripts/test/TestObj.idl:49
> +	   readonly attribute sequence<unsigned long> unsignedLongSequenceAttr;


Let's add test cases for other primitive types, e.g. sequence<long>,
sequence<float> etc.