Changeset 112318 in webkit


Ignore:
Timestamp:
Mar 27, 2012 2:38:33 PM (12 years ago)
Author:
adamk@chromium.org
Message:

Always set V8 wrappers via V8DOMWrapper::setJSWrapperFor* instead of WeakReferenceMap::set()
​https://bugs.webkit.org/show_bug.cgi?id=82256

Reviewed by Adam Barth.

This moves leakRef() calls out of generated code, centralizing them in
V8DOMWrapper implementation. Ideally, WeakReferenceMap::set would take
PassRefPtrs, but that's tricky given that some WeakReferenceMap's KeyType is 'void'
(which clearly can't be wrapped in a PassRefPtr).

Updated binding tests to reflect changes in CodeGeneratorV8.pm, no change in behavior.

Relanding r112207 with setJSWrapperForDOMSVGElementInstance defined
out-of-line to avoid SVG header dependencies.

  • bindings/scripts/CodeGeneratorV8.pm:

(GenerateConstructorCallback): Use GetDomMapFunction instead of custom logic.
(GenerateNamedConstructorCallback): ditto.
(GenerateToV8Converters): Call V8DOMWrapper::setJSWrapper* method
instead of directly accessing the wrapper maps and calling set.
(GetDomMapFunction): Refactored to call new GetDomWrapperMapName function.
(GetDomWrapperMapName): Helper pulled out of GetDomMapFunction.

  • bindings/scripts/test/V8/V8Float64Array.cpp:

(WebCore::V8Float64Array::wrapSlow):

  • bindings/scripts/test/V8/V8TestActiveDOMObject.cpp:

(WebCore::V8TestActiveDOMObject::wrapSlow):

  • bindings/scripts/test/V8/V8TestCustomNamedGetter.cpp:

(WebCore::V8TestCustomNamedGetter::wrapSlow):

  • bindings/scripts/test/V8/V8TestEventConstructor.cpp:

(WebCore::V8TestEventConstructor::wrapSlow):

  • bindings/scripts/test/V8/V8TestEventTarget.cpp:

(WebCore::V8TestEventTarget::wrapSlow):

  • bindings/scripts/test/V8/V8TestInterface.cpp:

(WebCore::V8TestInterface::wrapSlow):

  • bindings/scripts/test/V8/V8TestMediaQueryListListener.cpp:

(WebCore::V8TestMediaQueryListListener::wrapSlow):

  • bindings/scripts/test/V8/V8TestNamedConstructor.cpp:

(WebCore::V8TestNamedConstructor::wrapSlow):

  • bindings/scripts/test/V8/V8TestObj.cpp:

(WebCore::V8TestObj::wrapSlow):

  • bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp:

(WebCore::V8TestSerializedScriptValueInterface::wrapSlow):

  • bindings/v8/V8DOMWrapper.cpp: Moved setJSWrapperForDOMNode method to header to inline it.

(WebCore::V8DOMWrapper::setJSWrapperForDOMSVGElementInstance): New helper method for SVGElementInstances.
Not inline to avoid header dependency on SVGElementInstance.h.

  • bindings/v8/V8DOMWrapper.h:

(V8DOMWrapper):
(WebCore::V8DOMWrapper::setJSWrapperForDOMObject): Made inline.
(WebCore::V8DOMWrapper::setJSWrapperForActiveDOMObject): ditto.
(WebCore::V8DOMWrapper::setJSWrapperForDOMNode): Refactored into two methods;
this one handles non-active Nodes.
(WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode): Pulled out of previouse
DOMNode method, now handles only active Nodes.

Location:
trunk/Source/WebCore
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r112316 r112318  
     12012-03-26  Adam Klein  <adamk@chromium.org>
     2
     3        Always set V8 wrappers via V8DOMWrapper::setJSWrapperFor* instead of WeakReferenceMap::set()
     4        https://bugs.webkit.org/show_bug.cgi?id=82256
     5
     6        Reviewed by Adam Barth.
     7
     8        This moves leakRef() calls out of generated code, centralizing them in
     9        V8DOMWrapper implementation. Ideally, WeakReferenceMap::set would take
     10        PassRefPtrs, but that's tricky given that some WeakReferenceMap's KeyType is 'void'
     11        (which clearly can't be wrapped in a PassRefPtr).
     12
     13        Updated binding tests to reflect changes in CodeGeneratorV8.pm, no change in behavior.
     14
     15        Relanding r112207 with setJSWrapperForDOMSVGElementInstance defined
     16        out-of-line to avoid SVG header dependencies.
     17
     18        * bindings/scripts/CodeGeneratorV8.pm:
     19        (GenerateConstructorCallback): Use GetDomMapFunction instead of custom logic.
     20        (GenerateNamedConstructorCallback): ditto.
     21        (GenerateToV8Converters): Call V8DOMWrapper::setJSWrapper* method
     22        instead of directly accessing the wrapper maps and calling set.
     23        (GetDomMapFunction): Refactored to call new GetDomWrapperMapName function.
     24        (GetDomWrapperMapName): Helper pulled out of GetDomMapFunction.
     25        * bindings/scripts/test/V8/V8Float64Array.cpp:
     26        (WebCore::V8Float64Array::wrapSlow):
     27        * bindings/scripts/test/V8/V8TestActiveDOMObject.cpp:
     28        (WebCore::V8TestActiveDOMObject::wrapSlow):
     29        * bindings/scripts/test/V8/V8TestCustomNamedGetter.cpp:
     30        (WebCore::V8TestCustomNamedGetter::wrapSlow):
     31        * bindings/scripts/test/V8/V8TestEventConstructor.cpp:
     32        (WebCore::V8TestEventConstructor::wrapSlow):
     33        * bindings/scripts/test/V8/V8TestEventTarget.cpp:
     34        (WebCore::V8TestEventTarget::wrapSlow):
     35        * bindings/scripts/test/V8/V8TestInterface.cpp:
     36        (WebCore::V8TestInterface::wrapSlow):
     37        * bindings/scripts/test/V8/V8TestMediaQueryListListener.cpp:
     38        (WebCore::V8TestMediaQueryListListener::wrapSlow):
     39        * bindings/scripts/test/V8/V8TestNamedConstructor.cpp:
     40        (WebCore::V8TestNamedConstructor::wrapSlow):
     41        * bindings/scripts/test/V8/V8TestObj.cpp:
     42        (WebCore::V8TestObj::wrapSlow):
     43        * bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp:
     44        (WebCore::V8TestSerializedScriptValueInterface::wrapSlow):
     45        * bindings/v8/V8DOMWrapper.cpp: Moved setJSWrapperForDOMNode method to header to inline it.
     46        (WebCore::V8DOMWrapper::setJSWrapperForDOMSVGElementInstance): New helper method for SVGElementInstances.
     47        Not inline to avoid header dependency on SVGElementInstance.h.
     48        * bindings/v8/V8DOMWrapper.h:
     49        (V8DOMWrapper):
     50        (WebCore::V8DOMWrapper::setJSWrapperForDOMObject): Made inline.
     51        (WebCore::V8DOMWrapper::setJSWrapperForActiveDOMObject): ditto.
     52        (WebCore::V8DOMWrapper::setJSWrapperForDOMNode): Refactored into two methods;
     53        this one handles non-active Nodes.
     54        (WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode): Pulled out of previouse
     55        DOMNode method, now handles only active Nodes.
     56
    1572012-03-27  Levi Weintraub  <leviw@chromium.org>
    258

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

    r112218 r112318  
    17531753    }
    17541754
    1755     my $DOMObject = "DOMObject";
    1756     if (IsNodeSubType($dataNode)) {
    1757         $DOMObject = "DOMNode";
    1758     } elsif ($dataNode->extendedAttributes->{"ActiveDOMObject"}) {
    1759         $DOMObject = "ActiveDOMObject";
    1760     }
    1761 
     1755    my $DOMObject = GetDomWrapperMapName($dataNode, $implClassName);
    17621756    push(@implContent, <<END);
    17631757
    … …  
    19331927    }
    19341928
    1935     my $DOMObject = "DOMObject";
    1936     # A DOMObject that is an ActiveDOMObject and also a DOMNode should be treated as an DOMNode here.
    1937     # setJSWrapperForDOMNode() will look if node is active and choose correct map to add node to.
    1938     if (IsNodeSubType($dataNode)) {
    1939         $DOMObject = "DOMNode";
    1940     } elsif ($dataNode->extendedAttributes->{"ActiveDOMObject"}) {
    1941         $DOMObject = "ActiveDOMObject";
    1942     }
     1929    my $DOMObject = GetDomWrapperMapName($dataNode, $implClassName);
    19431930    push(@implContent, <<END);
    19441931
    … …  
    30943081    my $nativeType = shift;
    30953082
    3096     my $domMapFunction = GetDomMapFunction($dataNode, $interfaceName);
     3083    my $domMapName = GetDomWrapperMapName($dataNode, $interfaceName);
    30973084    my $forceNewObjectInput = IsDOMNodeType($interfaceName) ? ", bool forceNewObject" : "";
    30983085    my $forceNewObjectCall = IsDOMNodeType($interfaceName) ? ", forceNewObject" : "";
    … …  
    31833170    }
    31843171    push(@implContent, <<END);
    3185     ${domMapFunction}.set(impl.leakRef(), wrapperHandle);
     3172    V8DOMWrapper::setJSWrapperFor${domMapName}(impl, wrapperHandle);
    31863173    return wrapper;
    31873174}
    … …  
    31903177
    31913178sub GetDomMapFunction
     3179{
     3180    my $mapName = GetDomWrapperMapName(@_);
     3181    return "get${mapName}Map()";
     3182}
     3183
     3184sub GetDomWrapperMapName
    31923185{
    31933186    my $dataNode = shift;
    31943187    my $type = shift;
    3195     return "getDOMSVGElementInstanceMap()" if $type eq "SVGElementInstance";
    3196     return "getActiveDOMNodeMap()" if (IsNodeSubType($dataNode) && $dataNode->extendedAttributes->{"ActiveDOMObject"});
    3197     return "getDOMNodeMap()" if (IsNodeSubType($dataNode));
    3198     return "getActiveDOMObjectMap()" if $dataNode->extendedAttributes->{"ActiveDOMObject"};
    3199     return "getDOMObjectMap()";
     3188    return "DOMSVGElementInstance" if $type eq "SVGElementInstance";
     3189    return "ActiveDOMNode" if (IsNodeSubType($dataNode) && $dataNode->extendedAttributes->{"ActiveDOMObject"});
     3190    return "DOMNode" if (IsNodeSubType($dataNode));
     3191    return "ActiveDOMObject" if $dataNode->extendedAttributes->{"ActiveDOMObject"};
     3192    return "DOMObject";
    32003193}
    32013194

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8Float64Array.cpp

    r112207 r112318  
    133133    if (!hasDependentLifetime)
    134134        wrapperHandle.MarkIndependent();
    135     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     135    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    136136    return wrapper;
    137137}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.cpp

    r112218 r112318  
    189189    if (!hasDependentLifetime)
    190190        wrapperHandle.MarkIndependent();
    191     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     191    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    192192    return wrapper;
    193193}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCustomNamedGetter.cpp

    r112207 r112318  
    123123    if (!hasDependentLifetime)
    124124        wrapperHandle.MarkIndependent();
    125     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     125    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    126126    return wrapper;
    127127}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.cpp

    r112207 r112318  
    158158    if (!hasDependentLifetime)
    159159        wrapperHandle.MarkIndependent();
    160     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     160    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    161161    return wrapper;
    162162}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.cpp

    r112207 r112318  
    186186    if (!hasDependentLifetime)
    187187        wrapperHandle.MarkIndependent();
    188     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     188    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    189189    return wrapper;
    190190}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp

    r112207 r112318  
    318318    if (!hasDependentLifetime)
    319319        wrapperHandle.MarkIndependent();
    320     getActiveDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     320    V8DOMWrapper::setJSWrapperForActiveDOMObject(impl, wrapperHandle);
    321321    return wrapper;
    322322}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestMediaQueryListListener.cpp

    r112207 r112318  
    123123    if (!hasDependentLifetime)
    124124        wrapperHandle.MarkIndependent();
    125     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     125    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    126126    return wrapper;
    127127}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestNamedConstructor.cpp

    r112207 r112318  
    167167    if (!hasDependentLifetime)
    168168        wrapperHandle.MarkIndependent();
    169     getActiveDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     169    V8DOMWrapper::setJSWrapperForActiveDOMObject(impl, wrapperHandle);
    170170    return wrapper;
    171171}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp

    r112207 r112318  
    21732173    if (!hasDependentLifetime)
    21742174        wrapperHandle.MarkIndependent();
    2175     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     2175    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    21762176    return wrapper;
    21772177}

  • trunk/Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.cpp

    r112207 r112318  
    289289    if (!hasDependentLifetime)
    290290        wrapperHandle.MarkIndependent();
    291     getDOMObjectMap().set(impl.leakRef(), wrapperHandle);
     291    V8DOMWrapper::setJSWrapperForDOMObject(impl, wrapperHandle);
    292292    return wrapper;
    293293}

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp

    r112207 r112318  
    3737#include "EventTargetInterfaces.h"
    3838#include "FrameLoaderClient.h"
     39#include "SVGElementInstance.h"
    3940#include "StylePropertySet.h"
    4041#include "V8AbstractEventListener.h"
    … …  
    6768namespace WebCore {
    6869
    69 void V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
     70#if ENABLE(SVG)
     71void V8DOMWrapper::setJSWrapperForDOMSVGElementInstance(PassRefPtr<SVGElementInstance> element, v8::Persistent<v8::Object> wrapper)
    7072{
    7173    ASSERT(maybeDOMWrapper(wrapper));
    72     if (node->isActiveNode())
    73         getActiveDOMNodeMap().set(node.leakRef(), wrapper);
    74     else
    75         getDOMNodeMap().set(node.leakRef(), wrapper);
    76 }
     74    getDOMSVGElementInstanceMap().set(element.leakRef(), wrapper);
     75}
     76#endif
    7777
    7878v8::Local<v8::Function> V8DOMWrapper::getConstructor(WrapperTypeInfo* type, v8::Handle<v8::Value> objectPrototype)

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h

    r112207 r112318  
    111111        template<typename T> static void setJSWrapperForActiveDOMObject(PassRefPtr<T>, v8::Persistent<v8::Object>);
    112112        static void setJSWrapperForDOMNode(PassRefPtr<Node>, v8::Persistent<v8::Object>);
     113        static void setJSWrapperForActiveDOMNode(PassRefPtr<Node>, v8::Persistent<v8::Object>);
     114#if ENABLE(SVG)
     115        static void setJSWrapperForDOMSVGElementInstance(PassRefPtr<SVGElementInstance>, v8::Persistent<v8::Object>);
     116#endif
    113117
    114118        static bool isValidDOMObject(v8::Handle<v8::Value>);
    … …  
    151155
    152156    template<typename T>
    153     void V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
     157    inline void V8DOMWrapper::setJSWrapperForDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
    154158    {
    155159        ASSERT(maybeDOMWrapper(wrapper));
    … …  
    159163
    160164    template<typename T>
    161     void V8DOMWrapper::setJSWrapperForActiveDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
     165    inline void V8DOMWrapper::setJSWrapperForActiveDOMObject(PassRefPtr<T> object, v8::Persistent<v8::Object> wrapper)
    162166    {
    163167        ASSERT(maybeDOMWrapper(wrapper));
    … …  
    165169        getActiveDOMObjectMap().set(object.leakRef(), wrapper);
    166170    }
     171
     172    inline void V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
     173    {
     174        ASSERT(maybeDOMWrapper(wrapper));
     175        ASSERT(!domWrapperType(wrapper)->toActiveDOMObjectFunction);
     176        ASSERT(!node->isActiveNode());
     177        getDOMNodeMap().set(node.leakRef(), wrapper);
     178    }
     179
     180    inline void V8DOMWrapper::setJSWrapperForActiveDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
     181    {
     182        ASSERT(maybeDOMWrapper(wrapper));
     183        ASSERT(domWrapperType(wrapper)->toActiveDOMObjectFunction);
     184        ASSERT(node->isActiveNode());
     185        getActiveDOMNodeMap().set(node.leakRef(), wrapper);
     186    }
     187
    167188} // namespace WebCore
    168189
Note: See TracChangeset for help on using the changeset viewer.