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

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted 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 138305: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=138305&action=review

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


>>> Source/WebCore/bindings/scripts/CodeGenerator.pm:350
>>> +	 return 1 if $type eq "String";
>> 
>> AvoidInclusionOfType() is already used in CodeGenerator*.pm. I am a bit
worrying that this would change the existing behaviors. No worry?
> 
> Umm I think the places where AvoidInclusionOfType is used to check whether to
include header of type or not.
> Looking at the current implementation of AvoidInclusionOfType it is avoiding
inclusion only because it doesn't have headers like SVGPoint/SVGNumber adding
above code won't change existing behaviour because it doesn't have headers for
these type either.
> IMO worry is if $arrayType is "SVGPoint/SVGNumber" it will skip headers.
> 
> So should I use IsPrimitiveType subroutine adding "String" to it or Write a
new subroutine?

Thanks for the clarification. Currently there are no use cases for an
SVGPoint/SVGNumber array. Let's keep it as-is.

BTW, shall we rename AvoidInclusionOfType() to SkipHeaderInclude() (or
something clearer)?