Title: [293566] trunk/Source/WebCore
Revision
293566
Author
tyle...@apple.com
Date
2022-04-27 22:36:41 -0700 (Wed, 27 Apr 2022)

Log Message

AXObjectCache::childrenChanged modifies m_deferred* member variables but doesn't start timer to process them
https://bugs.webkit.org/show_bug.cgi?id=239787

Reviewed by Chris Fleizach.

This patch fixes this a bug where AXObjectCache::childrenChanged
methods modify m_deferred* member variables, but doesn't start
the m_performCacheUpdateTimer.

This patch also renames m_deferredChildrenChangedNodeList to
m_deferredNodeAddedOrRemovedList, since this more accurately
captures what this list is for.

Finally, this refactors a few methods that modify m_deferred* member
variables to not start the cache timer unless the variable has
actually changed.

Fixes these tests in ITM:
  - accessibility/mac/focus-moves-cursor.html
  - accessibility/image-load-on-delay.html
  - accessibility/legend-children-are-visible.html
  - accessibility/text-alternative-calculation-hidden-nodes.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::childrenChanged):
(WebCore::AXObjectCache::deferMenuListValueChange):
(WebCore::AXObjectCache::deferModalChange):
(WebCore::AXObjectCache::deferNodeAddedOrRemoved): Added.
(WebCore::AXObjectCache::prepareForDocumentDestruction):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
* accessibility/AXObjectCache.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (293565 => 293566)


--- trunk/Source/WebCore/ChangeLog	2022-04-28 05:32:05 UTC (rev 293565)
+++ trunk/Source/WebCore/ChangeLog	2022-04-28 05:36:41 UTC (rev 293566)
@@ -1,3 +1,38 @@
+2022-04-27  Tyler Wilcock  <tyle...@apple.com>
+
+        AXObjectCache::childrenChanged modifies m_deferred* member variables but doesn't start timer to process them
+        https://bugs.webkit.org/show_bug.cgi?id=239787
+
+        Reviewed by Chris Fleizach.
+
+        This patch fixes this a bug where AXObjectCache::childrenChanged
+        methods modify m_deferred* member variables, but doesn't start
+        the m_performCacheUpdateTimer.
+
+        This patch also renames m_deferredChildrenChangedNodeList to 
+        m_deferredNodeAddedOrRemovedList, since this more accurately
+        captures what this list is for.
+
+        Finally, this refactors a few methods that modify m_deferred* member
+        variables to not start the cache timer unless the variable has
+        actually changed.
+
+        Fixes these tests in ITM:
+          - accessibility/mac/focus-moves-cursor.html
+          - accessibility/image-load-on-delay.html
+          - accessibility/legend-children-are-visible.html
+          - accessibility/text-alternative-calculation-hidden-nodes.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::remove):
+        (WebCore::AXObjectCache::childrenChanged):
+        (WebCore::AXObjectCache::deferMenuListValueChange):
+        (WebCore::AXObjectCache::deferModalChange):
+        (WebCore::AXObjectCache::deferNodeAddedOrRemoved): Added.
+        (WebCore::AXObjectCache::prepareForDocumentDestruction):
+        (WebCore::AXObjectCache::performDeferredCacheUpdate):
+        * accessibility/AXObjectCache.h:
+
 2022-04-27  Patrick Angle  <pan...@apple.com>
 
         Web Inspector: [Flexbox] `<button>` and `<select>` elements are appearing in list of Flex containers

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (293565 => 293566)


--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-04-28 05:32:05 UTC (rev 293565)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2022-04-28 05:36:41 UTC (rev 293566)
@@ -903,7 +903,7 @@
         m_deferredModalChangedList.remove(downcast<Element>(node));
         m_deferredMenuListChange.remove(downcast<Element>(node));
     }
-    m_deferredChildrenChangedNodeList.remove(&node);
+    m_deferredNodeAddedOrRemovedList.remove(&node);
     m_deferredTextChangedList.remove(&node);
     // Remove the entry if the new focused node is being removed.
     m_deferredFocusedNodeChange.removeAllMatching([&node](auto& entry) -> bool {
@@ -1105,32 +1105,42 @@
     if (AccessibilityObject::liveRegionStatusIsEnabled(liveRegionStatus))
         postNotification(getOrCreate(node), &document(), AXLiveRegionCreated);
 }
-    
-void AXObjectCache::childrenChanged(Node* node, Node* newChild)
+
+void AXObjectCache::deferNodeAddedOrRemoved(Node* node)
 {
-    if (newChild)
-        m_deferredChildrenChangedNodeList.add(newChild);
+    if (!node)
+        return;
 
-    if (auto* object = get(node))
-        m_deferredChildrenChangedList.add(object);
+    m_deferredNodeAddedOrRemovedList.add(node);
+
+    if (!m_performCacheUpdateTimer.isActive())
+        m_performCacheUpdateTimer.startOneShot(0_s);
 }
 
-void AXObjectCache::childrenChanged(RenderObject* renderer, RenderObject* newChild)
+void AXObjectCache::childrenChanged(Node* node, Node* changedChild)
 {
+    childrenChanged(get(node));
+    deferNodeAddedOrRemoved(changedChild);
+}
+
+void AXObjectCache::childrenChanged(RenderObject* renderer, RenderObject* changedChild)
+{
     if (!renderer)
         return;
 
-    if (newChild && newChild->node())
-        m_deferredChildrenChangedNodeList.add(newChild->node());
-
-    if (auto* object = get(renderer))
-        m_deferredChildrenChangedList.add(object);
+    childrenChanged(get(renderer));
+    if (changedChild)
+        deferNodeAddedOrRemoved(changedChild->node());
 }
 
 void AXObjectCache::childrenChanged(AccessibilityObject* object)
 {
-    if (object)
-        m_deferredChildrenChangedList.add(object);
+    if (!object)
+        return;
+    m_deferredChildrenChangedList.add(object);
+
+    if (!m_performCacheUpdateTimer.isActive())
+        m_performCacheUpdateTimer.startOneShot(0_s);
 }
 
 void AXObjectCache::notificationPostTimerFired()
@@ -1295,8 +1305,10 @@
 
 void AXObjectCache::deferMenuListValueChange(Element* element)
 {
-    if (element)
-        m_deferredMenuListChange.add(*element);
+    if (!element)
+        return;
+
+    m_deferredMenuListChange.add(*element);
     if (!m_performCacheUpdateTimer.isActive())
         m_performCacheUpdateTimer.startOneShot(0_s);
 }
@@ -1303,14 +1315,15 @@
 
 void AXObjectCache::deferModalChange(Element* element)
 {
-    if (element) {
-        m_deferredModalChangedList.add(*element);
+    if (!element)
+        return;
 
-        // Notify that parent's children have changed.
-        if (auto* axParent = get(element->parentNode()))
-            m_deferredChildrenChangedList.add(axParent);
-    }
+    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);
 }
@@ -3232,7 +3245,7 @@
     filterListForRemoval(m_textMarkerNodes, document, nodesToRemove);
     filterListForRemoval(m_modalElementsSet, document, nodesToRemove);
     filterListForRemoval(m_deferredTextChangedList, document, nodesToRemove);
-    filterListForRemoval(m_deferredChildrenChangedNodeList, document, nodesToRemove);
+    filterListForRemoval(m_deferredNodeAddedOrRemovedList, document, nodesToRemove);
     filterWeakHashSetForRemoval(m_deferredRecomputeIsIgnoredList, document, nodesToRemove);
     filterWeakHashSetForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
     filterWeakHashSetForRemoval(m_deferredModalChangedList, document, nodesToRemove);
@@ -3277,11 +3290,11 @@
         return;
     SetForScope performingDeferredCacheUpdate(m_performingDeferredCacheUpdate, true);
 
-    for (auto* nodeChild : m_deferredChildrenChangedNodeList) {
+    for (auto* nodeChild : m_deferredNodeAddedOrRemovedList) {
         handleMenuOpened(nodeChild);
         handleLiveRegionCreated(nodeChild);
     }
-    m_deferredChildrenChangedNodeList.clear();
+    m_deferredNodeAddedOrRemovedList.clear();
 
     processDeferredChildrenChangedList();
 

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (293565 => 293566)


--- trunk/Source/WebCore/accessibility/AXObjectCache.h	2022-04-28 05:32:05 UTC (rev 293565)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h	2022-04-28 05:36:41 UTC (rev 293566)
@@ -196,6 +196,7 @@
     void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
     void deferModalChange(Element*);
     void deferMenuListValueChange(Element*);
+    void deferNodeAddedOrRemoved(Node*);
     void handleScrolledToAnchor(const Node* anchorNode);
     void handleScrollbarUpdate(ScrollView*);
     
@@ -523,7 +524,7 @@
     ListHashSet<Node*> m_deferredTextChangedList;
     WeakHashSet<Element> m_deferredSelectedChildredChangedList;
     ListHashSet<RefPtr<AccessibilityObject>> m_deferredChildrenChangedList;
-    ListHashSet<Node*> m_deferredChildrenChangedNodeList;
+    ListHashSet<Node*> m_deferredNodeAddedOrRemovedList;
     WeakHashSet<Element> m_deferredModalChangedList;
     WeakHashSet<Element> m_deferredMenuListChange;
     HashMap<Element*, String> m_deferredTextFormControlValue;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to