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

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);
     }

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
+    if (!this.props.supportsAddonsUninstall) {
-    // 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) {
       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,
   };
 };
 

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;

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

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"

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: {},
   };
 }
 

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
+// This actor is used by DevTools as well as external tools such as webext-run
-// webext-run and the Firefox VS-Code plugin. see bug #1578108
+// 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;

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,
     };
   }
 

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();
 });

devtools/shared/specs/addon/addons.js

@@ -20,6 +20,13 @@ const addonsSpec = generateActorSpec({
       },
       response: { addon: RetVal("json") },
     },
+
+    uninstallAddon: {
+      request: {
+        addonId: Arg(0, "string"),
+      },
+      response: {},
+    },
   },
 });