Changeset 263572 in webkit

Timestamp:

Jun 26, 2020 12:20:06 PM (4 years ago)

Author:

Andres Gonzalez

Message:

Access to AXIsolatedTree:m_readerThreadNodeMap should happen only on the secondary AX thread.
​https://bugs.webkit.org/show_bug.cgi?id=213575

Reviewed by Chris Fleizach.

After calling AXObjectCache::initializeSecondaryAXThread, there may
still be some client requests coming in on the main thread. Thus
AXIsolatedTree::applyPendingchanges and nodeForID can be called on the
main thread. Since these two methods access the member variable
m_readerThreadNodeMap which should be accessed only on the secondary AX
thread, we now check for this condition and bail out if needed.

• accessibility/isolatedtree/AXIsolatedTree.cpp:

(WebCore::AXIsolatedTree::nodeForID const):
(WebCore::AXIsolatedTree::applyPendingChanges):

• accessibility/isolatedtree/AXIsolatedTree.h: Reordered private methods

above member variables to comply with WebKit coding conventions,
pointed out by Darin Adler in review for ​https://bugs.webkit.org/show_bug.cgi?id=213435.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• accessibility/isolatedtree/AXIsolatedTree.cpp (modified) (2 diffs)
• accessibility/isolatedtree/AXIsolatedTree.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-06-26  Andres Gonzalez  <andresg_22@apple.com>
+
+        Access to AXIsolatedTree:m_readerThreadNodeMap should happen only on the secondary AX thread.
+        https://bugs.webkit.org/show_bug.cgi?id=213575
+
+        Reviewed by Chris Fleizach.
+
+        After calling AXObjectCache::initializeSecondaryAXThread, there may
+        still be some client requests coming in on the main thread. Thus
+        AXIsolatedTree::applyPendingchanges and nodeForID can be called on the
+        main thread. Since these two methods access the member variable
+        m_readerThreadNodeMap which should be accessed only on the secondary AX
+        thread, we now check for this condition and bail out if needed.
+
+        * accessibility/isolatedtree/AXIsolatedTree.cpp:
+        (WebCore::AXIsolatedTree::nodeForID const):
+        (WebCore::AXIsolatedTree::applyPendingChanges):
+        * accessibility/isolatedtree/AXIsolatedTree.h: Reordered private methods
+        above member variables to comply with WebKit coding conventions,
+        pointed out by Darin Adler in review for https://bugs.webkit.org/show_bug.cgi?id=213435.
+
 2020-06-26  Andres Gonzalez  <andresg_22@apple.com>
 

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

@@ -141 +141 @@
 RefPtr<AXIsolatedObject> AXIsolatedTree::nodeForID(AXID axID) const
 {
-    // FIXME: The following ASSERT should be met but it is commented out at the
-    // moment because of <rdar://problem/63985646> After calling _AXUIElementUseSecondaryAXThread(true),
-    // still receives client request on main thread.
-    // ASSERT(axObjectCache()->canUseSecondaryAXThread() ? !isMainThread() : isMainThread());
+    // In isolated tree mode 2, only access m_readerThreadNodeMap on the AX thread.
+    if (axObjectCache()->canUseSecondaryAXThread() && isMainThread())
+        return nullptr;
+
     return axID != InvalidAXID ? m_readerThreadNodeMap.get(axID) : nullptr;
 }
@@ -394 +394 @@
 {
     AXTRACE("AXIsolatedTree::applyPendingChanges");
+
+    // In isolated tree mode 2, only apply pending changes on the AX thread.
+    if (axObjectCache()->canUseSecondaryAXThread() && isMainThread())
+        return;
+
     LockHolder locker { m_changeLogLock };
 

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

@@ -99 +99 @@
     void clear();
 
-    AXIsolatedTreeID m_treeID;
     static HashMap<AXIsolatedTreeID, Ref<AXIsolatedTree>>& treeIDCache();
     static HashMap<PageIdentifier, Ref<AXIsolatedTree>>& treePageCache();
@@ -107 +106 @@
     // Queues all pending additions to the tree as the result of a subtree generation.
     void appendNodeChanges(Vector<NodeChange>&&);
+    // Called on main thread to update both m_nodeMap and m_pendingChildrenUpdates.
+    void updateChildrenIDs(AXID axID, Vector<AXID>&& childrenIDs);
 
+    AXIsolatedTreeID m_treeID;
     AXObjectCache* m_axObjectCache { nullptr };
 
@@ -124 +126 @@
     AXID m_focusedNodeID { InvalidAXID };
     Lock m_changeLogLock;
-
-    // Called on main thread to updates both m_nodeMap and m_pendingChildrenUpdates.
-    void updateChildrenIDs(AXID axID, Vector<AXID>&& childrenIDs);
 };