Diff
Modified: trunk/Source/WebCore/ChangeLog (90948 => 90949)
--- trunk/Source/WebCore/ChangeLog 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/ChangeLog 2011-07-13 21:44:29 UTC (rev 90949)
@@ -1,3 +1,55 @@
+2011-07-13 Vitaly Repeshko <[email protected]>
+
+ [V8] Avoid memory leaks with hidden references.
+ https://bugs.webkit.org/show_bug.cgi?id=64467
+
+ Reviewed by Adam Barth.
+
+ We used to have growing arrays of hidden references associated
+ with objects. The entries in this array had no keys and were never
+ removed. This patch changes the interface to require a reference
+ name. This way it's harder to leak an unbounded number of
+ objects. Also it makes our wrapper objects one machine word
+ smaller.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateNormalAttrGetter): Started using new interface.
+
+ Interface changes:
+ * bindings/v8/V8DOMWrapper.cpp:
+ (WebCore::V8DOMWrapper::setNamedHiddenReference):
+ (WebCore::V8DOMWrapper::setNamedHiddenWindowReference):
+ * bindings/v8/V8DOMWrapper.h:
+
+ Added a helper to compute hidden reference names as V8 strings:
+ * bindings/v8/V8HiddenPropertyName.cpp:
+ (WebCore::V8HiddenPropertyName::hiddenReferenceName):
+ * bindings/v8/V8HiddenPropertyName.h:
+
+ * bindings/v8/WrapperTypeInfo.h: Removed the hidden reference
+ array internal field.
+
+ Update usages of hidden references:
+ * bindings/v8/custom/V8CSSStyleSheetCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8DOMStringMapCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8DOMTokenListCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8LocationCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8MessageChannelConstructor.cpp:
+ (WebCore::V8MessageChannel::constructorCallback):
+ * bindings/v8/custom/V8NamedNodeMapCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8StyleSheetCustom.cpp:
+ (WebCore::toV8):
+ * bindings/v8/custom/V8WebGLRenderingContextCustom.cpp:
+ (WebCore::toV8Object):
+
+ * bindings/scripts/test/V8/V8TestObj.cpp:
+ (WebCore::TestObjInternal::readOnlyTestObjAttrAttrGetter): Updated.
+
2011-07-13 Joseph Pecoraro <[email protected]>
Improve names of some ApplicationCacheStorage accessor methods
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (90948 => 90949)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2011-07-13 21:44:29 UTC (rev 90949)
@@ -856,9 +856,9 @@
push(@implContentDecls, " wrapper = toV8(result.get());\n");
push(@implContentDecls, " if (!wrapper.IsEmpty())\n");
if ($dataNode->name eq "DOMWindow") {
- push(@implContentDecls, " V8DOMWrapper::setHiddenWindowReference(imp->frame(), wrapper);\n");
+ push(@implContentDecls, " V8DOMWrapper::setNamedHiddenWindowReference(imp->frame(), \"${attrName}\", wrapper);\n");
} else {
- push(@implContentDecls, " V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);\n");
+ push(@implContentDecls, " V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
}
push(@implContentDecls, " }\n");
push(@implContentDecls, " return wrapper;\n");
Modified: trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -76,7 +76,7 @@
if (wrapper.IsEmpty()) {
wrapper = toV8(result.get());
if (!wrapper.IsEmpty())
- V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);
+ V8DOMWrapper::setNamedHiddenReference(info.Holder(), "readOnlyTestObjAttr", wrapper);
}
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -31,6 +31,7 @@
#include "config.h"
#include "V8DOMWrapper.h"
+#include "ArrayBufferView.h"
#include "CSSMutableStyleDeclaration.h"
#include "DOMDataStore.h"
#include "DocumentLoader.h"
@@ -39,10 +40,10 @@
#include "V8AbstractEventListener.h"
#include "V8Binding.h"
#include "V8Collection.h"
-#include "V8DedicatedWorkerContext.h"
#include "V8DOMApplicationCache.h"
#include "V8DOMMap.h"
#include "V8DOMWindow.h"
+#include "V8DedicatedWorkerContext.h"
#include "V8EventListener.h"
#include "V8EventListenerList.h"
#include "V8EventSource.h"
@@ -51,6 +52,7 @@
#include "V8FileWriter.h"
#include "V8HTMLCollection.h"
#include "V8HTMLDocument.h"
+#include "V8HiddenPropertyName.h"
#include "V8IDBDatabase.h"
#include "V8IDBRequest.h"
#include "V8IDBTransaction.h"
@@ -75,7 +77,6 @@
#include "V8WorkerContext.h"
#include "V8WorkerContextEventListener.h"
#include "V8XMLHttpRequest.h"
-#include "ArrayBufferView.h"
#include "WebGLContextAttributes.h"
#include "WebGLUniformLocation.h"
#include "WorkerContextExecutionProxy.h"
@@ -189,18 +190,13 @@
}
#endif
-void V8DOMWrapper::setHiddenReference(v8::Handle<v8::Object> parent, v8::Handle<v8::Value> child)
+
+void V8DOMWrapper::setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child)
{
- v8::Local<v8::Value> hiddenReferenceObject = parent->GetInternalField(v8DOMHiddenReferenceArrayIndex);
- if (hiddenReferenceObject->IsNull() || hiddenReferenceObject->IsUndefined()) {
- hiddenReferenceObject = v8::Array::New();
- parent->SetInternalField(v8DOMHiddenReferenceArrayIndex, hiddenReferenceObject);
- }
- v8::Local<v8::Array> hiddenReferenceArray = v8::Local<v8::Array>::Cast(hiddenReferenceObject);
- hiddenReferenceArray->Set(v8::Integer::New(hiddenReferenceArray->Length()), child);
+ parent->SetHiddenValue(V8HiddenPropertyName::hiddenReferenceName(name), child);
}
-void V8DOMWrapper::setHiddenWindowReference(Frame* frame, v8::Handle<v8::Value> jsObject)
+void V8DOMWrapper::setNamedHiddenWindowReference(Frame* frame, const char* name, v8::Handle<v8::Value> jsObject)
{
// Get DOMWindow
if (!frame)
@@ -214,7 +210,7 @@
global = V8DOMWrapper::lookupDOMWrapper(V8DOMWindow::GetTemplate(), global);
ASSERT(!global.IsEmpty());
- setHiddenReference(global, jsObject);
+ setNamedHiddenReference(global, name, jsObject);
}
WrapperTypeInfo* V8DOMWrapper::domWrapperType(v8::Handle<v8::Object> object)
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2011-07-13 21:44:29 UTC (rev 90949)
@@ -114,11 +114,15 @@
// Check whether a V8 value is a wrapper of type |classType|.
static bool isWrapperOfType(v8::Handle<v8::Value>, WrapperTypeInfo*);
- static void setHiddenReference(v8::Handle<v8::Object> parent, v8::Handle<v8::Value> child);
+ // Proper object lifetime support.
+ //
+ // Helper functions to make sure the child object stays alive
+ // while the parent is alive. Using the name more than once
+ // overwrites previous references making it possible to free
+ // old children.
+ static void setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child);
+ static void setNamedHiddenWindowReference(Frame*, const char*, v8::Handle<v8::Value>);
- // Set hidden references in a DOMWindow object of a frame.
- static void setHiddenWindowReference(Frame*, v8::Handle<v8::Value>);
-
static v8::Local<v8::Object> instantiateV8Object(V8Proxy* proxy, WrapperTypeInfo*, void* impl);
static v8::Handle<v8::Object> getWrapper(Node* node)
Modified: trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -31,6 +31,9 @@
#include "config.h"
#include "V8HiddenPropertyName.h"
+#include <string.h>
+#include <wtf/Vector.h>
+
namespace WebCore {
#define V8_AS_STRING(x) V8_AS_STRING_IMPL(x)
@@ -39,12 +42,22 @@
#define V8_DEFINE_PROPERTY(name) \
v8::Handle<v8::String> V8HiddenPropertyName::name() \
{ \
- static v8::Persistent<v8::String>* string = createString("WebCore::V8HiddenPropertyName::" V8_AS_STRING(name)); \
+ static v8::Persistent<v8::String>* string = createString("WebCore::HiddenProperty::" V8_AS_STRING(name)); \
return *string; \
}
V8_HIDDEN_PROPERTIES(V8_DEFINE_PROPERTY);
+static const char hiddenReferenceNamePrefix[] = "WebCore::HiddenReference::";
+
+v8::Handle<v8::String> V8HiddenPropertyName::hiddenReferenceName(const char* name)
+{
+ Vector<char, 64> prefixedName;
+ prefixedName.append(hiddenReferenceNamePrefix, sizeof(hiddenReferenceNamePrefix) - 1);
+ prefixedName.append(name, strlen(name));
+ return v8::String::NewSymbol(prefixedName.data(), static_cast<int>(prefixedName.size()));
+}
+
v8::Persistent<v8::String>* V8HiddenPropertyName::createString(const char* key)
{
v8::HandleScope scope;
Modified: trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h 2011-07-13 21:44:29 UTC (rev 90949)
@@ -52,6 +52,8 @@
V8_HIDDEN_PROPERTIES(V8_DECLARE_PROPERTY);
#undef V8_DECLARE_PROPERTY
+ static v8::Handle<v8::String> hiddenReferenceName(const char* name);
+
private:
static v8::Persistent<v8::String>* createString(const char* key);
};
Modified: trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h 2011-07-13 21:44:29 UTC (rev 90949)
@@ -39,8 +39,7 @@
static const int v8DOMWrapperTypeIndex = 0;
static const int v8DOMWrapperObjectIndex = 1;
- static const int v8DOMHiddenReferenceArrayIndex = 2;
- static const int v8DefaultWrapperInternalFieldCount = 3;
+ static const int v8DefaultWrapperInternalFieldCount = 2;
static const uint16_t v8DOMSubtreeClassId = 1;
Modified: trunk/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -44,7 +44,7 @@
// Add a hidden reference from stylesheet object to its owner node.
Node* ownerNode = impl->ownerNode();
if (ownerNode && !wrapper.IsEmpty())
- V8DOMWrapper::setHiddenReference(wrapper, toV8(ownerNode));
+ V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(ownerNode));
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -105,7 +105,7 @@
if (!wrapper.IsEmpty() && element) {
v8::Handle<v8::Value> elementValue = toV8(element);
if (!elementValue.IsEmpty() && elementValue->IsObject())
- V8DOMWrapper::setHiddenReference(elementValue.As<v8::Object>(), wrapper);
+ V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domStringMap", wrapper);
}
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -48,7 +48,7 @@
if (!wrapper.IsEmpty() && element) {
v8::Handle<v8::Value> elementValue = toV8(element);
if (!elementValue.IsEmpty() && elementValue->IsObject())
- V8DOMWrapper::setHiddenReference(elementValue.As<v8::Object>(), wrapper);
+ V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domTokenList", wrapper);
}
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -280,7 +280,7 @@
if (wrapper.IsEmpty()) {
wrapper = V8Location::wrap(impl);
if (!wrapper.IsEmpty())
- V8DOMWrapper::setHiddenWindowReference(impl->frame(), wrapper);
+ V8DOMWrapper::setNamedHiddenWindowReference(impl->frame(), "location", wrapper);
}
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -67,8 +67,8 @@
// Create references from the MessageChannel wrapper to the two
// MessagePort wrappers to make sure that the MessagePort wrappers
// stay alive as long as the MessageChannel wrapper is around.
- V8DOMWrapper::setHiddenReference(messageChannel, toV8(obj->port1()));
- V8DOMWrapper::setHiddenReference(messageChannel, toV8(obj->port2()));
+ V8DOMWrapper::setNamedHiddenReference(messageChannel, "port1", toV8(obj->port1()));
+ V8DOMWrapper::setNamedHiddenReference(messageChannel, "port2", toV8(obj->port2()));
// Setup the standard wrapper object internal fields.
V8DOMWrapper::setDOMWrapper(messageChannel, &info, obj.get());
Modified: trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -83,7 +83,7 @@
// Add a hidden reference from named node map to its owner node.
Element* element = impl->element();
if (!wrapper.IsEmpty() && element)
- V8DOMWrapper::setHiddenReference(wrapper, toV8(element));
+ V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(element));
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -47,7 +47,7 @@
// Add a hidden reference from stylesheet object to its owner node.
Node* ownerNode = impl->ownerNode();
if (ownerNode && !wrapper.IsEmpty())
- V8DOMWrapper::setHiddenReference(wrapper, toV8(ownerNode));
+ V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(ownerNode));
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp (90948 => 90949)
--- trunk/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp 2011-07-13 21:40:54 UTC (rev 90948)
+++ trunk/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp 2011-07-13 21:44:29 UTC (rev 90949)
@@ -161,22 +161,27 @@
if (!extension)
return v8::Null();
v8::Handle<v8::Value> extensionObject;
+ const char* referenceName;
switch (extension->getName()) {
case WebGLExtension::WebKitLoseContextName:
extensionObject = toV8(static_cast<WebKitLoseContext*>(extension));
+ referenceName = "webKitLoseContextName";
break;
case WebGLExtension::OESStandardDerivativesName:
extensionObject = toV8(static_cast<OESStandardDerivatives*>(extension));
+ referenceName = "oesStandardDerivativesName";
break;
case WebGLExtension::OESTextureFloatName:
extensionObject = toV8(static_cast<OESTextureFloat*>(extension));
+ referenceName = "oesTextureFloatName";
break;
case WebGLExtension::OESVertexArrayObjectName:
extensionObject = toV8(static_cast<OESVertexArrayObject*>(extension));
+ referenceName = "oesVertexArrayObjectName";
break;
}
ASSERT(!extensionObject.IsEmpty());
- V8DOMWrapper::setHiddenReference(contextObject, extensionObject);
+ V8DOMWrapper::setNamedHiddenReference(contextObject, referenceName, extensionObject);
return extensionObject;
}
