Commit c997b5e8 authored by Katie D's avatar Katie D Committed by Commit Bot

Clean up and improve performance of findNodeMatchingPredicate.

Uses automation 'find' function and also only searches
changed subtrees after the initial search. This seems to improve
performance ~4x of the back button and menu, noticeable when
autoscan is on and set to high scanning speeds.

4x improvement measured on a webpage with a lot of nodes using
devtools 'performance' tab:
Before: Just under 4000ms on the back button moveForward call on
en.wikipedia.org/wiki/cat
After: Just under 1000ms on the back button moveForward call on
en.wikipedia.org/wiki/cat.

AX-Relnotes: N/A
Bug: 1108619
Change-Id: I9bd4bb1b92cde455949558c621700a6b900f2579
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324118Reviewed-by: default avatarAnastasia Helfinstein <anastasi@google.com>
Reviewed-by: default avatarJenny Zhang <jennyz@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792459}
parent 95a11344
...@@ -26,7 +26,6 @@ constexpr int kFocusRingSingleColorWidthDp = 2; ...@@ -26,7 +26,6 @@ constexpr int kFocusRingSingleColorWidthDp = 2;
// Additional buffer needed to prevent clipping at the focus ring's edges. // Additional buffer needed to prevent clipping at the focus ring's edges.
constexpr int kFocusRingBufferDp = 1; constexpr int kFocusRingBufferDp = 1;
constexpr char kUniqueId[] = "switch_access_back_button";
constexpr int kRadiusDp = 18; constexpr int kRadiusDp = 18;
} // namespace } // namespace
...@@ -77,7 +76,6 @@ void SwitchAccessBackButtonView::ButtonPressed(views::Button* sender, ...@@ -77,7 +76,6 @@ void SwitchAccessBackButtonView::ButtonPressed(views::Button* sender,
void SwitchAccessBackButtonView::GetAccessibleNodeData( void SwitchAccessBackButtonView::GetAccessibleNodeData(
ui::AXNodeData* node_data) { ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kButton; node_data->role = ax::mojom::Role::kButton;
node_data->html_attributes.push_back(std::make_pair("id", kUniqueId));
} }
int SwitchAccessBackButtonView::GetHeightForWidth(int w) const { int SwitchAccessBackButtonView::GetHeightForWidth(int w) const {
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
namespace ash { namespace ash {
namespace { namespace {
constexpr char kUniqueId[] = "switch_access_menu_view";
constexpr int kMaxColumns = 3; constexpr int kMaxColumns = 3;
struct ButtonInfo { struct ButtonInfo {
...@@ -129,7 +128,6 @@ int SwitchAccessMenuView::GetBubbleWidthDip() const { ...@@ -129,7 +128,6 @@ int SwitchAccessMenuView::GetBubbleWidthDip() const {
void SwitchAccessMenuView::GetAccessibleNodeData(ui::AXNodeData* node_data) { void SwitchAccessMenuView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kMenu; node_data->role = ax::mojom::Role::kMenu;
node_data->html_attributes.push_back(std::make_pair("id", kUniqueId));
} }
const char* SwitchAccessMenuView::GetClassName() const { const char* SwitchAccessMenuView::GetClassName() const {
......
...@@ -102,17 +102,6 @@ class MenuManager { ...@@ -102,17 +102,6 @@ class MenuManager {
MenuManager.instance.refreshActions_(); MenuManager.instance.refreshActions_();
} }
/**
* Checks if the given node is the Switch Access menu node.
* @param {AutomationNode} node
* @return {boolean}
* @private
*/
static isSwitchAccessMenuNode_(node) {
return !!node && node.role === chrome.automation.RoleType.MENU &&
node.htmlAttributes.id === 'switch_access_menu_view';
}
// ================= Private Methods ================== // ================= Private Methods ==================
/** /**
...@@ -165,8 +154,11 @@ class MenuManager { ...@@ -165,8 +154,11 @@ class MenuManager {
if (this.hasValidMenuAutomationNode_() && this.menuAutomationNode_) { if (this.hasValidMenuAutomationNode_() && this.menuAutomationNode_) {
this.jumpToMenuAutomationNode_(this.menuAutomationNode_); this.jumpToMenuAutomationNode_(this.menuAutomationNode_);
} }
SwitchAccess.findNodeMatchingPredicate( SwitchAccess.findNodeMatching(
MenuManager.isSwitchAccessMenuNode_, {
role: chrome.automation.RoleType.MENU,
attributes: {className: 'SwitchAccessMenuView'}
},
this.jumpToMenuAutomationNode_.bind(this)); this.jumpToMenuAutomationNode_.bind(this));
} }
......
...@@ -135,18 +135,12 @@ class BackButtonNode extends SAChildNode { ...@@ -135,18 +135,12 @@ class BackButtonNode extends SAChildNode {
if (BackButtonNode.automationNode_ && BackButtonNode.automationNode_.role) { if (BackButtonNode.automationNode_ && BackButtonNode.automationNode_.role) {
return; return;
} }
SwitchAccess.findNodeMatchingPredicate( SwitchAccess.findNodeMatching(
BackButtonNode.isBackButton_, BackButtonNode.saveAutomationNode_); {
} role: chrome.automation.RoleType.BUTTON,
attributes: {className: 'SwitchAccessBackButtonView'}
/** },
* Checks if the given node is the back button automation node. BackButtonNode.saveAutomationNode_);
* @param {!AutomationNode} node
* @return {boolean}
* @private
*/
static isBackButton_(node) {
return node.htmlAttributes.id === 'switch_access_back_button';
} }
/** /**
......
...@@ -118,9 +118,9 @@ class KeyboardRootNode extends RootNodeWrapper { ...@@ -118,9 +118,9 @@ class KeyboardRootNode extends RootNodeWrapper {
static startWatchingVisibility() { static startWatchingVisibility() {
const keyboardObject = KeyboardRootNode.getKeyboardObject(); const keyboardObject = KeyboardRootNode.getKeyboardObject();
if (!keyboardObject) { if (!keyboardObject) {
const isKeyboard = (n) => n.role === chrome.automation.RoleType.KEYBOARD; SwitchAccess.findNodeMatching(
SwitchAccess.findNodeMatchingPredicate( {role: chrome.automation.RoleType.KEYBOARD},
isKeyboard, KeyboardRootNode.startWatchingVisibility); KeyboardRootNode.startWatchingVisibility);
return; return;
} }
......
...@@ -47,26 +47,24 @@ class SwitchAccess { ...@@ -47,26 +47,24 @@ class SwitchAccess {
} }
/** /**
* Helper function to robustly find a node fitting a given predicate, even if * Helper function to robustly find a node fitting a given FindParams, even if
* that node has not yet been created. * that node has not yet been created.
* Used to find the menu and back button. * Used to find the menu and back button.
* @param {!function(!AutomationNode): boolean} predicate * @param {!chrome.automation.FindParams} findParams
* @param {!function(!AutomationNode): void} foundCallback * @param {!function(!AutomationNode): void} foundCallback
*/ */
static findNodeMatchingPredicate(predicate, foundCallback) { static findNodeMatching(findParams, foundCallback) {
const desktop = NavigationManager.desktopNode; const desktop = NavigationManager.desktopNode;
// First, check if the node is currently in the tree. // First, check if the node is currently in the tree.
const treeWalker = new AutomationTreeWalker( let node = desktop.find(findParams);
desktop, constants.Dir.FORWARD, {visit: predicate}); if (node) {
treeWalker.next(); foundCallback(node);
if (treeWalker.node) {
foundCallback(treeWalker.node);
return; return;
} }
// If it's not currently in the tree, listen for changes to the desktop // If it's not currently in the tree, listen for changes to the desktop
// tree. // tree.
const onDesktopChildrenChanged = (event) => { const onDesktopChildrenChanged = (event) => {
if (predicate(event.target)) { if (event.target.matches(findParams)) {
// If the event target is the node we're looking for, we've found it. // If the event target is the node we're looking for, we've found it.
desktop.removeEventListener( desktop.removeEventListener(
chrome.automation.EventType.CHILDREN_CHANGED, chrome.automation.EventType.CHILDREN_CHANGED,
...@@ -74,15 +72,12 @@ class SwitchAccess { ...@@ -74,15 +72,12 @@ class SwitchAccess {
foundCallback(event.target); foundCallback(event.target);
} else if (event.target.children.length > 0) { } else if (event.target.children.length > 0) {
// Otherwise, see if one of its children is the node we're looking for. // Otherwise, see if one of its children is the node we're looking for.
const treeWalker = new AutomationTreeWalker( node = event.target.find(findParams);
event.target, constants.Dir.FORWARD, if (node) {
{visit: predicate, root: (node) => node == event.target});
treeWalker.next();
if (treeWalker.node) {
desktop.removeEventListener( desktop.removeEventListener(
chrome.automation.EventType.CHILDREN_CHANGED, chrome.automation.EventType.CHILDREN_CHANGED,
onDesktopChildrenChanged, false); onDesktopChildrenChanged, false);
foundCallback(treeWalker.node); foundCallback(node);
} }
} }
}; };
......
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