Oilpan: Make the Node hierarchy RefCountedGarbageCollected instead of TreeShared.

Source/core/dom/ContainerNodeAlgorithms.h

Index: Source/core/dom/ContainerNodeAlgorithms.h
diff --git a/Source/core/dom/ContainerNodeAlgorithms.h b/Source/core/dom/ContainerNodeAlgorithms.h
index fbac15b690b481353814c5e386c50c2310d81fd0..da2c4fabf5d262566d61899d4bb0c59ac773613b 100644
--- a/Source/core/dom/ContainerNodeAlgorithms.h
+++ b/Source/core/dom/ContainerNodeAlgorithms.h
@@ -71,60 +71,7 @@ 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 {
@@ -171,28 +118,6 @@ namespace Private {
             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;
@@ -209,7 +134,6 @@ namespace Private {
                 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);
@@ -217,6 +141,70 @@ namespace Private {
 
 } // 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());