Correctly follows focus and moves to valid node when current becomes invalid

chrome/browser/resources/chromeos/switch_access/automation_manager.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 to manage interactions with the accessibility tree, including moving
  * to and selecting nodes.
  *
  * @constructor
+ * @param {!chrome.automation.AutomationNode} desktop
  */
-function AutomationManager() {
+function AutomationManager(desktop) {
   /**
    * Currently highlighted node.
    *
-   * @private {chrome.automation.AutomationNode}
+   * @private {!chrome.automation.AutomationNode}
    */
-  this.node_ = null;
+  this.node_ = desktop;
 
   /**
    * The root of the subtree that the user is navigating through.
    *
-   * @private {chrome.automation.AutomationNode}
+   * @private {!chrome.automation.AutomationNode}
    */
-  this.scope_ = null;
+  this.scope_ = desktop;
 
   /**
    * The desktop node.
    *
-   * @private {chrome.automation.AutomationNode}
+   * @private {!chrome.automation.AutomationNode}
    */
-  this.desktop_ = null;
+  this.desktop_ = desktop;
 
   /**
    * A stack of past scopes. Allows user to traverse back to previous groups
    * after selecting one or more groups. The most recent group is at the end
    * of the array.
    *
-   * @private {Array<chrome.automation.AutomationNode>}
+   * @private {Array<!chrome.automation.AutomationNode>}
    */
   this.scopeStack_ = [];
 
   /**
    * Moves to the appropriate node in the accessibility tree.
    *
-   * @private {AutomationTreeWalker}
+   * @private {!AutomationTreeWalker}
    */
-  this.treeWalker_ = null;
+  this.treeWalker_ = this.createTreeWalker_(desktop);
 
   this.init_();
 };
 
 /**
  * Highlight colors for the focus ring to distinguish between different types
  * of nodes.
  *
  * @const
  */
 AutomationManager.Color = {
   SCOPE: '#de742f', // dark orange
   GROUP: '#ffbb33', // light orange
   LEAF: '#78e428' //light green
 };
 
 AutomationManager.prototype = {
   /**
    * Set this.node_, this.root_, and this.desktop_ to the desktop node, and
    * creates an initial tree walker.
    *
    * @private
    */
   init_: function() {
-    chrome.automation.getDesktop(function(desktop) {
-      this.node_ = desktop;
-      this.scope_ = desktop;
-      this.desktop_ = desktop;
-      this.treeWalker_ = this.createTreeWalker_(desktop);
-      console.log('AutomationNode for desktop is loaded');
-      this.printNode_(this.node_);
-    }.bind(this));
+    console.log('AutomationNode for desktop is loaded');
+    this.printNode_(this.node_);
+
+    this.desktop_.addEventListener(
+        chrome.automation.EventType.FOCUS,
+        this.handleFocusChange_.bind(this),
+        false);
+
+    // TODO(elichtenberg): Eventually use a more specific filter than
+    // ALL_TREE_CHANGES.
+    chrome.automation.addTreeChangeObserver(
+        chrome.automation.TreeChangeObserverFilter.ALL_TREE_CHANGES,
+        this.handleNodeRemoved_.bind(this));
   },
 
   /**
+   * When an interesting element gains focus on the page, move to it. If an
+   * element gains focus but is not interesting, move to the next interesting
+   * node after it.
+   *
+   * @param {!chrome.automation.AutomationEvent} event
+   * @private
+   */
+  handleFocusChange_: function(event) {
+    if (this.node_ === event.target)
+      return;
+    console.log('Focus changed');
+
+    // Create list of ancestors of node that encountered focus change.
+    let node = event.target;
+    let ancestorList = [];
+    while (node.parent) {
+      ancestorList.push(node.parent);
+      node = node.parent;
+    }
+
+    // Rebuild scope stack.

[dmazzoni, 2017/05/18 16:25:52]

Might be simpler to just do this in one loop rather than
building a list and then popping from it.

[elichtenberg, 2017/05/18 18:31:01]

Need separate loops because need the node's scope in order to tell if that node
is a group. So need to start at desktop and walk down the ancestry chain.

Moved this to a helper function though.

+    this.scopeStack_ = [];
+    this.scope_ = this.desktop_;
+    while (ancestorList.length > 0) {
+      let newScope = ancestorList.pop();
+      if (newScope.role === chrome.automation.RoleType.DESKTOP)
+        continue;
+      if (AutomationPredicate.isGroup(newScope, this.scope_)) {
+        this.scopeStack_.push(this.scope_);
+        this.scope_ = newScope;
+      }
+    }
+
+    // Move to focused node.
+    this.node_ = event.target;
+    this.treeWalker_ = this.createTreeWalker_(this.scope_, this.node_);
+
+    // In case the node that gained focus is not a subtreeLeaf.
+    if (AutomationPredicate.isSubtreeLeaf(this.node_, this.scope_)) {
+      this.printNode_(this.node_);
+      this.setFocusRing_();
+    } else
+      this.moveToNode(true);
+  },
+
+  /**
+   * When a node is removed from the page, move to a new valid node.
+   *
+   * @param {!chrome.automation.TreeChange} treeChange
+   * @private
+   */
+  handleNodeRemoved_: function(treeChange) {
+    if (treeChange.type === chrome.automation.TreeChangeType.NODE_REMOVED

[dmazzoni, 2017/05/18 16:25:52]

Please split this up into a few separate checks, and
add a TODO comment next to the one that's wrong.

I'm thinking:

// TODO: only listen to NODE_REMOVED callbacks, we don't
// need any others
if (treeChange.type !== NODE_REMOVED)
  return;

// TODO: get rid of this, we should be getting NODE_REMOVED
// when a whole tree is deleted
if (treeChange.target.role === 'rootWebArea' && !this.node_.role)

...and so on

[elichtenberg, 2017/05/18 18:31:01]

Split it up and added some TODOs. Do you think this looks clean enough?

+        && ((treeChange.target.role === 'rootWebArea' && !this.node_.role)

[dmazzoni, 2017/05/18 16:25:52]

Use RoleType.ROOT_WEB_AREA here and elsewhere

[elichtenberg, 2017/05/18 18:31:01]

Done.

+          || treeChange.target === this.node_)) {
+      console.log('Node removed');
+      chrome.accessibilityPrivate.setFocusRing([]);
+
+      // Current node not invalid until after treeChange callback, so move to
+      // valid node after callback. Delay added to prevent moving to another
+      // node about to be made invalid. If already at a valid node (e.g., user
+      // moves to it or focus changes to it), won't need to move to a new node.
+      window.setTimeout(function() {
+        if (!this.node_.role)
+          this.moveToNode(true);
+      }.bind(this), 100);
+    }
+  },
+
+  /**
    * Set this.node_ to the next/previous interesting node, and then highlight
    * it on the screen. If no interesting node is found, set this.node_ to the
    * first/last interesting node. If |doNext| is true, will search for next
    * node. Otherwise, will search for previous node.
    *
    * @param {boolean} doNext
    */
   moveToNode: function(doNext) {
-    if (!this.treeWalker_)
-      return;
+    // If node is invalid, set node to last valid scope.
+    this.startAtValidNode_();
 
     let node = this.treeWalker_.moveToNode(doNext);
     if (node) {
       this.node_ = node;
       this.printNode_(this.node_);
-
-      let color;
-      if (this.node_ === this.scope_)
-        color = AutomationManager.Color.SCOPE;
-      else if (AutomationPredicate.isInteresting(this.node_))
-        color = AutomationManager.Color.LEAF;
-      else
-        color = AutomationManager.Color.GROUP;
-      chrome.accessibilityPrivate.setFocusRing([this.node_.location], color);
+      this.setFocusRing_();
     }
   },
 
   /**
    * Select the currently highlighted node. If the node is the current scope,
    * go back to the previous scope (i.e., create a new tree walker rooted at
    * the previous scope). If the node is a group other than the current scope,
    * create a new tree walker for the new subtree the user is scanning through.
    * Otherwise, meaning the node is interesting, perform the default action on
    * it.
    */
   selectCurrentNode: function() {
-    if (!this.node_ || !this.scope_ || !this.treeWalker_)
+    if (!this.node_.role)
       return;
 
     if (this.node_ === this.scope_) {
       // Don't let user select the top-level root node (i.e., the desktop node).
       if (this.scopeStack_.length === 0)
         return;
 
-      // Find a previous scope that is still valid.
-      let oldScope;
+      // Find a previous scope that is still valid. The stack here always has
+      // at least one valid scope (i.e., the desktop node).
       do {
-        oldScope = this.scopeStack_.pop();
-      } while (oldScope && !oldScope.role);
+        this.scope_ = this.scopeStack_.pop();
+      } while (!this.scope_.role && this.scopeStack_.length > 0);
 
-      // oldScope will always be valid here, so this will always be true.
-      if (oldScope) {
-        this.scope_ = oldScope;
-        this.treeWalker_ = this.createTreeWalker_(this.scope_, this.node_);
-        chrome.accessibilityPrivate.setFocusRing(
-            [this.node_.location], AutomationManager.Color.GROUP);
-      }
+      this.treeWalker_ = this.createTreeWalker_(this.scope_, this.node_);
+      chrome.accessibilityPrivate.setFocusRing(

[dmazzoni, 2017/05/18 16:25:52]

Can you call this.setFocusRing_() here?

[elichtenberg, 2017/05/18 18:31:01]

Yep. Just changed it.

+          [this.node_.location], AutomationManager.Color.GROUP);
+      console.log('Moved to previous scope');
+      this.printNode_(this.node_);
       return;
     }
 
     if (AutomationPredicate.isGroup(this.node_, this.scope_)) {
       this.scopeStack_.push(this.scope_);
       this.scope_ = this.node_;
       this.treeWalker_ = this.createTreeWalker_(this.scope_);
+      console.log('Entered scope');
       this.moveToNode(true);
       return;
     }
 
     this.node_.doDefault();
+    console.log('Performed default action');
+    console.log('\n');
   },
 
   /**
+   * Set the focus ring for the current node and determine the color for it.
+   *
+   * @private
+   */
+  setFocusRing_: function() {

[dmazzoni, 2017/05/18 16:25:52]

How about updateFocusRing(), since it sets the focus
ring based on the current state, rather than setting it
to some particular argument.

[elichtenberg, 2017/05/18 18:31:01]

Done.

+    let color;
+    if (this.node_ === this.scope_)
+      color = AutomationManager.Color.SCOPE;
+    else if (AutomationPredicate.isGroup(this.node_, this.scope_))
+      color = AutomationManager.Color.GROUP;
+    else
+      color = AutomationManager.Color.LEAF;
+    chrome.accessibilityPrivate.setFocusRing([this.node_.location], color);
+  },
+
+  /**
+   * If this.node_ is invalid, set this.node_ to a valid scope. Will check the
+   * current scope and past scopes until a valid scope is found. this.node_
+   * is set to that valid scope.
+   *
+   * @private
+   */
+  startAtValidNode_: function() {
+    if (this.node_.role)
+      return;
+    console.log('Finding new valid node');
+
+    // Current node is invalid, but current scope is still valid, so set node
+    // to the current scope.
+    if (this.scope_.role)
+      this.node_ = this.scope_;
+
+    // Current node and current scope are invalid, so set both to a valid scope
+    // from the scope stack. The stack here always has at least one valid scope
+    // (i.e., the desktop node).
+    while (!this.node_.role && this.scopeStack_.length > 0) {
+      this.node_ = this.scopeStack_.pop();
+      this.scope_ = this.node_;
+    }
+    this.treeWalker_ = this.createTreeWalker_(this.scope_);
+  },
+
+  /**
    * Create an AutomationTreeWalker for the subtree with |scope| as its root.
    * If |opt_start| is defined, the tree walker will start walking the tree
    * from |opt_start|; otherwise, it will start from |scope|.
    *
    * @param {!chrome.automation.AutomationNode} scope
    * @param {!chrome.automation.AutomationNode=} opt_start
+   * @private
    * @return {!AutomationTreeWalker}
    */
   createTreeWalker_: function(scope, opt_start) {
     // If no explicit start node, start walking the tree from |scope|.
     let start = opt_start || scope;
 
     let leafPred = function(node) {
       return (node !== scope && AutomationPredicate.isSubtreeLeaf(node, scope))
           || !AutomationPredicate.isInterestingSubtree(node);
     };
@@ skipping 80 matching lines @@
    */
   debugMoveToParent: function() {
     let parent = this.treeWalker_.debugMoveToParent(this.node_);
     if (parent) {
       this.node_ = parent;
       this.printNode_(this.node_);
       chrome.accessibilityPrivate.setFocusRing([this.node_.location]);
     }
   }
 };