Skip to content

Commit

Permalink
AX: Relations updates thrash between dirty and clean when multiple de…
Browse files Browse the repository at this point in the history
…ferred id attribute changes are processed

https://bugs.webkit.org/show_bug.cgi?id=260370
rdar://problem/114052085

Reviewed by Andres Gonzalez.

In AXObjectCache::handleAttributeChange, any change to the `id`
attribute causes AXObjectCache::m_relationsNeedUpdate to become true.

This is problematic when `m_deferredAttributeChange` contains multiple
`id` attribute changes, as we thrash between setting m_relationsNeedUpdate to true,
immediately resetting it to false as a result of an arbitrary` parentObject` call,
and then re-dirtying it with the next id attribute change.

With this patch (authored by Andres Gonzalez), we only update relations
once, even when a group of id attribute changes are processed.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::performDeferredCacheUpdate):
(WebCore::AXObjectCache::relationsNeedUpdate):
* Source/WebCore/accessibility/AXObjectCache.h:

Canonical link: https://commits.webkit.org/267190@main
  • Loading branch information
twilco authored and AndresGonzalezApple committed Aug 23, 2023
1 parent 1534dab commit f8ef167
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,8 @@ void AXObjectCache::deferAttributeChangeIfNeeded(Element* element, const Qualifi
}
RefPtr protectedElement { element };
handleAttributeChange(protectedElement.get(), attrName, oldValue, newValue);
if (attrName == idAttr)
relationsNeedUpdate(true);
}

bool AXObjectCache::shouldProcessAttributeChange(Element* element, const QualifiedName& attrName)
Expand Down Expand Up @@ -2373,7 +2375,6 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
else if (attrName == hrefAttr)
updateIsolatedTree(get(element), AXURLChanged);
else if (attrName == idAttr) {
relationsNeedUpdate(true);
#if !LOG_DISABLED
updateIsolatedTree(get(element), AXIdAttributeChanged);
#endif
Expand Down Expand Up @@ -3854,9 +3855,15 @@ void AXObjectCache::performDeferredCacheUpdate()
m_deferredTextFormControlValue.clear();

AXLOGDeferredCollection("AttributeChange"_s, m_deferredAttributeChange);
for (const auto& attributeChange : m_deferredAttributeChange)
bool idAttributeChanged = false;
for (const auto& attributeChange : m_deferredAttributeChange) {
handleAttributeChange(attributeChange.element.get(), attributeChange.attrName, attributeChange.oldValue, attributeChange.newValue);
if (attributeChange.attrName == idAttr)
idAttributeChanged = true;
}
m_deferredAttributeChange.clear();
if (idAttributeChanged)
relationsNeedUpdate(true);

if (m_deferredFocusedNodeChange) {
AXLOG(makeString(
Expand Down

0 comments on commit f8ef167

Please sign in to comment.