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

Source/core/dom/MutationObserverRegistration.cpp

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 26 matching lines @@
 
 namespace WebCore {
 
 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)
     : m_observer(observer)
-    , m_registrationNode(registrationNode)
+    , m_registrationNode(&registrationNode)

[Erik Corry, 2014/05/05 14:28:46]

This is called in exactly one place where we have a pointer (this pointer) for
the registration node.  There seems no benefit in having the argument be a
reference, we can just change this and the create() method to use pointers.

[sof, 2014/05/05 15:36:03]

Yes, impedance matching now done.

     , m_options(options)
     , m_attributeFilter(attributeFilter)
 {
     m_observer->observationStarted(this);
 }
 
 MutationObserverRegistration::~MutationObserverRegistration()
 {
-    // dispose() hasn't been called if this assert triggers.
-    ASSERT(!m_observer);
+#if !ENABLE(OILPAN)
+    dispose();
+#endif
 }
 
 void MutationObserverRegistration::dispose()
 {
     clearTransientRegistrations();
-    m_observer->observationEnded(this);
-    m_observer.clear();
+    if (m_observer) {

[Erik Corry, 2014/05/05 14:28:46]

I wonder why we did not need this 'if' before?

[sof, 2014/05/05 14:57:36]

We explicitly dispose() in Node::unregisterMutationObserver() when explicitly
disconnect()ing the observer registrations, as we have to. Which means we will
end up calling dispose() twice for those registration objects in the non-Oilpan
case.

The use of #if in unregisterMutationObserver() was nay'ed in the
MutationObserver CL, but perhaps we should reconsider that?

[Erik Corry, 2014/05/05 14:58:43]

I think it's cool as it is.

[sof, 2014/05/05 15:36:03]

Thinking about it a bit more, it is preferable to do away with the explicit null
check on m_observer here. It avoids having to rediscover that the check is
redundant once the codebase no longer supports both non-Oilpan and Oilpan.

IOW, made the call to dispose() in unregisterMutationObserver() #if-conditional.

+        m_observer->observationEnded(this);
+        m_observer.clear();
+    }
 }
 
 void MutationObserverRegistration::resetObservation(MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
 {
     clearTransientRegistrations();
     m_options = options;
     m_attributeFilter = attributeFilter;
 }
 
 void MutationObserverRegistration::observedSubtreeNodeWillDetach(Node& node)
 {
     if (!isSubtree())
         return;
 
     node.registerTransientMutationObserver(this);
     m_observer->setHasTransientRegistration();
 
     if (!m_transientRegistrationNodes) {
         m_transientRegistrationNodes = adoptPtr(new NodeHashSet);
 
         ASSERT(!m_registrationNodeKeepAlive);
-        m_registrationNodeKeepAlive = PassRefPtr<Node>(m_registrationNode); // Balanced in clearTransientRegistrations.
+        m_registrationNodeKeepAlive = PassRefPtr<Node>(m_registrationNode.get()); // Balanced in clearTransientRegistrations.
     }
     m_transientRegistrationNodes->add(&node);
 }
 
 void MutationObserverRegistration::clearTransientRegistrations()
 {
     if (!m_transientRegistrationNodes) {
         ASSERT(!m_registrationNodeKeepAlive);
         return;
     }
 
     for (NodeHashSet::iterator iter = m_transientRegistrationNodes->begin(); iter != m_transientRegistrationNodes->end(); ++iter)
         (*iter)->unregisterTransientMutationObserver(this);
 
     m_transientRegistrationNodes.clear();
 
     ASSERT(m_registrationNodeKeepAlive);
     m_registrationNodeKeepAlive = nullptr; // Balanced in observeSubtreeNodeWillDetach.
 }
 
 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.
 }
 
 bool MutationObserverRegistration::shouldReceiveMutationFrom(Node& node, MutationObserver::MutationType type, const QualifiedName* attributeName) const
 {
     ASSERT((type == MutationObserver::Attributes && attributeName) || !attributeName);
     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))
         return true;
 
     if (!attributeName->namespaceURI().isNull())
         return false;
 
     return m_attributeFilter.contains(attributeName->localName());
 }
 
 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());
 }
 
 void MutationObserverRegistration::trace(Visitor* visitor)
 {
     visitor->trace(m_observer);
+    visitor->trace(m_registrationNode);
 }
 
 } // namespace WebCore