Title: [100398] trunk/Source/WebCore
Revision
100398
Author
mhahnenb...@apple.com
Date
2011-11-15 20:29:17 -0800 (Tue, 15 Nov 2011)

Log Message

JS DOM wrappers depend on destructor to deref impl RefPtrs
https://bugs.webkit.org/show_bug.cgi?id=72341

Reviewed by Sam Weinig.

No new tests.

Added clearing of impl RefPtrs to JS DOM wrapper nodes and removed the default
wrapperOwner function in favor of generating all WeakHandleOwners and wrapperOwner functions.

* bindings/js/JSCSSValueCustom.cpp:
(WebCore::JSCSSValueOwner::finalize):
* bindings/js/JSDOMBinding.h:
* bindings/js/JSNodeCustom.cpp:
(WebCore::JSNodeOwner::finalize):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
(GenerateImplementation):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (100397 => 100398)


--- trunk/Source/WebCore/ChangeLog	2011-11-16 04:28:41 UTC (rev 100397)
+++ trunk/Source/WebCore/ChangeLog	2011-11-16 04:29:17 UTC (rev 100398)
@@ -1,3 +1,24 @@
+2011-11-15  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        JS DOM wrappers depend on destructor to deref impl RefPtrs
+        https://bugs.webkit.org/show_bug.cgi?id=72341
+
+        Reviewed by Sam Weinig.
+
+        No new tests.
+
+        Added clearing of impl RefPtrs to JS DOM wrapper nodes and removed the default 
+        wrapperOwner function in favor of generating all WeakHandleOwners and wrapperOwner functions.
+
+        * bindings/js/JSCSSValueCustom.cpp:
+        (WebCore::JSCSSValueOwner::finalize):
+        * bindings/js/JSDOMBinding.h:
+        * bindings/js/JSNodeCustom.cpp:
+        (WebCore::JSNodeOwner::finalize):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateHeader):
+        (GenerateImplementation):
+
 2011-11-15  Joseph Pecoraro  <pecor...@apple.com>
 
         Web Inspector: Share Highlight Code for Drawing Outlined Quad

Modified: trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp (100397 => 100398)


--- trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp	2011-11-16 04:28:41 UTC (rev 100397)
+++ trunk/Source/WebCore/bindings/js/JSCSSValueCustom.cpp	2011-11-16 04:29:17 UTC (rev 100398)
@@ -68,6 +68,7 @@
     DOMWrapperWorld* world = static_cast<DOMWrapperWorld*>(context);
     world->m_cssValueRoots.remove(jsCSSValue->impl());
     uncacheWrapper(world, jsCSSValue->impl(), jsCSSValue);
+    jsCSSValue->clearImpl();
 }
 
 JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, CSSValue* value)

Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (100397 => 100398)


--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2011-11-16 04:28:41 UTC (rev 100397)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2011-11-16 04:29:17 UTC (rev 100398)
@@ -125,10 +125,6 @@
     inline bool setInlineCachedWrapper(DOMWrapperWorld*, void*, JSDOMWrapper*) { return false; }
     inline bool clearInlineCachedWrapper(DOMWrapperWorld*, void*, JSDOMWrapper*) { return false; }
 
-    // Overload these functions to provide a custom WeakHandleOwner.
-    inline JSC::WeakHandleOwner* wrapperOwner(DOMWrapperWorld* world, void*) { return world->defaultWrapperOwner(); }
-    inline void* wrapperContext(DOMWrapperWorld*, void* domObject) { return domObject; }
-
     template <typename DOMClass> inline JSDOMWrapper* getCachedWrapper(DOMWrapperWorld* world, DOMClass* domObject)
     {
         if (JSDOMWrapper* wrapper = getInlineCachedWrapper(world, domObject))

Modified: trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp (100397 => 100398)


--- trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp	2011-11-16 04:28:41 UTC (rev 100397)
+++ trunk/Source/WebCore/bindings/js/JSNodeCustom.cpp	2011-11-16 04:29:17 UTC (rev 100398)
@@ -141,6 +141,7 @@
     JSNode* jsNode = static_cast<JSNode*>(handle.get().asCell());
     DOMWrapperWorld* world = static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, jsNode->impl(), jsNode);
+    jsNode->clearImpl();
 }
 
 JSValue JSNode::insertBefore(ExecState* exec)

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (100397 => 100398)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2011-11-16 04:28:41 UTC (rev 100397)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2011-11-16 04:29:17 UTC (rev 100398)
@@ -924,7 +924,8 @@
     }
 
     if (!$hasParent) {
-        push(@headerContent, "    $implType* impl() const { return m_impl.get(); }\n\n");
+        push(@headerContent, "    $implType* impl() const { return m_impl.get(); }\n");
+        push(@headerContent, "    void clearImpl() { m_impl.clear(); }\n\n");
         push(@headerContent, "private:\n");
         push(@headerContent, "    RefPtr<$implType> m_impl;\n");
     } elsif ($dataNode->extendedAttributes->{"GenerateNativeConverter"}) {
@@ -989,19 +990,20 @@
         push(@headerContent, "}\n\n");
     }
 
-    if ($dataNode->extendedAttributes->{"GenerateIsReachable"} || 
+    if (!$hasParent ||
+        $dataNode->extendedAttributes->{"GenerateIsReachable"} || 
         $dataNode->extendedAttributes->{"CustomIsReachable"} || 
         $dataNode->extendedAttributes->{"CustomFinalize"} ||
         $dataNode->extendedAttributes->{"ActiveDOMObject"}) {
-        push(@headerContent, "class JS${implType}Owner : public JSC::WeakHandleOwner {\n");
+        push(@headerContent, "class JS${implClassName}Owner : public JSC::WeakHandleOwner {\n");
         push(@headerContent, "    virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, JSC::SlotVisitor&);\n");
         push(@headerContent, "    virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);\n");
         push(@headerContent, "};\n");
         push(@headerContent, "\n");
         push(@headerContent, "inline JSC::WeakHandleOwner* wrapperOwner(DOMWrapperWorld*, $implType*)\n");
         push(@headerContent, "{\n");
-        push(@headerContent, "    DEFINE_STATIC_LOCAL(JS${implType}Owner, js${implType}Owner, ());\n");
-        push(@headerContent, "    return &js${implType}Owner;\n");
+        push(@headerContent, "    DEFINE_STATIC_LOCAL(JS${implClassName}Owner, js${implClassName}Owner, ());\n");
+        push(@headerContent, "    return &js${implClassName}Owner;\n");
         push(@headerContent, "}\n");
         push(@headerContent, "\n");
         push(@headerContent, "inline void* wrapperContext(DOMWrapperWorld* world, $implType*)\n");
@@ -2137,21 +2139,21 @@
         }
     }
 
-    if ($dataNode->extendedAttributes->{"GenerateIsReachable"} || $dataNode->extendedAttributes->{"ActiveDOMObject"}) {
-        push(@implContent, "static inline bool isObservable(JS${implType}* js${implType})\n");
+    if ((!$hasParent && !$dataNode->extendedAttributes->{"CustomIsReachable"})|| $dataNode->extendedAttributes->{"GenerateIsReachable"} || $dataNode->extendedAttributes->{"ActiveDOMObject"}) {
+        push(@implContent, "static inline bool isObservable(JS${implClassName}* js${implClassName})\n");
         push(@implContent, "{\n");
-        push(@implContent, "    if (js${implType}->hasCustomProperties())\n");
+        push(@implContent, "    if (js${implClassName}->hasCustomProperties())\n");
         push(@implContent, "        return true;\n");
         if ($eventTarget) {
-            push(@implContent, "    if (js${implType}->impl()->hasEventListeners())\n");
+            push(@implContent, "    if (js${implClassName}->impl()->hasEventListeners())\n");
             push(@implContent, "        return true;\n");
         }
         push(@implContent, "    return false;\n");
         push(@implContent, "}\n\n");
 
-        push(@implContent, "bool JS${implType}Owner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)\n");
+        push(@implContent, "bool JS${implClassName}Owner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)\n");
         push(@implContent, "{\n");
-        push(@implContent, "    JS${implType}* js${implType} = static_cast<JS${implType}*>(handle.get().asCell());\n");
+        push(@implContent, "    JS${implClassName}* js${implClassName} = static_cast<JS${implClassName}*>(handle.get().asCell());\n");
         # All ActiveDOMObjects implement hasPendingActivity(), but not all of them
         # increment their C++ reference counts when hasPendingActivity() becomes
         # true. As a result, ActiveDOMObjects can be prematurely destroyed before
@@ -2160,36 +2162,36 @@
         # FIXME: Fix this lifetime issue in the DOM, and move this hasPendingActivity
         # check below the isObservable check.
         if ($dataNode->extendedAttributes->{"ActiveDOMObject"}) {
-            push(@implContent, "    if (js${implType}->impl()->hasPendingActivity())\n");
+            push(@implContent, "    if (js${implClassName}->impl()->hasPendingActivity())\n");
             push(@implContent, "        return true;\n");
         }
-        push(@implContent, "    if (!isObservable(js${implType}))\n");
+        push(@implContent, "    if (!isObservable(js${implClassName}))\n");
         push(@implContent, "        return false;\n");
         if ($dataNode->extendedAttributes->{"GenerateIsReachable"}) {
             my $rootString;
             if ($dataNode->extendedAttributes->{"GenerateIsReachable"} eq "Impl") {
-                $rootString  = "    ${implType}* root = js${implType}->impl();\n";
+                $rootString  = "    ${implType}* root = js${implClassName}->impl();\n";
             } elsif ($dataNode->extendedAttributes->{"GenerateIsReachable"} eq "ImplContext") {
-                $rootString  = "    WebGLRenderingContext* root = js${implType}->impl()->context();\n";
+                $rootString  = "    WebGLRenderingContext* root = js${implClassName}->impl()->context();\n";
             } elsif ($dataNode->extendedAttributes->{"GenerateIsReachable"} eq "ImplFrame") {
-                $rootString  = "    Frame* root = js${implType}->impl()->frame();\n";
+                $rootString  = "    Frame* root = js${implClassName}->impl()->frame();\n";
                 $rootString .= "    if (!root)\n";
                 $rootString .= "        return false;\n";
             } elsif ($dataNode->extendedAttributes->{"GenerateIsReachable"} eq "ImplDocument") {
-                $rootString  = "    Document* root = js${implType}->impl()->document();\n";
+                $rootString  = "    Document* root = js${implClassName}->impl()->document();\n";
                 $rootString .= "    if (!root)\n";
                 $rootString .= "        return false;\n";
             } elsif ($dataNode->extendedAttributes->{"GenerateIsReachable"} eq "ImplElementRoot") {
-                $rootString  = "    Element* element = js${implType}->impl()->element();\n";
+                $rootString  = "    Element* element = js${implClassName}->impl()->element();\n";
                 $rootString .= "    if (!element)\n";
                 $rootString .= "        return false;\n";
                 $rootString .= "    void* root = WebCore::root(element);\n";
             } elsif ($interfaceName eq "CanvasRenderingContext") {
-                $rootString  = "    void* root = WebCore::root(js${implType}->impl()->canvas());\n";
+                $rootString  = "    void* root = WebCore::root(js${implClassName}->impl()->canvas());\n";
             } elsif ($interfaceName eq "HTMLCollection") {
-                $rootString  = "    void* root = WebCore::root(js${implType}->impl()->base());\n";
+                $rootString  = "    void* root = WebCore::root(js${implClassName}->impl()->base());\n";
             } else {
-                $rootString  = "    void* root = WebCore::root(js${implType}->impl());\n";
+                $rootString  = "    void* root = WebCore::root(js${implClassName}->impl());\n";
             }
 
             push(@implContent, $rootString);
@@ -2202,14 +2204,16 @@
     }
 
     if (!$dataNode->extendedAttributes->{"CustomFinalize"} &&
-        ($dataNode->extendedAttributes->{"GenerateIsReachable"} || 
+        (!$hasParent || 
+         $dataNode->extendedAttributes->{"GenerateIsReachable"} || 
          $dataNode->extendedAttributes->{"CustomIsReachable"} ||
          $dataNode->extendedAttributes->{"ActiveDOMObject"})) {
-        push(@implContent, "void JS${implType}Owner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)\n");
+        push(@implContent, "void JS${implClassName}Owner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)\n");
         push(@implContent, "{\n");
-        push(@implContent, "    JS${implType}* js${implType} = static_cast<JS${implType}*>(handle.get().asCell());\n");
+        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${implType}->impl(), js${implType});\n");
+        push(@implContent, "    uncacheWrapper(world, js${implClassName}->impl(), js${implClassName});\n");
+        push(@implContent, "    js${implClassName}->clearImpl();\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