[Webkit-unassigned] [Bug 102686] [V8] toFastV8 for non-Nodes

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Dan Carney <dcarney at google.com>  2012-12-03 08:25:37 PST ---
(From update of attachment 174973)
View in context: https://bugs.webkit.org/attachment.cgi?id=174973&action=review

>> Source/WebCore/ChangeLog:3
>> +        [V8] toFastV8 for non-Nodes
> 
> I'm a bit confused with the title. Is this patch intending to use toV8Fast() for a case where IsDOMNodeType($type) is true?
> 
> Nit: toV8Fast

fixed
it means that the callback is from for a node subclass, but the object requested could be anything with a wrapper

>> Source/WebCore/ChangeLog:8
>> +        This patch makes most generated bindings use toFastV8
> 
> Ditto.

ok

>> Source/WebCore/ChangeLog:10
>> +        Additionally, a toFastV8 for v8::Arguments was added
> 
> Ditto.

ok

>> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:539
>> +}
> 
> We might not want to duplicate this function for AccessorInfo and Arguments. Wouldn't it be possible to change the signature to
> 
>   v8::Handle<v8::Value> toV8Fast(${nativeType}* impl, v8::Handle<v8::Object> holder, Node*);
> 
> and thus eliminate the duplication?

ok

>> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:588
>> +        foreach my $argType ("AccessorInfo","Arguments") {
> 
> Nit: "AccessorInfo", "Arguments" (One space after ,)

ok

>> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:620
>> +inline v8::Handle<v8::Value> toV8Fast(PassRefPtr< ${nativeType} > impl, const v8::Arguments& info, Node* holder)
> 
> Nit: We are likely to use 'args' instead of 'info'.

ok

>> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:4144
>> +    if ($getFastArgs) {
> 
> Don't you need to check IsDOMNodeType($type) ?

not here, the presence of the argument implies there is a node to make the call from

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