Bug 1549075 Don't blow up on builtin addons while rebuilding the extensions database r=kmag a=lizzard CLOSED TREE
authorAndrew Swan <aswan@mozilla.com>
Sat, 04 May 2019 19:23:37 -0700
changeset 472594 74e72eb8369d8f494be2943cbf010d072d3e266c
parent 472593 023dd959512e2cfa685187616560f91efa91183c
child 472595 7995904ca18e2d85e768bc4317c56c1cddff44aa
push id113027
push userccoroiu@mozilla.com
push dateSun, 05 May 2019 21:45:51 +0000
treeherdermozilla-inbound@1e3244e602fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag, lizzard
bugs1549075
milestone68.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 1549075 Don't blow up on builtin addons while rebuilding the extensions database r=kmag a=lizzard CLOSED TREE Differential Revision: https://phabricator.services.mozilla.com/D29954
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
toolkit/mozapps/extensions/internal/XPIInstall.jsm
toolkit/mozapps/extensions/test/xpcshell/test_builtin_location.js
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -2405,19 +2405,17 @@ this.XPIDatabaseReconcile = {
       // Do not allow third party installs if xpinstall is disabled by policy
       if (isDetectedInstall && Services.policies &&
           !Services.policies.isAllowed("xpinstall")) {
         throw new Error("Extension installs are disabled by enterprise policy.");
       }
 
       if (!aNewAddon) {
         // Load the manifest from the add-on.
-        let file = new nsIFile(aAddonState.path);
-        aNewAddon = XPIInstall.syncLoadManifestFromFile(file, aLocation);
-        aNewAddon.rootURI = XPIInternal.getURIForResourceInFile(file, "").spec;
+        aNewAddon = XPIInstall.syncLoadManifest(aAddonState, aLocation);
       }
       // The add-on in the manifest should match the add-on ID.
       if (aNewAddon.id != aId) {
         throw new Error(`Invalid addon ID: expected addon ID ${aId}, found ${aNewAddon.id} in manifest`);
       }
 
       unsigned = XPIDatabase.mustSign(aNewAddon.type) && !aNewAddon.isCorrectlySigned;
       if (unsigned) {
@@ -2501,19 +2499,17 @@ this.XPIDatabaseReconcile = {
    *        changing this add-on
    */
   updateMetadata(aLocation, aOldAddon, aAddonState, aNewAddon) {
     logger.debug(`Add-on ${aOldAddon.id} modified in ${aLocation.name}`);
 
     try {
       // If there isn't an updated install manifest for this add-on then load it.
       if (!aNewAddon) {
-        let file = new nsIFile(aAddonState.path);
-        aNewAddon = XPIInstall.syncLoadManifestFromFile(file, aLocation, aOldAddon);
-        aNewAddon.rootURI = XPIInternal.getURIForResourceInFile(file, "").spec;
+        aNewAddon = XPIInstall.syncLoadManifest(aAddonState, aLocation, aOldAddon);
       } else {
         aNewAddon.rootURI = aOldAddon.rootURI;
       }
 
       // The ID in the manifest that was loaded must match the ID of the old
       // add-on.
       if (aNewAddon.id != aOldAddon.id)
         throw new Error(`Incorrect id in install manifest for existing add-on ${aOldAddon.id}`);
@@ -2581,19 +2577,17 @@ this.XPIDatabaseReconcile = {
     logger.debug(`Updating compatibility for add-on ${aOldAddon.id} in ${aLocation.name}`);
 
     let checkSigning = (aOldAddon.signedState === undefined &&
                         SIGNED_TYPES.has(aOldAddon.type));
 
     let manifest = null;
     if (checkSigning || aReloadMetadata) {
       try {
-        let file = new nsIFile(aAddonState.path);
-        manifest = XPIInstall.syncLoadManifestFromFile(file, aLocation);
-        manifest.rootURI = aOldAddon.rootURI;
+        manifest = XPIInstall.syncLoadManifest(aAddonState, aLocation);
       } catch (err) {
         // If we can no longer read the manifest, it is no longer compatible.
         aOldAddon.brokenManifest = true;
         aOldAddon.appDisabled = true;
         return aOldAddon;
       }
     }
 
@@ -2834,16 +2828,19 @@ this.XPIDatabaseReconcile = {
     }
 
     if (promises.some(p => p)) {
       XPIInternal.awaitPromise(Promise.all(promises));
     }
 
     for (let [id, addon] of previousVisible) {
       if (addon.location) {
+        if (addon.location.isBuiltin) {
+          continue;
+        }
         if (addonExists(addon)) {
           XPIInternal.BootstrapScope.get(addon).uninstall();
         }
         addon.location.removeAddon(id);
         addon.visible = false;
         addon.active = false;
       }
 
@@ -2904,18 +2901,19 @@ this.XPIDatabaseReconcile = {
     let isActive = !currentAddon.disabled;
     let wasActive = previousAddon ? previousAddon.active : currentAddon.active;
 
     if (previousAddon) {
       if (previousAddon !== currentAddon) {
         AddonManagerPrivate.addStartupChange(AddonManager.STARTUP_CHANGE_CHANGED, id);
 
         if (previousAddon.location &&
-            previousAddon._sourceBundle.exists() &&
-            !previousAddon._sourceBundle.equals(currentAddon._sourceBundle)) {
+            (!previousAddon._sourceBundle || 
+             (previousAddon._sourceBundle.exists() &&
+              !previousAddon._sourceBundle.equals(currentAddon._sourceBundle)))) {
           promise = XPIInternal.BootstrapScope.get(previousAddon).update(
             currentAddon);
         } else if (this.isSystemAddonLocation(currentAddon.location) &&
                    previousAddon.version == currentAddon.version &&
                    previousAddon.userDisabled != currentAddon.userDisabled) {
           // A system addon change, no need for install or update events.
         } else {
           let reason = XPIInstall.newVersionReason(previousAddon.version, currentAddon.version);
--- a/toolkit/mozapps/extensions/internal/XPIInstall.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIInstall.jsm
@@ -335,16 +335,46 @@ XPIPackage = class XPIPackage extends Pa
   }
 
   flushCache() {
     flushJarCache(this.file);
   }
 };
 
 /**
+ * Return an object that implements enough of the Package interface
+ * to allow loadManifest() to work for a built-in addon (ie, one loaded
+ * from a resource: url)
+ *
+ * @param {nsIURL} baseURL The URL for the root of the add-on.
+ * @returns {object}
+ */
+function builtinPackage(baseURL) {
+  return {
+    rootURI: baseURL,
+    filePath: baseURL.spec,
+    file: null,
+    verifySignedState() {
+      return {
+        signedState: AddonManager.SIGNEDSTATE_NOT_REQUIRED,
+        cert: null,
+      };
+    },
+    async hasResource(path) {
+      try {
+        let response = await fetch(this.rootURI.resolve(path));
+        return response.ok;
+      } catch (e) {
+        return false;
+      }
+    },
+  };
+}
+
+/**
  * Determine the reason to pass to an extension's bootstrap methods when
  * switch between versions.
  *
  * @param {string} oldVersion The version of the existing extension instance.
  * @param {string} newVersion The version of the extension being installed.
  *
  * @returns {integer}
  *        BOOSTRAP_REASONS.ADDON_UPGRADE or BOOSTRAP_REASONS.ADDON_DOWNGRADE
@@ -605,18 +635,33 @@ var loadManifestFromFile = async functio
     pkg.close();
   }
 };
 
 /*
  * A synchronous method for loading an add-on's manifest. Do not use
  * this.
  */
-function syncLoadManifestFromFile(aFile, aLocation, aOldAddon) {
-  return XPIInternal.awaitPromise(loadManifestFromFile(aFile, aLocation, aOldAddon));
+function syncLoadManifest(state, location, oldAddon) {
+  if (location.name == "app-builtin") {
+    let pkg = builtinPackage(Services.io.newURI(oldAddon.rootURI));
+    return XPIInternal.awaitPromise(loadManifest(pkg, location, oldAddon));
+  }
+
+  let file = new nsIFile(state.path);
+  let pkg = Package.get(file);
+  return XPIInternal.awaitPromise((async () => {
+    try {
+      let addon = await loadManifest(pkg, location, oldAddon);
+      addon.rootURI = getURIForResourceInFile(file, "").spec;
+      return addon;
+    } finally {
+      pkg.close();
+    }
+  })());
 }
 
 /**
  * Creates and returns a new unique temporary file. The caller should delete
  * the file when it is no longer needed.
  *
  * @returns {nsIFile}
  *       An nsIFile that points to a randomly named, initially empty file in
@@ -3221,17 +3266,17 @@ class SystemAddonInstaller extends Direc
 var XPIInstall = {
   // An array of currently active AddonInstalls
   installs: new Set(),
 
   createLocalInstall,
   flushJarCache,
   newVersionReason,
   recursiveRemove,
-  syncLoadManifestFromFile,
+  syncLoadManifest,
 
   // Keep track of in-progress operations that support cancel()
   _inProgress: [],
 
   doing(aCancellable) {
     this._inProgress.push(aCancellable);
   },
 
@@ -3690,37 +3735,17 @@ var XPIInstall = {
     // WebExtensions need to be able to iterate through the contents of
     // an extension (for localization).  It knows how to do this with
     // jar: and file: URLs, so translate the provided base URL to
     // something it can use.
     if (baseURL.scheme !== "resource") {
       throw new Error("Built-in addons must use resource: URLS");
     }
 
-    // Enough of the Package interface to allow loadManifest() to work.
-    let pkg = {
-      rootURI: baseURL,
-      filePath: baseURL.spec,
-      file: null,
-      verifySignedState() {
-        return {
-          signedState: AddonManager.SIGNEDSTATE_NOT_REQUIRED,
-          cert: null,
-        };
-      },
-      async hasResource(path) {
-        try {
-          let response = await fetch(this.rootURI.resolve(path));
-          return response.ok;
-        } catch (e) {
-          return false;
-        }
-      },
-    };
-
+    let pkg = builtinPackage(baseURL);
     let addon = await loadManifest(pkg, XPIInternal.BuiltInLocation);
     addon.rootURI = base;
 
     // If this is a theme, decide whether to enable it. Themes are
     // disabled by default. However:
     //
     // If a lightweight theme was selected in the last session, and this
     // theme has the same ID, then we clearly want to enable it.
--- a/toolkit/mozapps/extensions/test/xpcshell/test_builtin_location.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_builtin_location.js
@@ -38,16 +38,26 @@ add_task(async function test_builtin_loc
   await wrapper.awaitStartup();
   await wrapper.awaitMessage("started");
   ok(true, "Extension in built-in location ran after restart");
 
   addon = await promiseAddonByID(id);
   notEqual(addon, null, "Addon is installed");
   ok(addon.isActive, "Addon is active");
 
+  // After a restart that causes a database rebuild, it should still work
+  await promiseRestartManager("2");
+  await wrapper.awaitStartup();
+  await wrapper.awaitMessage("started");
+  ok(true, "Extension in built-in location ran after restart");
+
+  addon = await promiseAddonByID(id);
+  notEqual(addon, null, "Addon is installed");
+  ok(addon.isActive, "Addon is active");
+
   await wrapper.unload();
 
   addon = await promiseAddonByID(id);
   equal(addon, null, "Addon is gone after uninstall");
   await AddonTestUtils.promiseShutdownManager();
 });
 
 // Tests installing a hidden extension from the built-in location.