[webkit-reviews] review denied: [Bug 80696] Remove custom bindings for attribute type Array. : [Attachment 132575] 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 80696: Remove custom bindings for attribute type Array.
https://bugs.webkit.org/show_bug.cgi?id=80696

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

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


> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:170
> +    my @attributeType = split(/\W+/, $propType);
> +    if ($attributeType[0] eq "sequence") {
> +	   return 1;
> +    }

I think you want to skip all array types here. Maybe you can add
CodeGenerator::GetArrayType($type) to CodeGenerator.pm and use it in all code
generators. 

sub GetArrayType {
    my $type = shift;
    return $1 if $type =~ /^sequence<([\w\d_]+)>$/;
    return "";	 
}

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:248
> +    my @attributeType = split(/\W+/, $type);

Remove this

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:271
> +    } elsif ($attributeType[0] eq "sequence") {
>      } else {

You can just write

    } elsif (!$codeGenerator->GetArrayType($type)) {

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2953
> +    my @attributeType = split(/\W+/, $type);
> +    if ($attributeType[0] eq "sequence") {
> +	   return "";
> +    }

Use GetArrayType($type).

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3015
> +    my @attributeType = split(/\W+/, $type);
> +    if ($attributeType[0] eq "sequence") {

my $attributeType = $codeGenerator->GetArrayType($type);
if ($attributeType) {

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:929
> +	   my @attributeType = split(/\W+/, $returnType);
> +
> +	   if ($attributeType[0] eq "sequence") {

Ditto.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3572
> +    my @attributeType = split(/\W+/, $type);
> +    if ($attributeType[0] eq "sequence") {
> +	   return "";
> +    }

Ditto.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3792
> +    my @attributeType = split(/\W+/, $type);
> +    if ($attributeType[0] eq "sequence") {
> +	   AddToImplIncludes("V8ScriptProfile.h");
> +	   AddToImplIncludes("ScriptProfile.h");
> +	   return "v8Array($value)";
> +    }

Ditto.