[Webkit-unassigned] [Bug 100851] [V8] use toV8Fast in all relevant Node getters

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=100851


Kentaro Hara <haraken at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #171854|review+                     |review?
               Flag|                            |




--- Comment #6 from Kentaro Hara <haraken at chromium.org>  2012-11-01 08:25:41 PST ---
(From update of attachment 171854)
View in context: https://bugs.webkit.org/attachment.cgi?id=171854&action=review

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:527
> +    my $has_empty_toV8 = 0;
> +    my $has_fast_toV8 = 0;

WebKit uses camelCase for variable names.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:604
> +    die("HasFastToV8 is out of sync with respect to interface $interfaceName [$has_fast_toV8]\n") if HasFastToV8($interfaceName) != $has_fast_toV8;
> +    die("HasEmptyToV8 is out of sync with respect to interface $interfaceName [$has_fast_toV8]\n") if HasEmptyToV8($interfaceName) != $has_empty_toV8;

Basically we don't want to introduce hard-coded class lists (i.e. HasFastToV8() and HasEmptyToV8()). See below comments.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:4060
> +sub HasEmptyToV8
> +{
> +    my $type = shift;
> +
> +    return 1 if $type eq 'Element';
> +    return 1 if $type eq 'SVGRenderingIntent';
> +    return 1 if $type eq 'SVGUnitTypes';
> +    return 1 if $type eq 'SVGZoomAndPan';
> +
> +    return 0;
> +}

What is this?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:4071
> +    return 0 if $type eq 'Document';
> +    return 0 if $type eq 'Element';
> +    return 0 if $type eq 'HTMLDocument';
> +    return 0 if $type eq 'HTMLElement';
> +    return 0 if $type eq 'SVGDocument';
> +    return 0 if $type eq 'SVGElement';

Wouldn't it be possible to rename toV8() in custom bindings to toV8Fast() and thus remove this hard-coded class list from CodeGeneratorV8.pm? (It would be better if we could make the implementation of custom toV8Fast() *fast*er, but let's do it in follow-up patches.)

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:4076
>  sub IsDOMNodeType

Can't we just replace IsDOMNodeType() with IsNodeSubType(), and thus kill IsDOMNodeType() ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.