From 76f1d253013c72d10dba58122a0c8a239a6ae741 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Mon, 21 Jan 2019 11:04:31 +0000 Subject: [PATCH] Bug 1520772 - Construct the WebExtension target front before instantiating the Target object. r=yulia Differential Revision: https://phabricator.services.mozilla.com/D15829 --HG-- extra : moz-landing-system : lando --- .../src/modules/extensions-helper.js | 4 +++- devtools/client/framework/target.js | 15 --------------- .../framework/toolbox-process-window.js | 3 ++- ..._webextension-addon-debugging-connect.html | 7 +++++-- .../tests/mochitest/webextension-helpers.js | 3 ++- .../server/tests/unit/test_addon_reload.js | 16 ++++++++-------- devtools/shared/fronts/targets/addon.js | 19 +++++++++++++++---- 7 files changed, 35 insertions(+), 32 deletions(-) diff --git a/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js b/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js index 036936ee0262..8ff96554b830 100644 --- a/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js +++ b/devtools/client/aboutdebugging-new/src/modules/extensions-helper.js @@ -53,7 +53,9 @@ exports.debugLocalAddon = async function(addonID) { * 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(); diff --git a/devtools/client/framework/target.js b/devtools/client/framework/target.js index 22413fbd7c5f..b997af75d09a 100644 --- a/devtools/client/framework/target.js +++ b/devtools/client/framework/target.js @@ -530,9 +530,6 @@ class Target extends EventEmitter { * 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) { @@ -571,18 +568,6 @@ class Target extends EventEmitter { }; 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. diff --git a/devtools/client/framework/toolbox-process-window.js b/devtools/client/framework/toolbox-process-window.js index 00cb679524ee..142f818f63be 100644 --- a/devtools/client/framework/toolbox-process-window.js +++ b/devtools/client/framework/toolbox-process-window.js @@ -89,7 +89,8 @@ var connect = async function() { 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(); diff --git a/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html b/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html index b082a0fc566e..006e93774669 100644 --- a/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html +++ b/devtools/server/tests/mochitest/test_webextension-addon-debugging-connect.html @@ -37,8 +37,11 @@ async function test_connect_addon(oopMode) { 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({ diff --git a/devtools/server/tests/mochitest/webextension-helpers.js b/devtools/server/tests/mochitest/webextension-helpers.js index fda707115e8d..464418c2f0bc 100644 --- a/devtools/server/tests/mochitest/webextension-helpers.js +++ b/devtools/server/tests/mochitest/webextension-helpers.js @@ -98,7 +98,8 @@ async function attachAddon(addonId) { 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(); diff --git a/devtools/server/tests/unit/test_addon_reload.js b/devtools/server/tests/unit/test_addon_reload.js index d5d536ede780..3db76d411b4c 100644 --- a/devtools/server/tests/unit/test_addon_reload.js +++ b/devtools/server/tests/unit/test_addon_reload.js @@ -34,10 +34,10 @@ function promiseWebExtensionStartup() { }); } -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; } @@ -67,10 +67,10 @@ add_task(async function testReloadExitedAddon() { 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(), ]); @@ -82,10 +82,10 @@ add_task(async function testReloadExitedAddon() { // 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"); @@ -101,9 +101,9 @@ add_task(async function testReloadExitedAddon() { // 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"); diff --git a/devtools/shared/fronts/targets/addon.js b/devtools/shared/fronts/targets/addon.js index c72bcea44a7b..5b756d94e85f 100644 --- a/devtools/shared/fronts/targets/addon.js +++ b/devtools/shared/fronts/targets/addon.js @@ -56,10 +56,21 @@ class AddonTargetFront extends FrontClassWithSpec(addonTargetSpec) { * 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() {