Changeset 263378 in webkit

Timestamp:

Jun 22, 2020 4:54:58 PM (4 years ago)

Author:

Andres Gonzalez

Message:

AXIsolatedTree::generateSubtree should properly assign the generated subtree to its parent node.
​https://bugs.webkit.org/show_bug.cgi?id=213435

Reviewed by Darin Adler.

AXIsolatedTree::generateSubtree now properly updates the children IDs
of the parent node of the subtree being generated.

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::generateIsolatedTree):

• accessibility/isolatedtree/AXIsolatedTree.cpp:

(WebCore::AXIsolatedTree::updateChildrenIDs):
(WebCore::AXIsolatedTree::generateSubtree): Takes the parent object
instead of the parent ID. This allows to retrieve the children IDs of
the parent object.
(WebCore::AXIsolatedTree::createSubtree):
(WebCore::AXIsolatedTree::updateSubtree):
(WebCore::AXIsolatedTree::updateChildren):
(WebCore::AXIsolatedTree::setRootNode):
(WebCore::AXIsolatedTree::removeSubtree):
(WebCore::AXIsolatedTree::appendNodeChanges): R-value parameter instead
of by reference.

• accessibility/isolatedtree/AXIsolatedTree.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• accessibility/AXObjectCache.cpp (modified) (1 diff)
• accessibility/isolatedtree/AXIsolatedTree.cpp (modified) (7 diffs)
• accessibility/isolatedtree/AXIsolatedTree.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-06-22  Andres Gonzalez  <andresg_22@apple.com>
+
+        AXIsolatedTree::generateSubtree should properly assign the generated subtree to its parent node.
+        https://bugs.webkit.org/show_bug.cgi?id=213435
+
+        Reviewed by Darin Adler.
+
+        AXIsolatedTree::generateSubtree now properly updates the children IDs
+        of the parent node of the subtree being generated.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::generateIsolatedTree):
+        * accessibility/isolatedtree/AXIsolatedTree.cpp:
+        (WebCore::AXIsolatedTree::updateChildrenIDs):
+        (WebCore::AXIsolatedTree::generateSubtree): Takes the parent object
+        instead of the parent ID. This allows to retrieve the children IDs of
+        the parent object.
+        (WebCore::AXIsolatedTree::createSubtree):
+        (WebCore::AXIsolatedTree::updateSubtree):
+        (WebCore::AXIsolatedTree::updateChildren):
+        (WebCore::AXIsolatedTree::setRootNode):
+        (WebCore::AXIsolatedTree::removeSubtree):
+        (WebCore::AXIsolatedTree::appendNodeChanges): R-value parameter instead
+        of by reference.
+        * accessibility/isolatedtree/AXIsolatedTree.h:
+
 2020-06-22  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -3128 +3128 @@
     auto* axRoot = axObjectCache->getOrCreate(document.view());
     if (axRoot)
-        tree->generateSubtree(*axRoot, InvalidAXID, true);
+        tree->generateSubtree(*axRoot, nullptr, true);
 
     auto* axFocus = axObjectCache->focusedObject(document);

trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

@@ -162 +162 @@
 }
 
-void AXIsolatedTree::generateSubtree(AXCoreObject& axObject, AXID parentID, bool attachWrapper)
+void AXIsolatedTree::updateChildrenIDs(AXID axID, Vector<AXID>&& childrenIDs)
+{
+    ASSERT(isMainThread());
+    ASSERT(m_changeLogLock.isLocked());
+
+    m_nodeMap.set(axID, childrenIDs);
+    m_pendingChildrenUpdates.append(std::make_pair(axID, WTFMove(childrenIDs)));
+}
+
+void AXIsolatedTree::generateSubtree(AXCoreObject& axObject, AXCoreObject* axParent, bool attachWrapper)
 {
     AXTRACE("AXIsolatedTree::generateSubtree");
     ASSERT(isMainThread());
+
     Vector<NodeChange> nodeChanges;
-    auto object = createSubtree(axObject, parentID, attachWrapper, nodeChanges);
-    LockHolder locker { m_changeLogLock };
-    appendNodeChanges(nodeChanges);
-
-    if (parentID == InvalidAXID)
+    auto object = createSubtree(axObject, axParent ? axParent->objectID() : InvalidAXID, attachWrapper, nodeChanges);
+    LockHolder locker { m_changeLogLock };
+    appendNodeChanges(WTFMove(nodeChanges));
+
+    if (!axParent)
         setRootNode(object.ptr());
-    // FIXME: else attach the newly created subtree to its parent.
+    else
+        updateChildrenIDs(axParent->objectID(), axParent->childrenIDs());
 }
 
@@ -203 +214 @@
         childrenIDs.append(child->objectID());
     }
-    m_nodeMap.set(object->objectID(), childrenIDs);
+
     {
         LockHolder locker { m_changeLogLock };
-        m_pendingChildrenUpdates.append(std::make_pair(object->objectID(), childrenIDs));
+        updateChildrenIDs(object->objectID(), WTFMove(childrenIDs));
     }
 
@@ -238 +249 @@
     AXLOG(&axObject);
     ASSERT(isMainThread());
+
     removeSubtree(axObject.objectID());
-    auto* axParent = axObject.parentObject();
-    AXID parentID = axParent ? axParent->objectID() : InvalidAXID;
-    generateSubtree(axObject, parentID, false);
+    generateSubtree(axObject, axObject.parentObject(), false);
 }
 
@@ -287 +297 @@
             AXLOG("Adding a new child for:");
             AXLOG(axChildren[i]);
-            generateSubtree(*axChildren[i], axAncestor->objectID(), true);
+            generateSubtree(*axChildren[i], axAncestor, true);
         }
     }
@@ -297 +307 @@
 
     // Lastly, make the children IDs of the isolated object to be the same as the AXObject's.
-    m_nodeMap.set(axAncestor->objectID(), axChildrenIDs);
-    {
-        LockHolder locker { m_changeLogLock };
-        m_pendingChildrenUpdates.append(std::make_pair(axAncestor->objectID(), axChildrenIDs));
-    }
+    LockHolder locker { m_changeLogLock };
+    updateChildrenIDs(axAncestor->objectID(), WTFMove(axChildrenIDs));
 }
 
@@ -328 +335 @@
     ASSERT(isMainThread());
     ASSERT(m_changeLogLock.isLocked());
+    ASSERT(!m_rootNode);
     ASSERT(root);
 
@@ -371 +379 @@
 }
 
-void AXIsolatedTree::appendNodeChanges(const Vector<NodeChange>& changes)
+void AXIsolatedTree::appendNodeChanges(Vector<NodeChange>&& changes)
 {
     AXTRACE("AXIsolatedTree::appendNodeChanges");
     ASSERT(isMainThread());
-    m_pendingAppends.appendVector(changes);
+    ASSERT(m_changeLogLock.isLocked());
+
+    m_pendingAppends.appendVector(WTFMove(changes));
 }
 

trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

@@ -73 +73 @@
     };
 
-    void generateSubtree(AXCoreObject&, AXID parentID, bool attachWrapper);
+    void generateSubtree(AXCoreObject&, AXCoreObject*, bool attachWrapper);
     void updateNode(AXCoreObject&);
     void updateSubtree(AXCoreObject&);
@@ -99 +99 @@
     void clear();
 
+    AXIsolatedTreeID m_treeID;
     static HashMap<AXIsolatedTreeID, Ref<AXIsolatedTree>>& treeIDCache();
     static HashMap<PageIdentifier, Ref<AXIsolatedTree>>& treePageCache();
@@ -105 +106 @@
     Ref<AXIsolatedObject> createSubtree(AXCoreObject&, AXID parentID, bool attachWrapper, Vector<NodeChange>&);
     // Queues all pending additions to the tree as the result of a subtree generation.
-    void appendNodeChanges(const Vector<NodeChange>&);
+    void appendNodeChanges(Vector<NodeChange>&&);
 
     AXObjectCache* m_axObjectCache { nullptr };
@@ -121 +122 @@
     Vector<std::pair<AXID, Vector<AXID>>> m_pendingChildrenUpdates;
     AXID m_pendingFocusedNodeID { InvalidAXID };
+    AXID m_focusedNodeID { InvalidAXID };
     Lock m_changeLogLock;
 
-    AXIsolatedTreeID m_treeID;
-    AXID m_focusedNodeID { InvalidAXID };
+    // Called on main thread to updates both m_nodeMap and m_pendingChildrenUpdates.
+    void updateChildrenIDs(AXID axID, Vector<AXID>&& childrenIDs);
 };