Bug 1599806 - Move accessible walker creation into accessibility front's bootsrap method. Do not explicitly destroy accessibility walker front and let accessibility actor manage its lifecycle fully. r=ochameau,jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D58027

--HG--
extra : moz-landing-system : lando

devtools/client/accessibility/accessibility-startup.js

@@ -30,22 +30,19 @@ class AccessibilityStartup {
   }
 
   get walker() {
-    return this._walker;
+    return this._accessibility.accessibleWalkerFront;
   }
 
   /**
-   * Determine which features are supported based on the version of the server. Also, sync
-   * the state of the accessibility front/actor.
+   * Determine which features are supported based on the version of the server.
    * @return {Promise}
-   *         A promise that returns true when accessibility front is fully in sync with
-   *         the actor.
+   *         A promise that returns true when accessibility front is ready.
    */
   async prepareAccessibility() {
-    // We must call a method on an accessibility front here (such as getWalker), in
-    // oreder to be able to check actor's backward compatibility via actorHasMethod.
-    // See targe.js@getActorDescription for more information.
     try {
-      this._walker = await this._accessibility.getWalker();
+      // Finalize accessibility front initialization. See accessibility front
+      // bootstrap method description.
+      await this._accessibility.bootstrap();
       this._supports = {};
       // To add a check for backward compatibility add something similar to the
       // example below:
@@ -54,9 +51,6 @@ class AccessibilityStartup {
       //   // Please specify the version of Firefox when the feature was added.
       //   this.target.actorHasMethod("accessibility", "getSimulator"),
       // ]);
-
-      await this._accessibility.bootstrap();
-
       return true;
     } catch (e) {
       // toolbox may be destroyed during this step.
@@ -124,9 +118,7 @@ class AccessibilityStartup {
       this._accessibility.off("init", this._updateToolHighlight);
       this._accessibility.off("shutdown", this._updateToolHighlight);
 
-      await this._walker.destroy();
       this._accessibility = null;
-      this._walker = null;
     }.bind(this)();
     return this._destroyingAccessibility;
   }

devtools/server/tests/browser/browser_accessibility_highlighter_infobar.js

@@ -16,7 +16,7 @@ add_task(async function() {
     MAIN_DOMAIN + "doc_accessibility_infobar.html"
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
 
   info("Button front checks");

devtools/server/tests/browser/browser_accessibility_keyboard_audit.js

@@ -30,7 +30,7 @@ add_task(async function() {
     `${MAIN_DOMAIN}doc_accessibility_keyboard_audit.html`
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
 
   const tests = [

devtools/server/tests/browser/browser_accessibility_node.js

@@ -13,7 +13,7 @@ add_task(async function() {
   const modifiers =
     Services.appinfo.OS === "Darwin" ? "\u2303\u2325" : "Alt+Shift+";
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
   const buttonNode = await walker.querySelector(walker.rootNode, "#button");
   const accessibleFront = await a11yWalker.getAccessibleFor(buttonNode);

devtools/server/tests/browser/browser_accessibility_node_audit.js

@@ -61,7 +61,7 @@ add_task(async function() {
     MAIN_DOMAIN + "doc_accessibility_infobar.html"
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
 
   const headerNode = await walker.querySelector(walker.rootNode, "#h1");

devtools/server/tests/browser/browser_accessibility_node_events.js

@@ -13,7 +13,7 @@ add_task(async function() {
   const modifiers =
     Services.appinfo.OS === "Darwin" ? "\u2303\u2325" : "Alt+Shift+";
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
   const rootNode = await walker.getRootNode();
   const a11yDoc = await a11yWalker.getAccessibleFor(rootNode);

devtools/server/tests/browser/browser_accessibility_simple.js

@@ -35,8 +35,14 @@ add_task(async function() {
   ok(accessibility.getWalker, "The getWalker method exists");
   ok(accessibility.getSimulator, "The getSimulator method exists");
 
-  let a11yWalker = await accessibility.getWalker();
-  ok(a11yWalker, "The AccessibleWalkerFront was returned");
+  ok(accessibility.accessibleWalkerFront, "Accessible walker was initialized");
+
+  let a11yWalker = accessibility.accessibleWalkerFront;
+  is(
+    a11yWalker,
+    accessibility.accessibleWalkerFront,
+    "The AccessibleWalkerFront was returned"
+  );
 
   const a11ySimulator = await accessibility.getSimulator();
   const webRenderEnabled = isWebRenderEnabled(window);
@@ -83,7 +89,7 @@ add_task(async function() {
     canBeEnabled: true,
   });
 
-  a11yWalker = await accessibility.getWalker();
+  a11yWalker = accessibility.accessibleWalkerFront;
   const rootNode = await domWalker.getRootNode();
   const a11yDoc = await a11yWalker.getAccessibleFor(rootNode);
   ok(a11yDoc, "Accessible document actor is created");

devtools/server/tests/browser/browser_accessibility_text_label_audit.js

@@ -40,7 +40,7 @@ add_task(async function() {
     `${MAIN_DOMAIN}doc_accessibility_text_label_audit.html`
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
 
   const tests = [

devtools/server/tests/browser/browser_accessibility_text_label_audit_frame.js

@@ -24,7 +24,7 @@ add_task(async function() {
     `${MAIN_DOMAIN}doc_accessibility_text_label_audit_frame.html`
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   await accessibility.enable();
 
   const tests = [

devtools/server/tests/browser/browser_accessibility_walker.js

@@ -11,7 +11,7 @@ add_task(async function() {
     MAIN_DOMAIN + "doc_accessibility.html"
   );
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   ok(a11yWalker, "The AccessibleWalkerFront was returned");
 
   await accessibility.enable();

devtools/server/tests/browser/browser_accessibility_walker_audit.js

@@ -145,7 +145,7 @@ add_task(async function() {
     }
   }
 
-  const a11yWalker = await accessibility.getWalker();
+  const a11yWalker = accessibility.accessibleWalkerFront;
   ok(a11yWalker, "The AccessibleWalkerFront was returned");
   await accessibility.enable();
 

devtools/shared/fronts/accessibility.js

@@ -187,12 +187,23 @@ class AccessibilityFront extends FrontClassWithSpec(accessibilitySpec) {
     this.formAttributeName = "accessibilityActor";
   }
 
-  bootstrap() {
-    return super.bootstrap().then(state => {
-      this.enabled = state.enabled;
-      this.canBeEnabled = state.canBeEnabled;
-      this.canBeDisabled = state.canBeDisabled;
-    });
+  // We purposefully do not use initialize here and separate accessiblity
+  // front/actor initialization into two parts: getting the front from target
+  // and then separately bootstrapping the front. The reason for that is because
+  // accessibility front is always created as part of the accessibility panel
+  // startup when the toolbox is opened. If initialize was used, in rare cases,
+  // when the toolbox is destroyed before the accessibility tool startup is
+  // complete, the toolbox destruction would hang because the accessibility
+  // front will indefinitely wait for its initialize method to complete before
+  // being destroyed. With custom bootstrapping the front will be destroyed
+  // correctly.
+  async bootstrap() {
+    this.accessibleWalkerFront = await super.getWalker();
+    ({
+      enabled: this.enabled,
+      canBeEnabled: this.canBeEnabled,
+      canBeDisabled: this.canBeDisabled,
+    } = await super.bootstrap());
   }
 
   init() {