Oilpan: move mutation observers to the Oilpan 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 5051d1c27c11a558e84e81a3405e04172fe9d441..ac485cdce1f75b537b760884ec546b9eadce4ab4 100644
--- a/Source/core/dom/Node.cpp
+++ b/Source/core/dom/Node.cpp
@@ -328,10 +328,19 @@ void Node::clearRareData()
     ASSERT(!transientMutationObserverRegistry() || transientMutationObserverRegistry()->isEmpty());
 
     RenderObject* renderer = m_data.m_rareData->renderer();
-    if (isElementNode())
-        delete static_cast<ElementRareData*>(m_data.m_rareData);
-    else
-        delete static_cast<NodeRareData*>(m_data.m_rareData);
+    if (isElementNode()) {
+        ElementRareData* rareData = static_cast<ElementRareData*>(m_data.m_rareData);
+#if ENABLE(OILPAN)

[haraken, 2014/04/17 05:37:12]

Nit: You can drop the ENABLE(OILPAN) flag from here. You can just add the flag
inside NodeRareData::dispose() so that non-oilpan builds do nothing in
NodeRareData::dispose().

[sof, 2014/04/17 12:41:55]

ok, done that instead.

+        rareData->dispose();
+#endif
+        delete rareData;
+    } else {
+        NodeRareData* rareData = static_cast<NodeRareData*>(m_data.m_rareData);
+#if ENABLE(OILPAN)
+        rareData->dispose();
+#endif
+        delete rareData;
+    }
     m_data.m_renderer = renderer;
     clearFlag(HasRareDataFlag);
 }
@@ -1942,14 +1951,14 @@ void Node::didMoveToNewDocument(Document& oldDocument)
         }
     }
 
-    if (Vector<OwnPtr<MutationObserverRegistration> >* registry = mutationObserverRegistry()) {
+    if (WillBeHeapVector<OwnPtrWillBeMember<MutationObserverRegistration> >* registry = mutationObserverRegistry()) {
         for (size_t i = 0; i < registry->size(); ++i) {
             document().addMutationObserverTypes(registry->at(i)->mutationTypes());
         }
     }
 
-    if (HashSet<MutationObserverRegistration*>* transientRegistry = transientMutationObserverRegistry()) {
-        for (HashSet<MutationObserverRegistration*>::iterator iter = transientRegistry->begin(); iter != transientRegistry->end(); ++iter) {
+    if (WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >* transientRegistry = transientMutationObserverRegistry()) {
+        for (WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >::iterator iter = transientRegistry->begin(); iter != transientRegistry->end(); ++iter) {
             document().addMutationObserverTypes((*iter)->mutationTypes());
         }
     }
@@ -2039,7 +2048,7 @@ void Node::clearEventTargetData()
     eventTargetDataMap().remove(this);
 }
 
-Vector<OwnPtr<MutationObserverRegistration> >* Node::mutationObserverRegistry()
+WillBeHeapVector<OwnPtrWillBeMember<MutationObserverRegistration> >* Node::mutationObserverRegistry()
 {
     if (!hasRareData())
         return 0;
@@ -2049,7 +2058,7 @@ Vector<OwnPtr<MutationObserverRegistration> >* Node::mutationObserverRegistry()
     return &data->registry;
 }
 
-HashSet<MutationObserverRegistration*>* Node::transientMutationObserverRegistry()
+WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >* Node::transientMutationObserverRegistry()
 {
     if (!hasRareData())
         return 0;
@@ -2060,7 +2069,7 @@ HashSet<MutationObserverRegistration*>* Node::transientMutationObserverRegistry(
 }
 
 template<typename Registry>
-static inline void collectMatchingObserversForMutation(HashMap<MutationObserver*, MutationRecordDeliveryOptions>& observers, Registry* registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
+static inline void collectMatchingObserversForMutation(WillBeHeapHashMap<RawPtrWillBeMember<MutationObserver>, MutationRecordDeliveryOptions>& observers, Registry* registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
 {
     if (!registry)
         return;
@@ -2068,14 +2077,14 @@ static inline void collectMatchingObserversForMutation(HashMap<MutationObserver*
         const MutationObserverRegistration& registration = **iter;
         if (registration.shouldReceiveMutationFrom(target, type, attributeName)) {
             MutationRecordDeliveryOptions deliveryOptions = registration.deliveryOptions();
-            HashMap<MutationObserver*, MutationRecordDeliveryOptions>::AddResult result = observers.add(&registration.observer(), deliveryOptions);
+            WillBeHeapHashMap<RawPtrWillBeMember<MutationObserver>, MutationRecordDeliveryOptions>::AddResult result = observers.add(&registration.observer(), deliveryOptions);
             if (!result.isNewEntry)
                 result.storedValue->value |= deliveryOptions;
         }
     }
 }
 
-void Node::getRegisteredMutationObserversOfType(HashMap<MutationObserver*, MutationRecordDeliveryOptions>& observers, MutationObserver::MutationType type, const QualifiedName* attributeName)
+void Node::getRegisteredMutationObserversOfType(WillBeHeapHashMap<RawPtrWillBeMember<MutationObserver>, MutationRecordDeliveryOptions>& observers, MutationObserver::MutationType type, const QualifiedName* attributeName)
 {
     ASSERT((type == MutationObserver::Attributes && attributeName) || !attributeName);
     collectMatchingObserversForMutation(observers, mutationObserverRegistry(), *this, type, attributeName);
@@ -2089,7 +2098,7 @@ void Node::getRegisteredMutationObserversOfType(HashMap<MutationObserver*, Mutat
 void Node::registerMutationObserver(MutationObserver& observer, MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
 {
     MutationObserverRegistration* registration = 0;
-    Vector<OwnPtr<MutationObserverRegistration> >& registry = ensureRareData().ensureMutationObserverData().registry;
+    WillBeHeapVector<OwnPtrWillBeMember<MutationObserverRegistration> >& registry = ensureRareData().ensureMutationObserverData().registry;
     for (size_t i = 0; i < registry.size(); ++i) {
         if (&registry[i]->observer() == &observer) {
             registration = registry[i].get();
@@ -2107,7 +2116,7 @@ void Node::registerMutationObserver(MutationObserver& observer, MutationObserver
 
 void Node::unregisterMutationObserver(MutationObserverRegistration* registration)
 {
-    Vector<OwnPtr<MutationObserverRegistration> >* registry = mutationObserverRegistry();
+    WillBeHeapVector<OwnPtrWillBeMember<MutationObserverRegistration> >* registry = mutationObserverRegistry();
     ASSERT(registry);
     if (!registry)
         return;
@@ -2121,6 +2130,10 @@ void Node::unregisterMutationObserver(MutationObserverRegistration* registration
     // before that, in case |this| is destroyed (see MutationObserverRegistration::m_registrationNodeKeepAlive).
     // FIXME: Simplify the registration/transient registration logic to make this understandable by humans.
     RefPtr<Node> protect(this);
+#if ENABLE(OILPAN)
+    // Promptly dispose of the MutationObserverRegistration object.
+    registration->dispose();

[haraken, 2014/04/17 05:37:12]

Help me understand:

In my understanding, all MutaionObserverRegistration objects are registered in
NodeMutationObserverData::registry. And NodeMutationObserverData::registry is
responsible for calling dispose() for the MutaionObserverRegistration objects.
If so, why do we need to call dispose() here too?

[sof, 2014/04/17 12:41:55]

If you explicitly disconnect() a MutationObserver, you clear out the observer's
list of registrations. If we don't call dispose() here, but just clear out that
list/vector, MutationObserverRegistration objects are in a state where when we
later will call dispose(), they have a reference back to their observer, but not
the other way around. And I'm not sure what state the transient registrations
are in either, by that point.

So, to keep things simpler and more regular, explicitly disposing the
registration object here (to mimic what happens in the non-oilpan case), makes
good sense.

This question made me realize that the explicit clearing of m_registrations in
MutationObserver::disconnect() is quite redundant -- it will be empty. I've
replaced it with an assert.

[haraken, 2014/04/17 13:20:54]

Makes sense!

+#endif
     registry->remove(index);
 }
 
@@ -2131,7 +2144,7 @@ void Node::registerTransientMutationObserver(MutationObserverRegistration* regis
 
 void Node::unregisterTransientMutationObserver(MutationObserverRegistration* registration)
 {
-    HashSet<MutationObserverRegistration*>* transientRegistry = transientMutationObserverRegistry();
+    WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >* transientRegistry = transientMutationObserverRegistry();
     ASSERT(transientRegistry);
     if (!transientRegistry)
         return;
@@ -2146,14 +2159,14 @@ void Node::notifyMutationObserversNodeWillDetach()
         return;
 
     for (Node* node = parentNode(); node; node = node->parentNode()) {
-        if (Vector<OwnPtr<MutationObserverRegistration> >* registry = node->mutationObserverRegistry()) {
+        if (WillBeHeapVector<OwnPtrWillBeMember<MutationObserverRegistration> >* registry = node->mutationObserverRegistry()) {
             const size_t size = registry->size();
             for (size_t i = 0; i < size; ++i)
                 registry->at(i)->observedSubtreeNodeWillDetach(*this);
         }
 
-        if (HashSet<MutationObserverRegistration*>* transientRegistry = node->transientMutationObserverRegistry()) {
-            for (HashSet<MutationObserverRegistration*>::iterator iter = transientRegistry->begin(); iter != transientRegistry->end(); ++iter)
+        if (WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >* transientRegistry = node->transientMutationObserverRegistry()) {
+            for (WillBeHeapHashSet<RawPtrWillBeMember<MutationObserverRegistration> >::iterator iter = transientRegistry->begin(); iter != transientRegistry->end(); ++iter)
                 (*iter)->observedSubtreeNodeWillDetach(*this);
         }
     }