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

16 files changed, 50 insertions, 38 deletions

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
index 22051bf..c9ed679 100644
--- a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
+++ b/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");
diff --git a/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp b/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
index 5884e89..8030559 100644
--- a/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
+++ b/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;
 }
diff --git a/Source/WebCore/bindings/v8/V8DOMWrapper.cpp b/Source/WebCore/bindings/v8/V8DOMWrapper.cpp
index 249ca7c..7d0fe60 100644
--- a/Source/WebCore/bindings/v8/V8DOMWrapper.cpp
+++ b/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)
diff --git a/Source/WebCore/bindings/v8/V8DOMWrapper.h b/Source/WebCore/bindings/v8/V8DOMWrapper.h
index 7f4490a..0cae6db 100644
--- a/Source/WebCore/bindings/v8/V8DOMWrapper.h
+++ b/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);
 
diff --git a/Source/WebCore/bindings/v8/V8GCController.cpp b/Source/WebCore/bindings/v8/V8GCController.cpp
index c9e9dfd..ea017e8 100644
--- a/Source/WebCore/bindings/v8/V8GCController.cpp
+++ b/Source/WebCore/bindings/v8/V8GCController.cpp
@@ -377,13 +377,6 @@ public:
                     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));
         }
     }
 
diff --git a/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp b/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp
index d83573f..2909269 100644
--- a/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp
+++ b/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;
diff --git a/Source/WebCore/bindings/v8/V8HiddenPropertyName.h b/Source/WebCore/bindings/v8/V8HiddenPropertyName.h
index 7867b36..7c4aae0 100644
--- a/Source/WebCore/bindings/v8/V8HiddenPropertyName.h
+++ b/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);
     };
diff --git a/Source/WebCore/bindings/v8/WrapperTypeInfo.h b/Source/WebCore/bindings/v8/WrapperTypeInfo.h
index 166d642..c2684d6 100644
--- a/Source/WebCore/bindings/v8/WrapperTypeInfo.h
+++ b/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;
 
diff --git a/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp b/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp
index 9effca3..9b2988b 100644
--- a/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp
+++ b/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;
 }
 
diff --git a/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp b/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp
index 71ff357..1d6398e 100644
--- a/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp
+++ b/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;
 }
diff --git a/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp b/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp
index 171ff5c..08051ab 100644
--- a/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp
+++ b/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;
 }
diff --git a/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp b/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp
index e166323..e143a96 100644
--- a/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp
+++ b/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;
 }
diff --git a/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp b/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp
index b966e42..775e745 100644
--- a/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp
+++ b/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());
diff --git a/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp b/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp
index d9e1de0..c9014c0 100644
--- a/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp
+++ b/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;
 }
 
diff --git a/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp b/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp
index b3f6ff7..0fe6af8 100644
--- a/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp
+++ b/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;
 }
 
diff --git a/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp b/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
index af81a41..fa915b6 100644
--- a/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
+++ b/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;
 }