Title: [100517] trunk/Source/WebCore
Revision
100517
Author
wei...@apple.com
Date
2011-11-16 15:46:54 -0800 (Wed, 16 Nov 2011)

Log Message

JS wrappers of DOM objects should have no-op constructors
https://bugs.webkit.org/show_bug.cgi?id=72556

Reviewed by Geoffrey Garen.

Stop using a RefPtr to hold DOM objects contained by _javascript_
wrappers and instead use a raw pointer. We were already releasing
the underlying object before the destructor ran (via the finalizer)
so the default behavior of destroying the RefPtr is always unnecessary
busy work. 

* bindings/js/JSCSSValueCustom.cpp:
(WebCore::JSCSSValueOwner::finalize):
* bindings/js/JSNodeCustom.cpp:
(WebCore::JSNodeOwner::finalize):
(WebCore::JSNode::visitChildren):
Call releaseImpl() instead of clearImpl().

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
Stop storing m_impl in a RefPtr and instead use a raw pointer. Switch
clearImpl() to releaseImpl(), which explicitly derefs the pointer and
clear it.

(GenerateImplementation):
Use leakPtr() to explicitly adopt the PassRefPtr into the raw pointer.
Change default finalize to call releaseImpl() instead of clearImpl().

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (100516 => 100517)


--- trunk/Source/WebCore/ChangeLog	2011-11-16 23:37:15 UTC (rev 100516)
+++ trunk/Source/WebCore/ChangeLog	2011-11-16 23:46:54 UTC (rev 100517)
@@ -1,3 +1,33 @@
+2011-11-16  Sam Weinig  <s...@webkit.org>
+
+        JS wrappers of DOM objects should have no-op constructors
+        https://bugs.webkit.org/show_bug.cgi?id=72556
+
+        Reviewed by Geoffrey Garen.
+
+        Stop using a RefPtr to hold DOM objects contained by _javascript_
+        wrappers and instead use a raw pointer. We were already releasing
+        the underlying object before the destructor ran (via the finalizer)
+        so the default behavior of destroying the RefPtr is always unnecessary
+        busy work. 
+
+        * bindings/js/JSCSSValueCustom.cpp:
+        (WebCore::JSCSSValueOwner::finalize):
+        * bindings/js/JSNodeCustom.cpp:
+        (WebCore::JSNodeOwner::finalize):
+        (WebCore::JSNode::visitChildren):
+        Call releaseImpl() instead of clearImpl().
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateHeader):
+        Stop storing m_impl in a RefPtr and instead use a raw pointer. Switch
+        clearImpl() to releaseImpl(), which explicitly derefs the pointer and
+        clear it.
+
+        (GenerateImplementation):
+        Use leakPtr() to explicitly adopt the PassRefPtr into the raw pointer.
+        Change default finalize to call releaseImpl() instead of clearImpl().
+
 2011-11-16  Michael Saboff  <msab...@apple.com>
 
         Enable 8 Bit Strings in _javascript_Core

Modified: trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp (100516 => 100517)


--- trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp	2011-11-16 23:37:15 UTC (rev 100516)
+++ trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp	2011-11-16 23:46:54 UTC (rev 100517)
@@ -68,7 +68,7 @@
     DOMWrapperWorld* world = static_cast<DOMWrapperWorld*>(context);
     world->m_cssValueRoots.remove(jsCSSValue->impl());
     uncacheWrapper(world, jsCSSValue->impl(), jsCSSValue);
-    jsCSSValue->clearImpl();
+    jsCSSValue->releaseImpl();
 }
 
 JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, CSSValue* value)

Modified: trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp (100516 => 100517)


--- trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp	2011-11-16 23:37:15 UTC (rev 100516)
+++ trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp	2011-11-16 23:46:54 UTC (rev 100517)
@@ -141,7 +141,7 @@
     JSNode* jsNode = static_cast<JSNode*>(handle.get().asCell());
     DOMWrapperWorld* world = static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, jsNode->impl(), jsNode);
-    jsNode->clearImpl();
+    jsNode->releaseImpl();
 }
 
 JSValue JSNode::insertBefore(ExecState* exec)
@@ -201,7 +201,7 @@
     ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren());
     Base::visitChildren(thisObject, visitor);
 
-    Node* node = thisObject->m_impl.get();
+    Node* node = thisObject->impl();
     node->visitJSEventListeners(visitor);
 
     visitor.addOpaqueRoot(root(node));

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (100516 => 100517)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2011-11-16 23:37:15 UTC (rev 100516)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2011-11-16 23:46:54 UTC (rev 100517)
@@ -924,10 +924,10 @@
     }
 
     if (!$hasParent) {
-        push(@headerContent, "    $implType* impl() const { return m_impl.get(); }\n");
-        push(@headerContent, "    void clearImpl() { m_impl.clear(); }\n\n");
+        push(@headerContent, "    $implType* impl() const { return m_impl; }\n");
+        push(@headerContent, "    void releaseImpl() { m_impl->deref(); m_impl = 0; }\n\n");
         push(@headerContent, "private:\n");
-        push(@headerContent, "    RefPtr<$implType> m_impl;\n");
+        push(@headerContent, "    $implType* m_impl;\n");
     } elsif ($dataNode->extendedAttributes->{"GenerateNativeConverter"}) {
         push(@headerContent, "    $implClassName* impl() const\n");
         push(@headerContent, "    {\n");
@@ -1545,7 +1545,7 @@
             push(@implContent, "    : $parentClassName(structure, globalObject, impl)\n");
         } else {
             push(@implContent, "    : $parentClassName(structure, globalObject)\n");
-            push(@implContent, "    , m_impl(impl)\n");
+            push(@implContent, "    , m_impl(impl.leakRef())\n");
         }
         push(@implContent, "{\n");
         push(@implContent, "}\n\n");
@@ -2213,7 +2213,7 @@
         push(@implContent, "    JS${implClassName}* js${implClassName} = static_cast<JS${implClassName}*>(handle.get().asCell());\n");
         push(@implContent, "    DOMWrapperWorld* world = static_cast<DOMWrapperWorld*>(context);\n");
         push(@implContent, "    uncacheWrapper(world, js${implClassName}->impl(), js${implClassName});\n");
-        push(@implContent, "    js${implClassName}->clearImpl();\n");
+        push(@implContent, "    js${implClassName}->releaseImpl();\n");
         push(@implContent, "}\n\n");
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to