[Switch Access] Reorder node methods

I've realized that the ordering of the functions in the node classes,
while it makes sense to me, is fairly arbitrary. Reorder the functions
to group by type (private/setters and getters/static/etc.) and
alphabetize within those groups.

This is a pure refactor. It changes no logic nor behavior, it only
reorders the functions in the file.

Bug: None
Change-Id: I38d75a90df01070a817eb8847b7121d220193183
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902535
Commit-Queue: Anastasia Helfinstein <anastasi@google.com>
Reviewed-by: Akihiro Ota <akihiroota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715014}

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

@@ -21,14 +21,16 @@ class BackButtonNode extends SAChildNode {
     this.node_ = SwitchAccess.get().getBackButtonAutomationNode();
   }
 
+  // ================= Getters and setters =================
+
   /** @override */
-  equals(other) {
-    return other instanceof BackButtonNode;
+  get actions() {
+    return [SAConstants.MenuAction.SELECT];
   }
 
   /** @override */
-  get role() {
-    return chrome.automation.RoleType.BUTTON;
+  get automationNode() {
+    return this.node_;
   }
 
   /** @override */
@@ -39,25 +41,20 @@ class BackButtonNode extends SAChildNode {
   }
 
   /** @override */
-  get automationNode() {
-    return this.node_;
+  get role() {
+    return chrome.automation.RoleType.BUTTON;
   }
 
+  // ================= General methods =================
+
   /** @override */
-  get actions() {
-    return [SAConstants.MenuAction.SELECT];
+  asRootNode() {
+    return null;
   }
 
   /** @override */
-  performAction(action) {
-    if (action !== SAConstants.MenuAction.SELECT) {
-      return false;
-    }
-
-    if (this.node_) {
-      this.node_.doDefault();
-    }
-    return true;
+  equals(other) {
+    return other instanceof BackButtonNode;
   }
 
   /** @override */
@@ -70,11 +67,6 @@ class BackButtonNode extends SAChildNode {
     return false;
   }
 
-  /** @override */
-  asRootNode() {
-    return null;
-  }
-
   /** @override */
   onFocus() {
     chrome.accessibilityPrivate.setSwitchAccessMenuState(
@@ -87,6 +79,20 @@ class BackButtonNode extends SAChildNode {
         false, RectHelper.ZERO_RECT, 0 /* num_actions */);
   }
 
+  /** @override */
+  performAction(action) {
+    if (action !== SAConstants.MenuAction.SELECT) {
+      return false;
+    }
+
+    if (this.node_) {
+      this.node_.doDefault();
+    }
+    return true;
+  }
+
+  // ================= Debug methods =================
+
   /** @override */
   debugString() {
     return 'BackButtonNode';

chrome/browser/resources/chromeos/switch_access/nodes/group_node.js

@@ -21,27 +21,16 @@ class GroupNode extends SAChildNode {
     this.children_ = children;
   }
 
-  /** @override */
-  equals(other) {
-    if (!(other instanceof GroupNode)) {
-      return false;
-    }
+  // ================= Getters and setters =================
 
-    other = /** @type {GroupNode} */ (other);
-    if (other.children_.length !== this.children_.length) {
-      return false;
-    }
-    for (let i = 0; i < this.children_.length; i++) {
-      if (other.children_[i].equals(this.children_[i])) {
-        return false;
-      }
-    }
-    return true;
+  /** @override */
+  get actions() {
+    return [];
   }
 
   /** @override */
-  get role() {
-    return chrome.automation.RoleType.GROUP;
+  get automationNode() {
+    return null;
   }
 
   /** @override */
@@ -51,17 +40,42 @@ class GroupNode extends SAChildNode {
   }
 
   /** @override */
-  get automationNode() {
-    return null;
+  get role() {
+    return chrome.automation.RoleType.GROUP;
   }
 
+  // ================= General methods =================
+
   /** @override */
-  get actions() {
-    return [];
+  asRootNode() {
+    const root = new SARootNode();
+
+    let children = [];
+    for (const child of this.children_) {
+      children.push(child);
+    }
+
+    children.push(new BackButtonNode(root));
+    root.children = children;
+
+    return root;
   }
 
   /** @override */
-  performAction(action) {
+  equals(other) {
+    if (!(other instanceof GroupNode)) {
+      return false;
+    }
+
+    other = /** @type {GroupNode} */ (other);
+    if (other.children_.length !== this.children_.length) {
+      return false;
+    }
+    for (let i = 0; i < this.children_.length; i++) {
+      if (other.children_[i].equals(this.children_[i])) {
+        return false;
+      }
+    }
     return true;
   }
 
@@ -76,20 +90,12 @@ class GroupNode extends SAChildNode {
   }
 
   /** @override */
-  asRootNode() {
-    const root = new SARootNode();
-
-    let children = [];
-    for (const child of this.children_) {
-      children.push(child);
-    }
-
-    children.push(new BackButtonNode(root));
-    root.children = children;
-
-    return root;
+  performAction(action) {
+    return true;
   }
 
+  // ================= Static methods =================
+
   /**
    * Assumes nodes are visually in rows.
    * @param {!Array<!SAChildNode>} nodes

chrome/browser/resources/chromeos/switch_access/nodes/keyboard_node.js

@@ -15,6 +15,8 @@ class KeyboardNode extends NodeWrapper {
     super(node, parent);
   }
 
+  // ================= Getters and setters =================
+
   /** @override */
   get actions() {
     if (this.isGroup()) {
@@ -23,6 +25,24 @@ class KeyboardNode extends NodeWrapper {
     return [SAConstants.MenuAction.SELECT];
   }
 
+  // ================= General methods =================
+
+  /** @override */
+  asRootNode() {
+    if (!this.isGroup()) {
+      return null;
+    }
+
+    const node = this.automationNode;
+    if (!node) {
+      throw new TypeError('Keyboard nodes must have an automation node.');
+    }
+
+    const root = new RootNodeWrapper(node);
+    KeyboardNode.findAndSetChildren(root);
+    return root;
+  }
+
   /** @override */
   performAction(action) {
     if (this.isGroup()) {
@@ -45,21 +65,7 @@ class KeyboardNode extends NodeWrapper {
     return true;
   }
 
-  /** @override */
-  asRootNode() {
-    if (!this.isGroup()) {
-      return null;
-    }
-
-    const node = this.automationNode;
-    if (!node) {
-      throw new TypeError('Keyboard nodes must have an automation node.');
-    }
-
-    const root = new RootNodeWrapper(node);
-    KeyboardNode.findAndSetChildren(root);
-    return root;
-  }
+  // ================= Static methods =================
 
   /**
    * Helper function to connect tree elements, given the root node.
@@ -93,11 +99,15 @@ class KeyboardRootNode extends RootNodeWrapper {
     super(keyboard);
   }
 
+  // ================= General methods =================
+
   /** @override */
   onExit() {
     chrome.accessibilityPrivate.setVirtualKeyboardVisible(false);
   }
 
+  // ================= Private methods =================
+
   /**
    * Custom logic when entering the node.
    */
@@ -105,6 +115,8 @@ class KeyboardRootNode extends RootNodeWrapper {
     chrome.accessibilityPrivate.setVirtualKeyboardVisible(true);
   }
 
+  // ================= Static methods =================
+
   /**
    * Creates the tree structure for the system menu.
    * @param {!chrome.automation.AutomationNode} desktop

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

@@ -22,30 +22,7 @@ class NodeWrapper extends SAChildNode {
     this.isGroup_ = SwitchAccessPredicate.isGroup(this.baseNode_, parent);
   }
 
-  /** @override */
-  equals(other) {
-    if (!other || !(other instanceof NodeWrapper)) {
-      return false;
-    }
-
-    other = /** @type {!NodeWrapper} */ (other);
-    return other.baseNode_ === this.baseNode_;
-  }
-
-  /** @override */
-  get role() {
-    return this.baseNode_.role;
-  }
-
-  /** @override */
-  get location() {
-    return this.baseNode_.location;
-  }
-
-  /** @override */
-  get automationNode() {
-    return this.baseNode_;
-  }
+  // ================= Getters and setters =================
 
   /** @override */
   get actions() {
@@ -79,6 +56,51 @@ class NodeWrapper extends SAChildNode {
     return actions.concat(standardActions);
   }
 
+  /** @override */
+  get automationNode() {
+    return this.baseNode_;
+  }
+
+  /** @override */
+  get location() {
+    return this.baseNode_.location;
+  }
+
+  /** @override */
+  get role() {
+    return this.baseNode_.role;
+  }
+
+  // ================= General methods =================
+
+  /** @override */
+  asRootNode() {
+    if (!this.isGroup()) {
+      return null;
+    }
+    return RootNodeWrapper.buildTree(this.baseNode_);
+  }
+
+  /** @override */
+  equals(other) {
+    if (!other || !(other instanceof NodeWrapper)) {
+      return false;
+    }
+
+    other = /** @type {!NodeWrapper} */ (other);
+    return other.baseNode_ === this.baseNode_;
+  }
+
+  /** @override */
+  isEquivalentTo(node) {
+    return this.baseNode_ === node;
+  }
+
+  /** @override */
+  isGroup() {
+    return this.isGroup_;
+  }
+
   /** @override */
   performAction(action) {
     let ancestor;
@@ -125,6 +147,8 @@ class NodeWrapper extends SAChildNode {
     }
   }
 
+  // ================= Private methods =================
+
   /**
    * @return {AutomationNode}
    * @protected
@@ -135,24 +159,6 @@ class NodeWrapper extends SAChildNode {
       ancestor = ancestor.parent;
     return ancestor;
   }
-
-  /** @override */
-  isEquivalentTo(node) {
-    return this.baseNode_ === node;
-  }
-
-  /** @override */
-  isGroup() {
-    return this.isGroup_;
-  }
-
-  /** @override */
-  asRootNode() {
-    if (!this.isGroup()) {
-      return null;
-    }
-    return RootNodeWrapper.buildTree(this.baseNode_);
-  }
 }
 
 /**
@@ -170,6 +176,20 @@ class RootNodeWrapper extends SARootNode {
     this.baseNode_ = baseNode;
   }
 
+  // ================= Getters and setters =================
+
+  /** @override */
+  get automationNode() {
+    return this.baseNode_;
+  }
+
+  /** @override */
+  get location() {
+    return this.baseNode_.location || super.location;
+  }
+
+  // ================= General methods =================
+
   /** @override */
   equals(other) {
     if (!(other instanceof RootNodeWrapper)) {
@@ -181,8 +201,8 @@ class RootNodeWrapper extends SARootNode {
   }
 
   /** @override */
-  get location() {
-    return this.baseNode_.location || super.location;
+  isEquivalentTo(automationNode) {
+    return this.baseNode_ === automationNode;
   }
 
   /** @override */
@@ -190,14 +210,27 @@ class RootNodeWrapper extends SARootNode {
     return !!this.baseNode_.role;
   }
 
-  /** @override */
-  isEquivalentTo(automationNode) {
-    return this.baseNode_ === automationNode;
-  }
+  // ================= Static methods =================
 
-  /** @override */
-  get automationNode() {
-    return this.baseNode_;
+  /**
+   * @param {!AutomationNode} desktop
+   * @return {!RootNodeWrapper}
+   */
+  static buildDesktopTree(desktop) {
+    const root = new RootNodeWrapper(desktop);
+    const interestingChildren = RootNodeWrapper.getInterestingChildren(root);
+
+    if (interestingChildren.length < 1) {
+      throw SwitchAccess.error(
+          SAConstants.ErrorType.MALFORMED_DESKTOP,
+          'Desktop node must have at least 1 interesting child.');
+    }
+
+    const childConstructor = (autoNode) => new NodeWrapper(autoNode, root);
+    let children = interestingChildren.map(childConstructor);
+    root.children = children;
+
+    return root;
   }
 
   /**
@@ -232,27 +265,6 @@ class RootNodeWrapper extends SARootNode {
     root.children = children;
   }
 
-  /**
-   * @param {!AutomationNode} desktop
-   * @return {!RootNodeWrapper}
-   */
-  static buildDesktopTree(desktop) {
-    const root = new RootNodeWrapper(desktop);
-    const interestingChildren = RootNodeWrapper.getInterestingChildren(root);
-
-    if (interestingChildren.length < 1) {
-      throw SwitchAccess.error(
-          SAConstants.ErrorType.MALFORMED_DESKTOP,
-          'Desktop node must have at least 1 interesting child.');
-    }
-
-    const childConstructor = (autoNode) => new NodeWrapper(autoNode, root);
-    let children = interestingChildren.map(childConstructor);
-    root.children = children;
-
-    return root;
-  }
-
   /**
    * @param {!RootNodeWrapper} root
    * @return {!Array<!AutomationNode>}

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

@@ -21,34 +21,14 @@ class SAChildNode {
     this.next_ = null;
   }
 
-  /** @param {!SAChildNode} newVal */
-  set previous(newVal) {
-    this.previous_ = newVal;
-  }
-
-  /** @param {!SAChildNode} newVal */
-  set next(newVal) {
-    this.next_ = newVal;
-  }
+  // ================= Getters and setters =================
 
   /**
-   * @param {SAChildNode} other
-   * @return {boolean}
-   * @abstract
-   */
-  equals(other) {}
-
-  /**
-   * @return {chrome.automation.RoleType|undefined}
-   * @abstract
-   */
-  get role() {}
-
-  /**
-   * @return {chrome.accessibilityPrivate.ScreenRect|undefined}
+   * Returns a list of all the actions available for this node.
+   * @return {!Array<SAConstants.MenuAction>}
    * @abstract
    */
-  get location() {}
+  get actions() {}
 
   /**
    * Returns the underlying automation node, if one exists.
@@ -58,44 +38,16 @@ class SAChildNode {
   get automationNode() {}
 
   /**
-   * Returns whether this node should be displayed as a group.
-   * @return {boolean}
-   * @abstract
-   */
-  isGroup() {}
-
-  /**
-   * Returns a list of all the actions available for this node.
-   * @return {!Array<SAConstants.MenuAction>}
+   * @return {chrome.accessibilityPrivate.ScreenRect|undefined}
    * @abstract
    */
-  get actions() {}
+  get location() {}
 
-  /**
-   * Given a menu action, returns whether it can be performed on this node.
-   * @param {SAConstants.MenuAction} action
-   * @return {boolean}
-   */
-  hasAction(action) {
-    return this.actions.includes(action);
+  /** @param {!SAChildNode} newVal */
+  set next(newVal) {
+    this.next_ = newVal;
   }
 
-  /**
-   * Performs the specified action on the node, if it is available.
-   * @param {SAConstants.MenuAction} action
-   * @return {boolean} Whether to close the menu. True if the menu should close,
-   *     false otherwise.
-   * @abstract
-   */
-  performAction(action) {}
-
-  /**
-   * @param {!chrome.automation.AutomationNode} node
-   * @return {boolean}
-   * @abstract
-   */
-  isEquivalentTo(node) {}
-
   /**
    * Returns the next node in pre-order traversal.
    * @return {!SAChildNode}
@@ -109,6 +61,11 @@ class SAChildNode {
     return this.next_;
   }
 
+  /** @param {!SAChildNode} newVal */
+  set previous(newVal) {
+    this.previous_ = newVal;
+  }
+
   /**
    * Returns the previous node in pre-order traversal.
    * @return {!SAChildNode}
@@ -122,6 +79,14 @@ class SAChildNode {
     return this.previous_;
   }
 
+  /**
+   * @return {chrome.automation.RoleType|undefined}
+   * @abstract
+   */
+  get role() {}
+
+  // ================= General methods =================
+
   /**
    * If this node is a group, returns the analogous SARootNode.
    * @return {SARootNode}
@@ -129,6 +94,36 @@ class SAChildNode {
    */
   asRootNode() {}
 
+  /**
+   * @param {SAChildNode} other
+   * @return {boolean}
+   * @abstract
+   */
+  equals(other) {}
+
+  /**
+   * Given a menu action, returns whether it can be performed on this node.
+   * @param {SAConstants.MenuAction} action
+   * @return {boolean}
+   */
+  hasAction(action) {
+    return this.actions.includes(action);
+  }
+
+  /**
+   * @param {!chrome.automation.AutomationNode} node
+   * @return {boolean}
+   * @abstract
+   */
+  isEquivalentTo(node) {}
+
+  /**
+   * Returns whether this node should be displayed as a group.
+   * @return {boolean}
+   * @abstract
+   */
+  isGroup() {}
+
   /**
    * Called when this node becomes the primary highlighted node.
    */
@@ -139,6 +134,17 @@ class SAChildNode {
    */
   onUnfocus() {}
 
+  /**
+   * Performs the specified action on the node, if it is available.
+   * @param {SAConstants.MenuAction} action
+   * @return {boolean} Whether to close the menu. True if the menu should close,
+   *     false otherwise.
+   * @abstract
+   */
+  performAction(action) {}
+
+  // ================= Debug methods =================
+
   /**
    * String-ifies the node (for debugging purposes).
    * @param {boolean} wholeTree Whether to recursively include descendants.
@@ -181,36 +187,17 @@ class SARootNode {
     this.children_ = [];
   }
 
+  // ================= Getters and setters =================
+
+  /** @return {chrome.automation.AutomationNode} */
+  get automationNode() {}
+
   /** @param {!Array<!SAChildNode>} newVal */
   set children(newVal) {
     this.children_ = newVal;
     this.connectChildren_();
   }
 
-  /**
-   * @param {SARootNode} other
-   * @return {boolean}
-   */
-  equals(other) {
-    if (!other) {
-      return false;
-    }
-    if (this.children_.length !== other.children_.length) {
-      return false;
-    }
-
-    let result = true;
-    for (let i = 0; i < this.children_.length; i++) {
-      if (!this.children_[i]) {
-        throw SwitchAccess.error(
-            SAConstants.ErrorType.NULL_CHILD, 'Child cannot be null.');
-      }
-      result = result && this.children_[i].equals(other.children_[i]);
-    }
-
-    return result;
-  }
-
   /** @return {!Array<!SAChildNode>} */
   get children() {
     return this.children_;
@@ -238,11 +225,6 @@ class SARootNode {
     }
   }
 
-  /** @return {boolean} */
-  isValid() {
-    return true;
-  }
-
   /** @return {!chrome.accessibilityPrivate.ScreenRect} */
   get location() {
     let children = this.children_.filter((c) => !(c instanceof BackButtonNode));
@@ -250,6 +232,32 @@ class SARootNode {
     return RectHelper.unionAll(childLocations);
   }
 
+  // ================= General methods =================
+
+  /**
+   * @param {SARootNode} other
+   * @return {boolean}
+   */
+  equals(other) {
+    if (!other) {
+      return false;
+    }
+    if (this.children_.length !== other.children_.length) {
+      return false;
+    }
+
+    let result = true;
+    for (let i = 0; i < this.children_.length; i++) {
+      if (!this.children_[i]) {
+        throw SwitchAccess.error(
+            SAConstants.ErrorType.NULL_CHILD, 'Child cannot be null.');
+      }
+      result = result && this.children_[i].equals(other.children_[i]);
+    }
+
+    return result;
+  }
+
   /**
    * @param {chrome.automation.AutomationNode} automationNode
    * @return {boolean}
@@ -258,12 +266,16 @@ class SARootNode {
     return false;
   }
 
-  /** @return {chrome.automation.AutomationNode} */
-  get automationNode() {}
+  /** @return {boolean} */
+  isValid() {
+    return true;
+  }
 
   /** Called when a group is exiting. */
   onExit() {}
 
+  // ================= Debug methods =================
+
   /**
    * String-ifies the node (for debugging purposes).
    * @param {boolean=} wholeTree Whether to recursively descend the tree
@@ -295,6 +307,8 @@ class SARootNode {
     return str;
   }
 
+  // ================= Private methods =================
+
   /**
    * Helper function to connect children.
    * @private