Commit b386a958 authored by Kurt Catti-Schmidt's avatar Kurt Catti-Schmidt Committed by Commit Bot

UIA move-by-format performance improvements

This change introduces two main optimizations for move-by-format. Both
of these optimizations combined end up improving performance of Narrator
heading moves by 25% on NYTimes.com and 40% on a reduced test page.
I'm now seeing the format move cost dwarfed by the GetAttributeValue
cost for Narrator Scan Mode move-by-heading, which I'll look into next.

The first category of improvements in this change is from replacing
AsLeafTextPosition calls with AsLeafTreePosition. When we're moving
by format in intermediate steps, it doesn't matter whether the node
is a tree position or a text position, so this avoids expensive
MaxTextOffset calls.

The second category involves returning a reason why a format boundary
was determined. I saw that we were calling
Create(Next/Previous)TreePosition several times just to see if a move is
at the end of a document. So a more complex return value that provides
the reason of why a format boundary exists allows us to not have to do
that work multiple times. The AXBoundaryType enum class added with this
change provides this information, with the new method
'GetFormatEndBoundaryType' doing the work of determining format
boundaries and returning the appropriate AXBoundaryType.

Bug: 1029867
Change-Id: I12151ab2b4000227e1d5e2726223a7545d73bc2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1965865Reviewed-by: default avatarKevin Babbitt <kbabbitt@microsoft.com>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727375}
parent e68c02f6
...@@ -52,6 +52,20 @@ enum class AXBoundaryBehavior { ...@@ -52,6 +52,20 @@ enum class AXBoundaryBehavior {
StopAtLastAnchorBoundary StopAtLastAnchorBoundary
}; };
// Describes in further detail what type of boundary a current position is on.
// For complex boundaries such as format boundaries, it can be useful to know
// why a particular boundary was chosen.
enum class AXBoundaryType {
// Not at a unit boundary.
kNone,
// At a unit boundary (e.g. a format boundary).
kUnitBoundary,
// At the start of a document.
kDocumentStart,
// At the end of a document.
kDocumentEnd
};
// When converting to an unignored position, determines how to adjust the new // When converting to an unignored position, determines how to adjust the new
// position in order to make it valid, either moving backwards or forwards in // position in order to make it valid, either moving backwards or forwards in
// the accessibility tree. // the accessibility tree.
...@@ -706,38 +720,54 @@ class AXPosition { ...@@ -706,38 +720,54 @@ class AXPosition {
} }
} }
bool AtStartOfFormat() const { AXBoundaryType GetFormatStartBoundaryType() const {
// Since formats are stored on text anchors, the start of a format boundary // Since formats are stored on text anchors, the start of a format boundary
// must be at the start of an anchor. // must be at the start of an anchor.
if (IsNullPosition() || !AtStartOfAnchor()) if (IsNullPosition() || !AtStartOfAnchor())
return false; return AXBoundaryType::kNone;
// Treat the first iterable node as a format boundary. // Treat the first iterable node as a format boundary.
if (CreatePreviousLeafTreePosition()->IsNullPosition()) if (CreatePreviousLeafTreePosition()->IsNullPosition())
return true; return AXBoundaryType::kDocumentStart;
// Iterate over anchors until a format boundary is found. This will return a // Iterate over anchors until a format boundary is found. This will return a
// null position upon crossing a boundary. // null position upon crossing a boundary.
AXPositionInstance previous_position = CreatePreviousLeafTreePosition( AXPositionInstance previous_position = CreatePreviousLeafTreePosition(
base::BindRepeating(&AbortMoveAtFormatBoundary)); base::BindRepeating(&AbortMoveAtFormatBoundary));
return previous_position->IsNullPosition();
if (previous_position->IsNullPosition())
return AXBoundaryType::kUnitBoundary;
return AXBoundaryType::kNone;
} }
bool AtEndOfFormat() const { bool AtStartOfFormat() const {
return GetFormatStartBoundaryType() != AXBoundaryType::kNone;
}
AXBoundaryType GetFormatEndBoundaryType() const {
// Since formats are stored on text anchors, the end of a format break must // Since formats are stored on text anchors, the end of a format break must
// be at the end of an anchor. // be at the end of an anchor.
if (IsNullPosition() || !AtEndOfAnchor()) if (IsNullPosition() || !AtEndOfAnchor())
return false; return AXBoundaryType::kNone;
// Treat the last iterable node as a format boundary // Treat the last iterable node as a format boundary
if (CreateNextLeafTreePosition()->IsNullPosition()) if (CreateNextLeafTreePosition()->IsNullPosition())
return true; return AXBoundaryType::kDocumentEnd;
// Iterate over anchors until a format boundary is found. This will return a // Iterate over anchors until a format boundary is found. This will return a
// null position upon crossing a boundary. // null position upon crossing a boundary.
AXPositionInstance next_position = CreateNextLeafTreePosition( AXPositionInstance next_position = CreateNextLeafTreePosition(
base::BindRepeating(&AbortMoveAtFormatBoundary)); base::BindRepeating(&AbortMoveAtFormatBoundary));
return next_position->IsNullPosition();
if (next_position->IsNullPosition())
return AXBoundaryType::kUnitBoundary;
return AXBoundaryType::kNone;
}
bool AtEndOfFormat() const {
return GetFormatEndBoundaryType() != AXBoundaryType::kNone;
} }
bool AtStartOfInlineBlock() const { bool AtStartOfInlineBlock() const {
...@@ -1968,12 +1998,11 @@ class AXPosition { ...@@ -1968,12 +1998,11 @@ class AXPosition {
if (IsNullPosition()) if (IsNullPosition())
return Clone(); return Clone();
// AtStartOfFormat() always returns true if we are at the first iterable AXBoundaryType boundary_type = GetFormatStartBoundaryType();
// position, i.e. CreatePreviousLeafTreePosition()->IsNullPosition(). if (boundary_type != AXBoundaryType::kNone) {
if (AtStartOfFormat()) {
if (boundary_behavior == AXBoundaryBehavior::StopIfAlreadyAtBoundary || if (boundary_behavior == AXBoundaryBehavior::StopIfAlreadyAtBoundary ||
(boundary_behavior == AXBoundaryBehavior::StopAtLastAnchorBoundary && (boundary_behavior == AXBoundaryBehavior::StopAtLastAnchorBoundary &&
CreatePreviousLeafTreePosition()->IsNullPosition())) { boundary_type == AXBoundaryType::kDocumentStart)) {
AXPositionInstance clone = Clone(); AXPositionInstance clone = Clone();
// In order to make equality checks simpler, affinity should be reset so // In order to make equality checks simpler, affinity should be reset so
// that we would get consistent output from this function regardless of // that we would get consistent output from this function regardless of
...@@ -1981,7 +2010,7 @@ class AXPosition { ...@@ -1981,7 +2010,7 @@ class AXPosition {
clone->affinity_ = ax::mojom::TextAffinity::kDownstream; clone->affinity_ = ax::mojom::TextAffinity::kDownstream;
return clone; return clone;
} else if (boundary_behavior == AXBoundaryBehavior::CrossBoundary && } else if (boundary_behavior == AXBoundaryBehavior::CrossBoundary &&
CreatePreviousLeafTreePosition()->IsNullPosition()) { boundary_type == AXBoundaryType::kDocumentStart) {
// If we're at a format boundary and there are no more text positions // If we're at a format boundary and there are no more text positions
// to traverse, return a null position for cross-boundary moves. // to traverse, return a null position for cross-boundary moves.
return CreateNullPosition(); return CreateNullPosition();
...@@ -2004,7 +2033,8 @@ class AXPosition { ...@@ -2004,7 +2033,8 @@ class AXPosition {
// The first position in the document is also a format start boundary, so we // The first position in the document is also a format start boundary, so we
// should not return NullPosition unless we started from that location. // should not return NullPosition unless we started from that location.
while (!previous_tree_position->IsNullPosition() && while (boundary_type != AXBoundaryType::kDocumentStart &&
!previous_tree_position->IsNullPosition() &&
!tree_position->AtStartOfFormat()) { !tree_position->AtStartOfFormat()) {
tree_position = std::move(previous_tree_position); tree_position = std::move(previous_tree_position);
previous_tree_position = tree_position->CreatePreviousLeafTreePosition(); previous_tree_position = tree_position->CreatePreviousLeafTreePosition();
...@@ -2031,12 +2061,11 @@ class AXPosition { ...@@ -2031,12 +2061,11 @@ class AXPosition {
if (IsNullPosition()) if (IsNullPosition())
return Clone(); return Clone();
// AtEndOfFormat() always returns true if we are at the last iterable AXBoundaryType boundary_type = GetFormatEndBoundaryType();
// position, i.e. CreateNextLeafTreePosition()->IsNullPosition(). if (boundary_type != AXBoundaryType::kNone) {
if (AtEndOfFormat()) {
if (boundary_behavior == AXBoundaryBehavior::StopIfAlreadyAtBoundary || if (boundary_behavior == AXBoundaryBehavior::StopIfAlreadyAtBoundary ||
(boundary_behavior == AXBoundaryBehavior::StopAtLastAnchorBoundary && (boundary_behavior == AXBoundaryBehavior::StopAtLastAnchorBoundary &&
CreateNextLeafTreePosition()->IsNullPosition())) { boundary_type == AXBoundaryType::kDocumentEnd)) {
AXPositionInstance clone = Clone(); AXPositionInstance clone = Clone();
// In order to make equality checks simpler, affinity should be reset so // In order to make equality checks simpler, affinity should be reset so
// that we would get consistent output from this function regardless of // that we would get consistent output from this function regardless of
...@@ -2044,7 +2073,7 @@ class AXPosition { ...@@ -2044,7 +2073,7 @@ class AXPosition {
clone->affinity_ = ax::mojom::TextAffinity::kDownstream; clone->affinity_ = ax::mojom::TextAffinity::kDownstream;
return clone; return clone;
} else if (boundary_behavior == AXBoundaryBehavior::CrossBoundary && } else if (boundary_behavior == AXBoundaryBehavior::CrossBoundary &&
CreateNextLeafTreePosition()->IsNullPosition()) { boundary_type == AXBoundaryType::kDocumentEnd) {
// If we're at a format boundary and there are no more text positions // If we're at a format boundary and there are no more text positions
// to traverse, return a null position for cross-boundary moves. // to traverse, return a null position for cross-boundary moves.
return CreateNullPosition(); return CreateNullPosition();
...@@ -2068,7 +2097,8 @@ class AXPosition { ...@@ -2068,7 +2097,8 @@ class AXPosition {
// The last position in the document is also a format end boundary, so we // The last position in the document is also a format end boundary, so we
// should not return NullPosition unless we started from that location. // should not return NullPosition unless we started from that location.
while (!next_tree_position->IsNullPosition() && while (boundary_type != AXBoundaryType::kDocumentEnd &&
!next_tree_position->IsNullPosition() &&
!tree_position->AtEndOfFormat()) { !tree_position->AtEndOfFormat()) {
tree_position = std::move(next_tree_position); tree_position = std::move(next_tree_position);
next_tree_position = tree_position->CreateNextLeafTreePosition() next_tree_position = tree_position->CreateNextLeafTreePosition()
...@@ -3012,8 +3042,8 @@ class AXPosition { ...@@ -3012,8 +3042,8 @@ class AXPosition {
} }
// Stop moving when text styles differ. // Stop moving when text styles differ.
return move_from.AsLeafTextPosition()->GetTextStyles() != return move_from.AsLeafTreePosition()->GetTextStyles() !=
move_to.AsLeafTextPosition()->GetTextStyles(); move_to.AsLeafTreePosition()->GetTextStyles();
} }
// AbortMovePredicate function used to detect paragraph boundaries. // AbortMovePredicate function used to detect paragraph boundaries.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment