Commit 83031cb2 authored by Anastasia Helfinstein's avatar Anastasia Helfinstein Committed by Commit Bot

[Switch Access] Avoid recalculating values repeatedly

Switch Access has been experiencing performance issues on moderate to
large webpages. A large portion of this is due to the inefficiencies of
its traversal of the automation tree, recalculating the same values
sometimes hundreds of times for nodes deep in the tree.

This change utilizes techniques from dynamic programming to persist
intermediate values for continued use, reducing the asymptotic worst-
case run time from exponential in the depth of the node to linear in
the number of nodes in the subtree.

AX-Relnotes: n/a.
Bug: 1109970
Change-Id: I61368292f2e731245e4f733c2883b86293d8a74c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360340Reviewed-by: default avatarAbigail Klein <abigailbklein@google.com>
Commit-Queue: Anastasia Helfinstein <anastasi@google.com>
Cr-Commit-Position: refs/heads/master@{#799856}
parent 6b44cb09
......@@ -27,6 +27,7 @@ run_jsbundler("switch_access_copied_files") {
sources = [
"auto_scan_manager.js",
"background.js",
"cache.js",
"commands.js",
"event_helper.js",
"focus_ring_manager.js",
......@@ -133,6 +134,7 @@ js_type_check("closure_compile") {
":auto_scan_manager",
":back_button_node",
":background",
":cache",
":combo_box_node",
":commands",
":desktop_node",
......@@ -188,6 +190,10 @@ js_library("back_button_node") {
]
}
js_library("cache") {
externs_list = [ "$externs_path/automation.js" ]
}
js_library("combo_box_node") {
sources = [ "nodes/combo_box_node.js" ]
deps = [
......@@ -256,6 +262,7 @@ js_library("group_node") {
js_library("history") {
deps = [
":cache",
":desktop_node",
":node_wrapper",
":switch_access_node",
......@@ -338,6 +345,7 @@ js_library("node_wrapper") {
sources = [ "nodes/node_wrapper.js" ]
deps = [
":back_button_node",
":cache",
":switch_access_constants",
":switch_access_node",
":switch_access_predicate",
......@@ -404,6 +412,7 @@ js_library("switch_access_node") {
js_library("switch_access_predicate") {
deps = [
":cache",
":switch_access_constants",
":switch_access_node",
"../common:automation_predicate",
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
/**
* Saves computed values to avoid recalculating them repeatedly.
*
* Caches are single-use, and abandoned after the top-level question is answered
* (e.g. what are all the interesting descendants of this node?)
*/
class SACache {
constructor() {
/** @private {!Map<!AutomationNode, boolean>} */
this.isActionableMap_ = new Map();
/** @private {!Map<!AutomationNode, boolean>} */
this.isGroupMap_ = new Map();
/** @private {!Map<!AutomationNode, boolean>} */
this.isInterestingSubtreeMap_ = new Map();
}
/** @return {!Map<!AutomationNode, boolean>} */
get isActionable() {
return this.isActionableMap_;
}
/** @return {!Map<!AutomationNode, boolean>} */
get isGroup() {
return this.isGroupMap_;
}
/** @return {!Map<!AutomationNode, boolean>} */
get isInterestingSubtree() {
return this.isInterestingSubtreeMap_;
}
}
......@@ -45,6 +45,7 @@ class FocusHistory {
// No ancestors, cannot create stack.
return false;
}
const cache = new SACache();
// Create a list of ancestors.
const ancestorStack = [node];
while (node.parent) {
......@@ -54,7 +55,7 @@ class FocusHistory {
let group = DesktopNode.build(ancestorStack.pop());
const firstAncestor = ancestorStack[ancestorStack.length - 1];
if (!SwitchAccessPredicate.isInterestingSubtree(firstAncestor)) {
if (!SwitchAccessPredicate.isInterestingSubtree(firstAncestor, cache)) {
// If the topmost ancestor (other than the desktop) is entirely
// uninteresting, we leave the history as is.
return false;
......@@ -63,7 +64,7 @@ class FocusHistory {
const newDataStack = [];
while (ancestorStack.length > 0) {
const candidate = ancestorStack.pop();
if (!SwitchAccessPredicate.isInteresting(candidate, group)) {
if (!SwitchAccessPredicate.isInteresting(candidate, group, cache)) {
continue;
}
......
......@@ -14,12 +14,14 @@ SwitchAccessNavigationManagerTest = class extends SwitchAccessE2ETest {
}
moveToPageContents(pageContents) {
if (!SwitchAccessPredicate.isGroup(pageContents)) {
pageContents = new AutomationTreeWalker(
pageContents, constants.Dir.FORWARD,
{visit: SwitchAccessPredicate.isGroup})
.next()
.node;
const cache = new SACache();
if (!SwitchAccessPredicate.isGroup(pageContents, null, cache)) {
pageContents =
new AutomationTreeWalker(pageContents, constants.Dir.FORWARD, {
visit: (node) => SwitchAccessPredicate.isGroup(node, null, cache)
})
.next()
.node;
}
assertNotNullNorUndefined(
pageContents, 'Could not find group corresponding to page contents');
......
......@@ -102,7 +102,8 @@ class NodeWrapper extends SAChildNode {
/** @override */
isGroup() {
return SwitchAccessPredicate.isGroup(this.baseNode_, this.parent_);
const cache = new SACache();
return SwitchAccessPredicate.isGroup(this.baseNode_, this.parent_, cache);
}
/** @override */
......
......@@ -27,9 +27,14 @@ const SwitchAccessPredicate = {
* it in some way.
*
* @param {!AutomationNode} node
* @param {!SACache} cache
* @return {boolean}
*/
isActionable: (node) => {
isActionable: (node, cache) => {
if (cache.isActionable.has(node)) {
return cache.isActionable.get(node);
}
const defaultActionVerb = node.defaultActionVerb;
const loc = node.location;
const parent = node.parent;
......@@ -39,16 +44,19 @@ const SwitchAccessPredicate = {
// Skip things that are offscreen or invisible.
if (!SwitchAccessPredicate.isVisible(node)) {
cache.isActionable.set(node, false);
return false;
}
// Skip things that are disabled.
if (node.restriction === chrome.automation.Restriction.DISABLED) {
cache.isActionable.set(node, false);
return false;
}
// These web containers are not directly actionable.
if (role === RoleType.WEB_VIEW || role === RoleType.ROOT_WEB_AREA) {
cache.isActionable.set(node, false);
return false;
}
......@@ -57,17 +65,20 @@ const SwitchAccessPredicate = {
// Work around for browser tabs.
if (role === RoleType.TAB && parent.role === RoleType.TAB_LIST &&
root.role === RoleType.DESKTOP) {
cache.isActionable.set(node, true);
return true;
}
}
// Check various indicators that the node is actionable.
if (role === RoleType.BUTTON || role === RoleType.SLIDER) {
cache.isActionable.set(node, true);
return true;
}
if (AutomationPredicate.comboBox(node) ||
SwitchAccessPredicate.isTextInput(node)) {
cache.isActionable.set(node, true);
return true;
}
......@@ -78,11 +89,13 @@ const SwitchAccessPredicate = {
defaultActionVerb === DefaultActionVerb.PRESS ||
defaultActionVerb === DefaultActionVerb.SELECT ||
defaultActionVerb === DefaultActionVerb.UNCHECK)) {
cache.isActionable.set(node, true);
return true;
}
if (role === RoleType.LIST_ITEM &&
defaultActionVerb === DefaultActionVerb.CLICK) {
cache.isActionable.set(node, true);
return true;
}
......@@ -91,7 +104,10 @@ const SwitchAccessPredicate = {
// Current heuristic is to show as actionble any focusable item where no
// child is an interesting subtree.
if (state[StateType.FOCUSABLE] || role === RoleType.MENU_ITEM) {
return !node.children.some(SwitchAccessPredicate.isInterestingSubtree);
const result = !node.children.some(
(child) => SwitchAccessPredicate.isInterestingSubtree(child, cache));
cache.isActionable.set(node, result);
return result;
}
return false;
},
......@@ -105,38 +121,48 @@ const SwitchAccessPredicate = {
* box as its scope.
*
* @param {!AutomationNode} node
* @param {AutomationNode|SARootNode} scope
* @param {!AutomationNode|!SARootNode|null} scope
* @param {!SACache} cache
* @return {boolean}
*/
isGroup: (node, scope) => {
isGroup: (node, scope, cache) => {
if (cache.isGroup.has(node)) {
return cache.isGroup.get(node);
}
const scopeEqualsNode = scope &&
(scope instanceof SARootNode ? scope.isEquivalentTo(node) :
scope === node);
if (scope && !scopeEqualsNode &&
RectHelper.areEqual(node.location, scope.location)) {
cache.isGroup.set(node, false);
return false;
}
if (node.state[StateType.INVISIBLE]) {
cache.isGroup.set(node, false);
return false;
}
if (node.role === chrome.automation.RoleType.KEYBOARD) {
cache.isGroup.set(node, true);
return true;
}
let interestingBranchesCount =
SwitchAccessPredicate.isActionable(node) ? 1 : 0;
SwitchAccessPredicate.isActionable(node, cache) ? 1 : 0;
let child = node.firstChild;
while (child) {
if (SwitchAccessPredicate.isInterestingSubtree(child)) {
if (SwitchAccessPredicate.isInterestingSubtree(child, cache)) {
interestingBranchesCount += 1;
}
if (interestingBranchesCount >=
SwitchAccessPredicate.GROUP_INTERESTING_CHILD_THRESHOLD) {
cache.isGroup.set(node, true);
return true;
}
child = child.nextSibling;
}
cache.isGroup.set(node, false);
return false;
},
......@@ -146,10 +172,12 @@ const SwitchAccessPredicate = {
*
* @param {!AutomationNode} node
* @param {!AutomationNode|!SARootNode} scope
* @param {!SACache} cache
* @return {boolean}
*/
isInteresting: (node, scope) => SwitchAccessPredicate.isActionable(node) ||
SwitchAccessPredicate.isGroup(node, scope),
isInteresting: (node, scope, cache) =>
SwitchAccessPredicate.isActionable(node, cache) ||
SwitchAccessPredicate.isGroup(node, scope, cache),
/**
* Returns true if the element is visible to the user for any reason.
......@@ -170,10 +198,20 @@ const SwitchAccessPredicate = {
* isInterestingSubtree).
*
* @param {!AutomationNode} node
* @param {!SACache} cache
* @return {boolean}
*/
isInterestingSubtree: (node) => SwitchAccessPredicate.isActionable(node) ||
node.children.some(SwitchAccessPredicate.isInterestingSubtree),
isInterestingSubtree: (node, cache) => {
if (cache.isInterestingSubtree.has(node)) {
return cache.isInterestingSubtree.get(node);
}
const result = SwitchAccessPredicate.isActionable(node, cache) ||
node.children.some(
(child) =>
SwitchAccessPredicate.isInterestingSubtree(child, cache));
cache.isInterestingSubtree.set(node, result);
return result;
},
/**
* Returns true if |node| is an element that contains editable text.
......@@ -199,10 +237,11 @@ const SwitchAccessPredicate = {
* @return {!AutomationTreeWalkerRestriction}
*/
restrictions: (scope) => {
const cache = new SACache();
return {
leaf: SwitchAccessPredicate.leaf(scope),
leaf: SwitchAccessPredicate.leaf(scope, cache),
root: SwitchAccessPredicate.root(scope),
visit: SwitchAccessPredicate.visit(scope)
visit: SwitchAccessPredicate.visit(scope, cache)
};
},
......@@ -211,12 +250,14 @@ const SwitchAccessPredicate = {
* SwitchAccess scope tree when |scope| is the root.
*
* @param {!AutomationNode} scope
* @param {!SACache} cache
* @return {function(!AutomationNode): boolean}
*/
leaf(scope) {
leaf(scope, cache) {
return (node) => node.state[StateType.INVISIBLE] ||
!SwitchAccessPredicate.isInterestingSubtree(node) ||
(scope !== node && SwitchAccessPredicate.isInteresting(node, scope));
!SwitchAccessPredicate.isInterestingSubtree(node, cache) ||
(scope !== node &&
SwitchAccessPredicate.isInteresting(node, scope, cache));
},
/**
......@@ -235,10 +276,11 @@ const SwitchAccessPredicate = {
* SwitchAccess scope tree with |scope| as the root.
*
* @param {!AutomationNode} scope
* @param {!SACache} cache
* @return {function(!AutomationNode): boolean}
*/
visit(scope) {
visit(scope, cache) {
return (node) => node.role !== RoleType.DESKTOP &&
SwitchAccessPredicate.isInteresting(node, scope);
SwitchAccessPredicate.isInteresting(node, scope, cache);
}
};
......@@ -21,6 +21,7 @@
"common/repeated_tree_change_handler.js",
"common/tree_walker.js",
"switch_access/auto_scan_manager.js",
"switch_access/cache.js",
"switch_access/commands.js",
"switch_access/event_helper.js",
"switch_access/focus_ring_manager.js",
......
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