[webkit-reviews] review denied: [Bug 100851] [V8] use toV8Fast in all relevant getters : [Attachment 171640] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Dan Carney
<dcarney at google.com>'s request for review:
Bug 100851: [V8] use toV8Fast in all relevant getters
https://bugs.webkit.org/show_bug.cgi?id=100851

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

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


> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:297
> +    die("IsDOMNodeType is out of date with respect to $interfaceName") if
IsDOMNodeType($interfaceName) != IsNodeSubType($dataNode);

Why are both IsDOMNodeType() and IsNodeSubType() needed, given that they should
return the same result?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1121
> +    } elsif (IsDOMNodeType($attrType) && IsDOMNodeType($interfaceName)) {
> +	   AddToImplIncludes($attrType.".h");
>	   push(@implContentDecls, "	return toV8Fast($result, info,
imp);\n");

Some Node-type classes have custom implementation of toV8() in
bindings/v8/custom/*. Wouldn't it cause a problem? I mean, if you call
toV8Fast() here, you might end up failing to call custom toV8().

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-4031
> -    return 1 if $type eq 'HTMLElement';
> -    return 1 if $type eq 'HTMLUnknownElement';
> -    return 1 if $type eq 'HTMLFormElement';
> -    return 1 if $type eq 'HTMLTableCaptionElement';
> -    return 1 if $type eq 'HTMLTableSectionElement';

Isn't IsDOMNodeType() used by other places? I want to make sure that this
change won't affect other places.