Changeset 263571 in webkit

Timestamp:

Jun 26, 2020 11:07:35 AM (4 years ago)

Author:

Andres Gonzalez

Message:

Fix for crash in accessibility/roles-exposed.html in isolated tree mode.
​https://bugs.webkit.org/show_bug.cgi?id=213648

Reviewed by Chris Fleizach.

LayoutTest: accessibility/roles-exposed.html.

• In layout tests, when AXObjectCache::notificationPostTimerFired is

triggered, and we try to update the isolated tree, some of the
underlying objects may already be gone. So this change ensure we don't
try to update an isolated object that corresponds to an already detached
live object.

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::cacheAndInitializeWrapper): Sanity check.

• accessibility/AccessibilityListBox.cpp:

(WebCore::AccessibilityListBox::addChildren): m_renderer can be null
when trying to update the isolated tree.

• accessibility/AccessibilityListBoxOption.cpp:

(WebCore::AccessibilityListBoxOption::computeAccessibilityIsIgnored const):
Parent object may be gone when trying to update the isolated tree.

• accessibility/isolatedtree/AXIsolatedTree.cpp:

(WebCore::AXIsolatedTree::updateChildrenIDs):
(WebCore::AXIsolatedTree::generateSubtree):
(WebCore::AXIsolatedTree::updateChildren):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• accessibility/AXObjectCache.cpp (modified) (2 diffs)
• accessibility/AccessibilityListBox.cpp (modified) (1 diff)
• accessibility/AccessibilityListBoxOption.cpp (modified) (1 diff)
• accessibility/isolatedtree/AXIsolatedTree.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-06-26  Andres Gonzalez  <andresg_22@apple.com>
+
+        Fix for crash in accessibility/roles-exposed.html in isolated tree mode.
+        https://bugs.webkit.org/show_bug.cgi?id=213648
+
+        Reviewed by Chris Fleizach.
+
+        LayoutTest: accessibility/roles-exposed.html.
+
+        - In layout tests, when AXObjectCache::notificationPostTimerFired is
+        triggered, and we try to update the isolated tree, some of the
+        underlying objects may already be gone. So this change ensure we don't
+        try to update an isolated object that corresponds to an already detached
+        live object.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::cacheAndInitializeWrapper): Sanity check.
+        * accessibility/AccessibilityListBox.cpp:
+        (WebCore::AccessibilityListBox::addChildren): m_renderer can be null
+        when trying to update the isolated tree.
+        * accessibility/AccessibilityListBoxOption.cpp:
+        (WebCore::AccessibilityListBoxOption::computeAccessibilityIsIgnored const):
+        Parent object may be gone when trying to update the isolated tree.
+        * accessibility/isolatedtree/AXIsolatedTree.cpp:
+        (WebCore::AXIsolatedTree::updateChildrenIDs):
+        (WebCore::AXIsolatedTree::generateSubtree):
+        (WebCore::AXIsolatedTree::updateChildren):
+
 2020-06-26  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -588 +588 @@
     ASSERT(newObject);
     AXID axID = getAXID(newObject);
+    ASSERT(axID != InvalidAXID);
+
     WTF::switchOn(domObject,
         [&axID, this] (RenderObject* typedValue) { m_renderObjectMapping.set(typedValue, axID); },
@@ -594 +596 @@
         [] (auto&) { }
     );
+
     m_objects.set(axID, newObject);
     newObject->init();

trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp

@@ -65 +65 @@
 void AccessibilityListBox::addChildren()
 {
+    if (!m_renderer)
+        return;
+
     Node* selectNode = m_renderer->node();
     if (!selectNode)
         return;
-    
+
     m_haveChildren = true;
-    
+
     for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems()) {
         // The cast to HTMLElement below is safe because the only other possible listItem type

trunk/Source/WebCore/accessibility/AccessibilityListBoxOption.cpp

@@ -118 +118 @@
     if (accessibilityIsIgnoredByDefault())
         return true;
-    
-    return parentObject()->accessibilityIsIgnored();
+
+    auto* parent = parentObject();
+    return parent ? parent->accessibilityIsIgnored() : true;
 }
     

trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

@@ -167 +167 @@
     ASSERT(m_changeLogLock.isLocked());
 
-    m_nodeMap.set(axID, childrenIDs);
-    m_pendingChildrenUpdates.append(std::make_pair(axID, WTFMove(childrenIDs)));
+    if (axID != InvalidAXID) {
+        m_nodeMap.set(axID, childrenIDs);
+        m_pendingChildrenUpdates.append(std::make_pair(axID, WTFMove(childrenIDs)));
+    }
 }
 
@@ -183 +185 @@
     if (!axParent)
         setRootNode(object.ptr());
-    else
+    else if (axParent->objectID() != InvalidAXID) // Need to check for the objectID of axParent again because it may have been detached while traversing the tree.
         updateChildrenIDs(axParent->objectID(), axParent->childrenIDs());
 }
@@ -279 +281 @@
     });
     ASSERT(axAncestor && iterator != m_nodeMap.end());
-    if (!axAncestor || iterator == m_nodeMap.end())
+    if (!axAncestor || axAncestor->objectID() == InvalidAXID || iterator == m_nodeMap.end())
         return; // nothing to update.
 
@@ -289 +291 @@
     auto axChildrenIDs = axAncestor->childrenIDs();
 
-    for (size_t i = 0; i < axChildrenIDs.size(); ++i) {
+    for (size_t i = 0; i < axChildren.size() && i < axChildrenIDs.size(); ++i) {
         size_t index = removals.find(axChildrenIDs[i]);
         if (index != notFound)
             removals.remove(index);
         else {
+            ASSERT(axChildren[i]->objectID() == axChildrenIDs[i]);
             // This is a new child, add it to the tree.
             AXLOG("Adding a new child for:");