Add setJSWrapperForActiveDOMNode and use it for Nodes that are also ActiveDOMObjects

https://bugs.webkit.org/show_bug.cgi?id=83528

Reviewed by Kentaro Hara.

Instead of using a run-time call to isActiveNode to determine which
map to put a Node wrapper in, generate the proper call in the CodeGenerator.

This was originally part of r112318, which got rolled out due to OOM concerns.
I'm splitting it into smaller pieces so that each can be landed and
watched for issues seperately.

No new tests, no change in behavior.

* bindings/scripts/CodeGeneratorV8.pm:
(GenerateConstructorCallback): Use new GetDomMapName function to
figure out which setJSWrapper to call.
(GenerateNamedConstructorCallback): ditto.
(GetDomMapFunction): Delegate to GetDomMapName for logic.
(GetDomMapName): New helper factored out of GetDomMapFunction.
* bindings/v8/V8DOMWrapper.cpp:
(WebCore::V8DOMWrapper::setJSWrapperForDOMNode): Assert !isActiveNode instead of branching on it.
(WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode): New method split ouf of the above.
Assert isActiveNode instead of branching on it.
* bindings/v8/V8DOMWrapper.h:
(V8DOMWrapper):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@113754 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 41f594c..92c1b3f 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2012-04-10  Adam Klein  <adamk@chromium.org>
+
+        Add setJSWrapperForActiveDOMNode and use it for Nodes that are also ActiveDOMObjects
+        https://bugs.webkit.org/show_bug.cgi?id=83528
+
+        Reviewed by Kentaro Hara.
+
+        Instead of using a run-time call to isActiveNode to determine which
+        map to put a Node wrapper in, generate the proper call in the CodeGenerator.
+
+        This was originally part of r112318, which got rolled out due to OOM concerns.
+        I'm splitting it into smaller pieces so that each can be landed and
+        watched for issues seperately.
+
+        No new tests, no change in behavior.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateConstructorCallback): Use new GetDomMapName function to
+        figure out which setJSWrapper to call.
+        (GenerateNamedConstructorCallback): ditto.
+        (GetDomMapFunction): Delegate to GetDomMapName for logic.
+        (GetDomMapName): New helper factored out of GetDomMapFunction.
+        * bindings/v8/V8DOMWrapper.cpp:
+        (WebCore::V8DOMWrapper::setJSWrapperForDOMNode): Assert !isActiveNode instead of branching on it.
+        (WebCore::V8DOMWrapper::setJSWrapperForActiveDOMNode): New method split ouf of the above.
+        Assert isActiveNode instead of branching on it.
+        * bindings/v8/V8DOMWrapper.h:
+        (V8DOMWrapper):
+
 2012-04-10  Luke Macpherson  <macpherson@chromium.org>
 
         Pass PropertyHandler by reference in CSSStyleApplyProperty.h.

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
index 971cfa6..dc50502 100644 (file)
--- a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
+++ b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
@@ -1752,13 +1752,7 @@ END
         push(@implContent, "        goto fail;\n");
     }
 
-    my $DOMObject = "DOMObject";
-    if (IsNodeSubType($dataNode)) {
-        $DOMObject = "DOMNode";
-    } elsif ($dataNode->extendedAttributes->{"ActiveDOMObject"}) {
-        $DOMObject = "ActiveDOMObject";
-    }
-
+    my $DOMObject = GetDomMapName($dataNode, $implClassName);
     push(@implContent, <<END);
 
     V8DOMWrapper::setDOMWrapper(wrapper, &info, impl.get());
@@ -1932,14 +1926,7 @@ END
         push(@implContent, "        goto fail;\n");
     }
 
-    my $DOMObject = "DOMObject";
-    # A DOMObject that is an ActiveDOMObject and also a DOMNode should be treated as an DOMNode here.
-    # setJSWrapperForDOMNode() will look if node is active and choose correct map to add node to.
-    if (IsNodeSubType($dataNode)) {
-        $DOMObject = "DOMNode";
-    } elsif ($dataNode->extendedAttributes->{"ActiveDOMObject"}) {
-        $DOMObject = "ActiveDOMObject";
-    }
+    my $DOMObject = GetDomMapName($dataNode, $implClassName);
     push(@implContent, <<END);
 
     V8DOMWrapper::setDOMWrapper(wrapper, &V8${implClassName}Constructor::info, impl.get());
@@ -3190,13 +3177,19 @@ END
 
 sub GetDomMapFunction
 {
+    return "get" . GetDomMapName(@_) . "Map()";
+}
+
+sub GetDomMapName
+{
     my $dataNode = shift;
     my $type = shift;
-    return "getDOMSVGElementInstanceMap()" if $type eq "SVGElementInstance";
-    return "getActiveDOMNodeMap()" if (IsNodeSubType($dataNode) && $dataNode->extendedAttributes->{"ActiveDOMObject"});
-    return "getDOMNodeMap()" if (IsNodeSubType($dataNode));
-    return "getActiveDOMObjectMap()" if $dataNode->extendedAttributes->{"ActiveDOMObject"};
-    return "getDOMObjectMap()";
+
+    return "DOMSVGElementInstance" if $type eq "SVGElementInstance";
+    return "ActiveDOMNode" if (IsNodeSubType($dataNode) && $dataNode->extendedAttributes->{"ActiveDOMObject"});
+    return "DOMNode" if IsNodeSubType($dataNode);
+    return "ActiveDOMObject" if $dataNode->extendedAttributes->{"ActiveDOMObject"};
+    return "DOMObject";
 }
 
 sub GetNativeTypeForConversions

diff --git a/Source/WebCore/bindings/v8/V8DOMWrapper.cpp b/Source/WebCore/bindings/v8/V8DOMWrapper.cpp
index 84fd172..0fad5e8 100644 (file)
--- a/Source/WebCore/bindings/v8/V8DOMWrapper.cpp
+++ b/Source/WebCore/bindings/v8/V8DOMWrapper.cpp
@@ -72,10 +72,15 @@ namespace WebCore {
 void V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
 {
     ASSERT(maybeDOMWrapper(wrapper));
-    if (node->isActiveNode())
-        getActiveDOMNodeMap().set(node.leakRef(), wrapper);
-    else
-        getDOMNodeMap().set(node.leakRef(), wrapper);
+    ASSERT(!node->isActiveNode());
+    getDOMNodeMap().set(node.leakRef(), wrapper);
+}
+
+void V8DOMWrapper::setJSWrapperForActiveDOMNode(PassRefPtr<Node> node, v8::Persistent<v8::Object> wrapper)
+{
+    ASSERT(maybeDOMWrapper(wrapper));
+    ASSERT(node->isActiveNode());
+    getActiveDOMNodeMap().set(node.leakRef(), wrapper);
 }
 
 v8::Local<v8::Function> V8DOMWrapper::constructorForType(WrapperTypeInfo* type, DOMWindow* window)

diff --git a/Source/WebCore/bindings/v8/V8DOMWrapper.h b/Source/WebCore/bindings/v8/V8DOMWrapper.h
index d766307..16326b9 100644 (file)
--- a/Source/WebCore/bindings/v8/V8DOMWrapper.h
+++ b/Source/WebCore/bindings/v8/V8DOMWrapper.h
@@ -109,6 +109,7 @@ namespace WebCore {
         template<typename T> static void setJSWrapperForDOMObject(PassRefPtr<T>, v8::Persistent<v8::Object>);
         template<typename T> static void setJSWrapperForActiveDOMObject(PassRefPtr<T>, v8::Persistent<v8::Object>);
         static void setJSWrapperForDOMNode(PassRefPtr<Node>, v8::Persistent<v8::Object>);
+        static void setJSWrapperForActiveDOMNode(PassRefPtr<Node>, v8::Persistent<v8::Object>);
 
         static bool isValidDOMObject(v8::Handle<v8::Value>);