Commit d1830bbc authored by Ian Prest's avatar Ian Prest Committed by Commit Bot

UIA: Improve perf of TextRange::GetAttributeValue

When UIA was enabled, Narrator was timing out because it calls
TextRange::GetAttributeValue a lot when trying to navigate between
paragraphs.

This function was iterating using AXRange, which is relatively
expensive.  However, since we're just collecting text attribute values,
we don't actually need to iterate over all the text positions... we just
need to iterate the anchor positions.

This change also introduces a test-case for GetAttributeValue.

Bug: 1011057
Change-Id: I624e5fccefe88918ad289c0185a501ef290bde48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838373
Commit-Queue: Ian Prest <iapres@microsoft.com>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#702644}
parent 4f60d6a9
......@@ -381,6 +381,51 @@ class AXPlatformNodeTextRangeProviderWinBrowserTest
}
};
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
GetAttributeValue) {
LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML(
<!DOCTYPE html>
<html>
<body>
<div style="font-size: 16px">
<span style="font-size: 12px">Text1</span>
Text2
</div>
</body>
</html>
)HTML"));
ComPtr<IUnknown> mix_attribute_value;
EXPECT_HRESULT_SUCCEEDED(
UiaGetReservedMixedAttributeValue(&mix_attribute_value));
auto* node = FindNode(ax::mojom::Role::kStaticText, "Text1");
ASSERT_NE(nullptr, node);
EXPECT_TRUE(node->PlatformIsLeaf());
EXPECT_EQ(0u, node->PlatformChildCount());
ComPtr<ITextRangeProvider> text_range_provider;
GetTextRangeProviderFromTextNode(*node, &text_range_provider);
ASSERT_NE(nullptr, text_range_provider.Get());
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"Text1");
base::win::ScopedVariant value;
EXPECT_HRESULT_SUCCEEDED(text_range_provider->GetAttributeValue(
UIA_FontSizeAttributeId, value.Receive()));
EXPECT_EQ(value.type(), VT_R8);
EXPECT_EQ(V_R8(value.ptr()), 12.0);
EXPECT_HRESULT_SUCCEEDED(
text_range_provider->ExpandToEnclosingUnit(TextUnit_Paragraph));
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"Text1 Text2");
EXPECT_HRESULT_SUCCEEDED(text_range_provider->GetAttributeValue(
UIA_FontSizeAttributeId, value.Receive()));
EXPECT_EQ(value.type(), VT_UNKNOWN);
EXPECT_EQ(V_UNKNOWN(value.ptr()), mix_attribute_value.Get())
<< "expected 'mixed attribute value' interface pointer";
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
GetBoundingRectangles) {
LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML(
......
......@@ -399,15 +399,19 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::GetAttributeValue(
WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_TEXTRANGE_GETATTRIBUTEVALUE);
base::win::ScopedVariant attribute_value_variant;
AXNodeRange range(start_->Clone(), end_->Clone());
for (const AXNodeRange& leaf_text_range : range) {
AXPositionInstanceType* anchor_start = leaf_text_range.anchor();
AXPlatformNodeDelegate* delegate = GetDelegate(anchor_start);
DCHECK(anchor_start && delegate);
// The range is inclusive, so advance our endpoint to the next position
auto end = end_->CreateNextAnchorPosition();
// Iterate over anchor positions
for (auto it = start_->Clone();
it->anchor_id() != end->anchor_id() || it->tree_id() != end->tree_id();
it = it->CreateNextAnchorPosition()) {
AXPlatformNodeDelegate* delegate = GetDelegate(it.get());
DCHECK(it && delegate);
AXPlatformNodeWin* platform_node = static_cast<AXPlatformNodeWin*>(
delegate->GetFromNodeID(anchor_start->anchor_id()));
delegate->GetFromNodeID(it->anchor_id()));
DCHECK(platform_node);
base::win::ScopedVariant current_variant;
......
......@@ -3221,6 +3221,12 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
heading_data.id = 3;
heading_data.role = ax::mojom::Role::kHeading;
heading_data.AddIntAttribute(ax::mojom::IntAttribute::kHierarchicalLevel, 6);
heading_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
heading_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
heading_data.SetTextDirection(ax::mojom::TextDirection::kRtl);
heading_data.SetTextPosition(ax::mojom::TextPosition::kSuperscript);
heading_data.AddState(ax::mojom::State::kEditable);
heading_data.child_ids = {4};
ui::AXNodeData heading_text_data;
......@@ -3239,6 +3245,10 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
ui::AXNodeData mark_data;
mark_data.id = 5;
mark_data.role = ax::mojom::Role::kMark;
mark_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
mark_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
mark_data.SetTextDirection(ax::mojom::TextDirection::kRtl);
mark_data.child_ids = {6};
ui::AXNodeData mark_text_data;
......@@ -3254,6 +3264,9 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
list_data.id = 7;
list_data.role = ax::mojom::Role::kList;
list_data.child_ids = {8, 10};
list_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
list_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
ui::AXNodeData list_item_data;
list_item_data.id = 8;
......@@ -3262,6 +3275,9 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
list_item_data.AddIntAttribute(
ax::mojom::IntAttribute::kListStyle,
static_cast<int>(ax::mojom::ListStyle::kOther));
list_item_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
list_item_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
ui::AXNodeData list_item_text_data;
list_item_text_data.id = 9;
......@@ -3278,6 +3294,9 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
list_item2_data.AddIntAttribute(
ax::mojom::IntAttribute::kListStyle,
static_cast<int>(ax::mojom::ListStyle::kDisc));
list_item2_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
list_item2_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
ui::AXNodeData list_item2_text_data;
list_item2_text_data.id = 11;
......
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