Bug 1734987 - Part 4: Make addon.isPrivileged consistent with extension.isPrivileged r=rpl

addon.isPrivileged and extension.isPrivileged are currently almost
identical, except in the case of temporary extensions when experiments
are enabled.

This difference is inexplicable for those unfamiliar with the history,
so this patch makes them identical.

addon.isPrivileged was introduced in bug 1543204 to support hidden
add-ons, which was intended to be used by privileged add-ons only.
Hence, this patch updates the "hidden" getter to continue ignoring the
hidden flag for temporary add-ons.

With this change, the addon.isPrivileged check can be removed from
loadManifestFromWebManifest, in favor of checking extension.isPrivileged
at the location where addon.hidden is initialized for the first (and now
only) time.

BootstrapScope.callBootstrapMethod can now also use addon.isPrivileged
instead of invoking the ExtensionData.getIsPrivileged helper.

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

toolkit/mozapps/extensions/internal/XPIDatabase.jsm

@@ -26,6 +26,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   AddonRepository: "resource://gre/modules/addons/AddonRepository.jsm",
   AddonSettings: "resource://gre/modules/addons/AddonSettings.jsm",
   DeferredTask: "resource://gre/modules/DeferredTask.jsm",
+  ExtensionData: "resource://gre/modules/Extension.jsm",
   ExtensionUtils: "resource://gre/modules/ExtensionUtils.jsm",
   FileUtils: "resource://gre/modules/FileUtils.jsm",
   PermissionsUtils: "resource://gre/modules/PermissionsUtils.jsm",
@@ -480,17 +481,22 @@ class AddonInternal {
     return this.isCompatibleWith();
   }
 
-  // This matches Extension.isPrivileged with the exception of temporarily installed extensions.
   get isPrivileged() {
-    return (
-      this.signedState === AddonManager.SIGNEDSTATE_PRIVILEGED ||
-      this.signedState === AddonManager.SIGNEDSTATE_SYSTEM ||
-      this.location.isBuiltin
-    );
+    return ExtensionData.getIsPrivileged({
+      signedState: this.signedState,
+      builtIn: this.location.isBuiltin,
+      temporarilyInstalled: this.location.isTemporary,
+    });
   }
 
   get hidden() {
-    return this.location.hidden || (this._hidden && this.isPrivileged) || false;
+    return (
+      this.location.hidden ||
+      // The hidden flag is intended to only be used for features that are part
+      // of the application. Temporary add-ons should not be hidden.
+      (this._hidden && this.isPrivileged && !this.location.isTemporary) ||
+      false
+    );
   }
 
   set hidden(val) {

toolkit/mozapps/extensions/internal/XPIInstall.jsm

@@ -505,7 +505,7 @@ async function loadManifestFromWebManifest(aPackage, aLocation) {
   addon.aboutURL = null;
   addon.dependencies = Object.freeze(Array.from(extension.dependencies));
   addon.startupData = extension.startupData;
-  addon.hidden = manifest.hidden;
+  addon.hidden = extension.isPrivileged && manifest.hidden;
   addon.incognito = manifest.incognito;
 
   if (addon.type === "theme" && (await aPackage.hasResource("preview.png"))) {
@@ -700,9 +700,6 @@ var loadManifest = async function(aPackage, aLocation, aOldAddon) {
   let { signedState, cert } = verifiedSignedState;
   addon.signedState = signedState;
   addon.signedDate = cert?.validity?.notBefore / 1000 || null;
-  if (!addon.isPrivileged) {
-    addon.hidden = false;
-  }
 
   if (!addon.id) {
     if (cert) {

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -566,6 +566,14 @@ class XPIState {
     return this.loader == null;
   }
 
+  get isPrivileged() {
+    return ExtensionData.getIsPrivileged({
+      signedState: this.signedState,
+      builtIn: this.location.isBuiltin,
+      temporarilyInstalled: this.location.isTemporary,
+    });
+  }
+
   /**
    * Update the last modified time for an add-on on disk.
    *
@@ -1805,13 +1813,6 @@ class BootstrapScope {
           }
         }
       }
-      // TODO D128233: Replace AddonInternal's isPrivileged getter with a call to
-      // Extensions.getIsPrivileged, and use addon.isPrivileged instead of this.
-      const isPrivileged = ExtensionData.getIsPrivileged({
-        signedState: addon.signedState,
-        builtIn: addon.location.isBuiltin,
-        temporarilyInstalled: addon.location.isTemporary,
-      });
 
       let params = {
         id: addon.id,
@@ -1821,7 +1822,7 @@ class BootstrapScope {
         temporarilyInstalled: addon.location.isTemporary,
         builtIn: addon.location.isBuiltin,
         isSystem: addon.location.isSystem,
-        isPrivileged,
+        isPrivileged: addon.isPrivileged,
         recommendationState: addon.recommendationState,
       };
 

toolkit/mozapps/extensions/test/xpcshell/test_hidden.js

@@ -1,3 +1,8 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
 add_task(async function test_hidden() {
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "2");
   AddonTestUtils.usePrivilegedSignatures = id => id.startsWith("privileged");
@@ -54,5 +59,18 @@ add_task(async function test_hidden() {
   ok(!addon2.isPrivileged, "Unprivileged extension is not privileged");
   ok(!addon2.hidden, "Unprivileged extension should not be hidden");
 
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "temporary",
+    manifest: {
+      applications: { gecko: { id: "privileged@but-temporary" } },
+      hidden: true,
+    },
+  });
+  await extension.startup();
+  let tempAddon = extension.addon;
+  ok(tempAddon.isPrivileged, "Temporary add-on is privileged");
+  ok(!tempAddon.hidden, "Temporary add-on is not hidden despite privilige");
+  await extension.unload();
+
   await promiseShutdownManager();
 });

toolkit/mozapps/extensions/test/xpcshell/test_loadManifest_isPrivileged.js

@@ -80,6 +80,7 @@ async function testLoadManifest({ location, expectPrivileged }) {
   let { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
     let addon = await XPIInstall.loadManifestFromFile(xpi, location);
     actualPermissions = addon.userPermissions;
+    equal(addon.isPrivileged, expectPrivileged, "addon.isPrivileged");
   });
   if (expectPrivileged) {
     AddonTestUtils.checkMessages(messages, {

toolkit/mozapps/extensions/test/xpcshell/test_startup_isPrivileged.js

@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const ADDON_ID_PRIVILEGED = "@privileged-addon-id";
+const ADDON_ID_NO_PRIV = "@addon-without-privileges";
+AddonTestUtils.usePrivilegedSignatures = id => id === ADDON_ID_PRIVILEGED;
+
+function isExtensionPrivileged(addonId) {
+  const { extension } = WebExtensionPolicy.getByID(addonId);
+  return extension.isPrivileged;
+}
+
+add_task(async function setup() {
+  await ExtensionTestUtils.startAddonManager();
+});
+
+add_task(async function isPrivileged_at_install() {
+  {
+    let addon = await promiseInstallWebExtension({
+      manifest: {
+        permissions: ["mozillaAddons"],
+        applications: { gecko: { id: ADDON_ID_PRIVILEGED } },
+      },
+    });
+    ok(addon.isPrivileged, "Add-on is privileged");
+    ok(isExtensionPrivileged(ADDON_ID_PRIVILEGED), "Extension is privileged");
+  }
+
+  {
+    let addon = await promiseInstallWebExtension({
+      manifest: {
+        permissions: ["mozillaAddons"],
+        applications: { gecko: { id: ADDON_ID_NO_PRIV } },
+      },
+    });
+    ok(!addon.isPrivileged, "Add-on is not privileged");
+    ok(!isExtensionPrivileged(ADDON_ID_NO_PRIV), "Extension is not privileged");
+  }
+});
+
+// When the Add-on Manager is restarted, the extension is started using data
+// from XPIState. This test verifies that `extension.isPrivileged` is correctly
+// set in that scenario.
+add_task(async function isPrivileged_at_restart() {
+  await promiseRestartManager();
+  {
+    let addon = await AddonManager.getAddonByID(ADDON_ID_PRIVILEGED);
+    ok(addon.isPrivileged, "Add-on is privileged");
+    ok(isExtensionPrivileged(ADDON_ID_PRIVILEGED), "Extension is privileged");
+  }
+  {
+    let addon = await AddonManager.getAddonByID(ADDON_ID_NO_PRIV);
+    ok(!addon.isPrivileged, "Add-on is not privileged");
+    ok(!isExtensionPrivileged(ADDON_ID_NO_PRIV), "Extension is not privileged");
+  }
+});

toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini

@@ -110,6 +110,7 @@ skip-if = require_signing || !allow_legacy_extensions
 head = head_addons.js head_sideload.js
 skip-if = os == "linux" # Bug 1613268
 [test_startup_enable.js]
+[test_startup_isPrivileged.js]
 [test_startup_scan.js]
 head = head_addons.js head_sideload.js
 [test_strictcompatibility.js]