AX: Isolated objects in the full tree may be accessed before the tree…

… is completely built.

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

Reviewed by Tyler Wilcock.

When building the isolated tree for a given page, we first build a temporary tree consisting only of the ScrollView and the WebArea objects. The purpose of this temporary tree is to serve client's requests while the full tree is being built, which will keep the main thread busy and client's requests that required the main thread blocked. However that goal is not achieved in cases where the following sequence of events occur:
1. Clients holds a reference to the ScrollView object wrapper from the temporary tree. Let that wrapper be called W.
2. During building the full tree a new isolated object is created for the ScrollView object and attached to W since it represents the same underlying liveobject.
3. From that point on, all calls into W access the object that is part of the full isolated tree, not the temporary tree object.
This happens during the construction of the full tree. This issue defeats the purpose of the temporary tree. See comment in bugzilla for a log output showing the problem.

This patch solves the problem by not attaching the wrappers to newly created isolated objects immediately upon creation, but instead attaching all wrappers once the entire tree is built and the temporary tree is about to be replaced by the full tree. This apply to isolated objects whose wrappers were attached on the main thread. The same principle is applied to tree updates.

* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::nodeChangeForObject):
(WebCore::AXIsolatedTree::queueAppendsAndRemovals):
(WebCore::AXIsolatedTree::applyPendingChanges):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

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

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

@@ -251,15 +251,8 @@ std::optional<AXIsolatedTree::NodeChange> AXIsolatedTree::nodeChangeForObject(Re
         return std::nullopt;
 
     auto object = AXIsolatedObject::create(axObject, this);
-    NodeChange nodeChange { object, nullptr };
-
     ASSERT(axObject->wrapper());
-    if (attachWrapper == AttachWrapper::OnMainThread)
-        object->attachPlatformWrapper(axObject->wrapper());
-    else {
-        // Set the wrapper in the NodeChange so that it is set on the AX thread.
-        nodeChange.wrapper = axObject->wrapper();
-    }
+    NodeChange nodeChange { object, axObject->wrapper(), attachWrapper };
 
     m_nodeMap.set(axObject->objectID(), ParentChildrenIDs { nodeChange.isolatedObject->parent(), axObject->childrenIDs() });
 
@@ -370,13 +363,18 @@ void AXIsolatedTree::queueAppendsAndRemovals(Vector<NodeChange>&& appends, Vecto
     ASSERT(isMainThread());
 
     Locker locker { m_changeLogLock };
-    for (const auto& append : appends)
+    for (const auto& append : appends) {
+        if (append.attachWrapper == AttachWrapper::OnMainThread)
+            append.isolatedObject->attachPlatformWrapper(append.wrapper.get());
         queueChange(append);
+    }
+
     auto parentUpdateIDs = std::exchange(m_needsParentUpdate, { });
     for (const auto& axID : parentUpdateIDs) {
         ASSERT_WITH_MESSAGE(m_nodeMap.contains(axID), "An object marked as needing a parent update should've had an entry in the node map by now. ID was %s", axID.loggingString().utf8().data());
         m_pendingParentUpdates.set(axID, m_nodeMap.get(axID).parentID);
     }
+
     queueRemovalsLocked(WTFMove(subtreeRemovals));
 }
 
@@ -1081,7 +1079,7 @@ void AXIsolatedTree::applyPendingChanges()
         if (!axID.isValid())
             continue;
 
-        auto& wrapper = item.wrapper ? item.wrapper : item.isolatedObject->wrapper();
+        auto& wrapper = item.attachWrapper == AttachWrapper::OnAXThread ? item.wrapper : item.isolatedObject->wrapper();
         if (!wrapper)
             continue;
 

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

@@ -279,15 +279,6 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     RefPtr<AXIsolatedObject> objectForID(const AXID) const;
     Vector<RefPtr<AXCoreObject>> objectsForIDs(const Vector<AXID>&);
 
-    struct NodeChange {
-        Ref<AXIsolatedObject> isolatedObject;
-#if PLATFORM(COCOA)
-        RetainPtr<AccessibilityObjectWrapper> wrapper;
-#elif USE(ATSPI)
-        RefPtr<AccessibilityObjectWrapper> wrapper;
-#endif
-    };
-
     void generateSubtree(AccessibilityObject&);
     void labelCreated(AccessibilityObject&);
     void updateNode(AccessibilityObject&);
@@ -346,7 +337,18 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     void createEmptyContent(AccessibilityObject&);
     constexpr bool isUpdatingSubtree() const { return m_rootOfSubtreeBeingUpdated; }
     constexpr void updatingSubtree(AccessibilityObject* axObject) { m_rootOfSubtreeBeingUpdated = axObject; }
+
     enum class AttachWrapper : bool { OnMainThread, OnAXThread };
+    struct NodeChange {
+        Ref<AXIsolatedObject> isolatedObject;
+#if PLATFORM(COCOA)
+        RetainPtr<AccessibilityObjectWrapper> wrapper;
+#elif USE(ATSPI)
+        RefPtr<AccessibilityObjectWrapper> wrapper;
+#endif
+        AttachWrapper attachWrapper { AttachWrapper::OnMainThread };
+    };
+
     std::optional<NodeChange> nodeChangeForObject(Ref<AccessibilityObject>, AttachWrapper = AttachWrapper::OnMainThread);
     void collectNodeChangesForSubtree(AXCoreObject&);
     bool isCollectingNodeChanges() const { return m_collectingNodeChangesAtTreeLevel > 0; }