Bug 1207772: Add some better sanity checks for the bootstrap function calls. r=rhelmer

Unifies the methods we have to check that bootstrap add-ons are correctly loaded
and makes it easier to make changes to them all in the future without needing to
re-sign add-ons etc.

This code allows a bootstrap script to use a shared script in a single line of
code. The shared scripts sends out all the relevant info over the observer
service, the add-ons manager test harness receives and retains the current state
for every add-on also performing sanity checks like making sure an "install"
method is always called before any "startup" method etc. It also provides simple
functions to check the state of a given add-on.

toolkit/mozapps/extensions/internal/XPIProviderUtils.js

@@ -2060,11 +2060,11 @@ this.XPIDatabaseReconcile = {
                               BOOTSTRAP_REASONS.ADDON_UPGRADE :
                               BOOTSTRAP_REASONS.ADDON_DOWNGRADE;
 
-          // If the previous add-on was in a different location, bootstrapped
+          // If the previous add-on was in a different path, bootstrapped
           // and still exists then call its uninstall method.
           if (previousAddon.bootstrap && previousAddon._installLocation &&
-              currentAddon._installLocation != previousAddon._installLocation &&
-              previousAddon._sourceBundle.exists()) {
+              previousAddon._sourceBundle.exists() &&
+              currentAddon._sourceBundle.path != previousAddon._sourceBundle.path) {
 
             XPIProvider.callBootstrapMethod(previousAddon, previousAddon._sourceBundle,
                                             "uninstall", installReason,
@@ -2118,7 +2118,18 @@ this.XPIDatabaseReconcile = {
         continue;
 
       // This add-on vanished
+
+      // If the previous add-on was bootstrapped and still exists then call its
+      // uninstall method.
+      if (previousAddon.bootstrap && previousAddon._sourceBundle.exists()) {
+        XPIProvider.callBootstrapMethod(previousAddon, previousAddon._sourceBundle,
+                                        "uninstall", BOOTSTRAP_REASONS.ADDON_UNINSTALL);
+        XPIProvider.unloadBootstrapScope(previousAddon.id);
+      }
       AddonManagerPrivate.addStartupChange(AddonManager.STARTUP_CHANGE_UNINSTALLED, id);
+
+      // Make sure to flush the cache when an old add-on has gone away
+      flushStartupCache();
     }
 
     // Make sure add-ons from hidden locations are marked invisible and inactive

toolkit/mozapps/extensions/test/addons/test_bootstrap1_1/bootstrap.js

@@ -1,32 +1 @@
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-// Test steps chain from pref observers on *_reason,
-// so always set that last
-function install(data, reason) {
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.installed_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.install_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.install_reason", reason);
-}
-
-function startup(data, reason) {
-  Components.utils.reportError("bootstrap startup");
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.active_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.startup_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.startup_reason", reason);
-}
-
-function shutdown(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.active_version", 0);
-  Services.prefs.setIntPref("bootstraptest.shutdown_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.shutdown_reason", reason);
-}
-
-function uninstall(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.installed_version", 0);
-  Services.prefs.setIntPref("bootstraptest.uninstall_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.uninstall_reason", reason);
-}
+Components.utils.import("resource://xpcshell-data/BootstrapMonitor.jsm").monitor(this);

toolkit/mozapps/extensions/test/addons/test_bootstrap1_2/bootstrap.js

@@ -1,31 +1 @@
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-// Test steps chain from pref observers on *_reason,
-// so always set that last
-function install(data, reason) {
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.installed_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.install_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.install_reason", reason);
-}
-
-function startup(data, reason) {
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.active_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.startup_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.startup_reason", reason);
-}
-
-function shutdown(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.active_version", 0);
-  Services.prefs.setIntPref("bootstraptest.shutdown_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.shutdown_reason", reason);
-}
-
-function uninstall(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.installed_version", 0);
-  Services.prefs.setIntPref("bootstraptest.uninstall_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.uninstall_reason", reason);
-}
+Components.utils.import("resource://xpcshell-data/BootstrapMonitor.jsm").monitor(this);

toolkit/mozapps/extensions/test/addons/test_bootstrap1_3/bootstrap.js

@@ -1,31 +1 @@
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-// Test steps chain from pref observers on *_reason,
-// so always set that last
-function install(data, reason) {
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.installed_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.install_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.install_reason", reason);
-}
-
-function startup(data, reason) {
-  Components.utils.import(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.active_version", VERSION);
-  Services.prefs.setIntPref("bootstraptest.startup_oldversion", data.oldVersion);
-  Components.utils.unload(data.resourceURI.spec + "version.jsm");
-  Services.prefs.setIntPref("bootstraptest.startup_reason", reason);
-}
-
-function shutdown(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.active_version", 0);
-  Services.prefs.setIntPref("bootstraptest.shutdown_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.shutdown_reason", reason);
-}
-
-function uninstall(data, reason) {
-  Services.prefs.setIntPref("bootstraptest.installed_version", 0);
-  Services.prefs.setIntPref("bootstraptest.uninstall_newversion", data.newVersion);
-  Services.prefs.setIntPref("bootstraptest.uninstall_reason", reason);
-}
+Components.utils.import("resource://xpcshell-data/BootstrapMonitor.jsm").monitor(this);

toolkit/mozapps/extensions/test/addons/test_bootstrap2_1/bootstrap.js

@@ -1,17 +1 @@
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-function install(data, reason) {
-  Services.prefs.setIntPref("bootstraptest2.installed_version", 1);
-}
-
-function startup(data, reason) {
-  Services.prefs.setIntPref("bootstraptest2.active_version", 1);
-}
-
-function shutdown(data, reason) {
-  Services.prefs.setIntPref("bootstraptest2.active_version", 0);
-}
-
-function uninstall(data, reason) {
-  Services.prefs.setIntPref("bootstraptest2.installed_version", 0);
-}
+Components.utils.import("resource://xpcshell-data/BootstrapMonitor.jsm").monitor(this);

toolkit/mozapps/extensions/test/xpcshell/data/BootstrapMonitor.jsm

@@ -0,0 +1,28 @@
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+this.EXPORTED_SYMBOLS = [ "monitor" ];
+
+function notify(event, originalMethod, data, reason) {
+  let info = {
+    event,
+    data: Object.assign({}, data, {
+      installPath: data.installPath.path,
+      resourceURI: data.resourceURI.spec,
+    }),
+    reason
+  };
+
+  Services.obs.notifyObservers(null, "bootstrapmonitor-event", JSON.stringify(info));
+
+  // If the bootstrap scope already declares a method call it
+  if (originalMethod)
+    originalMethod(data, reason);
+}
+
+// Allows a simple one-line bootstrap script:
+// Components.utils.import("resource://xpcshelldata/bootstrapmonitor.jsm").monitor(this);
+this.monitor = function(scope) {
+  for (let event of ["install", "startup", "shutdown", "uninstall"]) {
+    scope[event] = notify.bind(null, event, scope[event]);
+  }
+}

toolkit/mozapps/extensions/test/xpcshell/data/system_addons/bootstrap.js

@@ -0,0 +1 @@
+Components.utils.import("resource://xpcshell-data/BootstrapMonitor.jsm").monitor(this);

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

@@ -61,6 +61,123 @@ var gUrlToFileMap = {};
 
 var TEST_UNPACKED = false;
 
+// Map resource://xpcshell-data/ to the data directory
+let resHandler = Services.io.getProtocolHandler("resource")
+                         .QueryInterface(AM_Ci.nsISubstitutingProtocolHandler);
+// Allow non-existent files because of bug 1207735
+let dataURI = NetUtil.newURI(do_get_file("data", true));
+resHandler.setSubstitution("xpcshell-data", dataURI);
+
+// Listens to messages from bootstrap.js telling us what add-ons were started
+// and stopped etc. and performs some sanity checks that only installed add-ons
+// are started etc.
+this.BootstrapMonitor = {
+  // Contain the current state of add-ons in the system
+  installed: new Map(),
+  started: new Map(),
+
+  // Contain the last state of shutdown and uninstall calls for an add-on
+  stopped: new Map(),
+  uninstalled: new Map(),
+
+  startupPromises: [],
+  installPromises: [],
+
+  init() {
+    Services.obs.addObserver(this, "bootstrapmonitor-event", false);
+  },
+
+  clear(id) {
+    this.installed.delete(id);
+    this.started.delete(id);
+    this.stopped.delete(id);
+    this.uninstalled.delete(id);
+  },
+
+  promiseAddonStartup(id) {
+    return new Promise(resolve => {
+      this.startupPromises.push(resolve);
+    });
+  },
+
+  promiseAddonInstall(id) {
+    return new Promise(resolve => {
+      this.installPromises.push(resolve);
+    });
+  },
+
+  checkMatches(cached, current) {
+    do_check_neq(cached, undefined);
+    do_check_eq(current.data.version, cached.data.version)
+    do_check_eq(current.data.installPath, cached.data.installPath)
+    do_check_eq(current.data.resourceURI, cached.data.resourceURI)
+  },
+
+  checkAddonStarted(id, version = undefined) {
+    let started = this.started.get(id);
+    do_check_neq(started, undefined);
+    if (version != undefined)
+      do_check_eq(started.data.version, version);
+  },
+
+  checkAddonNotStarted(id) {
+    do_check_false(this.started.has(id));
+  },
+
+  checkAddonInstalled(id, version = undefined) {
+    let installed = this.installed.get(id);
+    do_check_neq(installed, undefined);
+    if (version != undefined)
+      do_check_eq(installed.data.version, version);
+  },
+
+  checkAddonNotInstalled(id) {
+    do_check_false(this.installed.has(id));
+  },
+
+  observe(subject, topic, data) {
+    let info = JSON.parse(data);
+    let id = info.data.id;
+
+    // If this is the install event the add-ons shouldn't already be installed
+    if (info.event == "install") {
+      this.checkAddonNotInstalled(id);
+
+      this.installed.set(id, info);
+
+      for (let resolve of this.installPromises)
+        resolve();
+      this.installPromises = [];
+    }
+    else {
+      this.checkMatches(this.installed.get(id), info);
+    }
+
+    // If this is the shutdown event than the add-on should already be started
+    if (info.event == "shutdown") {
+      this.checkMatches(this.started.get(id), info);
+
+      this.started.delete(id);
+      this.stopped.set(id, info);
+    }
+    else {
+      this.checkAddonNotStarted(id);
+    }
+
+    if (info.event == "uninstall") {
+      this.installed.delete(id);
+      this.uninstalled.set(id, info)
+    }
+    else if (info.event == "startup") {
+      this.started.set(id, info);
+
+      for (let resolve of this.startupPromises)
+        resolve();
+      this.startupPromises = [];
+    }
+  }
+}
+
 function isNightlyChannel() {
   var channel = "default";
   try {

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

@@ -19,6 +19,8 @@ userExtDir.append("extensions2");
 userExtDir.append(gAppInfo.ID);
 registerDirectory("XREUSysExt", userExtDir.parent);
 
+BootstrapMonitor.init();
+
 function TestProvider(result) {
   this.result = result;
 }
@@ -50,18 +52,6 @@ function check_mapping(uri, id) {
   do_check_eq(val.value, id);
 }
 
-function resetPrefs() {
-  Services.prefs.setIntPref("bootstraptest.active_version", -1);
-}
-
-function waitForPref(aPref, aCallback) {
-  function prefChanged() {
-    Services.prefs.removeObserver(aPref, prefChanged);
-    aCallback();
-  }
-  Services.prefs.addObserver(aPref, prefChanged, false);
-}
-
 function getActiveVersion() {
   return Services.prefs.getIntPref("bootstraptest.active_version");
 }
@@ -69,8 +59,6 @@ function getActiveVersion() {
 function run_test() {
   do_test_pending();
 
-  resetPrefs();
-
   run_test_early();
 }
 
@@ -148,7 +136,7 @@ function run_test_1() {
       let uri = addon.getResourceURI(".");
       check_mapping(uri, addon.id);
 
-      waitForPref("bootstraptest.active_version", function() {
+      BootstrapMonitor.promiseAddonStartup("bootstrap1@tests.mozilla.org").then(function() {
         run_test_2(uri);
       });
     });

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

@@ -5,6 +5,8 @@ const PREF_SYSTEM_ADDON_SET = "extensions.systemAddonSet";
 // Enable signature checks for these tests
 Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true);
 
+BootstrapMonitor.init();
+
 const featureDir = FileUtils.getDir("ProfD", ["features"]);
 
 // Build the test sets
@@ -61,8 +63,7 @@ function* check_installed(inProfile, ...versions) {
       do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SYSTEM);
 
       // Verify the add-on actually started
-      let installed = Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
-      do_check_eq(installed, versions[i]);
+      BootstrapMonitor.checkAddonStarted(id, versions[i]);
     }
     else {
       if (inProfile) {
@@ -74,12 +75,12 @@ function* check_installed(inProfile, ...versions) {
         do_check_true(!addon || !addon.isActive);
       }
 
-      try {
-        Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
-        do_throw("Expected pref to be missing");
-      }
-      catch (e) {
-      }
+      BootstrapMonitor.checkAddonNotStarted(id);
+
+      if (addon)
+        BootstrapMonitor.checkAddonInstalled(id);
+      else
+        BootstrapMonitor.checkAddonNotInstalled(id);
     }
   }
 }
@@ -186,6 +187,9 @@ add_task(function* test_skips_additional() {
 add_task(function* test_revert() {
   manuallyUninstall(featureDir, "system2@tests.mozilla.org");
 
+  // With the add-on physically gone from disk we won't see uninstall events
+  BootstrapMonitor.clear("system2@tests.mozilla.org");
+
   startupManager(false);
 
   // With system add-on 2 gone the updated set is now invalid so it reverts to
@@ -258,7 +262,7 @@ add_task(function* test_bad_app_cert() {
   // Add-on will still be present just not active
   let addon = yield promiseAddonByID("system1@tests.mozilla.org");
   do_check_neq(addon, null);
-  do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SIGNED);
+  do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_BROKEN);
 
   yield check_installed(false, null, null, "1.0");
 

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

@@ -9,7 +9,9 @@ Components.utils.import("resource://testing-common/httpd.js");
 const { computeHash } = Components.utils.import("resource://gre/modules/addons/ProductAddonChecker.jsm");
 
 // Enable signature checks for these tests
-//Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true);
+Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true);
+
+BootstrapMonitor.init();
 
 const featureDir = FileUtils.getDir("ProfD", ["features"]);
 
@@ -142,6 +144,8 @@ function* check_installed(inProfile, ...versions) {
     let addon = yield promiseAddonByID(id);
 
     if (versions[i]) {
+      do_print(`Checking state of add-on ${id}, expecting version ${versions[i]}`);
+
       // Add-on should be installed
       do_check_neq(addon, null);
       do_check_eq(addon.version, versions[i]);
@@ -159,13 +163,14 @@ function* check_installed(inProfile, ...versions) {
       do_check_true(uri instanceof AM_Ci.nsIFileURL);
       do_check_eq(uri.file.path, file.path);
 
-      //do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SYSTEM);
+      do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_SYSTEM);
 
       // Verify the add-on actually started
-      let installed = Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
-      do_check_eq(installed, versions[i]);
+      BootstrapMonitor.checkAddonStarted(id, versions[i]);
     }
     else {
+      do_print(`Checking state of add-on ${id}, expecting it to be missing`);
+
       if (inProfile) {
         // Add-on should not be installed
         do_check_eq(addon, null);
@@ -175,12 +180,12 @@ function* check_installed(inProfile, ...versions) {
         do_check_true(!addon || !addon.isActive);
       }
 
-      try {
-        Services.prefs.getCharPref("bootstraptest." + id + ".active_version");
-        do_throw("Expected pref to be missing");
-      }
-      catch (e) {
-      }
+      BootstrapMonitor.checkAddonNotStarted(id);
+
+      if (addon)
+        BootstrapMonitor.checkAddonInstalled(id);
+      else
+        BootstrapMonitor.checkAddonNotInstalled(id);
     }
   }
 }
@@ -328,9 +333,9 @@ const TESTS = {
   // Correct sizes and hashes should work
   checkSizeHash: {
     updateList: [
-      { id: "system2@tests.mozilla.org", version: "3.0", path: "system2_3.xpi", size: 858 },
-      { id: "system3@tests.mozilla.org", version: "3.0", path: "system3_3.xpi", hashFunction: "sha1", hashValue: "105a4c49bd513ebd30594e380c19e86bba1f83e2" },
-      { id: "system5@tests.mozilla.org", version: "1.0", path: "system5_1.xpi", size: 857, hashFunction: "sha1", hashValue: "664e9218be3c9acbb9029e715c1e5d2fbb4ea2cc" }
+      { id: "system2@tests.mozilla.org", version: "3.0", path: "system2_3.xpi", size: 4672 },
+      { id: "system3@tests.mozilla.org", version: "3.0", path: "system3_3.xpi", hashFunction: "sha1", hashValue: "2df604b37b13766c0e04f1b7f59800e038f46cd5" },
+      { id: "system5@tests.mozilla.org", version: "1.0", path: "system5_1.xpi", size: 4671, hashFunction: "sha1", hashValue: "f13dcaa8bfacaa222189bcbb0074972c05ceb621" }
     ],
     finalState: [true, null, "3.0", "3.0", null, "1.0"]
   },