Diff
Modified: trunk/Source/WebCore/ChangeLog (113271 => 113272)
--- trunk/Source/WebCore/ChangeLog 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/ChangeLog 2012-04-05 01:38:28 UTC (rev 113272)
@@ -1,3 +1,46 @@
+2012-04-04 Adam Klein <[email protected]>
+
+ Use PassRefPtr in V8DOMWrapper interface to avoid explicit ref() calls
+ https://bugs.webkit.org/show_bug.cgi?id=82238
+
+ Reviewed by Adam Barth.
+
+ Relanding r112163 without modification, as it still seems valid.
+ Will watch Chrome Canaries closely for any memory issues.
+
+ The setJSWrapper* methods previously featured a comment that asked
+ callers to ref the objects before passing them in. This change makes
+ that contract explicit (and allows the removal of the comment).
+
+ In addition, for ConstructorCallbacks, this change slightly reduces
+ refcount churn by passing on the initial ref via RefPtr::release().
+
+ No new tests, no change in behavior.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateConstructorCallback): Use RefPtr::release() to avoid refcount churn and remove explicit ref() call.
+ (GenerateNamedConstructorCallback): ditto.
+ * bindings/v8/V8DOMWindowShell.cpp:
+ (WebCore::V8DOMWindowShell::installDOMWindow): Cast to a PassRefPtr and remove explicit ref call.
+ * bindings/v8/V8DOMWrapper.cpp:
+ (WebCore::V8DOMWrapper::setJSWrapperForDOMNode): Pass leaked refs into the DOMNodeMaps.
+ * bindings/v8/V8DOMWrapper.h:
+ (V8DOMWrapper): Make the setJSWrapperFor* methods take PassRefPtr<T>.
+ (WebCore::V8DOMWrapper::setJSWrapperForDOMObject): Pass leaked ref into the DOMObjectMap.
+ (WebCore::V8DOMWrapper::setJSWrapperForActiveDOMObject): Pass leaked ref into the ActiveDOMObjectMap.
+ * bindings/v8/V8Proxy.h:
+ (WebCore::toV8): Remove explicit ref.
+ * bindings/v8/WorkerContextExecutionProxy.cpp:
+ (WebCore::WorkerContextExecutionProxy::initContextIfNeeded): Cast to a PassRefPTr and remove explicit ref call.
+ * bindings/v8/custom/V8HTMLImageElementConstructor.cpp:
+ (WebCore::v8HTMLImageElementConstructorCallback): Use RefPtr::release() to avoid refcount churn and remove explicit ref.
+ * bindings/v8/custom/V8WebKitMutationObserverCustom.cpp:
+ (WebCore::V8WebKitMutationObserver::constructorCallback): ditto.
+ * bindings/v8/custom/V8WebSocketCustom.cpp:
+ (WebCore::V8WebSocket::constructorCallback): ditto.
+ * bindings/v8/custom/V8XMLHttpRequestConstructor.cpp:
+ (WebCore::V8XMLHttpRequest::constructorCallback): ditto.
+
2012-04-04 Chris Rogers <[email protected]>
WaveTable::waveDataForFundamentalFrequency() should properly interpret negative frequency
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (113271 => 113272)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-04-05 01:38:28 UTC (rev 113272)
@@ -1762,8 +1762,7 @@
push(@implContent, <<END);
V8DOMWrapper::setDOMWrapper(wrapper, &info, impl.get());
- impl->ref();
- V8DOMWrapper::setJSWrapperFor${DOMObject}(impl.get(), v8::Persistent<v8::Object>::New(wrapper));
+ V8DOMWrapper::setJSWrapperFor${DOMObject}(impl.release(), v8::Persistent<v8::Object>::New(wrapper));
return args.Holder();
END
@@ -1944,8 +1943,7 @@
push(@implContent, <<END);
V8DOMWrapper::setDOMWrapper(wrapper, &V8${implClassName}Constructor::info, impl.get());
- impl->ref();
- V8DOMWrapper::setJSWrapperFor${DOMObject}(impl.get(), v8::Persistent<v8::Object>::New(wrapper));
+ V8DOMWrapper::setJSWrapperFor${DOMObject}(impl.release(), v8::Persistent<v8::Object>::New(wrapper));
return args.Holder();
END
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -411,8 +411,7 @@
V8DOMWrapper::setDOMWrapper(jsWindow, &V8DOMWindow::info, window);
V8DOMWrapper::setDOMWrapper(v8::Handle<v8::Object>::Cast(jsWindow->GetPrototype()), &V8DOMWindow::info, window);
- window->ref();
- V8DOMWrapper::setJSWrapperForDOMObject(window, v8::Persistent<v8::Object>::New(jsWindow));
+ V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<DOMWindow>(window), v8::Persistent<v8::Object>::New(jsWindow));
// Insert the window instance as the prototype of the shadow object.
v8::Handle<v8::Object> v8RealGlobal = v8::Handle<v8::Object>::Cast(context->Global()->GetPrototype());
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -69,30 +69,13 @@
namespace WebCore {
-// The caller must have increased obj's ref count.
-void V8DOMWrapper::setJSWrapperForDOMObject(void* object, v8::Persistent<v8::Object> wrapper)
+void V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
{
- ASSERT(V8DOMWrapper::maybeDOMWrapper(wrapper));
- ASSERT(!domWrapperType(wrapper)->toActiveDOMObjectFunction);
- getDOMObjectMap().set(object, wrapper);
-}
-
-// The caller must have increased obj's ref count.
-void V8DOMWrapper::setJSWrapperForActiveDOMObject(void* object, v8::Persistent<v8::Object> wrapper)
-{
- ASSERT(V8DOMWrapper::maybeDOMWrapper(wrapper));
- ASSERT(domWrapperType(wrapper)->toActiveDOMObjectFunction);
- getActiveDOMObjectMap().set(object, wrapper);
-}
-
-// The caller must have increased node's ref count.
-void V8DOMWrapper::setJSWrapperForDOMNode(Node* node, v8::Persistent<v8::Object> wrapper)
-{
- ASSERT(V8DOMWrapper::maybeDOMWrapper(wrapper));
+ ASSERT(maybeDOMWrapper(wrapper));
if (node->isActiveNode())
- getActiveDOMNodeMap().set(node, wrapper);
+ getActiveDOMNodeMap().set(node.leakRef(), wrapper);
else
- getDOMNodeMap().set(node, wrapper);
+ getDOMNodeMap().set(node.leakRef(), wrapper);
}
v8::Local<v8::Function> V8DOMWrapper::constructorForType(WrapperTypeInfo* type, DOMWindow* window)
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-04-05 01:38:28 UTC (rev 113272)
@@ -38,6 +38,7 @@
#include "NodeFilter.h"
#include "PlatformString.h"
#include "V8CustomXPathNSResolver.h"
+#include "V8DOMMap.h"
#include "V8Event.h"
#include "V8IsolatedContext.h"
#include "V8Utilities.h"
@@ -46,6 +47,7 @@
#include "XPathNSResolver.h"
#include <v8.h>
#include <wtf/MainThread.h>
+#include <wtf/PassRefPtr.h>
namespace WebCore {
@@ -104,10 +106,9 @@
static v8::Local<v8::Function> constructorForType(WrapperTypeInfo*, WorkerContext*);
#endif
- // Set JS wrapper of a DOM object, the caller in charge of increase ref.
- static void setJSWrapperForDOMObject(void*, v8::Persistent<v8::Object>);
- static void setJSWrapperForActiveDOMObject(void*, v8::Persistent<v8::Object>);
- static void setJSWrapperForDOMNode(Node*, v8::Persistent<v8::Object>);
+ template<typename T> static void setJSWrapperForDOMObject(PassRefPtr<T>, v8::Persistent<v8::Object>);
+ template<typename T> static void setJSWrapperForActiveDOMObject(PassRefPtr<T>, v8::Persistent<v8::Object>);
+ static void setJSWrapperForDOMNode(PassRefPtr<Node>, v8::Persistent<v8::Object>);
static bool isValidDOMObject(v8::Handle<v8::Value>);
@@ -151,6 +152,22 @@
static V8BindingPerContextData* perContextData(WorkerContext*);
#endif
};
+
+ template<typename T>
+ void V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
+ {
+ ASSERT(maybeDOMWrapper(wrapper));
+ ASSERT(!domWrapperType(wrapper)->toActiveDOMObjectFunction);
+ getDOMObjectMap().set(object.leakRef(), wrapper);
+ }
+
+ template<typename T>
+ void V8DOMWrapper::setJSWrapperForActiveDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
+ {
+ ASSERT(maybeDOMWrapper(wrapper));
+ ASSERT(domWrapperType(wrapper)->toActiveDOMObjectFunction);
+ getActiveDOMObjectMap().set(object.leakRef(), wrapper);
+ }
}
#endif // V8DOMWrapper_h
Modified: trunk/Source/WebCore/bindings/v8/V8Proxy.h (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/V8Proxy.h 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/V8Proxy.h 2012-04-05 01:38:28 UTC (rev 113272)
@@ -354,11 +354,10 @@
template <class T> inline v8::Handle<v8::Object> toV8(PassRefPtr<T> object, v8::Local<v8::Object> holder, IndependentMode independent = DoNotMarkIndependent)
{
- object->ref();
v8::Persistent<v8::Object> handle = v8::Persistent<v8::Object>::New(holder);
if (independent == MarkIndependent)
handle.MarkIndependent();
- V8DOMWrapper::setJSWrapperForDOMObject(object.get(), handle);
+ V8DOMWrapper::setJSWrapperForDOMObject(object, handle);
return holder;
}
Modified: trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -179,8 +179,7 @@
// Wrap the object.
V8DOMWrapper::setDOMWrapper(jsWorkerContext, contextType, m_workerContext);
- V8DOMWrapper::setJSWrapperForDOMObject(m_workerContext, v8::Persistent<v8::Object>::New(jsWorkerContext));
- m_workerContext->ref();
+ V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<WorkerContext>(m_workerContext), v8::Persistent<v8::Object>::New(jsWorkerContext));
// Insert the object instance as the prototype of the shadow object.
v8::Handle<v8::Object> globalObject = v8::Handle<v8::Object>::Cast(m_context->Global()->GetPrototype());
Modified: trunk/Source/WebCore/bindings/v8/custom/V8HTMLImageElementConstructor.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/custom/V8HTMLImageElementConstructor.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/custom/V8HTMLImageElementConstructor.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -86,8 +86,7 @@
RefPtr<HTMLImageElement> image = HTMLImageElement::createForJSConstructor(document, optionalWidth, optionalHeight);
V8DOMWrapper::setDOMWrapper(args.Holder(), &V8HTMLImageElementConstructor::info, image.get());
- image->ref();
- V8DOMWrapper::setJSWrapperForDOMNode(image.get(), v8::Persistent<v8::Object>::New(args.Holder()));
+ V8DOMWrapper::setJSWrapperForDOMNode(image.release(), v8::Persistent<v8::Object>::New(args.Holder()));
return args.Holder();
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -75,8 +75,7 @@
RefPtr<WebKitMutationObserver> observer = WebKitMutationObserver::create(callback.release());
V8DOMWrapper::setDOMWrapper(args.Holder(), &info, observer.get());
- observer->ref();
- V8DOMWrapper::setJSWrapperForDOMObject(observer.get(), v8::Persistent<v8::Object>::New(args.Holder()));
+ V8DOMWrapper::setJSWrapperForDOMObject(observer.release(), v8::Persistent<v8::Object>::New(args.Holder()));
return args.Holder();
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8WebSocketCustom.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/custom/V8WebSocketCustom.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/custom/V8WebSocketCustom.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -107,13 +107,8 @@
if (ec)
return throwError(ec);
- // Setup the standard wrapper object internal fields.
V8DOMWrapper::setDOMWrapper(args.Holder(), &info, webSocket.get());
-
- // Add object to the wrapper map.
- webSocket->ref();
- V8DOMWrapper::setJSWrapperForActiveDOMObject(webSocket.get(), v8::Persistent<v8::Object>::New(args.Holder()));
-
+ V8DOMWrapper::setJSWrapperForActiveDOMObject(webSocket.release(), v8::Persistent<v8::Object>::New(args.Holder()));
return args.Holder();
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp (113271 => 113272)
--- trunk/Source/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp 2012-04-05 01:17:13 UTC (rev 113271)
+++ trunk/Source/WebCore/bindings/v8/custom/V8XMLHttpRequestConstructor.cpp 2012-04-05 01:38:28 UTC (rev 113272)
@@ -64,10 +64,7 @@
securityOrigin = isolatedContext->securityOrigin();
RefPtr<XMLHttpRequest> xmlHttpRequest = XMLHttpRequest::create(context, securityOrigin);
V8DOMWrapper::setDOMWrapper(args.Holder(), &info, xmlHttpRequest.get());
-
- // Add object to the wrapper map.
- xmlHttpRequest->ref();
- V8DOMWrapper::setJSWrapperForActiveDOMObject(xmlHttpRequest.get(), v8::Persistent<v8::Object>::New(args.Holder()));
+ V8DOMWrapper::setJSWrapperForActiveDOMObject(xmlHttpRequest.release(), v8::Persistent<v8::Object>::New(args.Holder()));
return args.Holder();
}
