Oilpan: Prepare moving AXObject to heap

Source/modules/accessibility/AXObject.cpp

Index: Source/modules/accessibility/AXObject.cpp
diff --git a/Source/modules/accessibility/AXObject.cpp b/Source/modules/accessibility/AXObject.cpp
index 21eb762cd0146c48d5b545358409f72b0fd92892..3e9c16e15a65d5e65b63ea6c2bab292a696368d3 100644
--- a/Source/modules/accessibility/AXObject.cpp
+++ b/Source/modules/accessibility/AXObject.cpp
@@ -375,7 +375,7 @@ AXObject::AXObject(AXObjectCacheImpl* axObjectCache)
     , m_role(UnknownRole)
     , m_lastKnownIsIgnoredValue(DefaultBehavior)
     , m_detached(false)
-    , m_parent(0)
+    , m_parent(nullptr)
     , m_lastModificationCount(-1)
     , m_cachedIsIgnored(false)
     , m_cachedIsInertOrAriaHidden(false)
@@ -383,7 +383,7 @@ AXObject::AXObject(AXObjectCacheImpl* axObjectCache)
     , m_cachedIsDescendantOfDisabledNode(false)
     , m_cachedHasInheritedPresentationalRole(false)
     , m_cachedIsPresentationalChild(false)
-    , m_cachedLiveRegionRoot(0)
+    , m_cachedLiveRegionRoot(nullptr)
     , m_axObjectCache(axObjectCache)
 {
 }
@@ -398,7 +398,7 @@ void AXObject::detach()
     // Clear any children and call detachFromParent on them so that
     // no children are left with dangling pointers to their parent.
     clearChildren();
-
+    m_axObjectCache = nullptr;
     m_detached = true;
 }
 
@@ -666,9 +666,9 @@ bool AXObject::isPresentationalChild() const
     return m_cachedIsPresentationalChild;
 }
 
-String AXObject::name(AXNameFrom& nameFrom, Vector<AXObject*>& nameObjects)
+String AXObject::name(AXNameFrom& nameFrom, WillBeHeapVector<RawPtrWillBeMember<AXObject>>& nameObjects)
 {
-    HashSet<AXObject*> visited;
+    WillBeHeapHashSet<RawPtrWillBeMember<AXObject>> visited;
     return textAlternative(false, false, visited, &nameFrom, &nameObjects);
 }
 
@@ -1477,4 +1477,12 @@ const AtomicString& AXObject::internalRoleName(AccessibilityRole role)
     return internalRoleNameVector->at(role);
 }
 
+DEFINE_TRACE(AXObject)
+{
+    visitor->trace(m_children);
+    visitor->trace(m_parent);
+    visitor->trace(m_cachedLiveRegionRoot);
+    visitor->trace(m_axObjectCache);

[dmazzoni, 2015/06/08 15:25:49]

Why is this circular reference needed? It doesn't seem like each AXObject needs
to visit its AXObjectCache.

Suppose a document is destroyed and it destroys its AXObjectCache. A few of the
AXObjects may stick around briefly because a WebAXObject still has a reference
to them - why does the AXObjectCache need to persist too?

I think this points out a possible bug in the existing code - I'd expect that
AXObject::detach() should be clearing m_axObjectCache.

[haraken, 2015/06/09 02:25:45]

We don't need to worry about circular references when tracing objects. We just
need to visit all object graph edges -- if the target node is already visited
(because of circular references etc), the visitor just stops the traversal
there. That's the way a GC system works and that's exactly why a GC system
allows the program to create circular references that haven't allowed in the
reference counting system.

[keishi, 2015/06/09 05:51:19]

Like haraken said we don't need to worry about circular references for GCed
objects. (and the rule is to always trace all Member references)
I didn't like that m_axObjectCache might be keeping a stale pointer to
AXObjectCache and thats why I added m_axObjectCache=nullptr to
AXObject::detach(). I haven't done it yet but I think we could get rid of
m_detached and use m_axObjectCache instead for isDetached().

[dmazzoni, 2015/06/09 18:07:28]

I like the idea of using m_axObjectCache instead of m_detached.

Could you consider landing that as a separate change?

Are there any other changes that are part of this CL right now that could be
split out? If this ends up having a bug in it it'd be a lot to revert.

[haraken, 2015/06/10 00:55:57]

I think the AccessibilityText part can be landed separately. Incremental landing
is a key to make a quick progress :)

+}
+
 } // namespace blink