Commit 1411ed7c authored by Benjamin Beaudry's avatar Benjamin Beaudry Committed by Commit Bot

Performance improvement of AXPosition::AsTextPosition()

This change is an optimization of AsTextPosition().

It's part of the larger performance improvement related to the expensive
calls to MaxTextOffset(). AXposition::CreatePositionAtStartOfDocument()
calls repetitively AxTextPosition() to allow converting from a tree
position and to a text position and back, but this required calling
MaxTextOffsetInParent() a lot.

In a simple navigate-by-word situation, every call to AsTextPosition()
would call MaxTextOffsetInParent uselessly. This change fixes it.

There is no need to get the MaxTextOffset of an AXPositionInstance when
the |text_offset_| will be positioned at the beginning of the anchor.
We expect to position the text_offset_ of a Text Position at the
beginning of the anchor when |text_offset_| is either invalid, already
at 0 or less than the actual new_offset. All I did was adding a check
for these conditions before calling MaxTextOffsetInParent().

I measured the perf difference with a trace captured with WPR and
analyzed with WPA. In both cases, the traces consist of 10 words read
by Narrator with navigate-by-word navigation.

Without patch:
  ExpandToEnclosingUnit: 2,169 ms
  CreatePositionAtStartOfDocument: 103 ms

With patch:
  ExpandToEnclosingUnit: 1,851 ms
  CreatePositionAtStartOfDocument: <1 ms

Bug: 928948
Change-Id: I41d896dd25519eaba6a8c82d0ffacd3fbdedd909
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811221
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699478}
parent a4acf933
...@@ -754,8 +754,11 @@ class AXPosition { ...@@ -754,8 +754,11 @@ class AXPosition {
// If the current text offset is valid, we don't touch it to potentially // If the current text offset is valid, we don't touch it to potentially
// allow converting from a text position to a tree position and back // allow converting from a text position to a tree position and back
// without losing information. // without losing information.
if (copy->text_offset_ < 0 || copy->text_offset_ >= copy->MaxTextOffset()) if (copy->text_offset_ < 0 ||
(copy->text_offset_ > 0 &&
copy->text_offset_ >= copy->MaxTextOffset())) {
copy->text_offset_ = 0; copy->text_offset_ = 0;
}
} else if (copy->child_index_ == copy->AnchorChildCount()) { } else if (copy->child_index_ == copy->AnchorChildCount()) {
copy->text_offset_ = copy->MaxTextOffset(); copy->text_offset_ = copy->MaxTextOffset();
} else { } else {
...@@ -765,15 +768,22 @@ class AXPosition { ...@@ -765,15 +768,22 @@ class AXPosition {
for (int i = 0; i <= child_index_; ++i) { for (int i = 0; i <= child_index_; ++i) {
AXPositionInstance child = copy->CreateChildPositionAt(i); AXPositionInstance child = copy->CreateChildPositionAt(i);
DCHECK(child); DCHECK(child);
int child_length = child->MaxTextOffsetInParent(); // If the current text offset is valid, we don't touch it to
// potentially allow converting from a text position to a tree
// position and back without losing information. Otherwise, if the
// text_offset is invalid, equals to 0 or is smaller than
// |new_offset|, we reset it to the beginning of the current child
// node.
if (i == child_index_ && copy->text_offset_ <= new_offset) {
copy->text_offset_ = new_offset;
break;
}
// If the current text offset is valid, we don't touch it to potentially int child_length = child->MaxTextOffsetInParent();
// allow converting from a text position to a tree position and back // Same comment as above: we don't touch the text offset if it's
// without losing information. Otherwise, we reset it to the beginning // already valid.
// of the current child node.
if (i == child_index_ && if (i == child_index_ &&
(copy->text_offset_ < new_offset || (copy->text_offset_ > (new_offset + child_length) ||
copy->text_offset_ > (new_offset + child_length) ||
// When the text offset is equal to the text's length but this is // When the text offset is equal to the text's length but this is
// not an "after text" position. // not an "after text" position.
(!copy->AtEndOfAnchor() && (!copy->AtEndOfAnchor() &&
......
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