Bug 1549075 Don't blow up on builtin addons while rebuilding the exte…

…nsions database r=kmag a=lizzard CLOSED TREE Differential Revision: https://phabricator.services.mozilla.com/D29954 --HG-- extra : amend_source : 0589d26ab75f9036034fb2d9f1e4280acf670b89 extra : histedit_source : fbb56252f19a30dd5c436f9cfe81a2b52ee037dc
marco-c · May 5, 2019 · acd424e · acd424e
1 parent 8aceea0
commit acd424e
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 35 deletions.
diff --git a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -2410,9 +2410,7 @@ this.XPIDatabaseReconcile = {
 
       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) {
@@ -2506,9 +2504,7 @@ this.XPIDatabaseReconcile = {
     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;
       }
@@ -2586,9 +2582,7 @@ this.XPIDatabaseReconcile = {
     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;
@@ -2839,6 +2833,9 @@ this.XPIDatabaseReconcile = {
 
     for (let [id, addon] of previousVisible) {
       if (addon.location) {
+        if (addon.location.isBuiltin) {
+          continue;
+        }
         if (addonExists(addon)) {
           XPIInternal.BootstrapScope.get(addon).uninstall();
         }
@@ -2909,8 +2906,9 @@ this.XPIDatabaseReconcile = {
         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) &&

diff --git a/toolkit/mozapps/extensions/internal/XPIInstall.jsm b/toolkit/mozapps/extensions/internal/XPIInstall.jsm
@@ -339,6 +339,36 @@ XPIPackage = class XPIPackage extends Package {
   }
 };
 
+/**
+ * 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.
@@ -610,8 +640,23 @@ var loadManifestFromFile = async function(aFile, aLocation, aOldAddon) {
  * 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();
+    }
+  })());
 }
 
 /**
@@ -3226,7 +3271,7 @@ var XPIInstall = {
   flushJarCache,
   newVersionReason,
   recursiveRemove,
-  syncLoadManifestFromFile,
+  syncLoadManifest,
 
   // Keep track of in-progress operations that support cancel()
   _inProgress: [],
@@ -3695,27 +3740,7 @@ var XPIInstall = {
       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;
 

diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_builtin_location.js b/toolkit/mozapps/extensions/test/xpcshell/test_builtin_location.js
@@ -43,6 +43,16 @@ add_task(async function test_builtin_location() {
   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);