Title: [285258] trunk/Source/WebCore
Revision
285258
Author
[email protected]
Date
2021-11-04 05:17:41 -0700 (Thu, 04 Nov 2021)

Log Message

Fix for AXObjectCache::postPlatformNotification in isolated tree mode, debug builds.
https://bugs.webkit.org/show_bug.cgi?id=232682
<rdar://problem/84991736>

Reviewed by Chris Fleizach.

In debug builds AXObjectCache::postPlatformNotification was calling the
object's wrapper accessibilityIsIgnored, which in turn calls
updateBackingStore. Since postPlatformNotification runs on the main
thread, and calls into the platform's wrapper should happen on the
secondary thread, this is hitting the asserts in
AXIsolatedTree::applyPendingChanges to prevent this situation.
Instead of calling through the wrapper, this patch adds a utility
function to exercise the same core methods invoked by
accessibilityIsIgnored on the AX object directly, and hence avoiding the
problem above.

* accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXObjectCache::postPlatformNotification):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (285257 => 285258)


--- trunk/Source/WebCore/ChangeLog	2021-11-04 09:31:00 UTC (rev 285257)
+++ trunk/Source/WebCore/ChangeLog	2021-11-04 12:17:41 UTC (rev 285258)
@@ -1,3 +1,25 @@
+2021-11-04  Andres Gonzalez  <[email protected]>
+
+        Fix for AXObjectCache::postPlatformNotification in isolated tree mode, debug builds.
+        https://bugs.webkit.org/show_bug.cgi?id=232682
+        <rdar://problem/84991736>
+
+        Reviewed by Chris Fleizach.
+
+        In debug builds AXObjectCache::postPlatformNotification was calling the
+        object's wrapper accessibilityIsIgnored, which in turn calls
+        updateBackingStore. Since postPlatformNotification runs on the main
+        thread, and calls into the platform's wrapper should happen on the
+        secondary thread, this is hitting the asserts in
+        AXIsolatedTree::applyPendingChanges to prevent this situation.
+        Instead of calling through the wrapper, this patch adds a utility
+        function to exercise the same core methods invoked by
+        accessibilityIsIgnored on the AX object directly, and hence avoiding the
+        problem above.
+
+        * accessibility/mac/AXObjectCacheMac.mm:
+        (WebCore::AXObjectCache::postPlatformNotification):
+
 2021-11-03  Antoine Quint  <[email protected]>
 
         REGRESSION (r268932): CPU usage higher than expected with sibling elements running WebAnimations

Modified: trunk/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm (285257 => 285258)


--- trunk/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm	2021-11-04 09:31:00 UTC (rev 285257)
+++ trunk/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm	2021-11-04 12:17:41 UTC (rev 285258)
@@ -295,11 +295,31 @@
     NSAccessibilityPostNotificationWithUserInfo(object, notification, userInfo);
 }
 
-void AXObjectCache::postPlatformNotification(AXCoreObject* obj, AXNotification notification)
+#ifndef NDEBUG
+// This function exercises, for debugging and testing purposes, the AXObject methods called in [WebAccessibilityObjectWrapper accessibilityIsIgnored].
+// It is useful in cases like AXObjectCache::postPlatformNotification which calls NSAccessibilityPostNotification, which in turn calls accessibilityIsIgnored, except when it is running layout tests.
+// Thus, calling exerciseIsIgnored during AXObjectCache::postPlatformNotification helps detect issues during layout tests.
+// Example of such issues is the crash described in: https://bugs.webkit.org/show_bug.cgi?id=46662.
+static void exerciseIsIgnored(AccessibilityObject& object)
 {
-    if (!obj)
+    object.updateBackingStore();
+    if (object.isAttachment()) {
+        ALLOW_DEPRECATED_DECLARATIONS_BEGIN
+        [[object.wrapper() attachmentView] accessibilityIsIgnored];
+        ALLOW_DEPRECATED_DECLARATIONS_END
+
         return;
+    }
+    object.accessibilityIsIgnored();
+}
+#endif
 
+void AXObjectCache::postPlatformNotification(AXCoreObject* object, AXNotification notification)
+{
+    ASSERT(is<AccessibilityObject>(object));
+    if (!is<AccessibilityObject>(object))
+        return;
+
     bool skipSystemNotification = false;
     // Some notifications are unique to Safari and do not have NSAccessibility equivalents.
     NSString *macNotification;
@@ -306,13 +326,13 @@
     switch (notification) {
     case AXActiveDescendantChanged:
         // An active descendant change for trees means a selected rows change.
-        if (obj->isTree() || obj->isTable())
+        if (object->isTree() || object->isTable())
             macNotification = NSAccessibilitySelectedRowsChangedNotification;
 
         // When a combobox uses active descendant, it means the selected item in its associated
         // list has changed. In these cases we should use selected children changed, because
         // we don't want the focus to change away from the combobox where the user is typing.
-        else if (obj->isComboBox() || obj->isList() || obj->isListBox())
+        else if (object->isComboBox() || object->isList() || object->isListBox())
             macNotification = NSAccessibilitySelectedChildrenChangedNotification;
         else
             macNotification = NSAccessibilityFocusedUIElementChangedNotification;
@@ -345,7 +365,7 @@
         macNotification = @"AXInvalidStatusChanged";
         break;
     case AXSelectedChildrenChanged:
-        if (obj->isTable() && obj->isExposable())
+        if (object->isTable() && object->isExposable())
             macNotification = NSAccessibilitySelectedRowsChangedNotification;
         else
             macNotification = NSAccessibilitySelectedChildrenChangedNotification;
@@ -419,13 +439,11 @@
         return;
     }
 
-    // NSAccessibilityPostNotification will call this method, (but not when running DRT), so ASSERT here to make sure it does not crash.
-    // https://bugs.webkit.org/show_bug.cgi?id=46662
-    ALLOW_DEPRECATED_DECLARATIONS_BEGIN
-    ASSERT([obj->wrapper() accessibilityIsIgnored] || true);
-    ALLOW_DEPRECATED_DECLARATIONS_END
+#ifndef NDEBUG
+    exerciseIsIgnored(downcast<AccessibilityObject>(*object));
+#endif
 
-    AXPostNotificationWithUserInfo(obj->wrapper(), macNotification, nil, skipSystemNotification);
+    AXPostNotificationWithUserInfo(object->wrapper(), macNotification, nil, skipSystemNotification);
 }
 
 void AXObjectCache::postTextStateChangePlatformNotification(AXCoreObject* object, const AXTextStateChangeIntent& intent, const VisibleSelection& selection)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to