Commit 86af2537 authored by Adam Ettenberger's avatar Adam Ettenberger Committed by Commit Bot

Performance improvement for ITextRangeProvider::GetEnclosingElement

This change is an optimizaiton for UIAs GetEnclosingElement.

The cost was in calling |LowestCommonAncestor| because it would compute
multiple parent AXPositions until reaching the common anchor. Creating
a parent Text position is expensive because it calls |MaxTextOffset|.

GetEnclosingElement didn't need an AXPosition so to reduce that cost
a new method |LowestCommonAnchor| has been added which returns the
AXNodeType* that we can use to find the relevant AXPlatformNodeDelegate
to do our work on. |LowestCommonAnchor| may also be used to optimize
other similar APIs which don't necessarily need an AXPosition object.

In order to derive the AXTreeID which is necessary to find the correct
AXPlatformNodeDelegate, AXNode::OwnerTree now exposes its AXTreeID.
This way we can know what the AXTreeID is for an AXNode of an
AXNodePosition anchor. This is already easily accessible from the other
position type BrowserAccessibilityPosition, since BrowserAccessibility
has access to the relevant AXTreeData. Now anchors of  both position
types can know what the relevant AXTreeID is for creating delegates!

I measured the perf difference with std::chrono::high_resolution_clock
on an unoptimized debug build.
With a document that contains 2 columns of 100 divs each, and a single
leaf text child in each column, and a text range that spans between the
two columns.

Without Patch :
439.006ms

With Patch :
0.1371ms

Bug: 928948
Change-Id: Ic9648ff34c7c7030c299c8484fb188a873e3af92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790066
Commit-Queue: Adam Ettenberger <adettenb@microsoft.com>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#697344}
parent d7f0ab31
...@@ -417,7 +417,7 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate { ...@@ -417,7 +417,7 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate {
// Gets the text offsets where new lines start. // Gets the text offsets where new lines start.
std::vector<int> GetLineStartOffsets() const; std::vector<int> GetLineStartOffsets() const;
virtual gfx::NativeViewAccessible GetNativeViewAccessible(); gfx::NativeViewAccessible GetNativeViewAccessible() override;
// AXPosition Support // AXPosition Support
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "ui/accessibility/ax_export.h" #include "ui/accessibility/ax_export.h"
#include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/ax_node_data.h"
#include "ui/accessibility/ax_tree_id.h"
namespace ui { namespace ui {
...@@ -48,6 +49,8 @@ class AX_EXPORT AXNode final { ...@@ -48,6 +49,8 @@ class AX_EXPORT AXNode final {
ax::mojom::TextAffinity focus_affinity; ax::mojom::TextAffinity focus_affinity;
}; };
// See AXTree.
virtual AXTreeID GetAXTreeID() const = 0;
// See AXTree. // See AXTree.
virtual AXTableInfo* GetTableInfo(const AXNode* table_node) const = 0; virtual AXTableInfo* GetTableInfo(const AXNode* table_node) const = 0;
// See AXTree. // See AXTree.
......
...@@ -670,17 +670,12 @@ class AXPosition { ...@@ -670,17 +670,12 @@ class AXPosition {
return CreateNextAnchorPosition()->IsNullPosition() && AtEndOfAnchor(); return CreateNextAnchorPosition()->IsNullPosition() && AtEndOfAnchor();
} }
// This method returns a position instead of a node because this allows us to // This method finds the lowest common AXNodeType of |this| and |second|.
// return the corresponding text offset or child index in the ancestor that AXNodeType* LowestCommonAnchor(const AXPosition& second) const {
// relates to the current position.
// Also, this method uses position instead of tree logic to traverse the tree,
// because positions can handle moving across multiple trees, while trees
// cannot.
AXPositionInstance LowestCommonAncestor(const AXPosition& second) const {
if (IsNullPosition() || second.IsNullPosition()) if (IsNullPosition() || second.IsNullPosition())
return CreateNullPosition(); return nullptr;
if (GetAnchor() == second.GetAnchor()) if (GetAnchor() == second.GetAnchor())
return Clone(); return GetAnchor();
base::stack<AXNodeType*> our_ancestors = GetAncestorAnchors(); base::stack<AXNodeType*> our_ancestors = GetAncestorAnchors();
base::stack<AXNodeType*> other_ancestors = second.GetAncestorAnchors(); base::stack<AXNodeType*> other_ancestors = second.GetAncestorAnchors();
...@@ -692,6 +687,17 @@ class AXPosition { ...@@ -692,6 +687,17 @@ class AXPosition {
our_ancestors.pop(); our_ancestors.pop();
other_ancestors.pop(); other_ancestors.pop();
} }
return common_anchor;
}
// This method returns a position instead of a node because this allows us to
// return the corresponding text offset or child index in the ancestor that
// relates to the current position.
// Also, this method uses position instead of tree logic to traverse the tree,
// because positions can handle moving across multiple trees, while trees
// cannot.
AXPositionInstance LowestCommonAncestor(const AXPosition& second) const {
AXNodeType* common_anchor = LowestCommonAnchor(second);
if (!common_anchor) if (!common_anchor)
return CreateNullPosition(); return CreateNullPosition();
......
...@@ -578,6 +578,10 @@ void AXTree::RemoveObserver(const AXTreeObserver* observer) { ...@@ -578,6 +578,10 @@ void AXTree::RemoveObserver(const AXTreeObserver* observer) {
observers_.RemoveObserver(observer); observers_.RemoveObserver(observer);
} }
AXTreeID AXTree::GetAXTreeID() const {
return data().tree_id;
}
AXNode* AXTree::GetFromId(int32_t id) const { AXNode* AXTree::GetFromId(int32_t id) const {
auto iter = id_map_.find(id); auto iter = id_map_.find(id);
return iter != id_map_.end() ? iter->second : nullptr; return iter != id_map_.end() ? iter->second : nullptr;
......
...@@ -54,6 +54,10 @@ class AX_EXPORT AXTree : public AXNode::OwnerTree { ...@@ -54,6 +54,10 @@ class AX_EXPORT AXTree : public AXNode::OwnerTree {
const AXTreeData& data() const { return data_; } const AXTreeData& data() const { return data_; }
// AXNode::OwnerTree override.
// Returns the globally unique ID of this accessibility tree.
AXTreeID GetAXTreeID() const override;
// AXNode::OwnerTree override. // AXNode::OwnerTree override.
// Returns the AXNode with the given |id| if it is part of this AXTree. // Returns the AXNode with the given |id| if it is part of this AXTree.
AXNode* GetFromId(int32_t id) const override; AXNode* GetFromId(int32_t id) const override;
......
...@@ -39,7 +39,7 @@ class AX_EXPORT AXFragmentRootWin : public ui::AXPlatformNodeDelegateBase { ...@@ -39,7 +39,7 @@ class AX_EXPORT AXFragmentRootWin : public ui::AXPlatformNodeDelegateBase {
gfx::AcceleratedWidget widget); gfx::AcceleratedWidget widget);
// Returns the NativeViewAccessible for this fragment root. // Returns the NativeViewAccessible for this fragment root.
gfx::NativeViewAccessible GetNativeViewAccessible(); gfx::NativeViewAccessible GetNativeViewAccessible() override;
// Assistive technologies will typically use UI Automation's control or // Assistive technologies will typically use UI Automation's control or
// content view rather than the raw view. // content view rather than the raw view.
......
...@@ -77,6 +77,10 @@ class AX_EXPORT AXPlatformNodeDelegate { ...@@ -77,6 +77,10 @@ class AX_EXPORT AXPlatformNodeDelegate {
// method is only meaningful on macOS. // method is only meaningful on macOS.
virtual gfx::NativeViewAccessible GetNSWindow() = 0; virtual gfx::NativeViewAccessible GetNSWindow() = 0;
// Get the node for this delegate, which may be an AXPlatformNode or it may
// be a native accessible object implemented by another class.
virtual gfx::NativeViewAccessible GetNativeViewAccessible() = 0;
// Get the parent of the node, which may be an AXPlatformNode or it may // Get the parent of the node, which may be an AXPlatformNode or it may
// be a native accessible object implemented by another class. // be a native accessible object implemented by another class.
virtual gfx::NativeViewAccessible GetParent() = 0; virtual gfx::NativeViewAccessible GetParent() = 0;
......
...@@ -44,6 +44,11 @@ gfx::NativeViewAccessible AXPlatformNodeDelegateBase::GetNSWindow() { ...@@ -44,6 +44,11 @@ gfx::NativeViewAccessible AXPlatformNodeDelegateBase::GetNSWindow() {
return nullptr; return nullptr;
} }
gfx::NativeViewAccessible
AXPlatformNodeDelegateBase::GetNativeViewAccessible() {
return nullptr;
}
gfx::NativeViewAccessible AXPlatformNodeDelegateBase::GetParent() { gfx::NativeViewAccessible AXPlatformNodeDelegateBase::GetParent() {
return nullptr; return nullptr;
} }
......
...@@ -44,6 +44,10 @@ class AX_EXPORT AXPlatformNodeDelegateBase : public AXPlatformNodeDelegate { ...@@ -44,6 +44,10 @@ class AX_EXPORT AXPlatformNodeDelegateBase : public AXPlatformNodeDelegate {
// See comments in AXPlatformNodeDelegate. // See comments in AXPlatformNodeDelegate.
gfx::NativeViewAccessible GetNSWindow() override; gfx::NativeViewAccessible GetNSWindow() override;
// Get the node for this delegate, which may be an AXPlatformNode or it may
// be a native accessible object implemented by another class.
gfx::NativeViewAccessible GetNativeViewAccessible() override;
// Get the parent of the node, which may be an AXPlatformNode or it may // Get the parent of the node, which may be an AXPlatformNode or it may
// be a native accessible object implemented by another class. // be a native accessible object implemented by another class.
gfx::NativeViewAccessible GetParent() override; gfx::NativeViewAccessible GetParent() override;
......
...@@ -71,6 +71,8 @@ class AXPlatformNodeTextChildProviderTest : public ui::AXPlatformNodeWinTest { ...@@ -71,6 +71,8 @@ class AXPlatformNodeTextChildProviderTest : public ui::AXPlatformNodeWinTest {
AXNode* root_node = GetRootNode(); AXNode* root_node = GetRootNode();
AXNodePosition::SetTree(tree_.get()); AXNodePosition::SetTree(tree_.get());
AXTreeManagerMap::GetInstance().AddTreeManager(update.tree_data.tree_id,
this);
AXNode* nontext_child_of_root_node = root_node->children()[0]; AXNode* nontext_child_of_root_node = root_node->children()[0];
AXNode* text_child_of_root_node = root_node->children()[1]; AXNode* text_child_of_root_node = root_node->children()[1];
AXNode* nontext_child_of_nontext_node = AXNode* nontext_child_of_nontext_node =
......
...@@ -453,17 +453,22 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::GetEnclosingElement( ...@@ -453,17 +453,22 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::GetEnclosingElement(
WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_TEXTRANGE_GETENCLOSINGELEMENT); WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_TEXTRANGE_GETENCLOSINGELEMENT);
UIA_VALIDATE_TEXTRANGEPROVIDER_CALL(); UIA_VALIDATE_TEXTRANGEPROVIDER_CALL();
AXPositionInstance common_ancestor = start_->LowestCommonAncestor(*end_); AXNode* common_anchor = start_->LowestCommonAnchor(*end_);
AXPlatformNode* node = GetDelegate(common_ancestor.get()) DCHECK(common_anchor);
->GetFromNodeID(common_ancestor->anchor_id()); if (!common_anchor)
DCHECK(node); return UIA_E_ELEMENTNOTAVAILABLE;
while (ui::IsIgnored(node->GetDelegate()->GetData())) {
node = static_cast<AXPlatformNodeWin*>( const AXTreeID tree_id = common_anchor->tree()->GetAXTreeID();
AXPlatformNode::FromNativeViewAccessible( const AXNode::AXID node_id = common_anchor->id();
node->GetDelegate()->GetParent())); AXPlatformNodeDelegate* delegate = GetDelegate(tree_id, node_id);
DCHECK(node); DCHECK(delegate);
while (ui::IsIgnored(delegate->GetData())) {
AXPlatformNodeWin* parent = static_cast<AXPlatformNodeWin*>(
AXPlatformNode::FromNativeViewAccessible(delegate->GetParent()));
DCHECK(parent);
delegate = parent->GetDelegate();
} }
node->GetNativeViewAccessible()->QueryInterface(IID_PPV_ARGS(element)); delegate->GetNativeViewAccessible()->QueryInterface(IID_PPV_ARGS(element));
DCHECK(*element); DCHECK(*element);
return S_OK; return S_OK;
...@@ -812,11 +817,15 @@ AXPlatformNodeWin* AXPlatformNodeTextRangeProviderWin::owner() const { ...@@ -812,11 +817,15 @@ AXPlatformNodeWin* AXPlatformNodeTextRangeProviderWin::owner() const {
AXPlatformNodeDelegate* AXPlatformNodeTextRangeProviderWin::GetDelegate( AXPlatformNodeDelegate* AXPlatformNodeTextRangeProviderWin::GetDelegate(
const AXPositionInstanceType* position) const { const AXPositionInstanceType* position) const {
AXTreeManager* manager = return GetDelegate(position->tree_id(), position->anchor_id());
AXTreeManagerMap::GetInstance().GetManager(position->tree_id()); }
return manager
? manager->GetDelegate(position->tree_id(), position->anchor_id()) AXPlatformNodeDelegate* AXPlatformNodeTextRangeProviderWin::GetDelegate(
: owner()->GetDelegate(); const AXTreeID tree_id,
const AXNode::AXID node_id) const {
AXTreeManager* manager = AXTreeManagerMap::GetInstance().GetManager(tree_id);
return manager ? manager->GetDelegate(tree_id, node_id)
: owner()->GetDelegate();
} }
AXPlatformNodeTextRangeProviderWin::AXPositionInstance AXPlatformNodeTextRangeProviderWin::AXPositionInstance
......
...@@ -92,6 +92,8 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1")) ...@@ -92,6 +92,8 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1"))
AXPlatformNodeWin* owner() const; AXPlatformNodeWin* owner() const;
AXPlatformNodeDelegate* GetDelegate( AXPlatformNodeDelegate* GetDelegate(
const AXPositionInstanceType* position) const; const AXPositionInstanceType* position) const;
AXPlatformNodeDelegate* GetDelegate(const AXTreeID tree_id,
const AXNode::AXID node_id) const;
template <typename AnchorIterator, typename ExpandMatchLambda> template <typename AnchorIterator, typename ExpandMatchLambda>
HRESULT FindAttributeRange(const TEXTATTRIBUTEID text_attribute_id, HRESULT FindAttributeRange(const TEXTATTRIBUTEID text_attribute_id,
......
...@@ -948,8 +948,11 @@ TEST_F(AXPlatformNodeTextRangeProviderTest, ...@@ -948,8 +948,11 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
TEST_F(AXPlatformNodeTextRangeProviderTest, TEST_F(AXPlatformNodeTextRangeProviderTest,
TestITextRangeProviderExpandToEnclosingCharacter) { TestITextRangeProviderExpandToEnclosingCharacter) {
Init(BuildTextDocument({"some text", "more text"})); ui::AXTreeUpdate update = BuildTextDocument({"some text", "more text"});
Init(update);
AXNodePosition::SetTree(tree_.get()); AXNodePosition::SetTree(tree_.get());
AXTreeManagerMap::GetInstance().AddTreeManager(update.tree_data.tree_id,
this);
AXNode* root_node = GetRootNode(); AXNode* root_node = GetRootNode();
ComPtr<ITextRangeProvider> text_range_provider; ComPtr<ITextRangeProvider> text_range_provider;
......
...@@ -111,6 +111,10 @@ AXNodePosition::AXPositionInstance TestAXNodeWrapper::CreateTextPositionAt( ...@@ -111,6 +111,10 @@ AXNodePosition::AXPositionInstance TestAXNodeWrapper::CreateTextPositionAt(
ax::mojom::TextAffinity::kDownstream); ax::mojom::TextAffinity::kDownstream);
} }
gfx::NativeViewAccessible TestAXNodeWrapper::GetNativeViewAccessible() {
return ax_platform_node()->GetNativeViewAccessible();
}
gfx::NativeViewAccessible TestAXNodeWrapper::GetParent() { gfx::NativeViewAccessible TestAXNodeWrapper::GetParent() {
TestAXNodeWrapper* parent_wrapper = TestAXNodeWrapper* parent_wrapper =
GetOrCreate(tree_, node_->GetUnignoredParent()); GetOrCreate(tree_, node_->GetUnignoredParent());
......
...@@ -55,6 +55,7 @@ class TestAXNodeWrapper : public AXPlatformNodeDelegateBase { ...@@ -55,6 +55,7 @@ class TestAXNodeWrapper : public AXPlatformNodeDelegateBase {
const AXTree::Selection GetUnignoredSelection() const override; const AXTree::Selection GetUnignoredSelection() const override;
AXNodePosition::AXPositionInstance CreateTextPositionAt( AXNodePosition::AXPositionInstance CreateTextPositionAt(
int offset) const override; int offset) const override;
gfx::NativeViewAccessible GetNativeViewAccessible() override;
gfx::NativeViewAccessible GetParent() override; gfx::NativeViewAccessible GetParent() override;
int GetChildCount() override; int GetChildCount() override;
gfx::NativeViewAccessible ChildAtIndex(int index) override; gfx::NativeViewAccessible ChildAtIndex(int index) override;
......
...@@ -245,6 +245,10 @@ gfx::NativeViewAccessible AXVirtualView::GetNSWindow() { ...@@ -245,6 +245,10 @@ gfx::NativeViewAccessible AXVirtualView::GetNSWindow() {
return nullptr; return nullptr;
} }
gfx::NativeViewAccessible AXVirtualView::GetNativeViewAccessible() {
return GetNativeObject();
}
gfx::NativeViewAccessible AXVirtualView::GetParent() { gfx::NativeViewAccessible AXVirtualView::GetParent() {
if (parent_view_) if (parent_view_)
return parent_view_->GetNativeObject(); return parent_view_->GetNativeObject();
......
...@@ -127,6 +127,7 @@ class VIEWS_EXPORT AXVirtualView : public ui::AXPlatformNodeDelegateBase { ...@@ -127,6 +127,7 @@ class VIEWS_EXPORT AXVirtualView : public ui::AXPlatformNodeDelegateBase {
int GetChildCount() override; int GetChildCount() override;
gfx::NativeViewAccessible ChildAtIndex(int index) override; gfx::NativeViewAccessible ChildAtIndex(int index) override;
gfx::NativeViewAccessible GetNSWindow() override; gfx::NativeViewAccessible GetNSWindow() override;
gfx::NativeViewAccessible GetNativeViewAccessible() override;
gfx::NativeViewAccessible GetParent() override; gfx::NativeViewAccessible GetParent() override;
gfx::Rect GetBoundsRect( gfx::Rect GetBoundsRect(
const ui::AXCoordinateSystem coordinate_system, const ui::AXCoordinateSystem coordinate_system,
......
...@@ -284,6 +284,11 @@ gfx::NativeViewAccessible ViewAXPlatformNodeDelegate::GetNSWindow() { ...@@ -284,6 +284,11 @@ gfx::NativeViewAccessible ViewAXPlatformNodeDelegate::GetNSWindow() {
return nullptr; return nullptr;
} }
gfx::NativeViewAccessible
ViewAXPlatformNodeDelegate::GetNativeViewAccessible() {
return GetNativeObject();
}
gfx::NativeViewAccessible ViewAXPlatformNodeDelegate::GetParent() { gfx::NativeViewAccessible ViewAXPlatformNodeDelegate::GetParent() {
if (view()->parent()) if (view()->parent())
return view()->parent()->GetNativeViewAccessible(); return view()->parent()->GetNativeViewAccessible();
......
...@@ -50,6 +50,7 @@ class ViewAXPlatformNodeDelegate : public ViewAccessibility, ...@@ -50,6 +50,7 @@ class ViewAXPlatformNodeDelegate : public ViewAccessibility,
int GetChildCount() override; int GetChildCount() override;
gfx::NativeViewAccessible ChildAtIndex(int index) override; gfx::NativeViewAccessible ChildAtIndex(int index) override;
gfx::NativeViewAccessible GetNSWindow() override; gfx::NativeViewAccessible GetNSWindow() override;
gfx::NativeViewAccessible GetNativeViewAccessible() override;
gfx::NativeViewAccessible GetParent() override; gfx::NativeViewAccessible GetParent() override;
gfx::Rect GetBoundsRect( gfx::Rect GetBoundsRect(
const ui::AXCoordinateSystem coordinate_system, const ui::AXCoordinateSystem coordinate_system,
......
...@@ -62,7 +62,7 @@ class AuraLinuxApplication : public ui::AXPlatformNodeDelegateBase, ...@@ -62,7 +62,7 @@ class AuraLinuxApplication : public ui::AXPlatformNodeDelegateBase,
window->AddObserver(this); window->AddObserver(this);
} }
gfx::NativeViewAccessible GetNativeViewAccessible() { gfx::NativeViewAccessible GetNativeViewAccessible() override {
return platform_node_->GetNativeViewAccessible(); return platform_node_->GetNativeViewAccessible();
} }
......
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