[Switch Access] Recalculate current group when nodes are removed.

When a node in the current group is removed we should check that
the current group is valid, and re-calculate its children.

This change also has some other bugfixes to improve stability
around edgecases.

AX-Relnotes: N/A
Bug: 1105196, 1106575
Change-Id: I0461150aadc42d5bd01099d96bf69d8d1f71119a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303056Reviewed-by: Anastasia Helfinstein <anastasi@google.com>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789643}

chrome/browser/resources/chromeos/accessibility/common/repeated_tree_change_handler.js

@@ -10,9 +10,9 @@ class RepeatedTreeChangeHandler {
   /**
    * @param {!chrome.automation.TreeChangeObserverFilter} filter
    * @param {!function(!chrome.automation.TreeChange)} callback
-   * @param {{predicate: (function(!chrome.automation.TreeChange): boolean)}}
-   *     options
-   *         predicate A generic predicate that filters for changes of interest.
+   * @param {{predicate: ((function(!chrome.automation.TreeChange): boolean) |
+   *     undefined)}} options predicate A generic predicate that filters for
+   *     changes of interest.
    */
   constructor(filter, callback, options = {}) {
     /** @private {!Array<!chrome.automation.TreeChange>} */

chrome/browser/resources/chromeos/accessibility/switch_access/BUILD.gn

@@ -161,6 +161,7 @@ js_type_check("closure_compile") {
     "../common:closure_shim",
     "../common:constants",
     "../common:repeated_event_handler",
+    "../common:repeated_tree_change_handler",
     "../common:tree_walker",
   ]
 }

chrome/browser/resources/chromeos/accessibility/switch_access/history.js

@@ -17,6 +17,11 @@ class FocusData {
 
   /** @return {boolean} */
   isValid() {
+    if (this.group.isValidGroup()) {
+      // Ensure it is still valid. Some nodes may have been added
+      // or removed since this was last used.
+      this.group.refreshChildren();
+    }
     return this.group.isValidGroup();
   }
 }

chrome/browser/resources/chromeos/accessibility/switch_access/navigation_manager.js

@@ -143,7 +143,11 @@ class NavigationManager {
    */
   static moveBackward() {
     const navigator = NavigationManager.instance;
+    if (navigator.node_.isValidAndVisible()) {
       navigator.setNode_(navigator.node_.previous);
+    } else {
+      NavigationManager.moveToValidNode();
+    }
   }
 
   /**
@@ -151,7 +155,11 @@ class NavigationManager {
    */
   static moveForward() {
     const navigator = NavigationManager.instance;
+    if (navigator.node_.isValidAndVisible()) {
       navigator.setNode_(navigator.node_.next);
+    } else {
+      NavigationManager.moveToValidNode();
+    }
   }
 
   /**
@@ -251,14 +259,20 @@ class NavigationManager {
   }
 
   /**
-   * When the automation tree changes, check if it affects any nodes we are
-   *     currently listening to.
+   * When the automation tree changes, ensure the group and node we are
+   * currently listening to are fresh. This is only called when the tree change
+   * occurred on the node or group which are currently active.
    * @param {!chrome.automation.TreeChange} treeChange
    * @private
    */
   onTreeChange_(treeChange) {
     if (treeChange.type === chrome.automation.TreeChangeType.NODE_REMOVED) {
+      this.group_.refresh();
       NavigationManager.moveToValidNode();
+    } else if (
+        treeChange.type ===
+        chrome.automation.TreeChangeType.SUBTREE_UPDATE_END) {
+      this.group_.refresh();
     }
   }
 
@@ -283,6 +297,14 @@ class NavigationManager {
         this.desktop_, chrome.automation.EventType.SCROLL_POSITION_CHANGED,
         this.onScrollChange_.bind(this));
 
+    new RepeatedTreeChangeHandler(
+        chrome.automation.TreeChangeObserverFilter.ALL_TREE_CHANGES,
+        this.onTreeChange_.bind(this), {
+          predicate: (treeChange) =>
+              this.group_.findChild(treeChange.target) != null ||
+              this.group_.isEquivalentTo(treeChange.target)
+        });
+
     // The status tray fires a SHOW event when it opens.
     this.desktop_.addEventListener(
         chrome.automation.EventType.SHOW, this.onModalDialog_.bind(this),
@@ -290,9 +312,6 @@ class NavigationManager {
     this.desktop_.addEventListener(
         chrome.automation.EventType.MENU_START, this.onModalDialog_.bind(this),
         false);
-    chrome.automation.addTreeChangeObserver(
-        chrome.automation.TreeChangeObserverFilter.ALL_TREE_CHANGES,
-        this.onTreeChange_.bind(this));
   }
 
   /**

chrome/browser/resources/chromeos/accessibility/switch_access/nodes/back_button_node.js

@@ -77,7 +77,7 @@ class BackButtonNode extends SAChildNode {
 
   /** @override */
   isValidAndVisible() {
-    return true;
+    return this.group_.isValidGroup();
   }
 
   /** @override */

chrome/browser/resources/chromeos/accessibility/switch_access/nodes/node_wrapper.js

@@ -304,6 +304,16 @@ class RootNodeWrapper extends SARootNode {
     }
   }
 
+  /** @override */
+  refreshChildren() {
+    const childConstructor = (node) => NodeWrapper.create(node, this);
+    try {
+      RootNodeWrapper.findAndSetChildren(this, childConstructor);
+    } catch (e) {
+      this.invalidated_ = true;
+    }
+  }
+
   /** @override */
   refresh() {
     // Find the currently focused child.
@@ -316,12 +326,9 @@ class RootNodeWrapper extends SARootNode {
     }
 
     // Update this RootNodeWrapper's children.
-    const childConstructor = (node) => NodeWrapper.create(node, this);
-    try {
-      RootNodeWrapper.findAndSetChildren(this, childConstructor);
-    } catch (e) {
+    this.refreshChildren();
+    if (this.invalidated_) {
       this.onUnfocus();
-      this.invalidated_ = true;
       NavigationManager.moveToValidNode();
       return;
     }

chrome/browser/resources/chromeos/accessibility/switch_access/nodes/switch_access_node.js

@@ -363,8 +363,8 @@ class SARootNode {
     // Must have one interesting child that is not the back button.
     return this.children_
                .filter(
-                   (child) => child.isValidAndVisible() &&
-                       !(child instanceof BackButtonNode))
+                   (child) => !(child instanceof BackButtonNode) &&
+                       child.isValidAndVisible())
                .length >= 1;
   }
 
@@ -384,6 +384,11 @@ class SARootNode {
   /** Called when a group is explicitly exited. */
   onExit() {}
 
+  /** Called when a group should recalculate its children. */
+  refreshChildren() {
+    this.children = this.children.filter((child) => child.isValidAndVisible());
+  }
+
   /** Called when the group's children may have changed. */
   refresh() {}
 

chrome/browser/resources/chromeos/accessibility/switch_access_manifest.json.jinja2

@@ -16,6 +16,7 @@
       "common/array_util.js",
       "common/automation_predicate.js",
       "common/repeated_event_handler.js",
+      "common/repeated_tree_change_handler.js",
       "common/tree_walker.js",
       "switch_access/auto_scan_manager.js",
       "switch_access/commands.js",