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

Source/core/dom/MutationObserverRegistration.h

 /*
  * 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 25 matching lines @@
 #include "wtf/HashSet.h"
 #include "wtf/text/AtomicString.h"
 #include "wtf/text/AtomicStringHash.h"
 
 namespace WebCore {
 
 class QualifiedName;
 
 class MutationObserverRegistration FINAL : public NoBaseWillBeGarbageCollectedFinalized<MutationObserverRegistration> {
 public:
-    static PassOwnPtrWillBeRawPtr<MutationObserverRegistration> create(MutationObserver&, Node&, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
+    static PassOwnPtrWillBeRawPtr<MutationObserverRegistration> create(MutationObserver&, Node*, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
     ~MutationObserverRegistration();
 
     void resetObservation(MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
     void observedSubtreeNodeWillDetach(Node&);
     void clearTransientRegistrations();
     bool hasTransientRegistrations() const { return m_transientRegistrationNodes && !m_transientRegistrationNodes->isEmpty(); }
     void unregister();
 
     bool shouldReceiveMutationFrom(Node&, MutationObserver::MutationType, const QualifiedName* attributeName) const;
     bool isSubtree() const { return m_options & MutationObserver::Subtree; }
 
     MutationObserver& observer() const { return *m_observer; }
     MutationRecordDeliveryOptions deliveryOptions() const { return m_options & (MutationObserver::AttributeOldValue | MutationObserver::CharacterDataOldValue); }
     MutationObserverOptions mutationTypes() const { return m_options & MutationObserver::AllMutationTypes; }
 
     void addRegistrationNodesToSet(HashSet<Node*>&) const;
 
     void trace(Visitor*);
 
     void dispose();
 
 private:
-    MutationObserverRegistration(MutationObserver&, Node&, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
+    MutationObserverRegistration(MutationObserver&, Node*, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
 
     RefPtrWillBeMember<MutationObserver> m_observer;
-    Node& m_registrationNode;
+    RawPtrWillBeMember<Node> m_registrationNode;
+    // FIXME: Oilpan: once a Member reference will keep a Node
+    // and its tree alive, this keep-alive RefPtr can be
+    // removed for Oilpan builds.

[Erik Corry, 2014/05/06 07:32:09]

The intended semantics is that m_registrationNode is sometimes strong and
sometimes weak.  The way that's realized in the pre-oilpan code is that it's
normally a raw pointer or C++ reference with clearing from the node destructor,
but sometimes we put the same pointer in the m_registrationNodeKeepAlive field
which is a RefPtr and keeps the node alive.  We could have the same thing in the
oilpan build by making m_registrationNode a WeakMember and
m_registrationNodeKeepAlive a Member.  You probably need an explicit callback
for the weak member in order to remove the registration.

Pre-oilpan the way it works is that we call dispose() on the registrations when
the node dies (via NodeRareData::dispose()).  Since the registration does not
keep the node alive, this makes it a weak observation.

[sof, 2014/05/06 22:03:25]

Excellent information & description.

Weakened the reference accordingly; let me know if that captures what you had in
mind.

     RefPtr<Node> m_registrationNodeKeepAlive;

[haraken, 2014/05/05 16:54:52]

I'd remove m_registrationNodeKeepAlive, even though it will lead to wrong
behavior because m_registrationNode does not keep the Node tree alive.

If you add RefPtr<Node> to the registration Node, I guess that the
MutationObserverRegistartion object will be never destructed because we have a
cycle: Node ==Member==> NodeRareData ==Member==> MutationObserverRegistartion
==RefPtr==> Node.

Rather than having an always leak, it would be better to allow wrong behavior
which will be fixed shortly once Mads' CL lands.

(A better idea would be to wait for Mads' CL to land.)

[sof, 2014/05/05 19:35:43]

Definitely, this will wait until it has settled on trunk. This is one reason why
I've held off on following up on some of your questions surrounding
ElementShadow -- the handling there will change once that CL has landed. (I
don't quite know how yet.. :) )

[haraken, 2014/05/06 06:32:23]

We're trying to land Mads' CL by the end of this week :)

[sof, 2014/05/06 07:13:10]

Thanks for the update, whenever it is ready :) This CL will just get in the way,
so better to wait.

     typedef HashSet<RefPtr<Node> > NodeHashSet;
     OwnPtr<NodeHashSet> m_transientRegistrationNodes;
 
     MutationObserverOptions m_options;
     HashSet<AtomicString> m_attributeFilter;
 };
 
 } // namespace WebCore
 
 #endif // MutationObserverRegistration_h