AX: Adding a symmetric relation is calling AXIsolatedTree::addUnconne…

…ctedNode unnecessarily.

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

Reviewed by Tyler Wilcock.

Follow up to https://bugs.webkit.org/show_bug.cgi?id=266608.

Since adding a symmetric relationship involves the same objects as the initial relation, there is no need to call AXIsolatedTree::addUnconnectedNode for symmetric relations. This addition to update the isolated tree belongs to the addRelation overload that takes AX objects. This patch also adds a check to ensure that the objects being related are still alive when adding a symmetric relationship.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::addRelation):

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

Source/WebCore/accessibility/AXObjectCache.cpp

@@ -4618,18 +4618,7 @@ void AXObjectCache::addRelation(Element* origin, Element* target, AXRelationType
         ASSERT_NOT_REACHED();
         return;
     }
-    RefPtr relationOrigin = getOrCreate(origin, IsPartOfRelation::Yes);
-    RefPtr relationTarget = getOrCreate(target, IsPartOfRelation::Yes);
-    addRelation(relationOrigin.get(), relationTarget.get(), relationType);
-
-#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
-    if (auto tree = AXIsolatedTree::treeForPageID(m_pageID)) {
-        if (relationOrigin && relationOrigin->accessibilityIsIgnored())
-            tree->addUnconnectedNode(relationOrigin.releaseNonNull());
-        if (relationTarget && relationTarget->accessibilityIsIgnored())
-            tree->addUnconnectedNode(relationTarget.releaseNonNull());
-    }
-#endif
+    addRelation(getOrCreate(origin, IsPartOfRelation::Yes), getOrCreate(target, IsPartOfRelation::Yes), relationType);
 }
 
 static bool canHaveRelations(Element& element)
@@ -4663,45 +4652,57 @@ void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject
     if (relationCausesCycle(origin, target, relationType))
         return;
 
-    auto relationsIterator = m_relations.find(origin->objectID());
+    AXID originID = origin->objectID();
+    AXID targetID = target->objectID();
+    auto relationsIterator = m_relations.find(originID);
     if (relationsIterator == m_relations.end()) {
         // No relations for this object, add the first one.
-        m_relations.add(origin->objectID(), AXRelations { { enumToUnderlyingType(relationType), { target->objectID() } } });
+        m_relations.add(originID, AXRelations { { enumToUnderlyingType(relationType), { targetID } } });
     } else if (auto targetsIterator = relationsIterator->value.find(enumToUnderlyingType(relationType)); targetsIterator == relationsIterator->value.end()) {
         // No relation of this type for this object, add the first one.
-        relationsIterator->value.add(enumToUnderlyingType(relationType), ListHashSet { target->objectID() });
+        relationsIterator->value.add(enumToUnderlyingType(relationType), ListHashSet { targetID });
     } 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.add(target->objectID());
+        targetsIterator->value.add(targetID);
     }
-    m_relationTargets.add(target->objectID());
+    m_relationTargets.add(targetID);
 
     if (relationType == AXRelationType::OwnerFor) {
         // First find and clear the old owner.
-        auto targetID = target->objectID();
         for (auto oldOwnerIterator = m_relations.begin(); oldOwnerIterator != m_relations.end(); ++oldOwnerIterator) {
-            if (oldOwnerIterator->key == origin->objectID())
+            if (oldOwnerIterator->key == originID)
                 continue;
-            
+
             removeRelationByID(oldOwnerIterator->key, targetID, AXRelationType::OwnerFor);
             if (auto* oldOwner = objectForID(oldOwnerIterator->key))
                 childrenChanged(oldOwner);
         }
-        
+
         childrenChanged(origin);
     } else if (relationType == AXRelationType::OwnedBy) {
         if (auto* parentObject = origin->parentObjectUnignored())
             childrenChanged(parentObject);
     }
 
-    if (addSymmetricRelation == AddSymmetricRelation::Yes) {
+    if (addSymmetricRelation == AddSymmetricRelation::Yes
+        && m_objects.contains(originID) && m_objects.contains(targetID)) {
+        // If the IDs are still in the m_objects map, the objects should be still alive.
         if (auto symmetric = symmetricRelation(relationType); symmetric != AXRelationType::None)
             addRelation(target, origin, symmetric, AddSymmetricRelation::No);
+
+#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
+        if (auto tree = AXIsolatedTree::treeForPageID(m_pageID)) {
+            if (origin && origin->accessibilityIsIgnored())
+                tree->addUnconnectedNode(*origin);
+            if (target && target->accessibilityIsIgnored())
+                tree->addUnconnectedNode(*target);
+        }
+#endif
     }
 }