From 475d4dd995674a202d1b0aef3f12e3b27314af53 Mon Sep 17 00:00:00 2001 From: William Durand Date: Thu, 23 Mar 2023 11:43:13 +0000 Subject: [PATCH] Bug 1823457 - Expose `uninstallAddon` method on addons actor. r=jdescottes,devtools-backward-compat-reviewers,devtools-reviewers Depends on D173053 Differential Revision: https://phabricator.services.mozilla.com/D173126 --- .../src/actions/debug-targets.js | 7 +++-- .../TemporaryExtensionAdditionalActions.js | 17 +++++----- .../src/modules/client-wrapper.js | 9 ++++++ .../src/modules/extensions-helper.js | 14 --------- ...er_aboutdebugging_addons_remote_runtime.js | 24 ++++++++++++-- .../mocks/helper-client-wrapper-mock.js | 2 ++ devtools/server/actors/addon/addons.js | 12 +++++-- devtools/server/actors/root.js | 3 ++ .../server/tests/xpcshell/test_webext_apis.js | 31 ++++++++++++++++++- devtools/shared/specs/addon/addons.js | 7 +++++ 10 files changed, 93 insertions(+), 33 deletions(-) diff --git a/devtools/client/aboutdebugging/src/actions/debug-targets.js b/devtools/client/aboutdebugging/src/actions/debug-targets.js index 26c51609f897..e8b4d92eea7a 100644 --- a/devtools/client/aboutdebugging/src/actions/debug-targets.js +++ b/devtools/client/aboutdebugging/src/actions/debug-targets.js @@ -21,7 +21,6 @@ const { const { openTemporaryExtension, - uninstallAddon, } = require("resource://devtools/client/aboutdebugging/src/modules/extensions-helper.js"); const { @@ -174,9 +173,11 @@ function reloadTemporaryExtension(id) { } function removeTemporaryExtension(id) { - return async () => { + return async ({ getState }) => { + const clientWrapper = getCurrentClient(getState().runtimes); + try { - await uninstallAddon(id); + await clientWrapper.uninstallAddon({ id }); } catch (e) { console.error(e); } diff --git a/devtools/client/aboutdebugging/src/components/debugtarget/TemporaryExtensionAdditionalActions.js b/devtools/client/aboutdebugging/src/components/debugtarget/TemporaryExtensionAdditionalActions.js index d7806926be5f..e1e54f0aeb55 100644 --- a/devtools/client/aboutdebugging/src/components/debugtarget/TemporaryExtensionAdditionalActions.js +++ b/devtools/client/aboutdebugging/src/components/debugtarget/TemporaryExtensionAdditionalActions.js @@ -31,11 +31,8 @@ const { } = require("resource://devtools/client/aboutdebugging/src/constants.js"); const { - getCurrentRuntimeDetails, + getCurrentClient, } = require("resource://devtools/client/aboutdebugging/src/modules/runtimes-state-helper.js"); -const { - RUNTIMES, -} = require("resource://devtools/client/aboutdebugging/src/constants.js"); /** * This component provides components that reload/remove temporary extension. @@ -46,7 +43,7 @@ class TemporaryExtensionAdditionalActions extends PureComponent { dispatch: PropTypes.func.isRequired, target: Types.debugTarget.isRequired, // Provided by redux state - runtimeDetails: Types.runtimeDetails.isRequired, + supportsAddonsUninstall: PropTypes.bool.isRequired, }; } @@ -145,10 +142,7 @@ class TemporaryExtensionAdditionalActions extends PureComponent { } renderRemoveButton() { - // TODO: Bug 1823457 - Uninstalling an add-on is currently limited to a - // local runtime. Once it becomes possible to use the RDP protocol, we can - // show this "Remove" button. - if (this.props.runtimeDetails.info.type !== RUNTIMES.THIS_FIREFOX) { + if (!this.props.supportsAddonsUninstall) { return null; } @@ -199,8 +193,11 @@ class TemporaryExtensionAdditionalActions extends PureComponent { } const mapStateToProps = state => { + const clientWrapper = getCurrentClient(state.runtimes); + return { - runtimeDetails: getCurrentRuntimeDetails(state.runtimes), + supportsAddonsUninstall: + clientWrapper.traits.supportsAddonsUninstall === true, }; }; diff --git a/devtools/client/aboutdebugging/src/modules/client-wrapper.js b/devtools/client/aboutdebugging/src/modules/client-wrapper.js index f2969b5de886..3344fa7258dc 100644 --- a/devtools/client/aboutdebugging/src/modules/client-wrapper.js +++ b/devtools/client/aboutdebugging/src/modules/client-wrapper.js @@ -146,6 +146,11 @@ class ClientWrapper { return this.client.mainRoot.getAddon({ id }); } + async uninstallAddon({ id }) { + const addonsFront = await this.getFront("addons"); + return addonsFront.uninstallAddon(id); + } + async getMainProcess() { return this.client.mainRoot.getMainProcess(); } @@ -206,6 +211,10 @@ class ClientWrapper { openRemoteDevTools ); } + + get traits() { + return { ...this.client.mainRoot.traits }; + } } exports.ClientWrapper = ClientWrapper; diff --git a/devtools/client/aboutdebugging/src/modules/extensions-helper.js b/devtools/client/aboutdebugging/src/modules/extensions-helper.js index 053adc209bbd..a2deec907aff 100644 --- a/devtools/client/aboutdebugging/src/modules/extensions-helper.js +++ b/devtools/client/aboutdebugging/src/modules/extensions-helper.js @@ -6,11 +6,6 @@ const lazy = {}; -ChromeUtils.defineModuleGetter( - lazy, - "AddonManager", - "resource://gre/modules/AddonManager.jsm" -); ChromeUtils.defineESModuleGetters(lazy, { FileUtils: "resource://gre/modules/FileUtils.sys.mjs", }); @@ -19,15 +14,6 @@ const { PREFERENCES, } = require("resource://devtools/client/aboutdebugging/src/constants.js"); -/** - * Uninstall the addon with the provided id. - * Resolves when the addon shutdown has completed. - */ -exports.uninstallAddon = async function(addonID) { - const addon = await lazy.AddonManager.getAddonByID(addonID); - return addon && addon.uninstall(); -}; - exports.parseFileUri = function(url) { // Strip a leading slash from Windows drive letter URIs. // file:///home/foo ~> /home/foo diff --git a/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_remote_runtime.js b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_remote_runtime.js index ea18c1ebbd0c..9679f14b5b08 100644 --- a/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_remote_runtime.js +++ b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_remote_runtime.js @@ -31,6 +31,12 @@ add_task(async function() { mocks.thisFirefoxClient, document ); + await testAddonsOnMockedRemoteClient( + usbClient, + mocks.thisFirefoxClient, + document, + /* supportsAddonsUninstall */ true + ); info("Prepare Network client mock"); const networkClient = mocks.createNetworkRuntime(NETWORK_RUNTIME_HOST, { @@ -45,6 +51,12 @@ add_task(async function() { mocks.thisFirefoxClient, document ); + await testAddonsOnMockedRemoteClient( + networkClient, + mocks.thisFirefoxClient, + document, + /* supportsAddonsUninstall */ true + ); await removeTab(tab); }); @@ -55,7 +67,8 @@ add_task(async function() { async function testAddonsOnMockedRemoteClient( remoteClient, firefoxClient, - document + document, + supportsAddonsUninstall = false ) { const extensionPane = getDebugTargetPane("Extensions", document); info("Check an empty target pane message is displayed"); @@ -73,6 +86,8 @@ async function testAddonsOnMockedRemoteClient( }; remoteClient.listAddons = () => [addon, temporaryAddon]; remoteClient._eventEmitter.emit("addonListChanged"); + // We use a mock client (wrapper) so we must set the trait ourselves. + remoteClient.traits.supportsAddonsUninstall = supportsAddonsUninstall; info("Wait until the extension appears"); await waitUntil( @@ -94,11 +109,14 @@ async function testAddonsOnMockedRemoteClient( "Temporary Extension target appeared for the remote runtime" ); - // TODO: Bug 1823457 - Allow to remove an extension using a non-local runtime. const removeButton = temporaryExtensionTarget.querySelector( ".qa-temporary-extension-remove-button" ); - ok(!removeButton, "No remove button expected for the temporary extension"); + if (supportsAddonsUninstall) { + ok(removeButton, "Remove button expected for the temporary extension"); + } else { + ok(!removeButton, "No remove button expected for the temporary extension"); + } const reloadButton = temporaryExtensionTarget.querySelector( ".qa-temporary-extension-reload-button" diff --git a/devtools/client/aboutdebugging/test/browser/mocks/helper-client-wrapper-mock.js b/devtools/client/aboutdebugging/test/browser/mocks/helper-client-wrapper-mock.js index cb80b867f89e..5748904bffe2 100644 --- a/devtools/client/aboutdebugging/test/browser/mocks/helper-client-wrapper-mock.js +++ b/devtools/client/aboutdebugging/test/browser/mocks/helper-client-wrapper-mock.js @@ -107,6 +107,8 @@ function createClientMock() { } = require("resource://devtools/client/shared/remote-debugging/version-checker.js"); return { status: COMPATIBILITY_STATUS.COMPATIBLE }; }, + // No traits by default but allow updates. + traits: {}, }; } diff --git a/devtools/server/actors/addon/addons.js b/devtools/server/actors/addon/addons.js index 0066f7f9011b..f31b38554539 100644 --- a/devtools/server/actors/addon/addons.js +++ b/devtools/server/actors/addon/addons.js @@ -16,8 +16,8 @@ const { FileUtils } = ChromeUtils.importESModule( "resource://gre/modules/FileUtils.sys.mjs" ); -// This actor is not used by DevTools, but is relied on externally by -// webext-run and the Firefox VS-Code plugin. see bug #1578108 +// This actor is used by DevTools as well as external tools such as webext-run +// and the Firefox VS-Code plugin. see bug #1578108 class AddonsActor extends Actor { constructor(conn) { super(conn, addonsSpec); @@ -65,6 +65,14 @@ class AddonsActor extends Actor { // gets upgraded to a real actor object. return { id: addon.id, actor: false }; } + + async uninstallAddon(addonId) { + const addon = await AddonManager.getAddonByID(addonId); + + if (addon) { + await addon.uninstall(); + } + } } exports.AddonsActor = AddonsActor; diff --git a/devtools/server/actors/root.js b/devtools/server/actors/root.js index dfced9faf3dd..fb3326800438 100644 --- a/devtools/server/actors/root.js +++ b/devtools/server/actors/root.js @@ -141,6 +141,9 @@ class RootActor extends Actor { supportsJavascriptTracing: true, // @backward-compat { version 112 } Checks if the server supports override isOverridesSupported: true, + // @backward-compat { version 113 } Fx 113 added support for uninstalling + // add-ons via the Addons actor. + supportsAddonsUninstall: true, }; } diff --git a/devtools/server/tests/xpcshell/test_webext_apis.js b/devtools/server/tests/xpcshell/test_webext_apis.js index f794c10d7826..bc9a3da03b79 100644 --- a/devtools/server/tests/xpcshell/test_webext_apis.js +++ b/devtools/server/tests/xpcshell/test_webext_apis.js @@ -3,6 +3,10 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; +const { AddonManager } = ChromeUtils.import( + "resource://gre/modules/AddonManager.jsm" +); + // The `AddonsManager` test helper can only be called once per test script. // This `setup` task will run first. add_task(async function setup() { @@ -73,11 +77,12 @@ add_task(async function test_webext_run_apis() { equal(addon.actor, false, "temporary add-on does not have an actor"); // listAddons - const { addons } = await sendRequest(transport, { + let { addons } = await sendRequest(transport, { to: "root", type: "listAddons", }); ok(Array.isArray(addons), "listAddons() returns a list of add-ons"); + equal(addons.length, 1, "expected an add-on installed"); const installedAddon = addons[0]; equal(installedAddon.id, addonId, "installed add-on is the expected one"); @@ -92,5 +97,29 @@ add_task(async function test_webext_run_apis() { }); await Promise.all([promiseReloaded, promiseRestarted]); + // uninstallAddon + const promiseUninstalled = new Promise(resolve => { + const listener = {}; + listener.onUninstalled = uninstalledAddon => { + if (uninstalledAddon.id == addonId) { + AddonManager.removeAddonListener(listener); + resolve(); + } + }; + AddonManager.addAddonListener(listener); + }); + await sendRequest(transport, { + to: getRootResponse.addonsActor, + type: "uninstallAddon", + addonId, + }); + await promiseUninstalled; + + ({ addons } = await sendRequest(transport, { + to: "root", + type: "listAddons", + })); + equal(addons.length, 0, "expected no add-on installed"); + transport.close(); }); diff --git a/devtools/shared/specs/addon/addons.js b/devtools/shared/specs/addon/addons.js index e0dc45c88a62..309a7acdf5e6 100644 --- a/devtools/shared/specs/addon/addons.js +++ b/devtools/shared/specs/addon/addons.js @@ -20,6 +20,13 @@ const addonsSpec = generateActorSpec({ }, response: { addon: RetVal("json") }, }, + + uninstallAddon: { + request: { + addonId: Arg(0, "string"), + }, + response: {}, + }, }, });