Oilpan: Make the Node hierarchy RefCountedGarbageCollected instead of TreeShared.

Source/core/dom/ContainerNodeAlgorithms.h

 /*
  * Copyright (C) 2007 Apple Inc. All rights reserved.
  *           (C) 2008 Nikolas Zimmermann <zimmermann@kde.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 53 matching lines @@
     void notifyNodeRemovedFromTree(ContainerNode&);
 
     ContainerNode& m_insertionPoint;
 };
 
 namespace Private {
 
     template<class GenericNode, class GenericNodeContainer>
     void addChildNodesToDeletionQueue(GenericNode*& head, GenericNode*& tail, GenericNodeContainer&);
 
-}
-
-// Helper functions for TreeShared-derived classes, which have a 'Node' style interface
-// This applies to 'ContainerNode' and 'SVGElementInstance'
-template<class GenericNode, class GenericNodeContainer>
-inline void removeDetachedChildrenInContainer(GenericNodeContainer& container)
-{
-    // List of nodes to be deleted.
-    GenericNode* head = 0;
-    GenericNode* tail = 0;
-
-    Private::addChildNodesToDeletionQueue<GenericNode, GenericNodeContainer>(head, tail, container);
-
-    GenericNode* n;
-    GenericNode* next;
-    while ((n = head) != 0) {
-#if !ENABLE(OILPAN)
-        ASSERT_WITH_SECURITY_IMPLICATION(n->m_deletionHasBegun);
-#endif
-
-        next = n->nextSibling();
-        n->setNextSibling(0);
-
-        head = next;
-        if (next == 0)
-            tail = 0;
-
-        if (n->hasChildren())
-            Private::addChildNodesToDeletionQueue<GenericNode, GenericNodeContainer>(head, tail, static_cast<GenericNodeContainer&>(*n));
-
-#if !ENABLE(OILPAN)
-        delete n;
-#endif
-    }
-}
-
-template<class GenericNode, class GenericNodeContainer>
-inline void appendChildToContainer(GenericNode& child, GenericNodeContainer& container)
-{
-    child.setParentOrShadowHostNode(&container);
-
-    GenericNode* lastChild = container.lastChild();
-    if (lastChild) {
-        child.setPreviousSibling(lastChild);
-        lastChild->setNextSibling(&child);
-    } else {
-        container.setFirstChild(&child);
-    }
-
-    container.setLastChild(&child);
-}
-
-// Helper methods for removeDetachedChildrenInContainer, hidden from WebCore namespace
-namespace Private {
+    // Helper methods for removeDetachedChildrenInContainer, hidden from WebCore namespace
 
     template<class GenericNode, class GenericNodeContainer, bool dispatchRemovalNotification>
     struct NodeRemovalDispatcher {
         static void dispatch(GenericNode&, GenericNodeContainer&)
         {
             // no-op, by default
         }
     };
 
     template<class GenericNode, class GenericNodeContainer>
@@ skipping 26 matching lines @@
         for (GenericNode* n = container.firstChild(); n; n = next) {
             ASSERT_WITH_SECURITY_IMPLICATION(!n->m_deletionHasBegun);
 
             next = n->nextSibling();
             n->setNextSibling(0);
             n->setParentOrShadowHostNode(0);
             container.setFirstChild(next);
             if (next)
                 next->setPreviousSibling(0);
 
-#if ENABLE(OILPAN)
-            {
-                // Always notify nodes of removal from the document even if they
-                // are going to die. Nodes do not immediately die when their
-                // refcounts reach zero with Oilpan. They die at the next garbage
-                // collection. The notifications when removed from document
-                // allows us to perform cleanup on the nodes when they are removed
-                // instead of when their destructors are called.
-                RefPtr<GenericNode> protect(n); // removedFromDocument may remove all references to this node.
-                NodeRemovalDispatcher<GenericNode, GenericNodeContainer, ShouldDispatchRemovalNotification<GenericNode>::value>::dispatch(*n, container);
-            }
-            if (!n->refCount()) {
-                // Add the node to the list of nodes to be deleted.
-                // Reuse the nextSibling pointer for this purpose.
-                if (tail)
-                    tail->setNextSibling(n);
-                else
-                    head = n;
-
-                tail = n;
-            }
-#else
             if (!n->refCount()) {
 #if SECURITY_ASSERT_ENABLED
                 n->m_deletionHasBegun = true;
 #endif
                 // Add the node to the list of nodes to be deleted.
                 // Reuse the nextSibling pointer for this purpose.
                 if (tail)
                     tail->setNextSibling(n);
                 else
                     head = n;
 
                 tail = n;
             } else {
                 RefPtr<GenericNode> protect(n); // removedFromDocument may remove all references to this node.
                 NodeRemovalDispatcher<GenericNode, GenericNodeContainer, ShouldDispatchRemovalNotification<GenericNode>::value>::dispatch(*n, container);
             }
-#endif // ENABLE(OILPAN)
         }
 
         container.setLastChild(0);
     }
 
 } // namespace Private
 
+// Helper functions for TreeShared-derived classes, which have a 'Node' style interface
+// This applies to 'ContainerNode' and 'SVGElementInstance'

[haraken, 2014/05/06 04:20:16]

FYI, SVGElementInstance is going to be removed soon :)

[Mads Ager (chromium), 2014/05/06 08:26:00]

That is great! Right now this code is only used for SVGElementInstance in the
oilpan build and can therefore be removed at that point.

+template<class GenericNode, class GenericNodeContainer>
+inline void removeDetachedChildrenInContainer(GenericNodeContainer& container)

[haraken, 2014/05/06 04:20:16]

Just help me understand (very fundamental question): Who calls
removeDetachedChildrenInContainer() in oilpan builds? removeDetachedChildren()
is removed in oilpan builds, and I cannot find any caller of
removeDetachedChildrenInContainer() for the Node hierarchy, which made me
confused.

[Mads Ager (chromium), 2014/05/06 08:26:00]

Yes, it is very confusing. This is still called by the SVGElementInstance detach
method. However, it is actually not doing anything. The only reason for doing
this is to dispatch removedFrom notifications. However, those notifications are
only dispatched for Node and not for SVGElementInstance. I can completely remove
this from the Oilpan build. It is not doing anything at this point.

+{
+#if ENABLE(OILPAN)
+    // Always dispatch node removal notifications when nodes are
+    // removed. This maintains the invariant that either nodes have
+    // been removed from their container and their removedFrom method has
+    // been called, or the nodes die with the their container.

[haraken, 2014/05/06 04:20:16]

the their container => their container

Just to confirm: If the nodes dies with their container,
removeDetachedChildrenInContainer() is not called and thus removedFrom() is not
called on the nodes. Right?

[Mads Ager (chromium), 2014/05/06 08:26:00]

That is correct. This is only called from SVGElementInstance destructor and I am
going to remove that since the removedFrom events are not dispatched for that
hierarchy anyway so this code just sets a few pointers to 0 which isn't needed.

+    GenericNode* next = 0;
+    for (GenericNode* n = container.firstChild(); n; n = next) {
+        next = n->nextSibling();
+        n->setNextSibling(0);
+        n->setParentOrShadowHostNode(0);
+        if (next)
+            next->setPreviousSibling(0);
+        Private::NodeRemovalDispatcher<GenericNode, GenericNodeContainer, Private::ShouldDispatchRemovalNotification<GenericNode>::value>::dispatch(*n, container);
+    }
+    container.setFirstChild(0);
+    container.setLastChild(0);
+#else
+    // List of nodes to be deleted.
+    GenericNode* head = 0;
+    GenericNode* tail = 0;
+
+    Private::addChildNodesToDeletionQueue<GenericNode, GenericNodeContainer>(head, tail, container);
+
+    GenericNode* n;
+    GenericNode* next;
+    while ((n = head) != 0) {
+        ASSERT_WITH_SECURITY_IMPLICATION(n->m_deletionHasBegun);
+
+        next = n->nextSibling();
+        n->setNextSibling(0);
+
+        head = next;
+        if (next == 0)
+            tail = 0;
+
+        if (n->hasChildren())
+            Private::addChildNodesToDeletionQueue<GenericNode, GenericNodeContainer>(head, tail, static_cast<GenericNodeContainer&>(*n));
+
+        delete n;
+    }
+#endif
+}
+
+template<class GenericNode, class GenericNodeContainer>
+inline void appendChildToContainer(GenericNode& child, GenericNodeContainer& container)
+{
+    child.setParentOrShadowHostNode(&container);
+
+    GenericNode* lastChild = container.lastChild();
+    if (lastChild) {
+        child.setPreviousSibling(lastChild);
+        lastChild->setNextSibling(&child);
+    } else {
+        container.setFirstChild(&child);
+    }
+
+    container.setLastChild(&child);
+}
+
 inline void ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument(Node& node)
 {
     ASSERT(m_insertionPoint.inDocument());
     RefPtr<Node> protect(node);
     if (Node::InsertionShouldCallDidNotifySubtreeInsertions == node.insertedInto(&m_insertionPoint))
         m_postInsertionNotificationTargets.append(&node);
     if (node.isContainerNode())
         notifyDescendantInsertedIntoDocument(toContainerNode(node));
 }
 
@@ skipping 128 matching lines @@
         for (Node* child = m_root.firstChild(); child; child = child->nextSibling())
             collectFrameOwners(*child);
     }
 
     disconnectCollectedFrameOwners();
 }
 
 } // namespace WebCore
 
 #endif // ContainerNodeAlgorithms_h