Make ChromeVox cursor robust to deleted nodes

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

 // Copyright 2014 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.
 
 /**
  * @fileoverview Classes related to cursors that point to and select parts of
  * the automation tree.
  */
 
 goog.provide('cursors.Cursor');
@@ skipping 53 matching lines @@
 
 /**
  * Represents a position within the automation tree.
  * @constructor
  * @param {!AutomationNode} node
  * @param {number} index A 0-based index into this cursor node's primary
  * accessible name. An index of |cursors.NODE_INDEX| means the node as a whole
  * is pointed to and covers the case where the accessible text is empty.
  */
 cursors.Cursor = function(node, index) {
-  /** @type {!AutomationNode} @private */
-  this.node_ = node;
   /** @type {number} @private */
   this.index_ = index;
+  /** @type {Array<AutomationNode>} @private */
+  this.ancestry_ = [];
+  var nodeWalker = node;
+  while (nodeWalker) {
+    this.ancestry_.push(nodeWalker);
+    nodeWalker = nodeWalker.parent;
+    if (nodeWalker == node.root)
+      break;
+  }
 };
 
 /**
  * Convenience method to construct a Cursor from a node.
  * @param {!AutomationNode} node
  * @return {!cursors.Cursor}
  */
 cursors.Cursor.fromNode = function(node) {
   return new cursors.Cursor(node, cursors.NODE_INDEX);
 };
 
 cursors.Cursor.prototype = {
   /**
    * Returns true if |rhs| is equal to this cursor.
    * @param {!cursors.Cursor} rhs
    * @return {boolean}
    */
   equals: function(rhs) {
-    return this.node_ === rhs.node &&
-        this.index_ === rhs.index;
+    return this.node === rhs.node &&
+        this.index === rhs.index;
   },
 
   /**
-   * @return {!AutomationNode}
+   * Returns the node. If the node is invalid since the last time it
+   * was accessed, moves the cursor to the nearest valid ancestor first.
+   * @return {?AutomationNode}

[David Tseng, 2016/05/25 20:58:10]

You don't need the '?' (objects are nullable by default).

[dmazzoni, 2016/06/01 16:36:50]

Done.

    */
   get node() {
-    return this.node_;
+    for (var index = 0; index < this.ancestry_.length; index++) {

[David Tseng, 2016/05/25 20:58:10]

The dual use of index here is confusing. Maybe use for (let x of y).

[dmazzoni, 2016/06/01 16:36:50]

Fixed, but the Closure compiler doesn't let us use ES6
syntax as currently configured so I kept it a simple
loop for now. Should we enable ES6 in a follow-on change?

+      var foo = this.ancestry_[index];

[David Tseng, 2016/05/25 20:58:10]

How about firstValidNode?

[dmazzoni, 2016/06/01 16:36:50]

Acknowledged.

+      if (foo != null && foo.role !== undefined)

[David Tseng, 2016/05/25 20:58:10]

This won't work. There are nodes that have a role but have no root; check for
the presence of root as well.

[dmazzoni, 2016/06/01 16:36:50]

Done.

+        return foo;

[David Tseng, 2016/05/25 20:58:10]

Do you need to do bookkeeping on the ancestry chain?

[dmazzoni, 2016/06/01 16:36:50]

I don't think so, because this object is basically immutable - if you call it
again it will search again, but as soon as you move to another node, it creates
a new Cursor with a new ancestry.

+      // If we have to walk up to an ancestor, reset the index to zero.
+      this.index_ = 0;

[David Tseng, 2016/05/25 20:58:10]

Reset to cursors.NODE_INDEX

[dmazzoni, 2016/06/01 16:36:50]

Thanks, that made it work better.

+    }
+    return null;
   },
 
   /**
    * @return {number}
    */
   get index() {
     return this.index_;
   },
 
   /**
@@ skipping 19 matching lines @@
   },
 
   /**
    * Gets the accessible text of the node associated with this cursor.
    *
    * @param {!AutomationNode=} opt_node Use this node rather than this cursor's
    * node.
    * @return {string}
    */
   getText: function(opt_node) {
-    var node = opt_node || this.node_;
+    var node = opt_node || this.node;
     if (node.role === RoleType.textField)
       return node.value;
     return node.name || '';
   },
 
   /**
    * Makes a Cursor which has been moved from this cursor by the unit in the
    * given direction using the given movement type.
    * @param {Unit} unit
    * @param {Movement} movement
    * @param {Dir} dir
    * @return {!cursors.Cursor} The moved cursor.
    */
   move: function(unit, movement, dir) {
-    var newNode = this.node_;
+    var originalNode = this.node;
+    if (!originalNode)
+      return this;
+
+    var newNode = originalNode;
     var newIndex = this.index_;
 
     if ((unit != Unit.NODE || unit != Unit.DOM_NODE) &&
         newIndex === cursors.NODE_INDEX)
       newIndex = 0;
 
     switch (unit) {
       case Unit.CHARACTER:
         // BOUND and DIRECTIONAL are the same for characters.
         var text = this.getText();
@@ skipping 80 matching lines @@
       case Unit.NODE:
       case Unit.DOM_NODE:
         switch (movement) {
           case Movement.BOUND:
             newIndex = dir == Dir.FORWARD ? this.getText().length - 1 : 0;
             break;
           case Movement.DIRECTIONAL:
             var pred = unit == Unit.NODE ?
                 AutomationPredicate.leaf : AutomationPredicate.element;
             newNode = AutomationUtil.findNextNode(
-                newNode, dir, pred) || this.node_;
+                newNode, dir, pred) || originalNode;
             newIndex = cursors.NODE_INDEX;
             break;
         }
         break;
       case Unit.LINE:
         newIndex = 0;
         switch (movement) {
           case Movement.BOUND:
             newNode = AutomationUtil.findNodeUntil(newNode, dir,
                 AutomationPredicate.linebreak, true);
-            newNode = newNode || this.node_;
+            newNode = newNode || originalNode;
             newIndex =
                 dir == Dir.FORWARD ? this.getText(newNode).length : 0;
             break;
           case Movement.DIRECTIONAL:
             newNode = AutomationUtil.findNodeUntil(
                 newNode, dir, AutomationPredicate.linebreak);
             break;
           }
       break;
       default:
         throw Error('Unrecognized unit: ' + unit);
     }
-    newNode = newNode || this.node_;
+    newNode = newNode || originalNode;
     newIndex = goog.isDef(newIndex) ? newIndex : this.index_;
     return new cursors.Cursor(newNode, newIndex);
   },
 
   /**
    * Returns whether this cursor points to a valid position.
    * @return {boolean}
    */
   isValid: function() {
     return this.node.role !== undefined;
@@ skipping 22 matching lines @@
 cursors.WrappingCursor.fromNode = function(node) {
   return new cursors.WrappingCursor(node, cursors.NODE_INDEX);
 };
 
 cursors.WrappingCursor.prototype = {
   __proto__: cursors.Cursor.prototype,
 
   /** @override */
   move: function(unit, movement, dir) {
     var result = this;
+    if (!result.node)
+      return this;
 
     // Regular movement.
     if (!AutomationUtil.isTraversalRoot(this.node) || dir == Dir.FORWARD)
       result = cursors.Cursor.prototype.move.call(this, unit, movement, dir);
 
     // There are two cases for wrapping:
     // 1. moving forwards from the last element.
     // 2. moving backwards from the document root.
     // Both result in |move| returning the same cursor.
     // For 1, simply place the new cursor on the document node.
     // For 2, try to descend to the first leaf-like object.
     if (movement == Movement.DIRECTIONAL && result.equals(this)) {
       var pred = unit == Unit.DOM_NODE ?
           AutomationPredicate.element : AutomationPredicate.leaf;
       var endpoint = this.node;
+      if (!endpoint)
+        return this;
 
       // Case 1: forwards (find the root-like node).
       while (!AutomationUtil.isTraversalRoot(endpoint) && endpoint.parent)
         endpoint = endpoint.parent;
 
       // Case 2: backward (sync downwards to a leaf).
       if (dir == Dir.BACKWARD)
         endpoint = AutomationUtil.findNodePre(endpoint, dir, pred) || endpoint;
 
       cvox.ChromeVox.earcons.playEarcon(cvox.Earcon.WRAP);
@@ skipping 33 matching lines @@
  * Given |rangeA| and |rangeB| in order, determine which |Dir|
  * relates them.
  * @param {!cursors.Range} rangeA
  * @param {!cursors.Range} rangeB
  * @return {Dir}
  */
 cursors.Range.getDirection = function(rangeA, rangeB) {
   if (!rangeA || !rangeB)
     return Dir.FORWARD;
 
+  if (!rangeA.start.node || !rangeA.end.node ||
+      !rangeB.start.node || !rangeB.end.node)
+    return Dir.FORWARD;
+
   // They are the same range.
   if (rangeA.start.node === rangeB.start.node &&
       rangeB.end.node === rangeA.end.node)
     return Dir.FORWARD;
 
   var testDirA =
       AutomationUtil.getDirection(
           rangeA.start.node, rangeB.end.node);
   var testDirB =
       AutomationUtil.getDirection(
@@ skipping 59 matching lines @@
 
   /**
    * Makes a Range which has been moved from this range by the given unit and
    * direction.
    * @param {Unit} unit
    * @param {Dir} dir
    * @return {cursors.Range}
    */
   move: function(unit, dir) {
     var newStart = this.start_;
+    if (!newStart.node)
+      return this;
+
     var newEnd;
     switch (unit) {
       case Unit.CHARACTER:
         newStart = newStart.move(unit, Movement.BOUND, dir);
         newEnd = newStart.move(unit, Movement.BOUND, Dir.FORWARD);
         // Character crossed a node; collapses to the end of the node.
         if (newStart.node !== newEnd.node)
           newEnd = new cursors.Cursor(newStart.node, newStart.index + 1);
         break;
       case Unit.WORD:
@@ skipping 13 matching lines @@
     return new cursors.Range(newStart, newEnd);
   },
 
   /**
    * Select the text contained within this range.
    */
   select: function() {
     var start = this.start.node;
     var end = this.end.node;
 
+    if (!start || !end)
+      return;
+
     // Find the most common root.
     var uniqueAncestors = AutomationUtil.getUniqueAncestors(start, end);
     var mcr = start.root;
     if (uniqueAncestors)
       mcr = uniqueAncestors.pop().parent.root;
 
     if (mcr.role == RoleType.desktop)
       return;
 
     if (mcr === start.root && mcr === end.root) {
@@ skipping 24 matching lines @@
   /**
    * Returns whether this range has valid start and end cursors.
    * @return {boolean}
    */
   isValid: function() {
     return this.start.isValid() && this.end.isValid();
   }
 };
 
 });  // goog.scope