Implemented scanning by group

chrome/browser/resources/chromeos/switch_access/automation_predicate.js

+// Copyright 2017 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.
+
+/**
+ * Class containing predicates for the chrome automation API. Each predicate
+ * can be run on one or more AutomationNodes and returns a boolean value.
+ *
+ * @constructor
+ */
+function AutomationPredicate() {};
+
+/**
+ * Returns true if |node| is a subtreeLeaf, meaning that |node| is either
+ * interesting or a group (both defined below).
+ *
+ * @param {!chrome.automation.AutomationNode} node
+ * @param {!chrome.automation.AutomationNode} scope
+ * @return {boolean}
+ */
+AutomationPredicate.isSubtreeLeaf = function(node, scope) {
+  return AutomationPredicate.isInteresting(node)
+      || AutomationPredicate.isGroup(node, scope);
+};
+
+/**
+ * Returns true if |node| is a group, meaning that the node has more than one
+ * interesting descendant, and that its interesting descendants exist in more
+ * than one subtree of its immediate children.
+ *
+ * Additionally, for |node| to be a group, it cannot have the same location as
+ * its outer group, |scope|.

[dmazzoni, 2017/05/05 18:23:19]

How about "it cannot have the same bounding box as its scope".

(Even though we use "location" as the api, intuitively some people
thing of location as just the x, y but here we mean the whole
bounding box.)

[elichtenberg, 2017/05/06 00:13:53]

Done.

+ *
+ * @param {!chrome.automation.AutomationNode} node
+ * @param {!chrome.automation.AutomationNode} scope
+ * @return {boolean}
+ */
+AutomationPredicate.isGroup = function(node, scope) {
+  if (node !== scope && AutomationPredicate.hasSameLocation_(node, scope))
+    return false;
+
+  // Work around for client nested in client. No need to have user select both
+  // clients for a window. Once locations for outer client updates correctly,
+  // this won't be needed.
+  if (node.role === chrome.automation.RoleType.CLIENT
+      && node.role === scope.role && node !== scope)
+    return false;
+
+  let interestingBranches = 0;
+  for (let child of node.children) {
+    if (AutomationPredicate.isInterestingSubtree(child))
+      interestingBranches += 1;
+    if (interestingBranches > 1)
+      return true;
+  }
+  return false;
+};
+
+/**
+ * Returns true if the two nodes have the same location.
+ *
+ * @param {!chrome.automation.AutomationNode} node1
+ * @param {!chrome.automation.AutomationNode} node2
+ * @return {boolean}
+ */
+AutomationPredicate.hasSameLocation_ = function(node1, node2) {

[dmazzoni, 2017/05/05 18:23:19]

Don't have to do anything now, but I wonder if this should be
slightly "fuzzy" - i.e. if the difference between them is less
than some number of pixels, consider them the same.

[elichtenberg, 2017/05/06 00:13:53]

Good idea. That's probably something we should look into.

+  let l1 = node1.location;
+  let l2 = node2.location;
+  return l1.left === l2.left && l1.top === l2.top && l1.width === l2.width
+      && l1.height === l2.height;
+};
+
+/**
+ * Returns true if there is an interesting node in the subtree containing
+ * |node| as its root (including |node| itself).
+ *
+ * @param {!chrome.automation.AutomationNode} node
+ * @return {boolean}
+ */
+AutomationPredicate.isInterestingSubtree = function(node) {
+  let children = node.children || [];
+  return AutomationPredicate.isInteresting(node)
+      || children.some(AutomationPredicate.isInterestingSubtree);
+};
+
+/**
+ * Returns true if |node| is interesting, meaning that a user can perform some
+ * type of action on it.
+ *
+ * @param {!chrome.automation.AutomationNode} node
+ * @return {boolean}
+ */
+AutomationPredicate.isInteresting = function(node) {
+  let loc = node.location;
+  let parent = node.parent;
+  let root = node.root;
+  let role = node.role;
+  let state = node.state;
+
+  // Skip things that are offscreen
+  if (state[chrome.automation.StateType.OFFSCREEN]

[dmazzoni, 2017/05/05 18:23:19]

Consider defining shorthand for chrome.automation.StateType
and chrome.automation.RoleType at the top of the file kind of
like how ChromeVox does it. I know there were difficulties
doing this with the Closure Compiler and tests, so you don't
have to worry about it for this change if it's not easy.

[elichtenberg, 2017/05/06 00:13:53]

Adding a TODO for this. Hopefully I can find an easier way to do this.

+      || loc.top < 0 || loc.left < 0)
+    return false;
+
+  // Should just leave these as groups
+  if (role === chrome.automation.RoleType.WEB_VIEW
+      || role === chrome.automation.RoleType.ROOT_WEB_AREA)
+    return false;
+
+  if (parent) {
+    // crbug.com/710559
+    // Work around for browser tabs
+    if (role === chrome.automation.RoleType.TAB
+        && parent.role === chrome.automation.RoleType.TAB_LIST
+        && root.role === chrome.automation.RoleType.DESKTOP)
+      return true;
+  }
+
+  // The general rule that applies to everything.
+  return state[chrome.automation.StateType.FOCUSABLE] === true;
+}