Oilpan: Prepare moving AXObject to heap

Source/modules/accessibility/AXObjectCacheImpl.cpp

 /*
  * Copyright (C) 2014, 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:
  *
  * 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 91 matching lines @@
 
 AXObjectCacheImpl::~AXObjectCacheImpl()
 {
     ASSERT(m_hasBeenDisposed);
 }
 
 void AXObjectCacheImpl::dispose()
 {
     m_notificationPostTimer.stop();
 
+#if ENABLE(OILPAN)
+    for (auto& entry : m_layoutObjectMapping) {

[haraken, 2015/05/28 10:48:41]

Don't we need to call detach() for AbstractInlineTextBoxs as well?

+        entry.value->detach();
+    }
+    m_layoutObjectMapping.clear();
+
+    HeapHashMap<AXID, WeakMember<AXObject>>::iterator end = m_objects.end();
+    for (HeapHashMap<AXID, WeakMember<AXObject>>::iterator it = m_objects.begin(); it != end; ++it) {
+        AXObject* obj = (*it).value.get();
+        obj->detach();
+    }

[haraken, 2015/05/28 10:48:41]

You'll need to call m_objects.clear(). Otherwise, the detach state and the
contents of the hashmap will be inconsistent until the next GC is triggered.

+#else
     HashMap<AXID, RefPtr<AXObject>>::iterator end = m_objects.end();
     for (HashMap<AXID, RefPtr<AXObject>>::iterator it = m_objects.begin(); it != end; ++it) {
         AXObject* obj = (*it).value.get();
         obj->detach();
         removeAXID(obj);
     }
+#endif
 
 #if ENABLE(ASSERT)
     m_hasBeenDisposed = true;
 #endif
 }
 
 AXObject* AXObjectCacheImpl::root()
 {
     return getOrCreate(&m_document);
 }
@@ skipping 59 matching lines @@
         obj = obj->parentObjectUnignored();
 
     return obj;
 }
 
 AXObject* AXObjectCacheImpl::get(Widget* widget)
 {
     if (!widget)
         return 0;
 
+#if ENABLE(OILPAN)
+    return m_widgetObjectMapping.get(widget);
+#else
     AXID axID = m_widgetObjectMapping.get(widget);
     ASSERT(!HashTraits<AXID>::isDeletedValue(axID));
     if (!axID)
         return 0;
 
     return m_objects.get(axID);
+#endif
 }
 
 AXObject* AXObjectCacheImpl::get(LayoutObject* layoutObject)
 {
     if (!layoutObject)
         return 0;
 
+#if ENABLE(OILPAN)
+    return m_layoutObjectMapping.get(layoutObject);
+#else
     AXID axID = m_layoutObjectMapping.get(layoutObject);
     ASSERT(!HashTraits<AXID>::isDeletedValue(axID));
     if (!axID)
         return 0;
 
     return m_objects.get(axID);
+#endif
 }
 
 // Returns true if |node| is an <option> element and its parent <select>
 // is a menu list (not a list box).
 static bool isMenuListOption(Node* node)
 {
     if (!isHTMLOptionElement(node))
         return false;
     HTMLSelectElement* select = toHTMLOptionElement(node)->ownerSelectElement();
     if (!select)
         return false;
     LayoutObject* layoutObject = select->layoutObject();
     return layoutObject && layoutObject->isMenuList();
 }
 
 AXObject* AXObjectCacheImpl::get(Node* node)
 {
     if (!node)
         return 0;
 
+#if ENABLE(OILPAN)
+    AXObject* layoutAXObject =  node->layoutObject() ? m_layoutObjectMapping.get(node->layoutObject()) : nullptr;
+    AXObject* nodeAXObject =  m_nodeObjectMapping.get(node);
+    if (node->layoutObject() && nodeAXObject && !layoutAXObject && !isMenuListOption(node)) {
+        // This can happen if an AXNodeObject is created for a node that's not
+        // laid out, but later something changes and it gets a layoutObject (like if it's
+        // reparented).
+        remove(nodeAXObject);
+        return nullptr;
+    }
+    return layoutAXObject ? layoutAXObject : nodeAXObject;
+#else
     AXID layoutID = node->layoutObject() ? m_layoutObjectMapping.get(node->layoutObject()) : 0;
     ASSERT(!HashTraits<AXID>::isDeletedValue(layoutID));
 
     AXID nodeID = m_nodeObjectMapping.get(node);
     ASSERT(!HashTraits<AXID>::isDeletedValue(nodeID));
 
     if (node->layoutObject() && nodeID && !layoutID && !isMenuListOption(node)) {
         // This can happen if an AXNodeObject is created for a node that's not
         // laid out, but later something changes and it gets a layoutObject (like if it's
         // reparented).
         remove(nodeID);
         return 0;
     }
 
     if (layoutID)
         return m_objects.get(layoutID);
 
     if (!nodeID)
         return 0;
 
     return m_objects.get(nodeID);
+#endif
 }
 
 AXObject* AXObjectCacheImpl::get(AbstractInlineTextBox* inlineTextBox)
 {
     if (!inlineTextBox)
         return 0;
 
+#if ENABLE(OILPAN)
+    return m_inlineTextBoxObjectMapping.get(inlineTextBox);
+#else
     AXID axID = m_inlineTextBoxObjectMapping.get(inlineTextBox);
     ASSERT(!HashTraits<AXID>::isDeletedValue(axID));
     if (!axID)
         return 0;
 
     return m_objects.get(axID);
+#endif
 }
 
 // FIXME: This probably belongs on Node.
 // FIXME: This should take a const char*, but one caller passes nullAtom.
 bool nodeHasRole(Node* node, const String& role)
 {
     if (!node || !node->isElementNode())
         return false;
 
     return equalIgnoringCase(toElement(node)->getAttribute(roleAttr), role);
 }
 
-PassRefPtr<AXObject> AXObjectCacheImpl::createFromRenderer(LayoutObject* layoutObject)
+PassRefPtrWillBeRawPtr<AXObject> AXObjectCacheImpl::createFromRenderer(LayoutObject* layoutObject)
 {
     // FIXME: How could layoutObject->node() ever not be an Element?
     Node* node = layoutObject->node();
 
     // If the node is aria role="list" or the aria role is empty and its a
     // ul/ol/dl type (it shouldn't be a list if aria says otherwise).
     if (nodeHasRole(node, "list") || nodeHasRole(node, "directory")
         || (nodeHasRole(node, nullAtom) && (isHTMLUListElement(node) || isHTMLOListElement(node) || isHTMLDListElement(node))))
         return AXList::create(layoutObject, this);
 
@@ skipping 35 matching lines @@
             return AXProgressIndicator::create(toLayoutProgress(cssBox), this);
 
         // input type=range
         if (cssBox->isSlider())
             return AXSlider::create(toLayoutSlider(cssBox), this);
     }
 
     return AXLayoutObject::create(layoutObject, this);
 }
 
-PassRefPtr<AXObject> AXObjectCacheImpl::createFromNode(Node* node)
+PassRefPtrWillBeRawPtr<AXObject> AXObjectCacheImpl::createFromNode(Node* node)
 {
     if (isMenuListOption(node))
         return AXMenuListOption::create(toHTMLOptionElement(node), this);
 
     return AXNodeObject::create(node, this);
 }
 
-PassRefPtr<AXObject> AXObjectCacheImpl::createFromInlineTextBox(AbstractInlineTextBox* inlineTextBox)
+PassRefPtrWillBeRawPtr<AXObject> AXObjectCacheImpl::createFromInlineTextBox(AbstractInlineTextBox* inlineTextBox)
 {
     return AXInlineTextBox::create(inlineTextBox, this);
 }
 
 AXObject* AXObjectCacheImpl::getOrCreate(Widget* widget)
 {
     if (!widget)
         return 0;
 
     if (AXObject* obj = get(widget))
         return obj;
 
-    RefPtr<AXObject> newObj = nullptr;
+    RefPtrWillBeRawPtr<AXObject> newObj = nullptr;
     if (widget->isFrameView())
         newObj = AXScrollView::create(toFrameView(widget), this);
     else if (widget->isScrollbar())
         newObj = AXScrollbar::create(toScrollbar(widget), this);
 
     // Will crash later if we have two objects for the same widget.
     ASSERT(!get(widget));
 
     // Catch the case if an (unsupported) widget type is used. Only FrameView and ScrollBar are supported now.
     ASSERT(newObj);
     if (!newObj)
         return 0;
 
     getAXID(newObj.get());
 
+#if ENABLE(OILPAN)
+    m_widgetObjectMapping.set(widget, newObj);
+#else
     m_widgetObjectMapping.set(widget, newObj->axObjectID());
+#endif
     m_objects.set(newObj->axObjectID(), newObj);
     newObj->init();
     return newObj.get();
 }
 
 AXObject* AXObjectCacheImpl::getOrCreate(Node* node)
 {
     if (!node)
         return 0;
 
     if (AXObject* obj = get(node))
         return obj;
 
     if (node->layoutObject())
         return getOrCreate(node->layoutObject());
 
     if (!node->parentElement())
         return 0;
 
     if (isHTMLHeadElement(node))
         return 0;
 
-    RefPtr<AXObject> newObj = createFromNode(node);
+    RefPtrWillBeRawPtr<AXObject> newObj = createFromNode(node);
 
     // Will crash later if we have two objects for the same node.
     ASSERT(!get(node));
 
     getAXID(newObj.get());
 
+#if ENABLE(OILPAN)
+    m_nodeObjectMapping.set(node, newObj);
+#else
     m_nodeObjectMapping.set(node, newObj->axObjectID());
+#endif
     m_objects.set(newObj->axObjectID(), newObj);
     newObj->init();
     newObj->setLastKnownIsIgnoredValue(newObj->accessibilityIsIgnored());
 
     return newObj.get();
 }
 
 AXObject* AXObjectCacheImpl::getOrCreate(LayoutObject* layoutObject)
 {
     if (!layoutObject)
         return 0;
 
     if (AXObject* obj = get(layoutObject))
         return obj;
 
-    RefPtr<AXObject> newObj = createFromRenderer(layoutObject);
+    RefPtrWillBeRawPtr<AXObject> newObj = createFromRenderer(layoutObject);
 
     // Will crash later if we have two objects for the same layoutObject.
     ASSERT(!get(layoutObject));
 
     getAXID(newObj.get());
 
+#if ENABLE(OILPAN)
+    m_layoutObjectMapping.set(layoutObject, newObj);
+#else
     m_layoutObjectMapping.set(layoutObject, newObj->axObjectID());
+#endif
     m_objects.set(newObj->axObjectID(), newObj);
     newObj->init();
     newObj->setLastKnownIsIgnoredValue(newObj->accessibilityIsIgnored());
 
     return newObj.get();
 }
 
 AXObject* AXObjectCacheImpl::getOrCreate(AbstractInlineTextBox* inlineTextBox)
 {
     if (!inlineTextBox)
         return 0;
 
     if (AXObject* obj = get(inlineTextBox))
         return obj;
 
-    RefPtr<AXObject> newObj = createFromInlineTextBox(inlineTextBox);
+    RefPtrWillBeRawPtr<AXObject> newObj = createFromInlineTextBox(inlineTextBox);
 
     // Will crash later if we have two objects for the same inlineTextBox.
     ASSERT(!get(inlineTextBox));
 
     getAXID(newObj.get());
 
+#if ENABLE(OILPAN)
+    m_inlineTextBoxObjectMapping.set(inlineTextBox, newObj);
+#else
     m_inlineTextBoxObjectMapping.set(inlineTextBox, newObj->axObjectID());
+#endif
     m_objects.set(newObj->axObjectID(), newObj);
     newObj->init();
     newObj->setLastKnownIsIgnoredValue(newObj->accessibilityIsIgnored());
 
     return newObj.get();
 }
 
 AXObject* AXObjectCacheImpl::rootObject()
 {
     if (!accessibilityEnabled())
         return 0;
 
     return getOrCreate(m_document.view());
 }
 
 AXObject* AXObjectCacheImpl::getOrCreate(AccessibilityRole role)
 {
-    RefPtr<AXObject> obj = nullptr;
+    RefPtrWillBeRawPtr<AXObject> obj = nullptr;
 
     // will be filled in...
     switch (role) {
     case ImageMapLinkRole:
         obj = AXImageMapLink::create(this);
         break;
     case ColumnRole:
         obj = AXTableColumn::create(this);
         break;
     case TableHeaderContainerRole:
@@ skipping 18 matching lines @@
     if (obj)
         getAXID(obj.get());
     else
         return 0;
 
     m_objects.set(obj->axObjectID(), obj);
     obj->init();
     return obj.get();
 }
 
+#if ENABLE(OILPAN)
+void AXObjectCacheImpl::remove(AXObject* obj)
+{
+    if (!obj)
+        return;
+
+    obj->detach();

[haraken, 2015/05/28 10:48:41]

I think you still need to explicitly remove the object from m_objects, even
though m_objects is a hashmap of weak members. Otherwise, the detach state and
the contents of the hashmap will be inconsistent until the next GC is triggered.

(But as commented in the header file, I don't fully understand why m_objects is
a hashmap of weak members, not members.)

+}
+#else
 void AXObjectCacheImpl::remove(AXID axID)
 {
     if (!axID)
         return;
 
     // first fetch object to operate some cleanup functions on it
     AXObject* obj = m_objects.get(axID);
     if (!obj)
         return;
 
     obj->detach();
     removeAXID(obj);
 
     // finally remove the object
     if (!m_objects.take(axID))
         return;
 
     ASSERT(m_objects.size() >= m_idsInUse.size());
 }
+#endif
 
 void AXObjectCacheImpl::remove(LayoutObject* layoutObject)
 {
     if (!layoutObject)
         return;
 
+#if ENABLE(OILPAN)
+    AXObject* axLayoutObject = m_layoutObjectMapping.get(layoutObject);
+    if (axLayoutObject) {
+        axLayoutObject->detach();
+        remove(axLayoutObject);
+    }
+#else
     AXID axID = m_layoutObjectMapping.get(layoutObject);
     remove(axID);
+#endif
     m_layoutObjectMapping.remove(layoutObject);
 }
 
 void AXObjectCacheImpl::remove(Node* node)
 {
     if (!node)
         return;
 
     removeNodeForUse(node);
 
     // This is all safe even if we didn't have a mapping.
+#if ENABLE(OILPAN)
+    remove(m_nodeObjectMapping.get(node));
+#else
     AXID axID = m_nodeObjectMapping.get(node);
     remove(axID);
+#endif
     m_nodeObjectMapping.remove(node);
 
     if (node->layoutObject()) {
         remove(node->layoutObject());
         return;
     }
 }
 
 void AXObjectCacheImpl::remove(Widget* view)
 {
     if (!view)
         return;
 
+#if ENABLE(OILPAN)
+    remove(m_widgetObjectMapping.get(view));
+#else
     AXID axID = m_widgetObjectMapping.get(view);
     remove(axID);
+#endif
     m_widgetObjectMapping.remove(view);
 }
 
 void AXObjectCacheImpl::remove(AbstractInlineTextBox* inlineTextBox)
 {
     if (!inlineTextBox)
         return;
 
+#if ENABLE(OILPAN)
+    remove(m_inlineTextBoxObjectMapping.get(inlineTextBox));
+#else
     AXID axID = m_inlineTextBoxObjectMapping.get(inlineTextBox);
     remove(axID);
+#endif
     m_inlineTextBoxObjectMapping.remove(inlineTextBox);
 }
 
-// FIXME: Oilpan: Use a weak hashmap for this instead.
 void AXObjectCacheImpl::clearWeakMembers(Visitor* visitor)
 {
     Vector<Node*> deadNodes;
-    for (HashMap<Node*, AXID>::iterator it = m_nodeObjectMapping.begin(); it != m_nodeObjectMapping.end(); ++it) {
-        if (!visitor->isHeapObjectAlive(it->key))
-            deadNodes.append(it->key);
+    for (auto& entry : m_nodeObjectMapping) {

[haraken, 2015/05/28 10:48:41]

Why is this needed? The weak processing should have been completed when you
reach here, because m_nodeObjectMapping is a hashmap of weak members. This
manual weak processing was needed before this CL because it was a hashmap of raw
pointers.

+        if (!visitor->isHeapObjectAlive(entry.key))
+            deadNodes.append(entry.key);
     }
     for (unsigned i = 0; i < deadNodes.size(); ++i)
         remove(deadNodes[i]);
 
     Vector<Widget*> deadWidgets;
-    for (HashMap<Widget*, AXID>::iterator it = m_widgetObjectMapping.begin();
-        it != m_widgetObjectMapping.end(); ++it) {
-        if (!visitor->isHeapObjectAlive(it->key))
-            deadWidgets.append(it->key);
+    for (auto& entry : m_widgetObjectMapping) {

[haraken, 2015/05/28 10:48:41]

Ditto. This weak processing won't be needed.

+        if (!visitor->isHeapObjectAlive(entry.key))
+            deadWidgets.append(entry.key);
     }
     for (unsigned i = 0; i < deadWidgets.size(); ++i)
         remove(deadWidgets[i]);
+
+#if ENABLE(OILPAN)

[haraken, 2015/05/28 10:48:41]

Remove the #if flag. AXObjectCacheImpl::clearWeakMembers is called only in
oilpan builds. Instead you can add the #if flag on clearWeakMembers.

+    Vector<AXID> deadIds;
+    for (auto& entry : m_objects) {
+        if (entry.value)

[haraken, 2015/05/28 10:48:41]

This should be !visitor->isHeapObjectAlive(entry.value).

However, I don't fully understand why m_objects needs to be a hashmap of weak
members, not members.

+            deadIds.append(entry.key);
+    }
+    for (auto& axID : deadIds) {
+        m_objects.take(axID);
+    }
+#endif
 }

[haraken, 2015/05/28 10:48:41]

If I'm not missing something, I guess we can remove the clearWeakMembers
completely.

 
 AXID AXObjectCacheImpl::platformGenerateAXID() const
 {
     static AXID lastUsedID = 0;
 
     // Generate a new ID.
     AXID objID = lastUsedID;
     do {
         ++objID;
     } while (!objID || HashTraits<AXID>::isDeletedValue(objID) || m_idsInUse.contains(objID));
@@ skipping 13 matching lines @@
     }
 
     objID = platformGenerateAXID();
 
     m_idsInUse.add(objID);
     obj->setAXObjectID(objID);
 
     return objID;
 }
 
+#if !ENABLE(OILPAN)
 void AXObjectCacheImpl::removeAXID(AXObject* object)
 {
     if (!object)
         return;
 
     AXID objID = object->axObjectID();
     if (!objID)
         return;
-    ASSERT(!HashTraits<AXID>::isDeletedValue(objID));
-    ASSERT(m_idsInUse.contains(objID));
+    removeAXID(objID);
     object->setAXObjectID(0);
-    m_idsInUse.remove(objID);
+}
+#endif
+
+void AXObjectCacheImpl::removeAXID(AXID axID)
+{
+    if (!axID)
+        return;
+    ASSERT(!HashTraits<AXID>::isDeletedValue(axID));
+    ASSERT(m_idsInUse.contains(axID));
+    m_idsInUse.remove(axID);
 }
 
 void AXObjectCacheImpl::selectionChanged(Node* node)
 {
     // Find the nearest ancestor that already has an accessibility object, since we
     // might be in the middle of a layout.
     while (node) {
         if (AXObject* obj = get(node)) {
             obj->selectionChanged();
             return;
@@ skipping 506 matching lines @@
     AXObject* obj = getOrCreate(element);
     if (!obj)
         return;
 
     obj->setElementRect(rect);
 }
 
 DEFINE_TRACE(AXObjectCacheImpl)
 {
     AXObjectCache::trace(visitor);
+#if ENABLE(OILPAN)
+    visitor->trace(m_objects);
+    visitor->trace(m_layoutObjectMapping);
+    visitor->trace(m_widgetObjectMapping);
+    visitor->trace(m_nodeObjectMapping);
+    visitor->trace(m_inlineTextBoxObjectMapping);
+    visitor->trace(m_notificationsToPost);
+    visitor->trace(m_textMarkerNodes);
     visitor->template registerWeakMembers<AXObjectCacheImpl, &AXObjectCacheImpl::clearWeakMembers>(this);
+#endif
 }
 
 } // namespace blink