Shadow tree TreeScope data is not removed by ContainerNode::removeAllChildren

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

Reviewed by Hajime Morita.

The ContainerNode::removeAllChildren method does fast and dirty node removal.
As compared to ContainerNode::removeChildren, it does not include the
method to adopt the removed child node into the root document's TreeScope.
Rather, it leaves the existing TreeScope in place. However, the existing
TreeScope may be removed, causing a crash when something prevents the child
node from being deleted immediately. The fix is to modify the code in
ContainerNodeAlgorithms to do the TreeScope adoption.

Could not reproduce crash in layout test.

* dom/ContainerNodeAlgorithms.h:
(Private):
(WebCore::Private::NodeRemovalDispatcher::dispatch):
(WebCore::Private::addChildNodesToDeletionQueue):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index c936042..6af0138 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2012-04-11  Stephen Chenney  <schenney@chromium.org>
+
+        Shadow tree TreeScope data is not removed by ContainerNode::removeAllChildren
+        https://bugs.webkit.org/show_bug.cgi?id=83484
+
+        Reviewed by Hajime Morita.
+
+        The ContainerNode::removeAllChildren method does fast and dirty node removal.
+        As compared to ContainerNode::removeChildren, it does not include the
+        method to adopt the removed child node into the root document's TreeScope.
+        Rather, it leaves the existing TreeScope in place. However, the existing
+        TreeScope may be removed, causing a crash when something prevents the child
+        node from being deleted immediately. The fix is to modify the code in
+        ContainerNodeAlgorithms to do the TreeScope adoption.
+
+        Could not reproduce crash in layout test.
+
+        * dom/ContainerNodeAlgorithms.h:
+        (Private):
+        (WebCore::Private::NodeRemovalDispatcher::dispatch):
+        (WebCore::Private::addChildNodesToDeletionQueue):
+
 2012-04-11  Yury Semikhatsky  <yurys@chromium.org>
 
         Web Inspector: showing summary view is very slow on a snapshot with thousands of constructors

diff --git a/Source/WebCore/dom/ContainerNodeAlgorithms.h b/Source/WebCore/dom/ContainerNodeAlgorithms.h
index ee69afd..39bfc19 100644 (file)
--- a/Source/WebCore/dom/ContainerNodeAlgorithms.h
+++ b/Source/WebCore/dom/ContainerNodeAlgorithms.h
@@ -83,18 +83,21 @@ void appendChildToContainer(GenericNode* child, GenericNodeContainer* container)
 // Helper methods for removeAllChildrenInContainer, hidden from WebCore namespace
 namespace Private {
 
-    template<class GenericNode, bool dispatchRemovalNotification>
+    template<class GenericNode, class GenericNodeContainer, bool dispatchRemovalNotification>
     struct NodeRemovalDispatcher {
-        static void dispatch(GenericNode*)
+        static void dispatch(GenericNode*, GenericNodeContainer*)
         {
             // no-op, by default
         }
     };
 
-    template<class GenericNode>
-    struct NodeRemovalDispatcher<GenericNode, true> {
-        static void dispatch(GenericNode* node)
+    template<class GenericNode, class GenericNodeContainer>
+    struct NodeRemovalDispatcher<GenericNode, GenericNodeContainer, true> {
+        static void dispatch(GenericNode* node, GenericNodeContainer* container)
         {
+            // Clean up any TreeScope to a removed tree.
+            if (Document* containerDocument = container->ownerDocument())
+                containerDocument->adoptIfNeeded(node);
             if (node->inDocument())
                 node->removedFromDocument();
         }
@@ -137,7 +140,7 @@ namespace Private {
                 tail = n;
             } else {
                 RefPtr<GenericNode> protect(n); // removedFromDocument may remove remove all references to this node.
-                NodeRemovalDispatcher<GenericNode, ShouldDispatchRemovalNotification<GenericNode>::value>::dispatch(n);
+                NodeRemovalDispatcher<GenericNode, GenericNodeContainer, ShouldDispatchRemovalNotification<GenericNode>::value>::dispatch(n, container);
             }
         }