Title: [261934] trunk/Source/WebCore
Revision
261934
Author
andresg...@apple.com
Date
2020-05-20 11:21:10 -0700 (Wed, 20 May 2020)

Log Message

Fix for accessibility-node-memory-management.html in isolated tree mode.
https://bugs.webkit.org/show_bug.cgi?id=212142

Reviewed by Chris Fleizach.

LayoutTests/accessibility/accessibility-node-memory-management.html.

- Fix in applyPendingChanges that was not removing removed nodes from
the nodes map. This was causing that some detached AXIsolatedObjects
were being returned.
- Also handle the case where pending changes can come from a detached
AXObject with invalid ID and no platform wrapper.
- updateChildren better handles the case when the notification target
is not in the isolated tree by walking up the object hierarchy until it
finds an associated object that does have a corresponding isolated object.

* accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::treeForPageID):
(WebCore::AXIsolatedTree::updateChildren):
(WebCore::AXIsolatedTree::applyPendingChanges):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (261933 => 261934)


--- trunk/Source/WebCore/ChangeLog	2020-05-20 18:01:49 UTC (rev 261933)
+++ trunk/Source/WebCore/ChangeLog	2020-05-20 18:21:10 UTC (rev 261934)
@@ -1,3 +1,26 @@
+2020-05-20  Andres Gonzalez  <andresg...@apple.com>
+
+        Fix for accessibility-node-memory-management.html in isolated tree mode.
+        https://bugs.webkit.org/show_bug.cgi?id=212142
+
+        Reviewed by Chris Fleizach.
+
+        LayoutTests/accessibility/accessibility-node-memory-management.html.
+
+        - Fix in applyPendingChanges that was not removing removed nodes from
+        the nodes map. This was causing that some detached AXIsolatedObjects
+        were being returned.
+        - Also handle the case where pending changes can come from a detached
+        AXObject with invalid ID and no platform wrapper.
+        - updateChildren better handles the case when the notification target
+        is not in the isolated tree by walking up the object hierarchy until it
+        finds an associated object that does have a corresponding isolated object.
+
+        * accessibility/isolatedtree/AXIsolatedTree.cpp:
+        (WebCore::AXIsolatedTree::treeForPageID):
+        (WebCore::AXIsolatedTree::updateChildren):
+        (WebCore::AXIsolatedTree::applyPendingChanges):
+
 2020-05-20  Antoine Quint  <grao...@apple.com>
 
         [Web Animations] Animation engine should not wake up every tick for steps timing functions

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (261933 => 261934)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2020-05-20 18:01:49 UTC (rev 261933)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp	2020-05-20 18:21:10 UTC (rev 261934)
@@ -129,7 +129,6 @@
 
 RefPtr<AXIsolatedTree> AXIsolatedTree::treeForPageID(PageIdentifier pageID)
 {
-    AXTRACE("AXIsolatedTree::treeForPageID");
     LockHolder locker(s_cacheLock);
 
     if (auto tree = treePageCache().get(pageID))
@@ -237,21 +236,27 @@
     if (!axObject.document() || !axObject.document()->hasLivingRenderTree())
         return;
 
-    AXID axObjectID = axObject.objectID();
-
     applyPendingChanges();
     LockHolder locker { m_changeLogLock };
-    auto object = nodeForID(axObjectID);
-    if (!object) {
-        AXLOG("No associated isolated object!");
+
+    // updateChildren may be called as the result of a children changed
+    // notification for an axObject that has no associated isolated object.
+    // An example of this is when an empty element such as a <canvas> or <div>
+    // is added a new child. So find the closest ancestor of axObject that has
+    // an associated isolated object and update its children.
+    RefPtr<AXIsolatedObject> object;
+    auto* axAncestor = Accessibility::findAncestor(axObject, true, [&object, this] (const AXCoreObject& ancestor) {
+        return object = nodeForID(ancestor.objectID());
+    });
+    ASSERT(object && object->objectID() != InvalidAXID);
+    if (!axAncestor || !object)
         return; // nothing to update.
-    }
 
     auto removals = object->m_childrenIDs;
     locker.unlockEarly();
 
-    const auto& axChildren = axObject.children();
-    auto axChildrenIDs = axObject.childrenIDs();
+    const auto& axChildren = axAncestor->children();
+    auto axChildrenIDs = axAncestor->childrenIDs();
 
     for (size_t i = 0; i < axChildrenIDs.size(); ++i) {
         size_t index = removals.find(axChildrenIDs[i]);
@@ -261,7 +266,7 @@
             // This is a new child, add it to the tree.
             AXLOG("Adding a new child for:");
             AXLOG(axChildren[i]);
-            generateSubtree(*axChildren[i], axObjectID, true);
+            generateSubtree(*axChildren[i], object->objectID(), true);
         }
     }
 
@@ -350,29 +355,23 @@
     while (m_pendingNodeRemovals.size()) {
         auto axID = m_pendingNodeRemovals.takeLast();
         AXLOG(makeString("removing axID ", axID));
-        if (axID == InvalidAXID)
-            continue;
-
-        if (auto object = nodeForID(axID))
+        if (auto object = nodeForID(axID)) {
             object->detach(AccessibilityDetachmentType::ElementDestroyed);
+            m_readerThreadNodeMap.remove(axID);
+        }
     }
 
     while (m_pendingSubtreeRemovals.size()) {
         auto axID = m_pendingSubtreeRemovals.takeLast();
         AXLOG(makeString("removing subtree axID ", axID));
-        if (axID == InvalidAXID)
-            continue;
-
         if (auto object = nodeForID(axID)) {
             object->detach(AccessibilityDetachmentType::ElementDestroyed);
             m_pendingSubtreeRemovals.appendVector(object->m_childrenIDs);
+            m_readerThreadNodeMap.remove(axID);
         }
     }
 
     for (const auto& item : m_pendingAppends) {
-        // Either the new object has a wrapper already attached, or one is passed to be attached, not both.
-        ASSERT((item.m_isolatedObject->wrapper() || item.m_wrapper)
-            && !(item.m_isolatedObject->wrapper() && item.m_wrapper));
         AXID axID = item.m_isolatedObject->objectID();
         AXLOG(makeString("appending axID ", axID));
         if (axID == InvalidAXID)
@@ -379,6 +378,8 @@
             continue;
 
         auto& wrapper = item.m_wrapper ? item.m_wrapper : item.m_isolatedObject->wrapper();
+        if (!wrapper)
+            continue;
 
         if (auto object = m_readerThreadNodeMap.get(axID)) {
             if (object != &item.m_isolatedObject.get()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to