Changeset 277434 in webkit

Timestamp:

May 13, 2021, 7:30:23 AM (5 years ago)

Author:

Chris Fleizach

Message:

AX: Crash at WebCore::Document::updateLayout
​https://bugs.webkit.org/show_bug.cgi?id=225677

Reviewed by Alan Bujtas.

Source/WebCore:

Handling notifications while during layout is risky because we can call back into an update method.
We have handled many other cases like this by deferring the notification, as we do here.
Special note: this test can't be made to crash because actual crash requires the accessibility
runtime to be initialized and posting real notifications to the system. But, we can verify that the
notification is still sent correctly under conditions that could lead to a crash.

Test: accessibility/mac/menu-selection-notification-crash.html

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::deferMenuListValueChange):
(WebCore::AXObjectCache::performDeferredCacheUpdate):

• accessibility/AXObjectCache.h:
• accessibility/AccessibilityMenuList.cpp:

(WebCore::AccessibilityMenuList::didUpdateActiveOption):

LayoutTests:

• accessibility/mac/menu-selection-notification-crash-expected.txt: Added.
• accessibility/mac/menu-selection-notification-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/mac/menu-selection-notification-crash-expected.txt (added)
• LayoutTests/accessibility/mac/menu-selection-notification-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AXObjectCache.cpp (modified) (2 diffs)
• Source/WebCore/accessibility/AXObjectCache.h (modified) (2 diffs)
• Source/WebCore/accessibility/AccessibilityMenuList.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-05-13  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: Crash at WebCore::Document::updateLayout
+        https://bugs.webkit.org/show_bug.cgi?id=225677
+
+        Reviewed by Alan Bujtas.
+
+        * accessibility/mac/menu-selection-notification-crash-expected.txt: Added.
+        * accessibility/mac/menu-selection-notification-crash.html: Added.
+
 2021-05-13  Enrique Ocaña González  <eocanha@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-13  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: Crash at WebCore::Document::updateLayout
+        https://bugs.webkit.org/show_bug.cgi?id=225677
+
+        Reviewed by Alan Bujtas.
+
+        Handling notifications while during layout is risky because we can call back into an update method.
+        We have handled many other cases like this by deferring the notification, as we do here.
+        Special note: this test can't be made to crash because actual crash requires the accessibility
+        runtime to be initialized and posting real notifications to the system. But, we can verify that the
+        notification is still sent correctly under conditions that could lead to a crash.
+
+        Test: accessibility/mac/menu-selection-notification-crash.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::deferMenuListValueChange):
+        (WebCore::AXObjectCache::performDeferredCacheUpdate):
+        * accessibility/AXObjectCache.h:
+        * accessibility/AccessibilityMenuList.cpp:
+        (WebCore::AccessibilityMenuList::didUpdateActiveOption):
+
 2021-05-13  Enrique Ocaña González  <eocanha@igalia.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -1229 +1229 @@
     } else
         handleFocusedUIElementChanged(oldNode, newNode);
+}
+
+void AXObjectCache::deferMenuListValueChange(Element* element)
+{
+    m_deferredMenuListChange.add(element);
+    if (!m_performCacheUpdateTimer.isActive())
+        m_performCacheUpdateTimer.startOneShot(0_s);
 }
 
@@ -3197 +3204 @@
     m_deferredModalChangedList.clear();
 
+    for (auto& deferredMenuListChangeElement : m_deferredMenuListChange)
+        postNotification(&deferredMenuListChangeElement, AXObjectCache::AXMenuListValueChanged);
+    m_deferredMenuListChange.clear();
+    
     platformPerformDeferredCacheUpdate();
 }

trunk/Source/WebCore/accessibility/AXObjectCache.h

@@ -188 +188 @@
     void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
     void deferModalChange(Element*);
+    void deferMenuListValueChange(Element*);
     void handleScrolledToAnchor(const Node* anchorNode);
     void handleScrollbarUpdate(ScrollView*);
@@ -514 +515 @@
     ListHashSet<Node*> m_deferredChildrenChangedNodeList;
     WeakHashSet<Element> m_deferredModalChangedList;
+    WeakHashSet<Element> m_deferredMenuListChange;
     HashMap<Element*, String> m_deferredTextFormControlValue;
     HashMap<Element*, QualifiedName> m_deferredAttributeChange;

trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp

@@ -137 +137 @@
 
     if (auto* cache = document->axObjectCache())
-        cache->postNotification(this, document.ptr(), AXObjectCache::AXMenuListValueChanged);
+        cache->deferMenuListValueChange(element());
 }