Title: [289723] trunk/Source/WebCore
Revision
289723
Author
andresg...@apple.com
Date
2022-02-13 19:49:00 -0800 (Sun, 13 Feb 2022)

Log Message

Decouple AXObjectCache handleChildrenChanged and postNotification.
https://bugs.webkit.org/show_bug.cgi?id=234059
<rdar://problem/86247404>

Reviewed by Chris Fleizach.

This fixes ~7 accessibility tests in isolated tree mode.
Also fixes flakiness in accessibility/dialog-showModal.html.

AXObjectCache::handleChildrenChanged was posting an AXChildrenChanged
notification (postNotification), which causes a double deferral of these
notifications, instead of actually handling the notification.
ChildrenChanged notifications should be handled before many other
notifications of property changes since they are DOM mutations, objects
added or removed, before properties in the resulting objects may be
updated. This patch fixes this problem by making handleChildrenChanged
to actually handle the notifications and update the isolated tree.
In addition, handling of the "open" attribute for <dialog> elements now
updates children, which fixes the flakiness observed in the
dialog-showModal.html test.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::handleChildrenChanged):
(WebCore::AXObjectCache::notificationPostTimerFired):
(WebCore::AXObjectCache::deferModalChange):
(WebCore::AXObjectCache::handleAttributeChange):
(WebCore::AXObjectCache::performCacheUpdateTimerFired):
* accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::collectNodeChangesForSubtree):
Removed alternative fix where we were processing the pending
ChildrenChanged Notifications before updating the isolated tree.
* accessibility/isolatedtree/AXIsolatedTree.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (289722 => 289723)


--- trunk/Source/WebCore/ChangeLog	2022-02-14 01:52:40 UTC (rev 289722)
+++ trunk/Source/WebCore/ChangeLog	2022-02-14 03:49:00 UTC (rev 289723)
@@ -1,3 +1,38 @@
+2022-02-13  Andres Gonzalez  <andresg...@apple.com>
+
+        Decouple AXObjectCache handleChildrenChanged and postNotification.
+        https://bugs.webkit.org/show_bug.cgi?id=234059
+        <rdar://problem/86247404>
+
+        Reviewed by Chris Fleizach.
+
+        This fixes ~7 accessibility tests in isolated tree mode.
+        Also fixes flakiness in accessibility/dialog-showModal.html.
+
+        AXObjectCache::handleChildrenChanged was posting an AXChildrenChanged
+        notification (postNotification), which causes a double deferral of these
+        notifications, instead of actually handling the notification.
+        ChildrenChanged notifications should be handled before many other
+        notifications of property changes since they are DOM mutations, objects
+        added or removed, before properties in the resulting objects may be
+        updated. This patch fixes this problem by making handleChildrenChanged
+        to actually handle the notifications and update the isolated tree.
+        In addition, handling of the "open" attribute for <dialog> elements now
+        updates children, which fixes the flakiness observed in the
+        dialog-showModal.html test.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::handleChildrenChanged):
+        (WebCore::AXObjectCache::notificationPostTimerFired):
+        (WebCore::AXObjectCache::deferModalChange):
+        (WebCore::AXObjectCache::handleAttributeChange):
+        (WebCore::AXObjectCache::performCacheUpdateTimerFired):
+        * accessibility/isolatedtree/AXIsolatedTree.cpp:
+        (WebCore::AXIsolatedTree::collectNodeChangesForSubtree):
+        Removed alternative fix where we were processing the pending
+        ChildrenChanged Notifications before updating the isolated tree.
+        * accessibility/isolatedtree/AXIsolatedTree.h:
+
 2022-02-13  Matt Woodrow  <mattwood...@apple.com>
 
         Add support for parsing 'subgrid' in grid-template-columns/row

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (289722 => 289723)


--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-02-14 01:52:40 UTC (rev 289722)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-02-14 03:49:00 UTC (rev 289723)
@@ -1010,15 +1010,16 @@
         return;
     }
 
-    // This method is meant as a quick way of marking a portion of the accessibility tree dirty.
     if (!object.node() && !object.renderer())
         return;
 
-    postNotification(&object, object.document(), AXChildrenChanged);
-
     // Should make the subtree dirty so that everything below will be updated correctly.
     object.setNeedsToUpdateSubtree();
 
+    // If isIgnored has changed for object, notify that ChildrenChanged for its parent.
+    if (object.parentObjectIfExists() && object.hasIgnoredValueChanged())
+        childrenChanged(object.parentObject());
+
     // Go up the existing ancestors chain and fire the appropriate notifications.
     bool shouldUpdateParent = true;
     for (auto* parent = &object; parent; parent = parent->parentObjectIfExists()) {
@@ -1040,6 +1041,12 @@
             shouldUpdateParent = false;
         }
     }
+
+#if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
+    updateIsolatedTree(object, AXChildrenChanged);
+#endif
+
+    postPlatformNotification(&object, AXChildrenChanged);
 }
 
 void AXObjectCache::handleMenuOpened(Node* node)
@@ -1129,24 +1136,17 @@
         }
 #endif
 
-        // Ensure that this menu really is a menu. We do this check here so that we don't have to create
-        // the axChildren when the menu is marked as opening.
         if (note.second == AXMenuOpened) {
+            // Only notify if the object is in fact a menu.
             note.first->updateChildrenIfNecessary();
             if (note.first->roleValue() != AccessibilityRole::Menu)
                 continue;
         }
 
-        if (note.second == AXChildrenChanged && note.first->parentObjectIfExists()
-            && downcast<AccessibilityObject>(*note.first).lastKnownIsIgnoredValue() != note.first->accessibilityIsIgnored())
-            childrenChanged(downcast<AccessibilityObject>(note.first->parentObject()));
-
         notificationsToPost.append(note);
     }
 
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
-    // FIXME: this updateIsolatedTree here may be premature in some cases.
-    // E.g., if the childrenChanged above is hit, we should updateIsolatedTree after performDeferredCacheUpdate.
     updateIsolatedTree(notificationsToPost);
 #endif
 
@@ -1271,8 +1271,14 @@
 
 void AXObjectCache::deferModalChange(Element* element)
 {
-    if (element)
+    if (element) {
         m_deferredModalChangedList.add(*element);
+
+        // Notify that parent's children have changed.
+        if (auto* axParent = get(element->parentNode()))
+            m_deferredChildrenChangedList.add(axParent);
+    }
+
     if (!m_performCacheUpdateTimer.isActive())
         m_performCacheUpdateTimer.startOneShot(0_s);
 }
@@ -1829,7 +1835,6 @@
         recomputeIsIgnored(element->parentNode());
     }
 
-
     if (!attrName.localName().string().startsWith("aria-"))
         return;
 
@@ -1848,7 +1853,9 @@
     else if (attrName == aria_expandedAttr)
         handleAriaExpandedChange(element);
     else if (attrName == aria_hiddenAttr) {
-        childrenChanged(element->parentNode(), element);
+        if (auto* parent = get(element->parentNode()))
+            handleChildrenChanged(*parent);
+
         if (m_currentModalElement && m_currentModalElement->isDescendantOf(element)) {
             m_modalNodesInitialized = false;
             deferModalChange(m_currentModalElement.get());
@@ -3228,8 +3235,6 @@
         return;
 
     performDeferredCacheUpdate();
-    // FIXME: need to update the isolated tree after the above cache update.
-    // This is most likely the cause of problems with the isolated tree updates..
 }
 
 void AXObjectCache::processDeferredChildrenChangedList()

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (289722 => 289723)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2022-02-14 01:52:40 UTC (rev 289722)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2022-02-14 03:49:00 UTC (rev 289723)
@@ -280,10 +280,6 @@
     AXTRACE("AXIsolatedTree::collectNodeChangesForSubtree");
     ASSERT(isMainThread());
 
-    if (!m_creatingSubtree)
-        axObjectCache()->processDeferredChildrenChangedList();
-    SetForScope<bool> creatingSubtree(m_creatingSubtree, true);
-
     auto nodeChange = nodeChangeForObject(axObject, parentID, attachWrapper);
     if (idsBeingChanged)
         idsBeingChanged->add(nodeChange.isolatedObject->objectID());

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h (289722 => 289723)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h	2022-02-14 01:52:40 UTC (rev 289722)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h	2022-02-14 03:49:00 UTC (rev 289723)
@@ -422,8 +422,6 @@
     double m_pendingLoadingProgress WTF_GUARDED_BY_LOCK(m_changeLogLock) { 0 };
     double m_loadingProgress { 0 };
     Lock m_changeLogLock;
-
-    bool m_creatingSubtree { false };
 };
 
 inline AXObjectCache* AXIsolatedTree::axObjectCache() const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to