Bug 93342 – [V8] Remove a bunch of Persistent::New()s on setJSWrapperForXXXObject()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

There are a lot of custom bindings like this:

    V8DOMWrapper::setJSWrapperForDOMObject(impl.release(), v8::Persistent<v8::Object>::New(args.Holder()));

Manually written Persistent::New() is error-prone. We can remove the Persistent::New() by allocating the Persistent handle inside setJSWrapperForDOMObject().

[Kentaro Hara]

Created attachment 156883 [details]
Patch

[Adam Barth]

Comment on attachment 156883 [details]
Patch

Ok.  Are you sure you got them all?  If we missed one, we'll create a leak.

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 156883 [details])
> Ok.  Are you sure you got them all?  If we missed one, we'll create a leak.

Looks OK. Let me land the patch tomorrow.

$ grep -r setJSWrapper WebCore/bindings/v8/* | grep Persistent
V8DOMWrapper.cpp:v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node> node, v8::Handle<v8::Object> wrapper, v8::Isolate* isolate)
V8DOMWrapper.cpp:v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForActiveDOMNode(PassRefPtr<Node> node, v8::Handle<v8::Object> wrapper, v8::Isolate* isolate)
V8DOMWrapper.h:        static v8::Persistent<v8::Object> setJSWrapperForDOMObject(PassRefPtr<T>, v8::Handle<v8::Object>, v8::Isolate* = 0);
V8DOMWrapper.h:        static v8::Persistent<v8::Object> setJSWrapperForActiveDOMObject(PassRefPtr<T>, v8::Handle<v8::Object>, v8::Isolate* = 0);
V8DOMWrapper.h:        static v8::Persistent<v8::Object> setJSWrapperForDOMNode(PassRefPtr<Node>, v8::Handle<v8::Object>, v8::Isolate* = 0);
V8DOMWrapper.h:        static v8::Persistent<v8::Object> setJSWrapperForActiveDOMNode(PassRefPtr<Node>, v8::Handle<v8::Object>, v8::Isolate* = 0);
V8DOMWrapper.h:    v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<T> object, v8::Handle<v8::Object> wrapper, v8::Isolate* isolate)
V8DOMWrapper.h:    v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForActiveDOMObject(PassRefPtr<T> object, v8::Handle<v8::Object> wrapper, v8::Isolate* isolate)
custom/V8ArrayBufferViewCustom.h:    v8::Persistent<v8::Object> wrapperHandle = V8DOMWrapper::setJSWrapperForDOMObject(array.release(), wrapper);
custom/V8ArrayBufferViewCustom.h:    v8::Persistent<v8::Object> wrapperHandle = V8DOMWrapper::setJSWrapperForDOMObject(array.release(), wrapper);

[WebKit Review Bot]

Comment on attachment 156883 [details]
Patch

Clearing flags on attachment: 156883

Committed r124968: <http://trac.webkit.org/changeset/124968>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.