Changeset 295779 in webkit


Ignore:
Timestamp:
Jun 23, 2022 9:44:06 AM (21 months ago)
Author:
Tyler Wilcock
Message:

AX: AXIsolatedTree::updateChildren sometimes fails to update isolated subtrees when the given live object is ignored
https://bugs.webkit.org/show_bug.cgi?id=241735

Reviewed by Andres Gonzalez.

Our current algorithm in AXIsolatedTree::updateChildren is:

  1. If the object we got an AXChildrenChanged notification for is in the isolated tree, update its isolated children
  1. Otherwise, ascend the ancestry to the nearest in-isolated-tree ancestor, and update the children of that object

This is not always adequate when the object passed to updateChildren
is ignored, as in some cases the ancestor has no children changes
but the subtrees of the ignored object do.

This patch fixes this by also checking the live-children of the ignored
object for any that have unitialized children. If so, we call
AXIsolatedTree::updateChildren on those too.

  • Source/WebCore/accessibility/AccessibilityObject.h:

(WebCore::AccessibilityObject::childrenInitialized const):
Move from private to public.

  • Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:

(WebCore::AXIsolatedTree::updateChildren):

Canonical link: https://commits.webkit.org/251784@main

Location:
trunk/Source/WebCore/accessibility
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/accessibility/AccessibilityObject.h

    r295139 r295779  
    498498
    499499    virtual void updateRole() { }
     500    bool childrenInitialized() const { return m_childrenInitialized; }
    500501    const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) override;
    501502    virtual void addChildren() { }
     
    828829   
    829830protected: // FIXME: Make the data members private.
    830     bool childrenInitialized() const { return m_childrenInitialized; }
    831831    AccessibilityChildrenVector m_children;
    832832    mutable bool m_childrenInitialized { false };
  • trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

    r295727 r295779  
    210210}
    211211
     212static bool shouldCreateNodeChange(AXCoreObject& axObject)
     213{
     214    // We should never create an isolated object from an ignored object or one with an invalid ID.
     215    return !axObject.accessibilityIsIgnored() && axObject.objectID().isValid();
     216}
     217
    212218std::optional<AXIsolatedTree::NodeChange> AXIsolatedTree::nodeChangeForObject(Ref<AXCoreObject> axObject, AttachWrapper attachWrapper)
    213219{
    214220    ASSERT(isMainThread());
    215 
    216     // We should never create an isolated object from an ignored object.
    217     if (axObject->accessibilityIsIgnored())
     221    ASSERT(axObject->objectID().isValid());
     222
     223    if (!shouldCreateNodeChange(axObject.get()))
    218224        return std::nullopt;
    219 
    220     if (!axObject->objectID().isValid()) {
    221         // Either the axObject has an invalid ID or something else went terribly wrong. Don't bother doing anything else.
    222         ASSERT_NOT_REACHED();
    223         return std::nullopt;
    224     }
    225225
    226226    auto object = AXIsolatedObject::create(axObject, this);
     
    459459}
    460460
    461 void AXIsolatedTree::updateChildren(AXCoreObject& axObject)
     461void AXIsolatedTree::updateChildren(AXCoreObject& axObject, ResolveNodeChanges resolveNodeChanges)
    462462{
    463463    AXTRACE("AXIsolatedTree::updateChildren"_s);
     
    490490    }
    491491
    492 #ifndef NDEBUG
    493492    if (axAncestor != &axObject) {
    494493        AXLOG(makeString("Original object with ID ", axObject.objectID().loggingString(), " wasn't in the isolated tree, so instead updating the closest in-isolated-tree ancestor:"));
    495494        AXLOG(axAncestor);
    496     }
    497 #endif
     495        for (auto& child : axObject.children()) {
     496            auto* liveChild = dynamicDowncast<AccessibilityObject>(child.get());
     497            if (!liveChild || liveChild->childrenInitialized())
     498                continue;
     499
     500            if (!m_nodeMap.contains(liveChild->objectID())) {
     501                if (!shouldCreateNodeChange(*liveChild))
     502                    continue;
     503
     504                // This child should be added to the isolated tree but hasn't been yet.
     505                // Add it to the nodemap so the recursive call to updateChildren below properly builds the subtree for this object.
     506                auto* parent = liveChild->parentObjectUnignored();
     507                m_nodeMap.set(liveChild->objectID(), ParentChildrenIDs { parent ? parent->objectID() : AXID(), liveChild->childrenIDs() });
     508                m_unresolvedPendingAppends.set(liveChild->objectID(), AttachWrapper::OnMainThread);
     509            }
     510
     511            AXLOG(makeString(
     512                "Child ID ", liveChild->objectID().loggingString(), " of original object ID ", axObject.objectID().loggingString(), " was found in the isolated tree with uninitialized live children. Updating its isolated children."
     513            ));
     514            // Don't immediately resolve node changes in these recursive calls to updateChildren. This avoids duplicate node change creation in this scenario:
     515            //   1. Some subtree is updated in the below call to updateChildren.
     516            //   2. Later in this function, when updating axAncestor, we update some higher subtree that includes the updated subtree from step 1.
     517            updateChildren(*liveChild, ResolveNodeChanges::No);
     518        }
     519    }
    498520
    499521    auto oldIDs = m_nodeMap.get(axAncestor->objectID());
     
    529551            removeSubtreeFromNodeMap(axID, axAncestor);
    530552    }
    531     queueRemovalsAndUnresolvedChanges(oldChildrenIDs);
     553
     554    if (resolveNodeChanges == ResolveNodeChanges::Yes)
     555        queueRemovalsAndUnresolvedChanges(oldChildrenIDs);
     556    else
     557        queueRemovals(oldChildrenIDs);
    532558
    533559    // Also queue updates to the target node itself and any properties that depend on children().
  • trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

    r295727 r295779  
    347347    void generateSubtree(AXCoreObject&);
    348348    void updateNode(AXCoreObject&);
    349     void updateChildren(AXCoreObject&);
     349    enum class ResolveNodeChanges : bool { No, Yes };
     350    void updateChildren(AXCoreObject&, ResolveNodeChanges = ResolveNodeChanges::Yes);
    350351    void updateNodeProperty(AXCoreObject&, AXPropertyName);
    351352    void updateNodeAndDependentProperties(AXCoreObject&);
Note: See TracChangeset for help on using the changeset viewer.