Skip to content

Commit

Permalink
AX: should be able to compute AXPosition using the isolated tree
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=267957
rdar://problem/121472048

Reviewed by Andres Gonzalez.

The relative frame that we're caching in the isolated tree is already
in the correct screen coordinates, all that's necessary is to cache
the screen relative position of the root, and then add them together.

We can update the cached root position when AXGeometryManager
updates object regions.

There isn't an easy way to write a layout test for this, as there
doesn't seem to be a way to move the window's origin.

In order to manually test this, I temporarily disabled AXRelativeFrame,
forcing VoiceOver to use AXPosition for every element, then I checked
that the bounds are correct when navigating and when moving and resizing
the window.

The main performance win I observe is when VoiceOver tries to focus on the
root web area when the page is busy. Without this patch, VoiceOver can
get blocked on the main thread, but with this patch it's no longer blocked.

* Source/WebCore/accessibility/AXGeometryManager.cpp:
(WebCore::AXGeometryManager::willUpdateObjectRegions):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::screenRelativePosition const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::updateNodeProperties):
(WebCore::AXIsolatedTree::updateRootScreenRelativePosition):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

Canonical link: https://commits.webkit.org/273716@main
  • Loading branch information
Dominic Mazzoni committed Jan 30, 2024
1 parent 3a4f8bd commit 551f6e6
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 0 deletions.
6 changes: 6 additions & 0 deletions Source/WebCore/accessibility/AXGeometryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ void AXGeometryManager::willUpdateObjectRegions()
{
if (m_updateObjectRegionsTimer.isActive())
m_updateObjectRegionsTimer.stop();

if (!m_cache)
return;

if (RefPtr tree = AXIsolatedTree::treeForPageID(m_cache->pageID()))
tree->updateRootScreenRelativePosition();
}

void AXGeometryManager::scheduleRenderingUpdate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,15 @@ FloatPoint AXIsolatedObject::screenRelativePosition() const
if (auto point = optionalAttributeValue<FloatPoint>(AXPropertyName::ScreenRelativePosition))
return *point;

if (auto rootNode = tree()->rootNode()) {
if (auto rootPoint = rootNode->optionalAttributeValue<FloatPoint>(AXPropertyName::ScreenRelativePosition)) {
// Relative frames are top-left origin, but screen relative positions are bottom-left origin.
FloatRect rootRelativeFrame = rootNode->relativeFrame();
FloatRect relativeFrame = this->relativeFrame();
return { rootPoint->x() + relativeFrame.x(), rootPoint->y() + (rootRelativeFrame.maxY() - relativeFrame.maxY()) };
}
}

return Accessibility::retrieveValueFromMainThread<FloatPoint>([&, this] () -> FloatPoint {
if (auto* axObject = associatedAXObject())
return axObject->screenRelativePosition();
Expand Down
15 changes: 15 additions & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const AXProper
case AXPropertyName::CellScope:
propertyMap.set(AXPropertyName::CellScope, axObject.cellScope().isolatedCopy());
break;
case AXPropertyName::ScreenRelativePosition:
propertyMap.set(AXPropertyName::ScreenRelativePosition, axObject.screenRelativePosition());
break;
case AXPropertyName::SelectedTextRange:
propertyMap.set(AXPropertyName::SelectedTextRange, axObject.selectedTextRange());
break;
Expand Down Expand Up @@ -1017,6 +1020,18 @@ void AXIsolatedTree::updateFrame(AXID axID, IntRect&& newFrame)
m_pendingPropertyChanges.append({ axID, WTFMove(propertyMap) });
}

void AXIsolatedTree::updateRootScreenRelativePosition()
{
AXTRACE("AXIsolatedTree::updateRootScreenRelativePosition"_s);
ASSERT(isMainThread());

if (!rootNode())
return;

if (auto* axRoot = rootNode()->associatedAXObject())
updateNodeProperties(*axRoot, { AXPropertyName::ScreenRelativePosition });
}

void AXIsolatedTree::removeNode(const AccessibilityObject& axObject)
{
AXTRACE("AXIsolatedTree::removeNode"_s);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
void updateDependentProperties(AccessibilityObject&);
void updatePropertiesForSelfAndDescendants(AccessibilityObject&, const AXPropertyNameSet&);
void updateFrame(AXID, IntRect&&);
void updateRootScreenRelativePosition();
void overrideNodeProperties(AXID, AXPropertyMap&&);

double loadingProgress() { return m_loadingProgress; }
Expand Down

0 comments on commit 551f6e6

Please sign in to comment.