Oilpan: Remove guardRef and guardDeref from TreeScope.

Source/core/dom/TreeScope.h

 /*
  * Copyright (C) 2011 Google Inc. All Rights Reserved.
  * Copyright (C) 2012 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 84 matching lines @@
 
     bool applyAuthorStyles() const;
 
     // Used by the basic DOM mutation methods (e.g., appendChild()).
     void adoptIfNeeded(Node&);
 
     Node& rootNode() const { return m_rootNode; }
 
     IdTargetObserverRegistry& idTargetObserverRegistry() const { return *m_idTargetObserverRegistry.get(); }
 
+
+#if !ENABLE(OILPAN)
     // Nodes belonging to this scope hold guard references -
     // these are enough to keep the scope from being destroyed, but
     // not enough to keep it from removing its children. This allows a
     // node that outlives its scope to still have a valid document
     // pointer without introducing reference cycles.
     void guardRef()
     {
-#if ENABLE(OILPAN)
-        if (!m_guardRefCount) {
-            ASSERT(!m_keepAlive);
-            m_keepAlive = adoptPtr(new Persistent<TreeScope>(this));
-        }
-#else
         ASSERT(!deletionHasBegun());
-#endif
         ++m_guardRefCount;
     }
 
     void guardDeref()
     {
         ASSERT(m_guardRefCount > 0);
-#if !ENABLE(OILPAN)
         ASSERT(!deletionHasBegun());
-#endif
         --m_guardRefCount;
-#if ENABLE(OILPAN)
-        if (!m_guardRefCount)
-            clearKeepAlive();
-#else
         if (!m_guardRefCount && !refCount() && !rootNodeHasTreeSharedParent()) {
             beginDeletion();
             delete this;
         }
+    }
 #endif
-    }
 
     void removedLastRefToScope();
 
-#if ENABLE(OILPAN)
-    void clearKeepAlive()
-    {
-        ASSERT(m_keepAlive);
-        m_keepAlive = nullptr;
-    }
-#endif
-
     bool isInclusiveAncestorOf(const TreeScope&) const;
     unsigned short comparePosition(const TreeScope&) const;
 
     const TreeScope* commonAncestorTreeScope(const TreeScope& other) const;
     TreeScope* commonAncestorTreeScope(TreeScope& other);
 
     Element* getElementByAccessKey(const String& key) const;
 
+    virtual void trace(Visitor*);
+
 protected:
     TreeScope(ContainerNode&, Document&);
     TreeScope(Document&);
     virtual ~TreeScope();
 
     void destroyTreeScopeData();
     void setDocument(Document& document) { m_document = &document; }
     void setParentTreeScope(TreeScope&);
 
+#if !ENABLE(OILPAN)
     bool hasGuardRefCount() const { return m_guardRefCount; }
+#endif
 
     void setNeedsStyleRecalcForViewportUnits();
 
 private:
     virtual void dispose() { }
 
     int refCount() const;
 #if SECURITY_ASSERT_ENABLED
     bool deletionHasBegun();
     void beginDeletion();
 #else
     bool deletionHasBegun() { return false; }
     void beginDeletion() { }
 #endif
 
     bool rootNodeHasTreeSharedParent() const;
 
     Node& m_rootNode;
     Document* m_document;
-    TreeScope* m_parentTreeScope;
+    RawPtrWillBeMember<TreeScope> m_parentTreeScope;
+
+#if !ENABLE(OILPAN)
     int m_guardRefCount;
+#endif
 
     OwnPtr<DocumentOrderedMap> m_elementsById;
     OwnPtr<DocumentOrderedMap> m_imageMapsByName;
     OwnPtr<DocumentOrderedMap> m_labelsByForAttribute;

[haraken, 2014/04/25 05:21:42]

Just help me understand: When do we need to change m_rootNode, m_document,
Element pointers in DocumentOrderedMap to Members? Is it when we remove all the
destruction ordering issues in the Node hierarchy and remove the TreeShared
algorithm?

[Mads Ager (chromium), 2014/04/25 10:58:25]

These are owned by the document and are updated in the same way that they used
to be. Things are explicitly added and removed from these sets and therefore the
pointers do not become stale. So raw pointers here are as OK as they were
before. We should move the DocumentOrderedMap to the heap and trace all pointers
in them. That will give us extra security. If there is a logic error that will
now lead to a stale pointer, with Oilpan it will lead to a leak of that Element
instead (and a leak is better than a stale pointer). So, yes, when we go through
the node hierarchy and replace RefPtr with Member we need to explicitly deal
with all raw pointer members and make sure that they are correctly updated to
oilpan types so they are properly traced.

 
     OwnPtr<IdTargetObserverRegistry> m_idTargetObserverRegistry;
 
-#if ENABLE(OILPAN)
-    // With Oilpan, a non-zero reference count will keep the TreeScope alive
-    // with a self-persistent handle. Whenever the ref count goes above zero
-    // we register the TreeScope as a root for garbage collection by allocating a
-    // persistent handle to the object itself. When the ref count goes to zero
-    // we deallocate the persistent handle again so the object can die if there
-    // are no other things keeping it alive.
-    //
-    // FIXME: Oilpan: Remove m_keepAlive and ref counting and use tracing instead.
-    GC_PLUGIN_IGNORE("359444")
-    OwnPtr<Persistent<TreeScope> > m_keepAlive;
-#endif
-
-    GC_PLUGIN_IGNORE("359444")
-    mutable RefPtrWillBePersistent<DOMSelection> m_selection;
+    mutable RefPtrWillBeMember<DOMSelection> m_selection;
 };
 
 inline bool TreeScope::hasElementWithId(StringImpl* id) const
 {
     ASSERT(id);
     return m_elementsById && m_elementsById->contains(id);
 }
 
 inline bool TreeScope::containsMultipleElementsWithId(const AtomicString& id) const
 {
     return m_elementsById && m_elementsById->containsMultiple(id.impl());
 }
 
 inline bool operator==(const TreeScope& a, const TreeScope& b) { return &a == &b; }
 inline bool operator==(const TreeScope& a, const TreeScope* b) { return &a == b; }
 inline bool operator==(const TreeScope* a, const TreeScope& b) { return a == &b; }
 inline bool operator!=(const TreeScope& a, const TreeScope& b) { return !(a == b); }
 inline bool operator!=(const TreeScope& a, const TreeScope* b) { return !(a == b); }
 inline bool operator!=(const TreeScope* a, const TreeScope& b) { return !(a == b); }
 
 HitTestResult hitTestInDocument(const Document*, int x, int y);
 TreeScope* commonTreeScope(Node*, Node*);
 
 } // namespace WebCore
 
 #endif // TreeScope_h