Title: [293186] trunk/Source/WebCore
Revision
293186
Author
tyle...@apple.com
Date
2022-04-21 13:51:37 -0700 (Thu, 21 Apr 2022)

Log Message

Only AXIsolatedTree::update{Children,Node} once when processing notifications in AXObjectCache::updateIsolatedTree
https://bugs.webkit.org/show_bug.cgi?id=239499

Reviewed by Andres Gonzalez.

Currently, we do try to prevent performing duplicate updates, but we do so only
if we have an exact match for a previous notification for any given object.
However, multiple notifications trigger AXIsolatedTree::updateNode and children,
so we'll still duplicate work if we get multiple notifications in the same group.
For example, before this patch, if the same object got both an AXValueChanged and
AXTextChanged notification, we'd call AXIsolatedTree::updateNode twice.

With this patch, we instead keep track of whether or not we've
performed a node or children update for each object to prevent
duplicate work.

No new tests because there is no behavior change.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::updateIsolatedTree):
(WebCore::appendIfNotContainsMatching): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (293185 => 293186)


--- trunk/Source/WebCore/ChangeLog	2022-04-21 20:48:20 UTC (rev 293185)
+++ trunk/Source/WebCore/ChangeLog	2022-04-21 20:51:37 UTC (rev 293186)
@@ -1,3 +1,27 @@
+2022-04-21  Tyler Wilcock  <tyle...@apple.com>
+
+        Only AXIsolatedTree::update{Children,Node} once when processing notifications in AXObjectCache::updateIsolatedTree
+        https://bugs.webkit.org/show_bug.cgi?id=239499
+
+        Reviewed by Andres Gonzalez.
+
+        Currently, we do try to prevent performing duplicate updates, but we do so only
+        if we have an exact match for a previous notification for any given object. 
+        However, multiple notifications trigger AXIsolatedTree::updateNode and children,
+        so we'll still duplicate work if we get multiple notifications in the same group.
+        For example, before this patch, if the same object got both an AXValueChanged and
+        AXTextChanged notification, we'd call AXIsolatedTree::updateNode twice.
+
+        With this patch, we instead keep track of whether or not we've
+        performed a node or children update for each object to prevent
+        duplicate work.
+
+        No new tests because there is no behavior change.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::updateIsolatedTree):
+        (WebCore::appendIfNotContainsMatching): Deleted.
+
 2022-04-21  Philippe Normand  <ph...@igalia.com>
 
         [GBM] Debug build broken

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (293185 => 293186)


--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-04-21 20:48:20 UTC (rev 293185)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-04-21 20:51:37 UTC (rev 293186)
@@ -3347,16 +3347,6 @@
 }
 
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
-// FIXME: should be added to WTF::Vector.
-template<typename T, typename F>
-static bool appendIfNotContainsMatching(Vector<T>& vector, const T& value, F matches)
-{
-    if (vector.findIf(matches) != notFound)
-        return false;
-    vector.append(value);
-    return true;
-}
-
 void AXObjectCache::updateIsolatedTree(AXCoreObject* object, AXNotification notification)
 {
     if (object)
@@ -3384,9 +3374,11 @@
         return;
     }
 
-    // Filter out multiple notifications for the same object. This avoids
-    // updating the isolated tree multiple times unnecessarily.
-    Vector<std::pair<RefPtr<AXCoreObject>, AXNotification>> filteredNotifications;
+    struct UpdatedFields {
+        bool children { false };
+        bool node { false };
+    };
+    HashMap<AXID, UpdatedFields> updatedObjects;
     for (const auto& notification : notifications) {
         AXLOG(notification);
         if (!notification.first || !notification.first->objectID().isValid())
@@ -3428,12 +3420,11 @@
         case AXSelectedChildrenChanged:
         case AXTextChanged:
         case AXValueChanged: {
-            bool needsUpdate = appendIfNotContainsMatching(filteredNotifications, notification, [&notification] (const std::pair<RefPtr<AXCoreObject>, AXNotification>& note) {
-                return note.second == notification.second && note.first.get() == notification.first.get();
-            });
-
-            if (needsUpdate)
+            auto updatedFields = updatedObjects.get(notification.first->objectID());
+            if (!updatedFields.node) {
+                updatedObjects.set(notification.first->objectID(), UpdatedFields { updatedFields.children, true });
                 tree->updateNode(*notification.first);
+            }
             break;
         }
         case AXChildrenChanged:
@@ -3441,12 +3432,11 @@
         case AXRowCountChanged:
         case AXRowCollapsed:
         case AXRowExpanded: {
-            bool needsUpdate = appendIfNotContainsMatching(filteredNotifications, notification, [&notification] (const std::pair<RefPtr<AXCoreObject>, AXNotification>& note) {
-                return note.second == notification.second && note.first.get() == notification.first.get();
-            });
-
-            if (needsUpdate)
+            auto updatedFields = updatedObjects.get(notification.first->objectID());
+            if (!updatedFields.children) {
+                updatedObjects.set(notification.first->objectID(), UpdatedFields { true, updatedFields.node });
                 tree->updateChildren(*notification.first);
+            }
             break;
         }
         default:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to