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

Source/core/dom/MutationObserverRegistration.cpp

Index: Source/core/dom/MutationObserverRegistration.cpp
diff --git a/Source/core/dom/MutationObserverRegistration.cpp b/Source/core/dom/MutationObserverRegistration.cpp
index 12816b0943bc7ef8a243660c0b9450f18fecb2a3..72af2d5c87d1d4741fdea100bfaa9c10f802c16e 100644
--- a/Source/core/dom/MutationObserverRegistration.cpp
+++ b/Source/core/dom/MutationObserverRegistration.cpp
@@ -37,14 +37,17 @@
 
 namespace WebCore {
 
-PassOwnPtrWillBeRawPtr<MutationObserverRegistration> MutationObserverRegistration::create(MutationObserver& observer, Node& registrationNode, MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
+PassOwnPtrWillBeRawPtr<MutationObserverRegistration> MutationObserverRegistration::create(MutationObserver& observer, Node* registrationNode, MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
 {
     return adoptPtrWillBeNoop(new MutationObserverRegistration(observer, registrationNode, options, attributeFilter));
 }
 
-MutationObserverRegistration::MutationObserverRegistration(MutationObserver& observer, Node& registrationNode, MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
+MutationObserverRegistration::MutationObserverRegistration(MutationObserver& observer, Node* registrationNode, MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
     : m_observer(observer)
     , m_registrationNode(registrationNode)
+#if ENABLE(OILPAN)
+    , m_keepAlive(false)
+#endif
     , m_options(options)
     , m_attributeFilter(attributeFilter)
 {
@@ -53,8 +56,9 @@ MutationObserverRegistration::MutationObserverRegistration(MutationObserver& obs
 
 MutationObserverRegistration::~MutationObserverRegistration()
 {
-    // dispose() hasn't been called if this assert triggers.
-    ASSERT(!m_observer);
+#if !ENABLE(OILPAN)
+    dispose();
+#endif
 }
 
 void MutationObserverRegistration::dispose()
@@ -82,8 +86,13 @@ void MutationObserverRegistration::observedSubtreeNodeWillDetach(Node& node)
     if (!m_transientRegistrationNodes) {
         m_transientRegistrationNodes = adoptPtr(new NodeHashSet);
 
+#if ENABLE(OILPAN)
+        ASSERT(!m_keepAlive);
+        m_keepAlive = true;
+#else
         ASSERT(!m_registrationNodeKeepAlive);
-        m_registrationNodeKeepAlive = PassRefPtr<Node>(m_registrationNode); // Balanced in clearTransientRegistrations.
+        m_registrationNodeKeepAlive = PassRefPtr<Node>(m_registrationNode.get()); // Balanced in clearTransientRegistrations.
+#endif
     }
     m_transientRegistrationNodes->add(&node);
 }
@@ -91,7 +100,11 @@ void MutationObserverRegistration::observedSubtreeNodeWillDetach(Node& node)
 void MutationObserverRegistration::clearTransientRegistrations()
 {
     if (!m_transientRegistrationNodes) {
+#if ENABLE(OILPAN)
+        ASSERT(!m_keepAlive);
+#else
         ASSERT(!m_registrationNodeKeepAlive);
+#endif
         return;
     }
 
@@ -100,13 +113,18 @@ void MutationObserverRegistration::clearTransientRegistrations()
 
     m_transientRegistrationNodes.clear();
 
+#if ENABLE(OILPAN)
+    ASSERT(m_keepAlive);
+    m_keepAlive = false;
+#else
     ASSERT(m_registrationNodeKeepAlive);
     m_registrationNodeKeepAlive = nullptr; // Balanced in observeSubtreeNodeWillDetach.
+#endif
 }
 
 void MutationObserverRegistration::unregister()
 {
-    m_registrationNode.unregisterMutationObserver(this);
+    m_registrationNode->unregisterMutationObserver(this);
     // The above line will cause this object to be deleted, so don't do any more in this function.
 }
 
@@ -116,7 +134,7 @@ bool MutationObserverRegistration::shouldReceiveMutationFrom(Node& node, Mutatio
     if (!(m_options & type))
         return false;
 
-    if (m_registrationNode != node && !isSubtree())
+    if (m_registrationNode != &node && !isSubtree())
         return false;
 
     if (type != MutationObserver::Attributes || !(m_options & MutationObserver::AttributeFilter))
@@ -130,16 +148,28 @@ bool MutationObserverRegistration::shouldReceiveMutationFrom(Node& node, Mutatio
 
 void MutationObserverRegistration::addRegistrationNodesToSet(HashSet<Node*>& nodes) const
 {
-    nodes.add(&m_registrationNode);
+    nodes.add(m_registrationNode.get());
     if (!m_transientRegistrationNodes)
         return;
     for (NodeHashSet::const_iterator iter = m_transientRegistrationNodes->begin(); iter != m_transientRegistrationNodes->end(); ++iter)
         nodes.add(iter->get());
 }
 
+#if ENABLE(OILPAN)
+void MutationObserverRegistration::clearWeakMembers(Visitor* visitor)
+{
+    if (!m_keepAlive && !visitor->isAlive(m_registrationNode))
+        dispose();

[haraken, 2014/05/07 02:10:26]

Would you elaborate on why we need to call dispose() here? I expected that this
line is 'm_registrationNode = nullptr'.

It seems a bit strange to me that you need to call dispose() in
clearWeakMember(), because the fact that m_keepAlive is false implies that
clearTransientRegistrations() is already called via dispose().

If the above observation is correct, you won't need clearWeakMembers.
Alternately, you can change trace() like this:

void trace(Visitor* visitor) {
  if (m_keepAlive)
    visitor->trace(m_registrationNode);
}

[sof, 2014/05/07 07:01:01]

The pair of weak&strong fields stays the same (see the latest patch; the use of
m_keepAlive wasn't valid).

Explicitly calling dispose() vs not, is subtle. If we don't remove a
registration, we run the risk of having a MutationObserver accessing a
registration without a Node (e.g., when it samples the current set of
registrations as part of a V8 GC). Supporting such near-dead access just makes
the implementation more complex for no apparent gain. Removing it eagerly
prevents that.

+}
+#endif
+
 void MutationObserverRegistration::trace(Visitor* visitor)
 {
     visitor->trace(m_observer);
+    visitor->trace(m_registrationNode);
+#if ENABLE(OILPAN)
+    visitor->registerWeakMembers<MutationObserverRegistration, &MutationObserverRegistration::clearWeakMembers>(this);
+#endif
 }
 
 } // namespace WebCore