[webkit-reviews] review denied: [Bug 28071] V8 bindings: speed up lookupDOMWrapper by using new V8 API function. : [Attachment 34283] patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Vitaly Repeshko
<vitalyr at chromium.org>'s request for review:
Bug 28071: V8 bindings: speed up lookupDOMWrapper by using new V8 API function.
https://bugs.webkit.org/show_bug.cgi?id=28071

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
This looks great, except for style nits:

> +
> +	   V8 bindings: speed up lookupDOMWrapper by using new V8 API function.


The convention we use is to use a bracketed V8 qualifier:

[V8] Speed up lookupD....

>  
> -	   static v8::Handle<v8::Object>
lookupDOMWrapper(V8ClassIndex::V8WrapperType, v8::Handle<v8::Value>);
> +	   static v8::Handle<v8::Object>
lookupDOMWrapper(V8ClassIndex::V8WrapperType type, v8::Handle<v8::Object>
object)
> +	   {
> +	       if (object.IsEmpty())
> +		   return object;
> +
> +	       v8::Handle<v8::FunctionTemplate> descriptor = getTemplate(type);

> +	       return object->FindInstanceInPrototypeChain(descriptor);

Great! Can we go even more succinct here?:

return object.IsEmpty() ? object :
object->FindInstanceInPrototypeChain(getTemplate(type));

> +	   }

>	   // A helper function extract native object pointer from a DOM
wrapper
>	   // and cast to the specified type.