Title: [291084] trunk/Source/WebCore
Revision
291084
Author
[email protected]
Date
2022-03-09 18:43:46 -0800 (Wed, 09 Mar 2022)

Log Message

In isolated tree mode, do not call NSAccessibilityUnregisterUniqueIdForUIElement until the isolated object is detached.
https://bugs.webkit.org/show_bug.cgi?id=237621
<rdar://problem/89992486>

Reviewed by Chris Fleizach.

When the live object is detached from its wrapper on the main thread,
the wrapper may be in the middle of serving a request on the AX thread.
Therefore calling NSAccessibilityUnregisterUniqueIdForUIElement may
interfere with the completiong of the request, or even cause undefined
behavior since many of the internal lookups for objects depend on a data
structure updated by system call. With this patch, we unregister a
wrapper when the isolated object is detached, which happens after the
live object is detached.

* accessibility/mac/WebAccessibilityObjectWrapperBase.h:
* accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase initWithAccessibilityObject:]):
(-[WebAccessibilityObjectWrapperBase attachIsolatedObject:]):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper detach]):
(-[WebAccessibilityObjectWrapper detachIsolatedObject:]):
(-[WebAccessibilityObjectWrapper unregisterUniqueIdForUIElement]): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (291083 => 291084)


--- trunk/Source/WebCore/ChangeLog	2022-03-10 02:05:03 UTC (rev 291083)
+++ trunk/Source/WebCore/ChangeLog	2022-03-10 02:43:46 UTC (rev 291084)
@@ -1,3 +1,29 @@
+2022-03-09  Andres Gonzalez  <[email protected]>
+
+        In isolated tree mode, do not call NSAccessibilityUnregisterUniqueIdForUIElement until the isolated object is detached.
+        https://bugs.webkit.org/show_bug.cgi?id=237621
+        <rdar://problem/89992486>
+
+        Reviewed by Chris Fleizach.
+
+        When the live object is detached from its wrapper on the main thread,
+        the wrapper may be in the middle of serving a request on the AX thread.
+        Therefore calling NSAccessibilityUnregisterUniqueIdForUIElement may
+        interfere with the completiong of the request, or even cause undefined
+        behavior since many of the internal lookups for objects depend on a data
+        structure updated by system call. With this patch, we unregister a
+        wrapper when the isolated object is detached, which happens after the
+        live object is detached.
+
+        * accessibility/mac/WebAccessibilityObjectWrapperBase.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
+        (-[WebAccessibilityObjectWrapperBase initWithAccessibilityObject:]):
+        (-[WebAccessibilityObjectWrapperBase attachIsolatedObject:]):
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper detach]):
+        (-[WebAccessibilityObjectWrapper detachIsolatedObject:]):
+        (-[WebAccessibilityObjectWrapper unregisterUniqueIdForUIElement]): Deleted.
+
 2022-03-09  Matt Woodrow  <[email protected]>
 
         Empty float rects should remain empty after integrally enclosing them.

Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h (291083 => 291084)


--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h	2022-03-10 02:05:03 UTC (rev 291083)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h	2022-03-10 02:43:46 UTC (rev 291084)
@@ -63,9 +63,13 @@
 
 @interface WebAccessibilityObjectWrapperBase : NSObject {
     WebCore::AXCoreObject* m_axObject;
+
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
     WebCore::AXCoreObject* m_isolatedObject;
+    // To be accessed only on the main thread.
+    bool m_isolatedObjectInitialized;
 #endif
+
     WebCore::AXID _identifier;
 }
 

Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm (291083 => 291084)


--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm	2022-03-10 02:05:03 UTC (rev 291083)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm	2022-03-10 02:43:46 UTC (rev 291084)
@@ -279,9 +279,10 @@
 
 - (id)initWithAccessibilityObject:(AXCoreObject*)axObject
 {
+    ASSERT(isMainThread());
+
     if (!(self = [super init]))
         return nil;
-
     [self attachAXObject:axObject];
     return self;
 }
@@ -299,6 +300,9 @@
 {
     ASSERT(isolatedObject && (!_identifier.isValid() || _identifier == isolatedObject->objectID()));
     m_isolatedObject = isolatedObject;
+    if (isMainThread())
+        m_isolatedObjectInitialized = true;
+
     if (!_identifier.isValid())
         _identifier = m_isolatedObject->objectID();
 }

Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (291083 => 291084)


--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2022-03-10 02:05:03 UTC (rev 291083)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2022-03-10 02:43:46 UTC (rev 291084)
@@ -524,21 +524,28 @@
 
 @implementation WebAccessibilityObjectWrapper
 
-- (void)unregisterUniqueIdForUIElement
+- (void)detach
 {
     ASSERT(isMainThread());
-    NSAccessibilityUnregisterUniqueIdForUIElement(self);
+
+    // If the IsolatedObject is initialized, do not UnregisterUniqueIdForUIElement here because the wrapper may be in the middle of serving a request on the AX thread.
+    // The IsolatedObject is capable to tend to some requests after the live object is gone.
+    // In regular mode, UnregisterUniqueIdForUIElement immediately.
+#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
+    if (!m_isolatedObjectInitialized)
+#endif
+        NSAccessibilityUnregisterUniqueIdForUIElement(self);
+
+    [super detach];
 }
 
-- (void)detach
+#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
+- (void)detachIsolatedObject:(AccessibilityDetachmentType)detachmentType
 {
-    // Send unregisterUniqueIdForUIElement unconditionally because if it is
-    // ever accidentally not done (via other bugs in our AX implementation) you
-    // end up with a crash like <rdar://problem/4273149>.  It is safe and not
-    // expensive to send even if the object is not registered.
-    [self unregisterUniqueIdForUIElement];
-    [super detach];
+    NSAccessibilityUnregisterUniqueIdForUIElement(self);
+    [super detachIsolatedObject:detachmentType];
 }
+#endif
 
 - (id)attachmentView
 {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to