Changeset 90949 in webkit


Ignore:
Timestamp:
Jul 13, 2011 2:44:29 PM (13 years ago)
Author:
vitalyr@chromium.org
Message:

2011-07-13 Vitaly Repeshko <vitalyr@chromium.org>

[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.

Location:
trunk/Source/WebCore
Files:
16 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r90948 r90949  
     12011-07-13  Vitaly Repeshko  <vitalyr@chromium.org>
     2
     3        [V8] Avoid memory leaks with hidden references.
     4        https://bugs.webkit.org/show_bug.cgi?id=64467
     5
     6        Reviewed by Adam Barth.
     7
     8        We used to have growing arrays of hidden references associated
     9        with objects. The entries in this array had no keys and were never
     10        removed. This patch changes the interface to require a reference
     11        name. This way it's harder to leak an unbounded number of
     12        objects. Also it makes our wrapper objects one machine word
     13        smaller.
     14
     15        * bindings/scripts/CodeGeneratorV8.pm:
     16        (GenerateNormalAttrGetter): Started using new interface.
     17
     18        Interface changes:
     19        * bindings/v8/V8DOMWrapper.cpp:
     20        (WebCore::V8DOMWrapper::setNamedHiddenReference):
     21        (WebCore::V8DOMWrapper::setNamedHiddenWindowReference):
     22        * bindings/v8/V8DOMWrapper.h:
     23
     24        Added a helper to compute hidden reference names as V8 strings:
     25        * bindings/v8/V8HiddenPropertyName.cpp:
     26        (WebCore::V8HiddenPropertyName::hiddenReferenceName):
     27        * bindings/v8/V8HiddenPropertyName.h:
     28
     29        * bindings/v8/WrapperTypeInfo.h: Removed the hidden reference
     30        array internal field.
     31
     32        Update usages of hidden references:
     33        * bindings/v8/custom/V8CSSStyleSheetCustom.cpp:
     34        (WebCore::toV8):
     35        * bindings/v8/custom/V8DOMStringMapCustom.cpp:
     36        (WebCore::toV8):
     37        * bindings/v8/custom/V8DOMTokenListCustom.cpp:
     38        (WebCore::toV8):
     39        * bindings/v8/custom/V8LocationCustom.cpp:
     40        (WebCore::toV8):
     41        * bindings/v8/custom/V8MessageChannelConstructor.cpp:
     42        (WebCore::V8MessageChannel::constructorCallback):
     43        * bindings/v8/custom/V8NamedNodeMapCustom.cpp:
     44        (WebCore::toV8):
     45        * bindings/v8/custom/V8StyleSheetCustom.cpp:
     46        (WebCore::toV8):
     47        * bindings/v8/custom/V8WebGLRenderingContextCustom.cpp:
     48        (WebCore::toV8Object):
     49
     50        * bindings/scripts/test/V8/V8TestObj.cpp:
     51        (WebCore::TestObjInternal::readOnlyTestObjAttrAttrGetter): Updated.
     52
    1532011-07-13  Joseph Pecoraro  <joepeck@webkit.org>
    254
  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

    r90680 r90949  
    857857        push(@implContentDecls, "        if (!wrapper.IsEmpty())\n");
    858858        if ($dataNode->name eq "DOMWindow") {
    859             push(@implContentDecls, "            V8DOMWrapper::setHiddenWindowReference(imp->frame(), wrapper);\n");
     859            push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenWindowReference(imp->frame(), \"${attrName}\", wrapper);\n");
    860860        } else {
    861             push(@implContentDecls, "            V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);\n");
     861            push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
    862862        }
    863863        push(@implContentDecls, "    }\n");
  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp

    r89358 r90949  
    7777        wrapper = toV8(result.get());
    7878        if (!wrapper.IsEmpty())
    79             V8DOMWrapper::setHiddenReference(info.Holder(), wrapper);
     79            V8DOMWrapper::setNamedHiddenReference(info.Holder(), "readOnlyTestObjAttr", wrapper);
    8080    }
    8181    return wrapper;
  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp

    r89567 r90949  
    3232#include "V8DOMWrapper.h"
    3333
     34#include "ArrayBufferView.h"
    3435#include "CSSMutableStyleDeclaration.h"
    3536#include "DOMDataStore.h"
     
    4041#include "V8Binding.h"
    4142#include "V8Collection.h"
    42 #include "V8DedicatedWorkerContext.h"
    4343#include "V8DOMApplicationCache.h"
    4444#include "V8DOMMap.h"
    4545#include "V8DOMWindow.h"
     46#include "V8DedicatedWorkerContext.h"
    4647#include "V8EventListener.h"
    4748#include "V8EventListenerList.h"
     
    5253#include "V8HTMLCollection.h"
    5354#include "V8HTMLDocument.h"
     55#include "V8HiddenPropertyName.h"
    5456#include "V8IDBDatabase.h"
    5557#include "V8IDBRequest.h"
     
    7678#include "V8WorkerContextEventListener.h"
    7779#include "V8XMLHttpRequest.h"
    78 #include "ArrayBufferView.h"
    7980#include "WebGLContextAttributes.h"
    8081#include "WebGLUniformLocation.h"
     
    190191#endif
    191192
    192 void V8DOMWrapper::setHiddenReference(v8::Handle<v8::Object> parent, v8::Handle<v8::Value> child)
    193 {
    194     v8::Local<v8::Value> hiddenReferenceObject = parent->GetInternalField(v8DOMHiddenReferenceArrayIndex);
    195     if (hiddenReferenceObject->IsNull() || hiddenReferenceObject->IsUndefined()) {
    196         hiddenReferenceObject = v8::Array::New();
    197         parent->SetInternalField(v8DOMHiddenReferenceArrayIndex, hiddenReferenceObject);
    198     }
    199     v8::Local<v8::Array> hiddenReferenceArray = v8::Local<v8::Array>::Cast(hiddenReferenceObject);
    200     hiddenReferenceArray->Set(v8::Integer::New(hiddenReferenceArray->Length()), child);
    201 }
    202 
    203 void V8DOMWrapper::setHiddenWindowReference(Frame* frame, v8::Handle<v8::Value> jsObject)
     193
     194void V8DOMWrapper::setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child)
     195{
     196    parent->SetHiddenValue(V8HiddenPropertyName::hiddenReferenceName(name), child);
     197}
     198
     199void V8DOMWrapper::setNamedHiddenWindowReference(Frame* frame, const char* name, v8::Handle<v8::Value> jsObject)
    204200{
    205201    // Get DOMWindow
     
    215211    ASSERT(!global.IsEmpty());
    216212
    217     setHiddenReference(global, jsObject);
     213    setNamedHiddenReference(global, name, jsObject);
    218214}
    219215
  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h

    r74014 r90949  
    115115        static bool isWrapperOfType(v8::Handle<v8::Value>, WrapperTypeInfo*);
    116116
    117         static void setHiddenReference(v8::Handle<v8::Object> parent, v8::Handle<v8::Value> child);
    118 
    119         // Set hidden references in a DOMWindow object of a frame.
    120         static void setHiddenWindowReference(Frame*, v8::Handle<v8::Value>);
     117        // Proper object lifetime support.
     118        //
     119        // Helper functions to make sure the child object stays alive
     120        // while the parent is alive. Using the name more than once
     121        // overwrites previous references making it possible to free
     122        // old children.
     123        static void setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child);
     124        static void setNamedHiddenWindowReference(Frame*, const char*, v8::Handle<v8::Value>);
    121125
    122126        static v8::Local<v8::Object> instantiateV8Object(V8Proxy* proxy, WrapperTypeInfo*, void* impl);
  • trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.cpp

    r49074 r90949  
    3232#include "V8HiddenPropertyName.h"
    3333
     34#include <string.h>
     35#include <wtf/Vector.h>
     36
    3437namespace WebCore {
    3538
     
    4043v8::Handle<v8::String> V8HiddenPropertyName::name() \
    4144{ \
    42     static v8::Persistent<v8::String>* string = createString("WebCore::V8HiddenPropertyName::" V8_AS_STRING(name)); \
     45    static v8::Persistent<v8::String>* string = createString("WebCore::HiddenProperty::" V8_AS_STRING(name)); \
    4346    return *string; \
    4447}
    4548
    4649V8_HIDDEN_PROPERTIES(V8_DEFINE_PROPERTY);
     50
     51static const char hiddenReferenceNamePrefix[] = "WebCore::HiddenReference::";
     52
     53v8::Handle<v8::String> V8HiddenPropertyName::hiddenReferenceName(const char* name)
     54{
     55    Vector<char, 64> prefixedName;
     56    prefixedName.append(hiddenReferenceNamePrefix, sizeof(hiddenReferenceNamePrefix) - 1);
     57    prefixedName.append(name, strlen(name));
     58    return v8::String::NewSymbol(prefixedName.data(), static_cast<int>(prefixedName.size()));
     59}
    4760
    4861v8::Persistent<v8::String>* V8HiddenPropertyName::createString(const char* key)
  • trunk/Source/WebCore/bindings/v8/V8HiddenPropertyName.h

    r65730 r90949  
    5353#undef V8_DECLARE_PROPERTY
    5454
     55        static v8::Handle<v8::String> hiddenReferenceName(const char* name);
     56
    5557    private:
    5658        static v8::Persistent<v8::String>* createString(const char* key);
  • trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h

    r81265 r90949  
    4040    static const int v8DOMWrapperTypeIndex = 0;
    4141    static const int v8DOMWrapperObjectIndex = 1;
    42     static const int v8DOMHiddenReferenceArrayIndex = 2;
    43     static const int v8DefaultWrapperInternalFieldCount = 3;
     42    static const int v8DefaultWrapperInternalFieldCount = 2;
    4443
    4544    static const uint16_t v8DOMSubtreeClassId = 1;
  • trunk/Source/WebCore/bindings/v8/custom/V8CSSStyleSheetCustom.cpp

    r57004 r90949  
    4545    Node* ownerNode = impl->ownerNode();
    4646    if (ownerNode && !wrapper.IsEmpty())
    47         V8DOMWrapper::setHiddenReference(wrapper, toV8(ownerNode));
     47        V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(ownerNode));
    4848    return wrapper;
    4949}
  • trunk/Source/WebCore/bindings/v8/custom/V8DOMStringMapCustom.cpp

    r68064 r90949  
    106106        v8::Handle<v8::Value> elementValue = toV8(element);
    107107        if (!elementValue.IsEmpty() && elementValue->IsObject())
    108             V8DOMWrapper::setHiddenReference(elementValue.As<v8::Object>(), wrapper);
     108            V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domStringMap", wrapper);
    109109    }
    110110    return wrapper;
  • trunk/Source/WebCore/bindings/v8/custom/V8DOMTokenListCustom.cpp

    r68440 r90949  
    4949        v8::Handle<v8::Value> elementValue = toV8(element);
    5050        if (!elementValue.IsEmpty() && elementValue->IsObject())
    51             V8DOMWrapper::setHiddenReference(elementValue.As<v8::Object>(), wrapper);
     51            V8DOMWrapper::setNamedHiddenReference(elementValue.As<v8::Object>(), "domTokenList", wrapper);
    5252    }
    5353    return wrapper;
  • trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp

    r84371 r90949  
    281281        wrapper = V8Location::wrap(impl);
    282282        if (!wrapper.IsEmpty())
    283             V8DOMWrapper::setHiddenWindowReference(impl->frame(), wrapper);
     283            V8DOMWrapper::setNamedHiddenWindowReference(impl->frame(), "location", wrapper);
    284284    }
    285285    return wrapper;
  • trunk/Source/WebCore/bindings/v8/custom/V8MessageChannelConstructor.cpp

    r57004 r90949  
    6868    // MessagePort wrappers to make sure that the MessagePort wrappers
    6969    // stay alive as long as the MessageChannel wrapper is around.
    70     V8DOMWrapper::setHiddenReference(messageChannel, toV8(obj->port1()));
    71     V8DOMWrapper::setHiddenReference(messageChannel, toV8(obj->port2()));
     70    V8DOMWrapper::setNamedHiddenReference(messageChannel, "port1", toV8(obj->port1()));
     71    V8DOMWrapper::setNamedHiddenReference(messageChannel, "port2", toV8(obj->port2()));
    7272
    7373    // Setup the standard wrapper object internal fields.
  • trunk/Source/WebCore/bindings/v8/custom/V8NamedNodeMapCustom.cpp

    r60022 r90949  
    8484    Element* element = impl->element();
    8585    if (!wrapper.IsEmpty() && element)
    86         V8DOMWrapper::setHiddenReference(wrapper, toV8(element));
     86        V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(element));
    8787    return wrapper;
    8888}
  • trunk/Source/WebCore/bindings/v8/custom/V8StyleSheetCustom.cpp

    r57004 r90949  
    4848    Node* ownerNode = impl->ownerNode();
    4949    if (ownerNode && !wrapper.IsEmpty())
    50         V8DOMWrapper::setHiddenReference(wrapper, toV8(ownerNode));
     50        V8DOMWrapper::setNamedHiddenReference(wrapper, "ownerNode", toV8(ownerNode));
    5151    return wrapper;
    5252}
  • trunk/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp

    r87837 r90949  
    162162        return v8::Null();
    163163    v8::Handle<v8::Value> extensionObject;
     164    const char* referenceName;
    164165    switch (extension->getName()) {
    165166    case WebGLExtension::WebKitLoseContextName:
    166167        extensionObject = toV8(static_cast<WebKitLoseContext*>(extension));
     168        referenceName = "webKitLoseContextName";
    167169        break;
    168170    case WebGLExtension::OESStandardDerivativesName:
    169171        extensionObject = toV8(static_cast<OESStandardDerivatives*>(extension));
     172        referenceName = "oesStandardDerivativesName";
    170173        break;
    171174    case WebGLExtension::OESTextureFloatName:
    172175        extensionObject = toV8(static_cast<OESTextureFloat*>(extension));
     176        referenceName = "oesTextureFloatName";
    173177        break;
    174178    case WebGLExtension::OESVertexArrayObjectName:
    175179        extensionObject = toV8(static_cast<OESVertexArrayObject*>(extension));
     180        referenceName = "oesVertexArrayObjectName";
    176181        break;
    177182    }
    178183    ASSERT(!extensionObject.IsEmpty());
    179     V8DOMWrapper::setHiddenReference(contextObject, extensionObject);
     184    V8DOMWrapper::setNamedHiddenReference(contextObject, referenceName, extensionObject);
    180185    return extensionObject;
    181186}
Note: See TracChangeset for help on using the changeset viewer.