[Webkit-unassigned] [Bug 107207] [V8] Support selectively wrapping DOM accesses from certain V8 contexts.

[bugzilla-daemon at webkit.org]

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


Kentaro Hara <haraken at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #191309|review?                     |review-
               Flag|                            |




--- Comment #11 from Kentaro Hara <haraken at chromium.org>  2013-03-04 16:10:11 PST ---
(From update of attachment 191309)
View in context: https://bugs.webkit.org/attachment.cgi?id=191309&action=review

Overall the approach looks good to me.

I'm interested in performance impacts when you enable the wrapping? Did you observe performance impacts on Dromaeo/dom-*.html or Bindings/* ?

> Source/WebCore/ChangeLog:7
> +

Please describe what your change is doing.

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

You need to update run-bindings-tests.

> Source/WebCore/bindings/v8/V8DOMCoverWrapping.cpp:104
> +    v8::Persistent<v8::Value> dataHandle = v8::Persistent<v8::Value>::New(isolate, v8::External::New(instanceData));
> +    dataHandle.MakeWeak(isolate, instanceData, weakReferenceCallback);

Can't you use ScopedPersistent to manage the lifetime? (although the lifetime will be almost the same as the lifetime of WebKit.)

> Source/WebCore/bindings/v8/V8DOMCoverWrapping.cpp:107
> +    // Override the method callback, redirecting it to the coverwrapping dispatcher.
> +    callbackTemplate->SetCallHandler(dispatchCoverWrappedMethod, dataHandle);

It's a bit inconsistent that you are setting callbacks in V8DOMCoverWrapping.cpp whereas you are setting getters/setters in V8DOMConfigurations.cpp. Could you make it more consistent?

> Source/WebCore/bindings/v8/WrapperTypeInfo.h:78
> +        static WrapperTypeInfo* unwrapAddr(v8::Handle<v8::Value> typeInfoAddrWrapper)

Nit: "unwrapInfoAddr" would be a better name.

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