Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix memory leak caused by CSSRuleList wrapper

Accessing cssrulelist in javascript will create a new
CSSRuleList object and wrapper JS object. The wrapper
JS object will be added into hidden array in parent JS
object which is alive during the whole execution. Thus
memory leak happens (CSSRuleList in webkit, wrapper object
and weak global handle in v8).

Cherry pick 2 patches from upstream solves the problem:

http://trac.webkit.org/changeset/90949

This patch changes hidden reference from array to named
property. So new wrapper will replace old wrappper. But the
memory leak still exists because the CSSRuleList wrapper
will be added into an object group of current document. So
they will still be alive during execution.

http://trac.webkit.org/changeset/91256

This patch avoids to adding CSSRuleList wrapper into document
object group. Combined with the first patch, it can resolve
the memory leak problem.

Change-Id: Icb523db52963726f27b6c02596822cfb6e8d5049
Author: Vitaly Repeshko <vitalyr@chromium.org>
Signed-off-by: Xi Qian <xi.qian@intel.com>
Signed-off-by: Shuo Gao <shuo.gao@intel.com>
Signed-off-by: Bruce Beare <bruce.j.beare@intel.com>
Signed-off-by: Jack Ren <jack.ren@intel.com>
Author-tracking-BZ: 32630
  • Loading branch information...
commit 7ab7560f0eceb2629a22082c8544a40ce713b7e2 1 parent 2acb0f9
xqian6 authored Whitehawkx committed
View
4 Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
@@ -817,9 +817,9 @@ END
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");
View
2  Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
@@ -75,7 +75,7 @@ static v8::Handle<v8::Value> readOnlyTestObjAttrAttrGetter(v8::Local<v8::String>
if (wrapper.IsEmpty()) {
wrapper = toV8(result.get());
if (!wrapper.IsEmpty())
- V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);
+ V8DOMWrapper::setNamedHiddenReference(info.Holder(), "readOnlyTestObjAttr", wrapper);
}
return wrapper;
}
View
20 Source/WebCore/bindings/v8/V8DOMWrapper.cpp
@@ -31,6 +31,7 @@
#include "config.h"
#include "V8DOMWrapper.h"
+#include "ArrayBufferView.h"
#include "CSSMutableStyleDeclaration.h"
#include "DOMDataStore.h"
#include "DocumentLoader.h"
@@ -40,10 +41,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"
@@ -71,7 +73,6 @@
#include "V8WorkerContext.h"
#include "V8WorkerContextEventListener.h"
#include "V8XMLHttpRequest.h"
-#include "ArrayBufferView.h"
#include "WebGLContextAttributes.h"
#include "WebGLUniformLocation.h"
#include "WorkerContextExecutionProxy.h"
@@ -185,18 +186,13 @@ v8::Local<v8::Function> V8DOMWrapper::getConstructor(WrapperTypeInfo* type, Work
}
#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)
@@ -210,7 +206,7 @@ void V8DOMWrapper::setHiddenWindowReference(Frame* frame, v8::Handle<v8::Value>
global = V8DOMWrapper::lookupDOMWrapper(V8DOMWindow::GetTemplate(), global);
ASSERT(!global.IsEmpty());
- setHiddenReference(global, jsObject);
+ setNamedHiddenReference(global, name, jsObject);
}
WrapperTypeInfo* V8DOMWrapper::domWrapperType(v8::Handle<v8::Object> object)
View
12 Source/WebCore/bindings/v8/V8DOMWrapper.h
@@ -114,10 +114,14 @@ namespace WebCore {
// 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);
-
- // Set hidden references in a DOMWindow object of a frame.
- static void setHiddenWindowReference(Frame*, v8::Handle<v8::Value>);
+ // 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>);
static v8::Local<v8::Object> instantiateV8Object(V8Proxy* proxy, WrapperTypeInfo*, void* impl);
View
7 Source/WebCore/bindings/v8/V8GCController.cpp
@@ -377,13 +377,6 @@ class GrouperVisitor : public DOMWrapperMap<Node>::Visitor, public DOMWrapperMap
v8::V8::AddImplicitReferences(wrapper, values.data(), values.size());
}
- } else if (typeInfo->isSubclass(&V8CSSRuleList::info)) {
- CSSRuleList* cssRuleList = static_cast<CSSRuleList*>(object);
- GroupId groupId(cssRuleList);
- StyleList* styleList = cssRuleList->styleList();
- if (styleList)
- groupId = calculateGroupId(styleList);
- m_grouper.append(GrouperItem(groupId, wrapper));
}
}
View
15 Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp
@@ -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 @@ namespace WebCore {
#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;
View
2  Source/WebCore/bindings/v8/V8HiddenPropertyName.h
@@ -52,6 +52,8 @@ namespace WebCore {
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);
};
View
3  Source/WebCore/bindings/v8/WrapperTypeInfo.h
@@ -39,8 +39,7 @@ namespace WebCore {
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;
View
2  Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp
@@ -44,7 +44,7 @@ v8::Handle<v8::Value> toV8(CSSStyleSheet* impl)
// 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;
}
View
2  Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp
@@ -105,7 +105,7 @@ v8::Handle<v8::Value> toV8(DOMStringMap* impl)
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;
}
View
2  Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp
@@ -48,7 +48,7 @@ v8::Handle<v8::Value> toV8(DOMTokenList* impl)
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;
}
View
2  Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp
@@ -281,7 +281,7 @@ v8::Handle<v8::Value> toV8(Location* impl)
if (wrapper.IsEmpty()) {
wrapper = V8Location::wrap(impl);
if (!wrapper.IsEmpty())
- V8DOMWrapper::setHiddenWindowReference(impl->frame(), wrapper);
+ V8DOMWrapper::setNamedHiddenWindowReference(impl->frame(), "location", wrapper);
}
return wrapper;
}
View
4 Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp
@@ -67,8 +67,8 @@ v8::Handle<v8::Value> V8MessageChannel::constructorCallback(const v8::Arguments&
// 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());
View
2  Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp
@@ -83,7 +83,7 @@ v8::Handle<v8::Value> toV8(NamedNodeMap* impl)
// 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;
}
View
2  Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp
@@ -47,7 +47,7 @@ v8::Handle<v8::Value> toV8(StyleSheet* impl)
// 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;
}
View
7 Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
@@ -161,22 +161,27 @@ static v8::Handle<v8::Value> toV8Object(WebGLExtension* extension, v8::Handle<v8
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;
}
Please sign in to comment.
Something went wrong with that request. Please try again.