Oilpan: Prepare moving AXObject to heap

Source/modules/accessibility/AXObject.cpp

 /*
  * Copyright (C) 2008, 2009, 2011 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 357 matching lines @@
 }
 
 } // namespace
 
 AXObject::AXObject(AXObjectCacheImpl* axObjectCache)
     : m_id(0)
     , m_haveChildren(false)
     , m_role(UnknownRole)
     , m_lastKnownIsIgnoredValue(DefaultBehavior)
     , m_detached(false)
-    , m_parent(0)
+    , m_parent(nullptr)
     , m_lastModificationCount(-1)
     , m_cachedIsIgnored(false)
     , m_cachedIsInertOrAriaHidden(false)
     , m_cachedIsDescendantOfLeafNode(false)
     , m_cachedIsDescendantOfDisabledNode(false)
     , m_cachedHasInheritedPresentationalRole(false)
     , m_cachedIsPresentationalChild(false)
-    , m_cachedLiveRegionRoot(0)
+    , m_cachedLiveRegionRoot(nullptr)
     , m_axObjectCache(axObjectCache)
 {
 }
 
 AXObject::~AXObject()
 {
     ASSERT(isDetached());
 }
 
 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;
 }
 
 bool AXObject::isDetached() const
 {
     return m_detached;
 }
 
 bool AXObject::isARIATextControl() const
 {
@@ skipping 247 matching lines @@
     updateCachedAttributeValuesIfNeeded();
     return m_cachedHasInheritedPresentationalRole;
 }
 
 bool AXObject::isPresentationalChild() const
 {
     updateCachedAttributeValuesIfNeeded();
     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);
 }
 
 // In ARIA 1.1, the default value for aria-orientation changed from horizontal to undefined.
 AccessibilityOrientation AXObject::orientation() const
 {
     return AccessibilityOrientationUndefined;
 }
 
 static String queryString(WebLocalizedString::Name name)
@@ skipping 788 matching lines @@
     return roleNameVector->at(role);
 }
 
 const AtomicString& AXObject::internalRoleName(AccessibilityRole role)
 {
     static const Vector<AtomicString>* internalRoleNameVector = createInternalRoleNameVector();
 
     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