Oilpan: move node/element rare data objects to the heap.

Source/core/dom/Node.cpp

Index: Source/core/dom/Node.cpp
diff --git a/Source/core/dom/Node.cpp b/Source/core/dom/Node.cpp
index 0798dda6c5b079fdd35680c844fb3a3dd533d867..1fe3197c7e1c419b2a478832e7cd38b93e97b590 100644
--- a/Source/core/dom/Node.cpp
+++ b/Source/core/dom/Node.cpp
@@ -266,14 +266,18 @@ Node::~Node()
     liveNodeSet.remove(this);
 #endif
 
+#if !ENABLE(OILPAN)
     if (hasRareData())
         clearRareData();
 
     RELEASE_ASSERT(!renderer());
 
-#if !ENABLE(OILPAN)
     if (!isContainerNode())
         willBeDeletedFromDocument();
+#else
+    // With Oilpan, the rare data finalizer also asserts for
+    // this condition (we cannot directly access it here.)
+    RELEASE_ASSERT(hasRareData() || !renderer());
 #endif
 
     if (m_previous)
@@ -326,36 +330,38 @@ NodeRareData& Node::ensureRareData()
     if (hasRareData())
         return *rareData();
 
-    NodeRareData* data;
+    OwnPtrWillBeRawPtr<NodeRareData> data;

[Erik Corry, 2014/05/05 09:31:27]

I can't see the point of this OwnPtr.  Things would be simpler if this was just
a raw pointer, and you just immediately leakPtr after create. If RawPtr had a
leakPtr method you could avid the ifdef completely, otherwise you have to move
it to the create() calls.

[sof, 2014/05/05 09:41:38]

It minimized the #if'ery required, but I'll take another look.

[sof, 2014/05/05 13:38:58]

Jumped ahead a bit and made the create() return a raw pointer (in the form of a
RawPtr<>) to avoid the #if's here, which is its only call site.

Acceptable type for create() to have in a non-Oilpan setting?

     if (isElementNode())
-        data = ElementRareData::create(m_data.m_renderer).leakPtr();
+        data = ElementRareData::create(m_data.m_renderer);
     else
-        data = NodeRareData::create(m_data.m_renderer).leakPtr();
+        data = NodeRareData::create(m_data.m_renderer);
+
     ASSERT(data);
 
+#if ENABLE(OILPAN)
     m_data.m_rareData = data;
+#else
+    m_data.m_rareData = data.leakPtr();
+#endif
     setFlag(HasRareDataFlag);
-    return *data;
+    return *rareData();
 }
 
+#if !ENABLE(OILPAN)
 void Node::clearRareData()
 {
     ASSERT(hasRareData());
     ASSERT(!transientMutationObserverRegistry() || transientMutationObserverRegistry()->isEmpty());
 
     RenderObject* renderer = m_data.m_rareData->renderer();
-    if (isElementNode()) {
-        ElementRareData* rareData = static_cast<ElementRareData*>(m_data.m_rareData);
-        rareData->dispose();
-        delete rareData;
-    } else {
-        NodeRareData* rareData = static_cast<NodeRareData*>(m_data.m_rareData);
-        rareData->dispose();
-        delete rareData;
-    }
+    if (isElementNode())
+        delete static_cast<ElementRareData*>(m_data.m_rareData);
+    else
+        delete static_cast<NodeRareData*>(m_data.m_rareData);
     m_data.m_renderer = renderer;
     clearFlag(HasRareDataFlag);
 }
+#endif
 
 Node* Node::toNode()
 {
@@ -2571,6 +2577,9 @@ void Node::setCustomElementState(CustomElementState newState)
 
 void Node::trace(Visitor* visitor)
 {
+    if (hasRareData())
+        visitor->trace(rareData());
+
     visitor->trace(m_treeScope);
 }