Bug 1452618 - track force-disabling things so we don't accidentally re-enable them by using `isUsableAddon` later, r?kmag draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 13 Apr 2018 15:01:05 +0100
changeset 783167 5a6b5a5deb62eaa7e442a251ce861d19f3313eee
parent 782895 6276ec7ebbf33e3484997b189f20fc1511534187
child 783168 ed4419c6e1db27955a08441ec00862d8ab3793a5
push id106628
push usergijskruitbosch@gmail.com
push dateMon, 16 Apr 2018 18:02:56 +0000
reviewerskmag
bugs1452618
milestone61.0a1
Bug 1452618 - track force-disabling things so we don't accidentally re-enable them by using `isUsableAddon` later, r?kmag MozReview-Commit-ID: B7ZAw04cVaZ
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/internal/XPIProviderUtils.js
toolkit/mozapps/extensions/test/xpcshell/test_upgrade_incompatible.js
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -803,16 +803,21 @@ function isUsableAddon(aAddon) {
     return false;
   }
 
   if (aAddon.blocklistState == nsIBlocklistService.STATE_BLOCKED) {
     logger.warn(`Add-on ${aAddon.id} is blocklisted.`);
     return false;
   }
 
+  // If we can't read it, it's not usable:
+  if (aAddon.brokenManifest) {
+    return false;
+  }
+
   if (AddonManager.checkUpdateSecurity && !aAddon.providesUpdatesSecurely) {
     logger.warn(`Updates for add-on ${aAddon.id} must be provided over HTTPS.`);
     return false;
   }
 
 
   if (!aAddon.isPlatformCompatible) {
     logger.warn(`Add-on ${aAddon.id} is not compatible with platform.`);
--- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
@@ -1306,16 +1306,17 @@ this.XPIDatabaseReconcile = {
 
     let manifest = null;
     if (checkSigning || aReloadMetadata) {
       try {
         let file = new nsIFile(aAddonState.path);
         manifest = syncLoadManifestFromFile(file, aInstallLocation);
       } catch (err) {
         // If we can no longer read the manifest, it is no longer compatible.
+        aOldAddon.brokenManifest = true;
         aOldAddon.appDisabled = true;
         return aOldAddon;
       }
     }
 
     // If updating from a version of the app that didn't support signedState
     // then update that property now
     if (checkSigning) {
--- a/toolkit/mozapps/extensions/test/xpcshell/test_upgrade_incompatible.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_upgrade_incompatible.js
@@ -3,17 +3,17 @@
 // unparseable after an application update, the extension becomes
 // disabled.  (See bug 1439600 for a concrete example of a situation where
 // this happened).
 add_task(async function test_upgrade_incompatible() {
   const ID = "incompatible-upgrade@tests.mozilla.org";
 
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
 
-  await promiseStartupManager();
+  await promiseStartupManager(false);
 
   let file = createTempWebExtensionFile({
     manifest: {
       applications: {gecko: {id: ID}},
     },
   });
 
   await Promise.all([promiseInstallFile(file), promiseWebExtensionStartup()]);
@@ -46,10 +46,31 @@ add_task(async function test_upgrade_inc
   Services.prefs.setIntPref("extensions.databaseSchema", 0);
   await promiseStartupManager(true);
 
   addon = await promiseAddonByID(ID);
   notEqual(addon, null);
   equal(addon.appDisabled, true);
 
   await promiseShutdownManager();
+
+  file = createTempWebExtensionFile({
+    manifest: {
+      applications: {gecko: {id: ID}},
+    },
+  });
+
+  // swap the old extension back in and check that we don't persist the disabled state forever.
+  await OS.File.move(file.path, path);
+  await promiseSetExtensionModifiedTime(path, timestamp);
+  Services.obs.notifyObservers(new FileUtils.File(path), "flush-cache-entry");
+
+  // Restart.  With the change to the DB schema we recompute compatibility.
+  Services.prefs.setIntPref("extensions.databaseSchema", 0);
+  await promiseStartupManager(true);
+
+  addon = await promiseAddonByID(ID);
+  notEqual(addon, null);
+  equal(addon.appDisabled, false);
+
+  await promiseShutdownManager();
 });