Cleanup AutomationUtil.findNodeUntil, fix wrapping when moving by line

1. AutomationUtil.findNodeUntil isn't entirely clear due to the use of a callback that wants to keep track of the previous node being visited. This has since become wrong because of the changes made to AutomationTreeWalker. Instead, let's just call AutomationUtil.findNextNode which should gibve us the proper linearization. Take pairs in order and apply the predicate.

2. wrapping by line didn't quite work as expected because of the asymmetric nature of the linearization. We need to get to the end when wrapping from the beginning to the end and start applying our predicate. Note that this is subject to some weird corner cases depending on how we write the predicates (e.g. ones that check for children).

3. Ensure we consider invisible nodes to be leaves. They should also be ignored when encountered (this is already done).

TEST=navigate around using ChromeVox Next on the shelf. Observe desirable behaviors.

Review URL: https://codereview.chromium.org/1622803002

Cr-Commit-Position: refs/heads/master@{#371620}

chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_predicate.js

@@ -143,6 +143,7 @@ AutomationPredicate.leaf = function(node) {
       node.role == RoleType.popUpButton ||
       node.role == RoleType.slider ||
       node.role == RoleType.textField ||
+      node.state.invisible ||
       node.children.every(function(n) {
         return n.state.invisible;
       });

chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_util.js

@@ -115,44 +115,22 @@ AutomationUtil.findNextNode = function(cur, dir, pred, opt_restrictions) {
 /**
  * Given nodes a_1, ..., a_n starting at |cur| in pre order traversal, apply
  * |pred| to a_i and a_(i - 1) until |pred| is satisfied.  Returns a_(i - 1) or
- * a_i (depending on opt_options.before) or null if no match was found.
+ * a_i (depending on opt_before) or null if no match was found.
  * @param {!AutomationNode} cur
  * @param {Dir} dir
  * @param {AutomationPredicate.Binary} pred
- * @param {{filter: (AutomationPredicate.Unary|undefined),
- *      before: boolean?}=} opt_options
- *     filter - Filters which candidate nodes to consider. Defaults to leaf
- *         only.
- *     before - True to return a_(i - 1); a_i otherwise. Defaults to false.
+ * @param {boolean=} opt_before True to return a_(i - 1); a_i otherwise.
+ *                              Defaults to false.
  * @return {AutomationNode}
  */
-AutomationUtil.findNodeUntil = function(cur, dir, pred, opt_options) {
-  opt_options =
-      opt_options || {filter: AutomationPredicate.leaf, before: false};
-  if (!opt_options.filter)
-    opt_options.filter = AutomationPredicate.leaf;
-
-  var before = null;
-  var after = null;
-  var prev = cur;
-  AutomationUtil.findNextNode(cur,
-      dir,
-      function(candidate) {
-        if (!opt_options.filter(candidate))
-          return false;
-
-        var satisfied = pred(prev, candidate);
-
-        prev = candidate;
-        if (!satisfied)
-          before = candidate;
-        else
-          after = candidate;
-        return satisfied;
-      },
-      {leaf: AutomationPredicate.leaf, skipInitialSubtree: true});
-
-  return opt_options.before ? before : after;
+AutomationUtil.findNodeUntil = function(cur, dir, pred, opt_before) {
+  var before = cur;
+  var after = before;
+  do {
+    before = after;
+    after = AutomationUtil.findNextNode(before, dir, AutomationPredicate.leaf);
+  } while (after && !pred(before, after));
+  return opt_before ? before : after;
 };
 
 /**

chrome/browser/resources/chromeos/chromevox/cvox2/background/cursors.js

@@ -238,7 +238,7 @@ cursors.Cursor.prototype = {
         switch (movement) {
           case Movement.BOUND:
             newNode = AutomationUtil.findNodeUntil(newNode, dir,
-                AutomationPredicate.linebreak, {before: true});
+                AutomationPredicate.linebreak, true);
             newNode = newNode || this.node_;
             newIndex =
                 dir == Dir.FORWARD ? this.getText(newNode).length : 0;
@@ -290,10 +290,17 @@ cursors.WrappingCursor.prototype = {
     if (movement == Movement.DIRECTIONAL && result.equals(this)) {
       var pred = unit == Unit.DOM_NODE ?
           AutomationPredicate.leafDomNode : AutomationPredicate.leaf;
-      var root = this.node;
-      while (!AutomationUtil.isTraversalRoot(root) && root.parent)
-        root = root.parent;
-      var wrappedNode = AutomationUtil.findNodePre(root, dir, pred);
+      var endpoint = this.node;
+      while (!AutomationUtil.isTraversalRoot(endpoint) && endpoint.parent)
+        endpoint = endpoint.parent;
+      if (dir == Dir.BACKWARD) {
+        while (endpoint.lastChild)
+          endpoint = endpoint.lastChild;
+      }
+      var wrappedNode = endpoint;
+      if (!pred(wrappedNode))
+        wrappedNode = AutomationUtil.findNextNode(endpoint, dir, pred);
+
       if (wrappedNode) {
         cvox.ChromeVox.earcons.playEarcon(cvox.Earcon.WRAP);
         return new cursors.WrappingCursor(wrappedNode, cursors.NODE_INDEX);

chrome/browser/resources/chromeos/chromevox/cvox2/background/cursors_test.extjs

@@ -298,7 +298,7 @@ TEST_F('CursorsTest', 'WrappingCursors', function() {
     var para = root.firstChild;
     var cursor = new cursors.WrappingCursor(para, -1);
     cursor = cursor.move(DOM_NODE, DIRECTIONAL, BACKWARD);
-    assertEquals(root.lastChild.firstChild, cursor.node);
+    assertEquals(root.lastChild.firstChild.firstChild, cursor.node);
     cursor = cursor.move(DOM_NODE, DIRECTIONAL, FORWARD);
     assertEquals(root.firstChild.firstChild, cursor.node);
   });