AX: WebAccessibilityObjectWrapperMac and AXIsolatedTree do many unnec…

…essary isMainThread() calls

https://bugs.webkit.org/show_bug.cgi?id=270866
rdar://problem/124468129

Reviewed by Andres Gonzalez.

Many of our hottest codepaths (various wrapper functions, AXIsolatedTree::{objectForID, applyPendingChanges})
call isMainThread() either completely unnecessarily (i.e. in the case of the AXIsolatedTree functions), or
more than necessary. These show up in samples.

This patch downgrades the isMainThread() checks in AXIsolatedTree::{objectForID, applyPendingChanges} to ASSERT,
as no codepath exists for these to be called on the main-thread.

This patch also removes unnecessary calls to axBackingObject in WebAccessibilityObjectWrapperMac and instead
passes a single RefPtr object around (which can also be more correct, i.e. https://bugs.webkit.org/show_bug.cgi?id=267786).

Furthermore, -[WebAccessibilityObjectWrapper accessibilityPresenterProcessIdentifier] now directly calls presentingApplicationPID(),
rather than getting axBackingObject first and calling presentingApplicationPID() on that.

* Source/WebCore/accessibility/AXCoreObject.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::processID const): Deleted.
* Source/WebCore/accessibility/AccessibilityObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::detachRemoteParts):
(WebCore::AXIsolatedObject::updateBackingStore):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::objectForID const):
(WebCore::AXIsolatedTree::applyPendingChanges):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::processID const): Deleted.
* Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXPostNotificationWithUserInfo):
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.h:
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper _additionalAccessibilityAttributeNames:]):
(-[WebAccessibilityObjectWrapper ALLOW_DEPRECATED_IMPLEMENTATIONS_END]):
(-[WebAccessibilityObjectWrapper _associatedPluginParent]):
(-[WebAccessibilityObjectWrapper _associatedPluginParentWith:]):
(-[WebAccessibilityObjectWrapper _isEmptyGroup:]):
(-[WebAccessibilityObjectWrapper subrole]):
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
(-[WebAccessibilityObjectWrapper accessibilityPresenterProcessIdentifier]):
(-[WebAccessibilityObjectWrapper accessibilityArrayAttributeValues:index:maxCount:]):
(-[WebAccessibilityObjectWrapper additionalAccessibilityAttributeNames]): Deleted.
(-[WebAccessibilityObjectWrapper associatedPluginParent]): Deleted.
(-[WebAccessibilityObjectWrapper isEmptyGroup]): Deleted.

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

Source/WebCore/accessibility/AXCoreObject.h

@@ -38,7 +38,6 @@
 #include <variant>
 #include <wtf/HashSet.h>
 #include <wtf/ObjectIdentifier.h>
-#include <wtf/ProcessID.h>
 #include <wtf/RefCounted.h>
 #include <wtf/ThreadSafeWeakPtr.h>
 #include <wtf/WallTime.h>
@@ -809,7 +808,6 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
     void setObjectID(AXID axID) { m_id = axID; }
     AXID objectID() const { return m_id; }
     virtual AXID treeID() const = 0;
-    virtual ProcessID processID() const = 0;
 
     // When the corresponding WebCore object that this accessible object
     // represents is deleted, it must be detached.

Source/WebCore/accessibility/AccessibilityObject.cpp

@@ -85,7 +85,6 @@
 #include "RenderView.h"
 #include "RenderWidget.h"
 #include "RenderedPosition.h"
-#include "RuntimeApplicationChecks.h"
 #include "Settings.h"
 #include "TextCheckerClient.h"
 #include "TextCheckingHelper.h"
@@ -119,11 +118,6 @@ AXID AccessibilityObject::treeID() const
     return cache ? cache->treeID() : AXID();
 }
 
-inline ProcessID AccessibilityObject::processID() const
-{
-    return presentingApplicationPID();
-}
-
 String AccessibilityObject::dbg() const
 {
     String backingEntityDescription;

Source/WebCore/accessibility/AccessibilityObject.h

@@ -65,7 +65,6 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     virtual ~AccessibilityObject();
 
     AXID treeID() const final;
-    ProcessID processID() const override;
     String dbg() const final;
 
     // After constructing an AccessibilityObject, it must be given a

Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp

@@ -500,6 +500,8 @@ void AXIsolatedObject::setProperty(AXPropertyName propertyName, AXPropertyValueV
 
 void AXIsolatedObject::detachRemoteParts(AccessibilityDetachmentType)
 {
+    ASSERT(!isMainThread());
+
     for (const auto& childID : m_childrenIDs) {
         if (RefPtr child = tree()->objectForID(childID))
             child->detachFromParent();
@@ -1070,7 +1072,8 @@ void AXIsolatedObject::fillChildrenVectorForProperty(AXPropertyName propertyName
 
 void AXIsolatedObject::updateBackingStore()
 {
-    // This method can be called on either the main or the AX threads.
+    ASSERT(!isMainThread());
+
     if (RefPtr tree = this->tree())
         tree->applyPendingChanges();
     // AXIsolatedTree::applyPendingChanges can cause this object and / or the AXIsolatedTree to be destroyed.

Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h

@@ -56,7 +56,6 @@ class AXIsolatedObject final : public AXCoreObject {
     ~AXIsolatedObject();
 
     AXID treeID() const final { return tree()->treeID(); }
-    ProcessID processID() const final { return tree()->processID(); }
     String dbg() const final;
 
     AccessibilityRole roleValue() const final { return static_cast<AccessibilityRole>(intAttributeValue(AXPropertyName::RoleValue)); }

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

@@ -228,11 +228,8 @@ RefPtr<AXIsolatedTree> AXIsolatedTree::treeForPageID(PageIdentifier pageID)
 
 RefPtr<AXIsolatedObject> AXIsolatedTree::objectForID(const AXID axID) const
 {
-    // In isolated tree mode, only access m_readerThreadNodeMap on the AX thread.
-    if (isMainThread()) {
-        ASSERT_NOT_REACHED();
-        return nullptr;
-    }
+    ASSERT(!isMainThread());
+
     return axID.isValid() ? m_readerThreadNodeMap.get(axID) : nullptr;
 }
 
@@ -1169,12 +1166,7 @@ std::optional<ListHashSet<AXID>> AXIsolatedTree::relatedObjectIDsFor(const AXIso
 void AXIsolatedTree::applyPendingChanges()
 {
     AXTRACE("AXIsolatedTree::applyPendingChanges"_s);
-
-    // In isolated tree mode, only apply pending changes on the AX thread.
-    if (isMainThread()) {
-        ASSERT_NOT_REACHED();
-        return;
-    }
+    ASSERT(!isMainThread());
 
     Locker locker { m_changeLogLock };
 

Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

@@ -33,7 +33,6 @@
 #include "AXTreeStore.h"
 #include "PageIdentifier.h"
 #include "RenderStyleConstants.h"
-#include "RuntimeApplicationChecks.h"
 #include <wtf/HashMap.h>
 #include <wtf/Lock.h>
 #include <wtf/RefPtr.h>
@@ -325,7 +324,6 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
 
     static RefPtr<AXIsolatedTree> treeForPageID(std::optional<PageIdentifier>);
     static RefPtr<AXIsolatedTree> treeForPageID(PageIdentifier);
-    constexpr ProcessID processID() const { return m_processID; }
     AXObjectCache* axObjectCache() const;
     constexpr AXGeometryManager* geometryManager() const { return m_geometryManager.get(); }
 
@@ -429,7 +427,6 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     Vector<NodeChange> resolveAppends();
     void queueAppendsAndRemovals(Vector<NodeChange>&&, Vector<AXID>&&);
 
-    const ProcessID m_processID { presentingApplicationPID() };
     unsigned m_maxTreeDepth { 0 };
     WeakPtr<AXObjectCache> m_axObjectCache;
     OptionSet<ActivityState> m_pageActivityState;

Source/WebCore/accessibility/mac/AXObjectCacheMac.mm

@@ -287,7 +287,7 @@ static AXTextSelectionGranularity platformGranularityForWebCoreGranularity(WebCo
 
 static void AXPostNotificationWithUserInfo(AccessibilityObjectWrapper *object, NSString *notification, id userInfo, bool skipSystemNotification = false)
 {
-    if (id associatedPluginParent = [object associatedPluginParent])
+    if (id associatedPluginParent = [object _associatedPluginParent])
         object = associatedPluginParent;
 
     // To simplify monitoring for notifications in tests, repost as a simple NSNotification instead of forcing test infrastucture to setup an IPC client and do all the translation between WebCore types and platform specific IPC types and back

Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.h

@@ -139,7 +139,7 @@ extern "C" AXUIElementRef NSAccessibilityCreateAXUIElementRef(id element);
 - (RetainPtr<AXTextMarkerRef>)textMarkerForFirstPositionInTextControl:(WebCore::HTMLTextFormControlElement&)textControl;
 
 // When a plugin uses a WebKit control to act as a surrogate view (e.g. PDF use WebKit to create text fields).
-- (id)associatedPluginParent;
+- (id)_associatedPluginParent;
 
 @end
 

Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

@@ -72,6 +72,7 @@
 #import "RenderTextControl.h"
 #import "RenderView.h"
 #import "RenderWidget.h"
+#import "RuntimeApplicationChecks.h"
 #import "ScrollView.h"
 #import "TextIterator.h"
 #import "VisibleUnits.h"
@@ -759,12 +760,8 @@ - (NSArray*)accessibilityActionNames
     return actions;
 }
 
-- (NSArray*)additionalAccessibilityAttributeNames
+- (NSArray*)_additionalAccessibilityAttributeNames:(const RefPtr<AXCoreObject>&)backingObject
 {
-    RefPtr<AXCoreObject> backingObject = self.axBackingObject;
-    if (!backingObject)
-        return nil;
-
     NSMutableArray *additional = [NSMutableArray array];
     if (backingObject->supportsARIAOwns())
         [additional addObject:NSAccessibilityOwnsAttribute];
@@ -1245,7 +1242,7 @@ - (NSArray*)accessibilityAttributeNames
     else if (backingObject->isVideo())
         objectAttributes = videoAttrs.get().get();
 
-    NSArray *additionalAttributes = [self additionalAccessibilityAttributeNames];
+    NSArray *additionalAttributes = [self _additionalAccessibilityAttributeNames:backingObject];
     if ([additionalAttributes count])
         objectAttributes = [objectAttributes arrayByAddingObjectsFromArray:additionalAttributes];
 
@@ -1312,9 +1309,15 @@ - (AXTextMarkerRangeRef)selectedTextMarkerRange
     return range;
 }
 
-- (id)associatedPluginParent
+- (id)_associatedPluginParent
+{
+    RefPtr<AXCoreObject> backingObject = self.axBackingObject;
+    return [self _associatedPluginParentWith:self.axBackingObject];
+}
+
+- (id)_associatedPluginParentWith:(const RefPtr<AXCoreObject>&)backingObject
 {
-    if (!self.axBackingObject || !self.axBackingObject->hasApplePDFAnnotationAttribute())
+    if (!backingObject || !backingObject->hasApplePDFAnnotationAttribute())
         return nil;
 
     return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
@@ -1388,22 +1391,18 @@ - (NSString*)role
     return NSAccessibilityUnknownRole;
 }
 
-- (BOOL)isEmptyGroup
+- (BOOL)_isEmptyGroup:(const RefPtr<AXCoreObject>&)object
 {
-    RefPtr<AXCoreObject> backingObject = self.axBackingObject;
-    if (!backingObject)
-        return false;
-
 #if ENABLE(MODEL_ELEMENT)
-    if (backingObject->isModel())
+    if (object->isModel())
         return false;
 #endif
 
-    if (backingObject->isRemoteFrame())
+    if (object->isRemoteFrame())
         return false;
 
     return [[self role] isEqual:NSAccessibilityGroupRole]
-        && backingObject->children().isEmpty()
+        && object->children().isEmpty()
         && ![[self renderWidgetChildren] count];
 }
 
@@ -1414,7 +1413,7 @@ - (NSString*)subrole
     if (!backingObject)
         return nil;
 
-    if ([self isEmptyGroup])
+    if ([self _isEmptyGroup:backingObject])
         return @"AXEmptyGroup";
 
     auto subrole = backingObject->subrolePlatformString();
@@ -2184,7 +2183,7 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName
 
     // This allows us to connect to a plugin that creates a shadow node for editing (like PDFs).
     if ([attributeName isEqualToString:@"_AXAssociatedPluginParent"])
-        return [self associatedPluginParent];
+        return [self _associatedPluginParentWith:backingObject];
 
     // this is used only by DumpRenderTree for testing
     if ([attributeName isEqualToString:@"AXClickPoint"])
@@ -3839,8 +3838,7 @@ - (NSUInteger)accessibilityArrayAttributeCount:(NSString *)attribute
 // until it finds something that responds to this method.
 - (pid_t)accessibilityPresenterProcessIdentifier
 {
-    RefPtr<AXCoreObject> backingObject = self.axBackingObject;
-    return backingObject ? backingObject->processID() : 0;
+    return presentingApplicationPID();
 }
 
 - (NSArray *)accessibilityArrayAttributeValues:(NSString *)attribute index:(NSUInteger)index maxCount:(NSUInteger)maxCount
@@ -3883,11 +3881,11 @@ - (NSArray *)accessibilityArrayAttributeValues:(NSString *)attribute index:(NSUI
 
             // The attachment view should be returned, otherwise AX palindrome errors occur.
             id attachmentView = nil;
-            if ([wrapper isKindOfClass:[WebAccessibilityObjectWrapper class]] && wrapper.axBackingObject) {
-                if (wrapper.axBackingObject->isAttachment())
+            if (RefPtr childObject = [wrapper isKindOfClass:[WebAccessibilityObjectWrapper class]] ? wrapper.axBackingObject : nullptr) {
+                if (childObject->isAttachment())
                     attachmentView = [wrapper attachmentView];
-                else if (wrapper.axBackingObject->isRemoteFrame())
-                    attachmentView = wrapper.axBackingObject->remoteFramePlatformElement().get();
+                else if (childObject->isRemoteFrame())
+                    attachmentView = childObject->remoteFramePlatformElement().get();
             }
 
             [subarray addObject:attachmentView ? attachmentView : wrapper];