AX: Duplicated relationships in AXObjectCache::m_relations.

https://bugs.webkit.org/show_bug.cgi?id=265044
<rdar://problem/118566254>

Reviewed by Tyler Wilcock.

Under certain conditions it is possible to add more than one entry to the AXObjectCache::m_relations data structure for the same pair of objects and the same relation type. To avoid this, this patch replaces the Vector in AXRelation with a ListHashSet. This also has some performance gain, since find and remove opeartions are o(1) in ListHashSets.

In addition, changed the enum AddingSymmetricRelation to AddSymmetricRelation which makes the code more readable.

* Source/WebCore/accessibility/AXCoreObject.h:
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::addRelation):
(WebCore::AXObjectCache::removeRelationByID):
(WebCore::AXObjectCache::relatedObjectIDsFor):
(WebCore::AXObjectCache::objectsForIDs const): Deleted.
* Source/WebCore/accessibility/AXObjectCache.h:
(WebCore::AXObjectCache::objectsForIDs const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::objectsForIDs):
(WebCore::AXIsolatedTree::relatedObjectIDsFor):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

Canonical link: https://commits.webkit.org/270916@main

Source/WebCore/accessibility/AXCoreObject.h

@@ -753,7 +753,7 @@ enum class AXRelationType : uint8_t {
     OwnedBy,
     OwnerFor,
 };
-using AXRelations = HashMap<AXRelationType, Vector<AXID>, DefaultHash<uint8_t>, WTF::UnsignedWithZeroKeyHashTraits<uint8_t>>;
+using AXRelations = HashMap<AXRelationType, ListHashSet<AXID>, DefaultHash<uint8_t>, WTF::UnsignedWithZeroKeyHashTraits<uint8_t>>;
 
 enum class SpinButtonType : bool {
     // The spin button is standalone. It has no separate controls, and should receive and perform actions itself.

Source/WebCore/accessibility/AXObjectCache.cpp

@@ -1122,17 +1122,6 @@ AXID AXObjectCache::generateNewObjectID() const
     return axID;
 }
 
-Vector<RefPtr<AXCoreObject>> AXObjectCache::objectsForIDs(const Vector<AXID>& axIDs) const
-{
-    ASSERT(isMainThread());
-
-    return WTF::compactMap(axIDs, [&](auto& axID) -> std::optional<RefPtr<AXCoreObject>> {
-        if (auto* object = objectForID(axID))
-            return RefPtr { object };
-        return std::nullopt;
-    });
-}
-
 AXID AXObjectCache::getAXID(AccessibilityObject* object)
 {
     // check for already-assigned ID
@@ -4634,7 +4623,7 @@ static bool relationCausesCycle(AccessibilityObject* origin, AccessibilityObject
     return false;
 }
 
-void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject* target, AXRelationType relationType, AddingSymmetricRelation addingSymmetricRelation)
+void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject* target, AXRelationType relationType, AddSymmetricRelation addSymmetricRelation)
 {
     if (!validRelation(origin, target, relationType))
         return;
@@ -4648,15 +4637,15 @@ void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject
         m_relations.add(origin->objectID(), AXRelations { { static_cast<uint8_t>(relationType), { target->objectID() } } });
     } else if (auto targetsIterator = relationsIterator->value.find(static_cast<uint8_t>(relationType)); targetsIterator == relationsIterator->value.end()) {
         // No relation of this type for this object, add the first one.
-        relationsIterator->value.add(static_cast<uint8_t>(relationType), Vector<AXID> { target->objectID() });
+        relationsIterator->value.add(static_cast<uint8_t>(relationType), ListHashSet { target->objectID() });
     } else {
         // There are already relations of this type for the object. Add the new relation.
         if (relationType == AXRelationType::ActiveDescendant
             || relationType == AXRelationType::OwnedBy) {
             // There should be only one active descendant and only one owner. Enforce that by removing any existing targets.
             targetsIterator->value.clear();
         }
-        targetsIterator->value.append(target->objectID());
+        targetsIterator->value.add(target->objectID());
     }
     m_relationTargets.add(target->objectID());
 
@@ -4678,9 +4667,9 @@ void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject
             childrenChanged(parentObject);
     }
 
-    if (addingSymmetricRelation == AddingSymmetricRelation::No) {
+    if (addSymmetricRelation == AddSymmetricRelation::Yes) {
         if (auto symmetric = symmetricRelation(relationType); symmetric != AXRelationType::None)
-            addRelation(target, origin, symmetric, AddingSymmetricRelation::Yes);
+            addRelation(target, origin, symmetric, AddSymmetricRelation::No);
     }
 }
 
@@ -4715,10 +4704,7 @@ void AXObjectCache::removeRelationByID(AXID originID, AXID targetID, AXRelationT
     auto targetsIterator = relationsIterator->value.find(static_cast<uint8_t>(relationType));
     if (targetsIterator == relationsIterator->value.end())
         return;
-
-    targetsIterator->value.removeAllMatching([targetID] (const AXID axID) {
-        return axID == targetID;
-    });
+    targetsIterator->value.remove(targetID);
 }
 
 void AXObjectCache::updateRelationsIfNeeded()
@@ -4834,7 +4820,7 @@ const HashSet<AXID>& AXObjectCache::relationTargetIDs()
     return m_relationTargets;
 }
 
-std::optional<Vector<AXID>> AXObjectCache::relatedObjectIDsFor(const AXCoreObject& object, AXRelationType relationType)
+std::optional<ListHashSet<AXID>> AXObjectCache::relatedObjectIDsFor(const AXCoreObject& object, AXRelationType relationType)
 {
     updateRelationsIfNeeded();
     auto relationsIterator = m_relations.find(object.objectID());

Source/WebCore/accessibility/AXObjectCache.h

@@ -277,7 +277,7 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
     bool nodeIsTextControl(const Node*);
 
     AccessibilityObject* objectForID(const AXID id) const { return m_objects.get(id); }
-    Vector<RefPtr<AXCoreObject>> objectsForIDs(const Vector<AXID>&) const;
+    template<typename U> Vector<RefPtr<AXCoreObject>> objectsForIDs(const U&) const;
 
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
     void onPaint(const RenderObject&, IntRect&&) const;
@@ -483,7 +483,7 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
     AXTreeData treeData();
 
     // Returns the IDs of the objects that relate to the given object with the specified relationship.
-    std::optional<Vector<AXID>> relatedObjectIDsFor(const AXCoreObject&, AXRelationType);
+    std::optional<ListHashSet<AXID>> relatedObjectIDsFor(const AXCoreObject&, AXRelationType);
     void updateRelations(Element&, const QualifiedName&);
 
 #if PLATFORM(IOS_FAMILY)
@@ -632,10 +632,10 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
     // Relationships between objects.
     static Vector<QualifiedName>& relationAttributes();
     static AXRelationType attributeToRelationType(const QualifiedName&);
-    enum class AddingSymmetricRelation : bool { No, Yes };
+    enum class AddSymmetricRelation : bool { No, Yes };
     static AXRelationType symmetricRelation(AXRelationType);
     void addRelation(Element*, Element*, AXRelationType);
-    void addRelation(AccessibilityObject*, AccessibilityObject*, AXRelationType, AddingSymmetricRelation = AddingSymmetricRelation::No);
+    void addRelation(AccessibilityObject*, AccessibilityObject*, AXRelationType, AddSymmetricRelation = AddSymmetricRelation::Yes);
     void removeRelationByID(AXID originID, AXID targetID, AXRelationType);
     void addRelations(Element&, const QualifiedName&);
     void removeRelations(Element&, AXRelationType);
@@ -732,6 +732,18 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
 #endif
 };
 
+template<typename U>
+inline Vector<RefPtr<AXCoreObject>> AXObjectCache::objectsForIDs(const U& axIDs) const
+{
+    ASSERT(isMainThread());
+
+    return WTF::compactMap(axIDs, [&](auto& axID) -> std::optional<RefPtr<AXCoreObject>> {
+        if (auto* object = objectForID(axID))
+            return RefPtr { object };
+        return std::nullopt;
+    });
+}
+
 class AXAttributeCacheEnabler
 {
 public:

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

@@ -222,7 +222,8 @@ RefPtr<AXIsolatedObject> AXIsolatedTree::objectForID(const AXID axID) const
     return axID.isValid() ? m_readerThreadNodeMap.get(axID) : nullptr;
 }
 
-Vector<RefPtr<AXCoreObject>> AXIsolatedTree::objectsForIDs(const Vector<AXID>& axIDs)
+template<typename U>
+Vector<RefPtr<AXCoreObject>> AXIsolatedTree::objectsForIDs(const U& axIDs)
 {
     AXTRACE("AXIsolatedTree::objectsForIDs"_s);
     ASSERT(!isMainThread());
@@ -1074,7 +1075,7 @@ void AXIsolatedTree::removeSubtreeFromNodeMap(AXID objectID, AccessibilityObject
     }
 }
 
-std::optional<Vector<AXID>> AXIsolatedTree::relatedObjectIDsFor(const AXIsolatedObject& object, AXRelationType relationType)
+std::optional<ListHashSet<AXID>> AXIsolatedTree::relatedObjectIDsFor(const AXIsolatedObject& object, AXRelationType relationType)
 {
     ASSERT(!isMainThread());
 

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

@@ -282,7 +282,7 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     WEBCORE_EXPORT RefPtr<AXIsolatedObject> focusedNode();
 
     RefPtr<AXIsolatedObject> objectForID(const AXID) const;
-    Vector<RefPtr<AXCoreObject>> objectsForIDs(const Vector<AXID>&);
+    template<typename U> Vector<RefPtr<AXCoreObject>> objectsForIDs(const U&);
 
     void generateSubtree(AccessibilityObject&);
     void labelCreated(AccessibilityObject&);
@@ -313,7 +313,7 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     void setFocusedNodeID(AXID);
 
     // Relationships between objects.
-    std::optional<Vector<AXID>> relatedObjectIDsFor(const AXIsolatedObject&, AXRelationType);
+    std::optional<ListHashSet<AXID>> relatedObjectIDsFor(const AXIsolatedObject&, AXRelationType);
     void relationsNeedUpdate(bool needUpdate) { m_relationsNeedUpdate = needUpdate; }
 
     // Called on AX thread from WebAccessibilityObjectWrapper methods.