Bug 1520772 - Construct the WebExtension target front before instantiating the Target object. r=yulia
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 21 Jan 2019 11:04:31 +0000
changeset 514699 11325875967aeff353c93e6cfc38fc614ce211b1
parent 514698 909b904600e079bc0fe2867956f2d067f1f9ec48
child 514700 5bda17f8f936f89149ab5fbe76b63836cf053d06
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia
bugs1520772
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520772 - Construct the WebExtension target front before instantiating the Target object. r=yulia Differential Revision: https://phabricator.services.mozilla.com/D15829
devtools/client/aboutdebugging-new/src/modules/extensions-helper.js
devtools/client/framework/target.js
devtools/client/framework/toolbox-process-window.js
devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html
devtools/server/tests/mochitest/webextension-helpers.js
devtools/server/tests/unit/test_addon_reload.js
devtools/shared/fronts/targets/addon.js
--- a/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js
+++ b/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js
@@ -48,17 +48,19 @@ exports.debugLocalAddon = async function
  * Start debugging an addon in a remote instance of Firefox.
  *
  * @param {String} id
  *        The addon id to debug.
  * @param {DebuggerClient} client
  *        Required for remote debugging.
  */
 exports.debugRemoteAddon = async function(id, client) {
-  const addonTargetFront = await client.mainRoot.getAddon({ id });
+  const addonFront = await client.mainRoot.getAddon({ id });
+
+  const addonTargetFront = await addonFront.connect();
 
   // Close previous addon debugging toolbox.
   closeToolbox();
 
   const options = {
     activeTab: addonTargetFront,
     chrome: true,
     client,
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -525,19 +525,16 @@ class Target extends EventEmitter {
   /**
    * Attach the target and its console actor.
    *
    * This method will mainly call `attach` request on the target actor as well
    * as the console actor.
    * See DebuggerClient.attachTarget and DebuggerClient.attachConsole for more info.
    * It also starts listenings to events the target actor will start emitting
    * after being attached, like `tabDetached` and `frameUpdate`
-   *
-   * For webextension, it also preliminary converts addonTargetActor to a
-   * WebExtensionTargetActor.
    */
   attach() {
     if (this._attach) {
       return this._attach;
     }
 
     // Attach the target actor
     const attachBrowsingContextTarget = async () => {
@@ -566,28 +563,16 @@ class Target extends EventEmitter {
         this.form.consoleActor, []);
       this.activeConsole = consoleClient;
 
       this._onInspectObject = packet => this.emit("inspect-object", packet);
       this.activeConsole.on("inspectObject", this._onInspectObject);
     };
 
     this._attach = (async () => {
-      if (this.form.isWebExtension &&
-          this.client.mainRoot.traits.webExtensionAddonConnect) {
-        // The addonTargetActor form is related to a WebExtensionActor instance,
-        // which isn't a target actor on its own, it is an actor living in the parent
-        // process with access to the addon metadata, it can control the addon (e.g.
-        // reloading it) and listen to the AddonManager events related to the lifecycle of
-        // the addon (e.g. when the addon is disabled or uninstalled).
-        // To retrieve the target actor instance, we call its "connect" method, (which
-        // fetches the target actor form from a WebExtensionTargetActor instance).
-        this.activeTab = await this.activeTab.connect();
-      }
-
       // AddonTargetActor and ContentProcessTargetActor don't inherit from
       // BrowsingContextTargetActor (i.e. this.isBrowsingContext=false) and don't need
       // to be attached via DebuggerClient.attachTarget.
       if (this.isBrowsingContext) {
         await attachBrowsingContextTarget();
 
       // Addon Worker and Content process targets are the first targets to have their
       // front already instantiated. The plan is to have all targets to have their front
--- a/devtools/client/framework/toolbox-process-window.js
+++ b/devtools/client/framework/toolbox-process-window.js
@@ -84,17 +84,18 @@ var connect = async function() {
     webSocket,
   });
   gClient = new DebuggerClient(transport);
   appendStatusMessage("Start protocol client for connection");
   await gClient.connect();
 
   appendStatusMessage("Get root form for toolbox");
   if (addonID) {
-    const addonTargetFront = await gClient.mainRoot.getAddon({ id: addonID });
+    const addonFront = await gClient.mainRoot.getAddon({ id: addonID });
+    const addonTargetFront = await addonFront.connect();
     await openToolbox({activeTab: addonTargetFront, chrome: true});
   } else {
     const front = await gClient.mainRoot.getMainProcess();
     await openToolbox({activeTab: front, chrome: true});
   }
 };
 
 // Certain options should be toggled since we can assume chrome debugging here
--- a/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html
+++ b/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html
@@ -32,18 +32,21 @@ async function test_connect_addon(oopMod
   await extension.awaitMessage("background page ready");
 
   // Connect a DebuggerClient.
   const transport = DebuggerServer.connectPipe();
   const client = new DebuggerClient(transport);
   await client.connect();
 
   // List addons and assertions on the expected addon actor.
-  const addonTargetFront = await client.mainRoot.getAddon({ id: extension.id });
-  ok(addonTargetFront, "The expected webextension addon actor has been found");
+  const addonFront = await client.mainRoot.getAddon({ id: extension.id });
+  ok(addonFront, "The expected webextension addon actor has been found");
+
+  const addonTargetFront = await addonFront.connect();
+  ok(addonTargetFront, "The expected webextension target addon actor has been found");
 
   // Connect to the target addon actor and wait for the updated list of frames.
   const addonTarget = await TargetFactory.forRemoteTab({
     activeTab: addonTargetFront,
     client,
     chrome: true,
   });
   is(addonTarget.targetForm.isOOP, oopMode,
--- a/devtools/server/tests/mochitest/webextension-helpers.js
+++ b/devtools/server/tests/mochitest/webextension-helpers.js
@@ -93,17 +93,18 @@ function collectFrameUpdates({client}, m
 }
 
 async function attachAddon(addonId) {
   const transport = DebuggerServer.connectPipe();
   const client = new DebuggerClient(transport);
 
   await client.connect();
 
-  const addonTargetFront = await client.mainRoot.getAddon({ id: addonId });
+  const addonFront = await client.mainRoot.getAddon({ id: addonId });
+  const addonTargetFront = await addonFront.connect();
 
   if (!addonTargetFront) {
     client.close();
     throw new Error(`No WebExtension Actor found for ${addonId}`);
   }
 
   const addonTarget = await TargetFactory.forRemoteTab({
     activeTab: addonTargetFront,
--- a/devtools/server/tests/unit/test_addon_reload.js
+++ b/devtools/server/tests/unit/test_addon_reload.js
@@ -29,20 +29,20 @@ function promiseWebExtensionStartup() {
       Management.off("ready", listener);
       resolve(extension);
     };
 
     Management.on("ready", listener);
   });
 }
 
-async function reloadAddon(addonTargetFront) {
+async function reloadAddon(addonFront) {
   // The add-on will be re-installed after a successful reload.
   const onInstalled = promiseAddonEvent("onInstalled");
-  await addonTargetFront.reload();
+  await addonFront.reload();
   await onInstalled;
 }
 
 function getSupportFile(path) {
   const allowMissing = false;
   return do_get_file(path, allowMissing);
 }
 
@@ -62,51 +62,51 @@ add_task(async function testReloadExited
 
   // Install a decoy add-on.
   const addonFile2 = getSupportFile("addons/web-extension2");
   const [installedAddon2] = await Promise.all([
     AddonManager.installTemporaryAddon(addonFile2),
     promiseWebExtensionStartup(),
   ]);
 
-  const addonTargetFront = await client.mainRoot.getAddon({ id: installedAddon.id });
+  const addonFront = await client.mainRoot.getAddon({ id: installedAddon.id });
 
   await Promise.all([
-    reloadAddon(addonTargetFront),
+    reloadAddon(addonFront),
     promiseWebExtensionStartup(),
   ]);
 
   // Uninstall the decoy add-on, which should cause its actor to exit.
   const onUninstalled = promiseAddonEvent("onUninstalled");
   installedAddon2.uninstall();
   await onUninstalled;
 
   // Try to re-list all add-ons after a reload.
   // This was throwing an exception because of the exited actor.
   const newAddonFront = await client.mainRoot.getAddon({ id: installedAddon.id });
-  equal(newAddonFront.id, addonTargetFront.id);
+  equal(newAddonFront.id, addonFront.id);
 
   // The fronts should be the same after the reload
-  equal(newAddonFront, addonTargetFront);
+  equal(newAddonFront, addonFront);
 
   const onAddonListChanged = client.mainRoot.once("addonListChanged");
 
   // Install an upgrade version of the first add-on.
   const addonUpgradeFile = getSupportFile("addons/web-extension-upgrade");
   const [upgradedAddon] = await Promise.all([
     AddonManager.installTemporaryAddon(addonUpgradeFile),
     promiseWebExtensionStartup(),
   ]);
 
   // Waiting for addonListChanged unsolicited event
   await onAddonListChanged;
 
   // re-list all add-ons after an upgrade.
   const upgradedAddonFront = await client.mainRoot.getAddon({ id: upgradedAddon.id });
-  equal(upgradedAddonFront.id, addonTargetFront.id);
+  equal(upgradedAddonFront.id, addonFront.id);
   // The fronts should be the same after the upgrade.
-  equal(upgradedAddonFront, addonTargetFront);
+  equal(upgradedAddonFront, addonFront);
 
   // The addon metadata has been updated.
   equal(upgradedAddonFront.name, "Test Addons Actor Upgrade");
 
   await close(client);
 });
--- a/devtools/shared/fronts/targets/addon.js
+++ b/devtools/shared/fronts/targets/addon.js
@@ -51,20 +51,31 @@ class AddonTargetFront extends FrontClas
    *
    * AddonTargetActor is used for WebExtensions, but this isn't the final target actor
    * we want to use for it. AddonTargetActor only expose metadata about the Add-on, like
    * its name, type, ... Instead, we want to use a WebExtensionTargetActor, which
    * inherits from BrowsingContextTargetActor. This connect method is used to retrive
    * the final target actor to use.
    */
   async connect() {
-    const { form } = await super.connect();
-    const front = new BrowsingContextTargetFront(this.client, form);
-    this.manage(front);
-    return front;
+    if (this.isWebExtension &&
+        this.client.mainRoot.traits.webExtensionAddonConnect) {
+      // The AddonTargetFront form is related to a WebExtensionActor instance,
+      // which isn't a target actor on its own, it is an actor living in the parent
+      // process with access to the addon metadata, it can control the addon (e.g.
+      // reloading it) and listen to the AddonManager events related to the lifecycle of
+      // the addon (e.g. when the addon is disabled or uninstalled).
+      // To retrieve the target actor instance, we call its "connect" method, (which
+      // fetches the target actor targetForm from a WebExtensionTargetActor instance).
+      const { form } = await super.connect();
+      const front = new BrowsingContextTargetFront(this.client, form);
+      this.manage(front);
+      return front;
+    }
+    return this;
   }
 
   async attach() {
     const response = await super.attach();
 
     this.threadActor = response.threadActor;
 
     return response;