Bug 1640338 - simplify accessibility proxy startup since we no longer need to initialize it until the panel is opened. r=mtigley

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

devtools/client/accessibility/accessibility-proxy.js

@@ -5,6 +5,7 @@
 "use strict";
 
 const Services = require("Services");
+const EventEmitter = require("devtools/shared/event-emitter");
 
 loader.lazyRequireGetter(
   this,
@@ -29,6 +30,7 @@ class AccessibilityProxy {
     this._accessibilityWalkerFronts = new Set();
     this.lifecycleEvents = new Map();
     this.accessibilityEvents = new Map();
+    this._updateTargetListeners = new EventEmitter();
     this.supports = {};
 
     this.audit = this.audit.bind(this);
@@ -45,6 +47,9 @@ class AccessibilityProxy {
     this.startListeningForParentLifecycleEvents = this.startListeningForParentLifecycleEvents.bind(
       this
     );
+    this.startListeningForTargetUpdated = this.startListeningForTargetUpdated.bind(
+      this
+    );
     this.stopListeningForAccessibilityEvents = this.stopListeningForAccessibilityEvents.bind(
       this
     );
@@ -54,6 +59,9 @@ class AccessibilityProxy {
     this.stopListeningForParentLifecycleEvents = this.stopListeningForParentLifecycleEvents.bind(
       this
     );
+    this.stopListeningForTargetUpdated = this.stopListeningForTargetUpdated.bind(
+      this
+    );
     this.highlightAccessible = this.highlightAccessible.bind(this);
     this.unhighlightAccessible = this.unhighlightAccessible.bind(this);
     this.onTargetAvailable = this.onTargetAvailable.bind(this);
@@ -134,6 +142,14 @@ class AccessibilityProxy {
     return combinedAudit;
   }
 
+  startListeningForTargetUpdated(onTargetUpdated) {
+    this._updateTargetListeners.on("target-updated", onTargetUpdated);
+  }
+
+  stopListeningForTargetUpdated(onTargetUpdated) {
+    this._updateTargetListeners.off("target-updated", onTargetUpdated);
+  }
+
   async disableAccessibility() {
     // Accessibility service is shut down using the parent accessibility front.
     // That, in turn, shuts down accessibility service in all content processes.
@@ -280,8 +296,7 @@ class AccessibilityProxy {
 
   async resetAccessiblity() {
     const { enabled } = this.accessibilityFront;
-    const { canBeEnabled, canBeDisabled } =
-      this.parentAccessibilityFront || this.accessibilityFront;
+    const { canBeEnabled, canBeDisabled } = this.parentAccessibilityFront;
     return { enabled, canBeDisabled, canBeEnabled };
   }
 
@@ -387,58 +402,22 @@ class AccessibilityProxy {
     });
   }
 
-  /**
-   * Part of the proxy initialization only needs to be done when the accessibility panel starts.
-   * To avoid performance issues, the panel will explicitly call this method every time a new
-   * target becomes available.
-   */
-  async initializeProxyForPanel(targetFront) {
-    await this.onTargetAvailable({ targetFront });
-
-    // No need to retrieve parent accessibility front since root front does not
-    // change.
-    if (!this.parentAccessibilityFront) {
-      this.parentAccessibilityFront = await this._currentTarget.client.mainRoot.getFront(
-        "parentaccessibility"
-      );
-    }
-
-    this.simulatorFront = this.accessibilityFront.simulatorFront;
-    if (this.simulatorFront) {
-      this.simulate = types => this.simulatorFront.simulate({ types });
-    } else {
-      this.simulate = null;
-    }
-
-    // Move accessibility front lifecycle event listeners to a new top level
-    // front.
-    for (const [type, listeners] of this.lifecycleEvents.entries()) {
-      for (const listener of listeners.values()) {
-        this.accessibilityFront.on(type, listener);
-      }
-    }
-  }
-
   async initialize() {
-    try {
-      await this.toolbox.targetList.watchTargets(
-        [this.toolbox.targetList.TYPES.FRAME],
-        this.onTargetAvailable,
-        this.onTargetDestroyed
-      );
-      // Bug 1602075: auto init feature definition is used for an experiment to
-      // determine if we can automatically enable accessibility panel when it
-      // opens.
-      this.supports.autoInit = Services.prefs.getBoolPref(
-        "devtools.accessibility.auto-init.enabled",
-        false
-      );
-
-      return true;
-    } catch (e) {
-      // toolbox may be destroyed during this step.
-      return false;
-    }
+    await this.toolbox.targetList.watchTargets(
+      [this.toolbox.targetList.TYPES.FRAME],
+      this.onTargetAvailable,
+      this.onTargetDestroyed
+    );
+    this.parentAccessibilityFront = await this._currentTarget.client.mainRoot.getFront(
+      "parentaccessibility"
+    );
+    // Bug 1602075: auto init feature definition is used for an experiment to
+    // determine if we can automatically enable accessibility panel when it
+    // opens.
+    this.supports.autoInit = Services.prefs.getBoolPref(
+      "devtools.accessibility.auto-init.enabled",
+      false
+    );
   }
 
   destroy() {
@@ -450,6 +429,7 @@ class AccessibilityProxy {
 
     this.lifecycleEvents.clear();
     this.accessibilityEvents.clear();
+    this._updateTargetListeners = null;
 
     this.accessibilityFront = null;
     this.parentAccessibilityFront = null;
@@ -532,7 +512,7 @@ class AccessibilityProxy {
     }
   }
 
-  async onTargetAvailable({ targetFront }) {
+  async onTargetAvailable({ targetFront, isTargetSwitching }) {
     targetFront.watchFronts(
       "accessibility",
       this.onAccessibilityFrontAvailable,
@@ -540,33 +520,41 @@ class AccessibilityProxy {
     );
 
     if (!targetFront.isTopLevel) {
-      return null;
-    }
-
-    if (this._updatePromise && this._currentTarget === targetFront) {
-      return this._updatePromise;
+      return;
     }
 
     this._currentTarget = targetFront;
     this._accessibilityWalkerFronts.clear();
 
-    this._updatePromise = (async () => {
-      this.accessibilityFront = await this._currentTarget.getFront(
-        "accessibility"
-      );
-      // Finalize accessibility front initialization. See accessibility front
-      // bootstrap method description.
-      await this.accessibilityFront.bootstrap();
-      // To add a check for backward compatibility add something similar to the
-      // example below:
-      //
-      // [this.supports.simulation] = await Promise.all([
-      //   // Please specify the version of Firefox when the feature was added.
-      //   this._currentTarget.actorHasMethod("accessibility", "getSimulator"),
-      // ]);
-    })();
+    this.accessibilityFront = await this._currentTarget.getFront(
+      "accessibility"
+    );
+    // To add a check for backward compatibility add something similar to the
+    // example below:
+    //
+    // [this.supports.simulation] = await Promise.all([
+    //   // Please specify the version of Firefox when the feature was added.
+    //   this._currentTarget.actorHasMethod("accessibility", "getSimulator"),
+    // ]);
+    this.simulatorFront = this.accessibilityFront.simulatorFront;
+    if (this.simulatorFront) {
+      this.simulate = types => this.simulatorFront.simulate({ types });
+    } else {
+      this.simulate = null;
+    }
 
-    return this._updatePromise;
+    // Move accessibility front lifecycle event listeners to a new top level
+    // front.
+    for (const [type, listeners] of this.lifecycleEvents.entries()) {
+      for (const listener of listeners.values()) {
+        this.accessibilityFront.on(type, listener);
+      }
+    }
+
+    this._updateTargetListeners.emit("target-updated", {
+      targetFront: this._currentTarget,
+      isTargetSwitching,
+    });
   }
 
   async onTargetDestroyed({ targetFront }) {

devtools/client/accessibility/panel.js

@@ -55,7 +55,7 @@ function AccessibilityPanel(iframeWindow, toolbox) {
   this._toolbox = toolbox;
 
   this.onTabNavigated = this.onTabNavigated.bind(this);
-  this.onTargetAvailable = this.onTargetAvailable.bind(this);
+  this.onTargetUpdated = this.onTargetUpdated.bind(this);
   this.onPanelVisibilityChange = this.onPanelVisibilityChange.bind(this);
   this.onNewAccessibleFrontSelected = this.onNewAccessibleFrontSelected.bind(
     this
@@ -106,12 +106,10 @@ AccessibilityPanel.prototype = {
     this.shouldRefresh = true;
 
     this.accessibilityProxy = new AccessibilityProxy(this._toolbox);
-    await this.accessibilityProxy.initialize();
-
-    await this._toolbox.targetList.watchTargets(
-      [this._toolbox.targetList.TYPES.FRAME],
-      this.onTargetAvailable
+    this.accessibilityProxy.startListeningForTargetUpdated(
+      this.onTargetUpdated
     );
+    await this.accessibilityProxy.initialize();
 
     // Bug 1602075: if auto init feature is enabled, enable accessibility
     // service if necessary.
@@ -181,10 +179,9 @@ AccessibilityPanel.prototype = {
     this._opening.then(() => this.refresh());
   },
 
-  async onTargetAvailable({ targetFront, isTargetSwitching }) {
+  onTargetUpdated({ targetFront, isTargetSwitching }) {
     if (targetFront.isTopLevel) {
-      await this.accessibilityProxy.initializeProxyForPanel(targetFront);
-      this.accessibilityProxy.currentTarget.on("navigate", this.onTabNavigated);
+      targetFront.on("navigate", this.onTabNavigated);
     }
 
     if (isTargetSwitching) {
@@ -329,14 +326,12 @@ AccessibilityPanel.prototype = {
     }
     this._destroyed = true;
 
-    this._toolbox.targetList.unwatchTargets(
-      [this._toolbox.targetList.TYPES.FRAME],
-      this.onTargetAvailable
-    );
-
     this.postContentMessage("destroy");
 
     if (this.accessibilityProxy) {
+      this.accessibilityProxy.stopListeningForTargetUpdated(
+        this.onTargetUpdated
+      );
       this.accessibilityProxy.currentTarget.off(
         "navigate",
         this.onTabNavigated

devtools/client/fronts/accessibility.js

@@ -192,7 +192,6 @@ class AccessibleFront extends FrontClassWithSpec(accessibleSpec) {
     const accessibilityFront = await documentNodeFront.targetFront.getFront(
       "accessibility"
     );
-    await accessibilityFront.bootstrap();
 
     return accessibilityFront.accessibleWalkerFront.children();
   }
@@ -330,7 +329,6 @@ class AccessibleWalkerFront extends FrontClassWithSpec(accessibleWalkerSpec) {
     );
     const frameNodeFront = (await domWalkerFront.getRootNode()).parentNode();
     const accessibilityFront = await parentTarget.getFront("accessibility");
-    await accessibilityFront.bootstrap();
     const { accessibleWalkerFront } = accessibilityFront;
     const frameAccessibleFront = await accessibleWalkerFront.getAccessibleFor(
       frameNodeFront
@@ -443,20 +441,11 @@ class AccessibilityFront extends FrontClassWithSpec(accessibilitySpec) {
     this.formAttributeName = "accessibilityActor";
   }
 
-  // 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() {
+  async initialize() {
     this.accessibleWalkerFront = await super.getWalker();
     this.simulatorFront = await super.getSimulator();
-    ({ enabled: this.enabled } = await super.bootstrap());
+    const { enabled } = await super.bootstrap();
+    this.enabled = enabled;
   }
 
   init() {

devtools/server/tests/browser/head.js

@@ -88,7 +88,6 @@ async function initAccessibilityFrontsForUrl(
     "parentaccessibility"
   );
   const accessibility = await target.getFront("accessibility");
-  await accessibility.bootstrap();
   const a11yWalker = accessibility.accessibleWalkerFront;
   if (enableByDefault) {
     await parentAccessibility.enable();