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..2fe34ed1d8ad508988c4efff2fdfb0077a27e442 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)

[Mads Ager (chromium), 2014/05/05 10:47:42]

I had to move this code around so the Private namespace parts are declared
before use. The changes here are much smaller than they appear.

-{
-    // 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

[zerny-chromium, 2014/05/05 11:07:52]

Nit: reference to "TreeShared-derived classes" is not accurate for the two
builds anymore.

[Mads Ager (chromium), 2014/05/05 11:35:54]

Yes, I would prefer to not change the comment at this point, but you are right
that it doesn't make sense for the oilpan build.

+// This applies to 'ContainerNode' and 'SVGElementInstance'
+template<class GenericNode, class GenericNodeContainer>
+inline void removeDetachedChildrenInContainer(GenericNodeContainer& container)
+{
+#if ENABLE(OILPAN)
+    // Always dispatch node removal notifications when nodes are

[Erik Corry, 2014/05/05 11:35:48]

This looks relevant for regular container nodes, but this function is not called
for them in the oilpan build any more(?)

[Mads Ager (chromium), 2014/05/05 11:49:16]

Yeah, its complicated...

This is still called for SVGElementInstances on detach. This was never called
for explicit removal of nodes from a container node. See
ContainerNode::removeChild/removeBetween which does this work for the explicit
removal case.

+    // removed. This maintains the invariant that either nodes have
+    // been removed from the ducument and their removedFrom method has

[Erik Corry, 2014/05/05 11:35:48]

ducument -> duckument!

[Mads Ager (chromium), 2014/05/05 11:49:16]

Yeah, the ducument -> their container.

+    // been called, or the nodes die with the document.
+    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());