Changeset 20151 in webkit

Timestamp:

Mar 13, 2007 9:53:26 AM (17 years ago)

Author:

weinig

Message:

Reviewed by Maciej.

Fixes: ​http://bugs.webkit.org/show_bug.cgi?id=12974

Call forgetGenericContext in JSSVGPathSeg destructor, otherwhise
we'll hit an ASSERT in a debug build, when running svg/custom/js-update-path-changes.svg
a few dozen times in a single WebKit instance. The ASSERT is good and just warns that
there was already a generic context pointer registered, and the new "to be registered"
object already exists, but pointing to a different object. That's because garbage collection
calls the JSSVGPathSeg destructor, but that didn't cleanup the generic context map.

Only JSSVGPathSeg is hit by this problem, as it's the only non SVGAnimated* type
using the generic context system while using a custom JSSVGPathSegList implementation.

Also cleanup JSSVGPathSegListCustom code to call the static forgetGenericContext
method instead of doing the same using custom code.

• bindings/js/JSSVGPathSegListCustom.cpp: (WebCore::removeFromPathSegContextMap): (WebCore::JSSVGPathSegList::clear): (WebCore::JSSVGPathSegList::removeItem):
• bindings/scripts/CodeGeneratorJS.pm:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSSVGPathSegListCustom.cpp (modified) (3 diffs)
• bindings/scripts/CodeGeneratorJS.pm (modified) (4 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-03-13  Nikolas Zimmermann  <zimmermann@kde.org>
+
+        Reviewed by Maciej.
+
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=12974
+
+        Call forgetGenericContext in JSSVGPathSeg destructor, otherwhise
+        we'll hit an ASSERT in a debug build, when running svg/custom/js-update-path-changes.svg
+        a few dozen times in a single WebKit instance. The ASSERT is good and just warns that
+        there was already a generic context pointer registered, and the new "to be registered"
+        object already exists, but pointing to a different object. That's because garbage collection
+        calls the JSSVGPathSeg destructor, but that didn't cleanup the generic context map.
+
+        Only JSSVGPathSeg is hit by this problem, as it's the only non SVGAnimated* type
+        using the generic context system while using a custom JSSVGPathSegList implementation.
+
+        Also cleanup JSSVGPathSegListCustom code to call the static forgetGenericContext
+        method instead of doing the same using custom code.
+
+        * bindings/js/JSSVGPathSegListCustom.cpp:
+        (WebCore::removeFromPathSegContextMap):
+        (WebCore::JSSVGPathSegList::clear):
+        (WebCore::JSSVGPathSegList::removeItem):
+        * bindings/scripts/CodeGeneratorJS.pm:
+
 2007-03-13  Darin Adler  <darin@apple.com>
 

trunk/WebCore/bindings/js/JSSVGPathSegListCustom.cpp

@@ -58 +58 @@
 }
 
-static void removeFromPathSegContextMap(ExecState* exec, SVGPathSegList* list, SVGPathSeg* obj)
+static void removeFromPathSegContextMap(SVGPathSegList* list, SVGPathSeg* obj)
 {
-    ASSERT(exec && exec->dynamicInterpreter());
-    Frame* activeFrame = static_cast<ScriptInterpreter*>(exec->dynamicInterpreter())->frame();
-    if (!activeFrame)
-        return;
-
     const SVGElement* context = list->context();
     ASSERT(context);
 
-    SVGDocumentExtensions* extensions = (activeFrame->document() ? activeFrame->document()->accessSVGExtensions() : 0);
-    if (extensions) {
-        if (extensions->hasGenericContext<SVGPathSeg>(obj))
-            extensions->removeGenericContext<SVGPathSeg>(obj);
-    }
-
+    SVGDocumentExtensions::forgetGenericContext(obj);
     context->notifyAttributeChange();
 }
@@ -85 +75 @@
     unsigned int nr = imp->numberOfItems();
     for (unsigned int i = 0; i < nr; i++)
-        removeFromPathSegContextMap(exec, imp, imp->getItem(i, ec).get());
+        removeFromPathSegContextMap(imp, imp->getItem(i, ec).get());
 
     imp->clear(ec);
@@ -182 +172 @@
 
     RefPtr<SVGPathSeg> obj(imp->removeItem(index, ec));
-    removeFromPathSegContextMap(exec, imp, obj.get());
+    removeFromPathSegContextMap(imp, obj.get());
 
     KJS::JSValue* result = toJS(exec, obj.get());

trunk/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@ -1 +1 @@
 #
-# Copyright (C) 2005, 2006 Nikolas Zimmermann <zimmermann@kde.org>
+# Copyright (C) 2005, 2006, 2007 Nikolas Zimmermann <zimmermann@kde.org>
 # Copyright (C) 2006 Anders Carlsson <andersca@mac.com>
 # Copyright (C) 2006 Samuel Weinig <sam.weinig@gmail.com>
@@ -167 +167 @@
 }
 
+sub CreateSVGContextInterfaceName
+{
+    my $type = shift;
+
+    return $type if $codeGenerator->IsSVGAnimatedType($type);
+    return "SVGPathSeg" if $type =~ /^SVGPathSeg/ and $type ne "SVGPathSegList";
+
+    return "";
+}
+
 sub AddIncludesForType
 {
@@ -730 +740 @@
         push(@implContent, "${className}::~$className()\n");
 
-        if ($codeGenerator->IsSVGAnimatedType($interfaceName)) {
-            push(@implContent, "{\n    SVGDocumentExtensions::forgetGenericContext<$interfaceName>(m_impl.get());\n");   
+        my $contextInterfaceName = CreateSVGContextInterfaceName($interfaceName);
+        if ($contextInterfaceName ne "") {
+            push(@implContent, "{\n    SVGDocumentExtensions::forgetGenericContext<$contextInterfaceName>(m_impl.get());\n");   
             push(@implContent, "    ScriptInterpreter::forgetDOMObject(m_impl.get());\n}\n\n");
         } else {
@@ -949 +960 @@
             push(@implContent, "    }\n"); # end switch
 
+            my $contextInterfaceName = CreateSVGContextInterfaceName($interfaceName);
             if ($interfaceName eq "DOMWindow") {
                 push(@implContent, "    // FIXME: Hack to prevent unused variable warning -- remove once DOMWindow includes a settable property\n");
                 push(@implContent, "    (void)imp;\n");
-            } elsif ($interfaceName =~ /^SVGPathSeg/ or $codeGenerator->IsSVGAnimatedType($interfaceName)) {
+            } elsif ($contextInterfaceName ne "") {
                 push(@implContent, "    ASSERT(exec && exec->dynamicInterpreter());\n");
                 push(@implContent, "    Frame* activeFrame = static_cast<ScriptInterpreter*>(exec->dynamicInterpreter())->frame();\n");
                 push(@implContent, "    if (!activeFrame)\n        return;\n\n");
                 push(@implContent, "    SVGDocumentExtensions* extensions = (activeFrame->document() ? activeFrame->document()->accessSVGExtensions() : 0);\n");
-                push(@implContent, "    if (extensions && extensions->hasGenericContext<$interfaceName>(imp)) {\n");
-                push(@implContent, "        const SVGElement* context = extensions->genericContext<$interfaceName>(imp);\n");
+                push(@implContent, "    if (extensions && extensions->hasGenericContext<$contextInterfaceName>(imp)) {\n");
+                push(@implContent, "        const SVGElement* context = extensions->genericContext<$contextInterfaceName>(imp);\n");
                 push(@implContent, "        ASSERT(context);\n\n");
                 push(@implContent, "        context->notifyAttributeChange();\n");