use new temporary install location instead of passing around isTemporary bool draft
authorRobert Helmer <rhelmer@mozilla.com>
Fri, 30 Oct 2015 15:58:21 -0700
changeset 304787 a13b97b34c25de30f031c138439a69601b79144b
parent 304786 a9d41ddfd928336a3733486a0a1f53b143978f3e
child 510731 c216619fd4ef8df74a38b4e956880d50821dcc8c
push id6998
push userrhelmer@mozilla.com
push dateFri, 30 Oct 2015 23:39:17 +0000
milestone44.0a1
use new temporary install location instead of passing around isTemporary bool
.eslintrc
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
--- a/.eslintrc
+++ b/.eslintrc
@@ -1,5 +1,143 @@
+// Note: there are extra allowances for files used solely in Firefox desktop,
+// see content/js/.eslintrc and modules/.eslintrc
 {
   "plugins": [
-    "mozilla"
-  ]
+    "react"
+  ],
+  "ecmaFeatures": {
+    "forOf": true,
+    "jsx": true,
+  },
+  "env": {
+    "browser": true,
+    "mocha": true,
+    "es6": true,
+  },
+  "extends": "eslint:recommended",
+  "globals": {
+    "_": false,
+    "Backbone": false,
+    "chai": false,
+    "console": false,
+    "loop": true,
+    "MozActivity": false,
+    "RTCSessionDescription": false,
+    "OT": false,
+    "performance": false,
+    "Promise": false,
+    "React": false,
+    "sinon": false
+  },
+  "rules": {
+    // turn off all kinds of stuff that we actually do want, because
+    // right now, we're bootstrapping the linting infrastructure.  We'll
+    // want to audit these rules, and start turning them on and fixing the
+    // problems they find, one at a time.
+
+    // Eslint built-in rules are documented at <http://eslint.org/docs/rules/>
+    "array-bracket-spacing": [2, "never"],
+    "block-spacing": [2, "always"],
+    "callback-return": 0,         // TBD
+    "camelcase": 0,               // TODO: set to 2
+    "comma-spacing": 2,
+    "comma-style": 2,
+    "computed-property-spacing": [2, "never"],
+    "consistent-return": 2,
+    "curly": [2, "all"],
+    "dot-location": [2, "property"],
+    "eol-last": 2,
+    "eqeqeq": [2, "smart"],
+    "jsx-quotes": [2, "prefer-double"],
+    "key-spacing": [2, {"beforeColon": false, "afterColon": true }],
+    "linebreak-style": [2, "unix"],
+    "new-cap": 0,                 // TODO: set to 2
+    "new-parens": 2,
+    "no-alert": 2,
+    "no-array-constructor": 2,
+    "no-caller": 2,
+    "no-catch-shadow": 2,
+    "no-class-assign": 2,
+    "no-const-assign": 2,
+    "no-console": 0,              // Leave as 0. We use console logging in content code.
+    "no-empty": 2,
+    "no-empty-label": 2,
+    "no-eval": 2,
+    "no-extend-native": 2, // XXX
+    "no-extra-bind": 0,           // Leave as 0
+    "no-extra-parens": 0,         // TODO: (bug?) [2, "functions"],
+    "no-extra-semi": 2,
+    "no-implied-eval": 2,
+    "no-invalid-this": 0,         // TBD
+    "no-iterator": 2,
+    "no-label-var": 2,
+    "no-labels": 2,
+    "no-lone-blocks": 2,
+    "no-loop-func": 2,
+    "no-mixed-spaces-and-tabs": 2,
+    "no-multi-spaces": 2,
+    "no-multi-str": 2,
+    "no-multiple-empty-lines": 2,
+    "no-native-reassign": 2,
+    "no-new": 2,
+    "no-new-func": 2,
+    "no-new-object": 2,
+    "no-new-wrappers": 2,
+    "no-octal-escape": 2,
+    "no-process-exit": 2,
+    "no-proto": 2,
+    "no-return-assign": 2,
+    "no-script-url": 2,
+    "no-sequences": 2,
+    "no-shadow": 2,
+    "no-shadow-restricted-names": 2,
+    "no-spaced-func": 2,
+    "no-trailing-spaces": 2,
+    "no-undef-init": 2,
+    "no-underscore-dangle": 0,    // Leave as 0. Commonly used for private variables.
+    "no-unexpected-multiline": 2,
+    "no-unneeded-ternary": 2,
+    "no-unused-expressions": 0,   // TODO: Set to 2
+    "no-unused-vars": 0,          // TODO: Set to 2
+    "no-use-before-define": 0,    // TODO: Set to 2
+    "no-useless-call": 2,
+    "no-with": 2,
+    "object-curly-spacing": [2, "always"],
+    "operator-assignment": [2, "always"],
+    "quotes": [2, "double", "avoid-escape"],
+    "semi": 2,
+    "semi-spacing": [2, {"before": false, "after": true}],
+    "space-after-keywords": 2,
+    "space-before-blocks": 2,
+    "space-before-function-paren": [2, "never"],
+    "space-before-keywords": 2,
+    "space-infix-ops": 2,
+    "space-in-parens": [2, "never"],
+    "space-return-throw-case": 2,
+    "space-unary-ops": [2, {"words": true, "nonwords": false}],
+    "spaced-comment": [2, "always"],
+    "strict": [2, "function"],
+    "yoda": [2, "never"],
+    // eslint-plugin-react rules. These are documented at
+    // <https://github.com/yannickcr/eslint-plugin-react#list-of-supported-rules>
+    "react/jsx-no-undef": 2,
+    "react/jsx-sort-props": 2,
+    "react/jsx-sort-prop-types": 2,
+    "react/jsx-uses-vars": 2,
+    "react/jsx-no-duplicate-props": 2,
+    "react/no-did-mount-set-state": 2,
+    "react/no-did-update-set-state": 2,
+    "react/no-unknown-property": 2,
+    "react/prop-types": 2,
+    "react/self-closing-comp": 2,
+    "react/wrap-multilines": 2,
+    "react/jsx-curly-spacing": [2, "never"],
+    // Not worth it: React is defined globally
+    "react/jsx-uses-react": 0,
+    "react/react-in-jsx-scope": 0,
+    // These ones we don't want to ever enable
+    "react/display-name": 0,
+    "react/jsx-boolean-value": 0,
+    "react/no-danger": 0,
+    "react/no-multi-comp": 0
+  }
 }
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -3023,18 +3023,20 @@ this.AddonManager = {
   // Installed in this profile.
   SCOPE_PROFILE: 1,
   // Installed for all of this user's profiles.
   SCOPE_USER: 2,
   // Installed and owned by the application.
   SCOPE_APPLICATION: 4,
   // Installed for all users of the computer.
   SCOPE_SYSTEM: 8,
+  // Installed temporarily
+  SCOPE_TEMPORARY: 16,
   // The combination of all scopes.
-  SCOPE_ALL: 15,
+  SCOPE_ALL: 31,
 
   // 1-15 are different built-in views for the add-on type
   VIEW_TYPE_LIST: "list",
 
   TYPE_UI_HIDE_EMPTY: 16,
   // Indicates that this add-on type supports the ask-to-activate state.
   // That is, add-ons of this type can be set to be optionally enabled
   // on a case-by-case basis.
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -198,17 +198,17 @@ function loadView(aViewId) {
     gViewController.loadInitialView(aViewId);
   } else {
     gViewController.loadView(aViewId);
   }
 }
 
 function isCorrectlySigned(aAddon) {
   // temporary add-ons do not require signing
-  if (aAddon.isTemporary)
+  if (aAddon.scope == AddonManager.SCOPE_TEMPORARY)
       return true;
   if (aAddon.signedState <= AddonManager.SIGNEDSTATE_MISSING)
     return false;
   if (aAddon.foreignInstall && aAddon.signedState < AddonManager.SIGNEDSTATE_SIGNED)
     return false;
   return true;
 }
 
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -139,16 +139,17 @@ const KEY_APP_FEATURES                = 
 
 const KEY_APP_PROFILE                 = "app-profile";
 const KEY_APP_SYSTEM_ADDONS           = "app-system-addons";
 const KEY_APP_SYSTEM_DEFAULTS         = "app-system-defaults";
 const KEY_APP_GLOBAL                  = "app-global";
 const KEY_APP_SYSTEM_LOCAL            = "app-system-local";
 const KEY_APP_SYSTEM_SHARE            = "app-system-share";
 const KEY_APP_SYSTEM_USER             = "app-system-user";
+const KEY_APP_TEMPORARY               = "app-temporary";
 
 const NOTIFICATION_FLUSH_PERMISSIONS  = "flush-pending-permissions";
 const XPI_PERMISSION                  = "install";
 
 const RDFURI_INSTALL_MANIFEST_ROOT    = "urn:mozilla:install-manifest";
 const PREFIX_NS_EM                    = "http://www.mozilla.org/2004/em-rdf#";
 
 const TOOLKIT_ID                      = "toolkit@mozilla.org";
@@ -640,22 +641,25 @@ function applyBlocklistChanges(aOldAddon
  *         The add-on to check
  * @return true if the add-on should run in safe mode
  */
 function canRunInSafeMode(aAddon) {
   // Even though the updated system add-ons aren't generally run in safe mode we
   // include them here so their uninstall functions get called when switching
   // back to the default set.
 
-  // TODO product should make the call whether temporary add-ons can
-  // run in safe mode - assume for now they cannot
-  if (aAddon.isTemporary)
-    return false;
-
-  return aAddon._installLocation.name == KEY_APP_SYSTEM_DEFAULTS ||
+  // TODO product should make the call about temporary add-ons running
+  // in safe mode. assuming for now that they are.
+
+  // FIXME not sure why temp addons only have location not _installLocation
+  // after restart..
+  if (aAddon.location == KEY_APP_TEMPORARY)
+    return true;
+
+  return aAddon._installLocation.name == KEY_APP_SYSTEM_DEFAULTS &&
          aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS;
 }
 
 /**
  * Calculates whether an add-on should be appDisabled or not.
  *
  * @param  aAddon
  *         The add-on to check
@@ -665,25 +669,24 @@ function isUsableAddon(aAddon) {
   // Hack to ensure the default theme is always usable
   if (aAddon.type == "theme" && aAddon.internalName == XPIProvider.defaultSkin)
     return true;
 
   if (aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS &&
       aAddon.signedState != AddonManager.SIGNEDSTATE_SYSTEM) {
     return false;
   }
-  // temporary add-ons do not require signing
-  if (!aAddon.isTemporary) {
-      if (aAddon._installLocation.name != KEY_APP_SYSTEM_DEFAULTS &&
-           mustSign(aAddon.type)) {
-        if (aAddon.signedState <= AddonManager.SIGNEDSTATE_MISSING)
-          return false;
-        if (aAddon.foreignInstall && aAddon.signedState < AddonManager.SIGNEDSTATE_SIGNED)
-          return false;
-      }
+  // temporary and system add-ons do not require signing
+  if ((aAddon._installLocation.name != KEY_APP_SYSTEM_DEFAULTS &&
+       aAddon._installLocation.name != KEY_APP_TEMPORARY) &&
+       mustSign(aAddon.type)) {
+    if (aAddon.signedState <= AddonManager.SIGNEDSTATE_MISSING)
+      return false;
+    if (aAddon.foreignInstall && aAddon.signedState < AddonManager.SIGNEDSTATE_SIGNED)
+      return false;
   }
   if (aAddon.blocklistState == Blocklist.STATE_BLOCKED)
     return false;
 
   if (AddonManager.checkUpdateSecurity && !aAddon.providesUpdatesSecurely)
     return false;
 
   if (!aAddon.isPlatformCompatible)
@@ -706,17 +709,16 @@ function isUsableAddon(aAddon) {
         let minCompatVersion = Services.prefs.getCharPref(PREF_CHECKCOMAT_THEMEOVERRIDE);
         if (minCompatVersion &&
             Services.vc.compare(minCompatVersion, app.maxVersion) > 0) {
           return false;
         }
       } catch (e) {}
     }
   }
-
   return true;
 }
 
 XPCOMUtils.defineLazyServiceGetter(this, "gRDF", "@mozilla.org/rdf/rdf-service;1",
                                    Ci.nsIRDFService);
 
 function EM_R(aProperty) {
   return gRDF.GetResource(PREFIX_NS_EM + aProperty);
@@ -3044,25 +3046,22 @@ this.XPIProvider = {
     });
   },
 
   /**
    * Persists changes to XPIProvider.bootstrappedAddons to its store (a pref).
    */
   persistBootstrappedAddons: function XPI_persistBootstrappedAddons() {
     // Experiments are disabled upon app load, so don't persist references.
-    // Temporary add-ons should not persist across restarts.
     let filtered = {};
     for (let id in this.bootstrappedAddons) {
       let entry = this.bootstrappedAddons[id];
       if (entry.type == "experiment") {
         continue;
       }
-      if (entry.isTemporary)
-        continue;
 
       filtered[id] = entry;
     }
 
     Services.prefs.setCharPref(PREF_BOOTSTRAP_ADDONS,
                                JSON.stringify(filtered));
   },
 
@@ -3761,67 +3760,67 @@ this.XPIProvider = {
    *
    * @param aDirectory
    *        The directory containing the unpacked add-on
    *
    * @throws if add-on is not restartless
    * @throws if an add-on with this ID is already temporarily installed
    */
   installTemporaryAddon: function XPI_installTemporaryAddon(aDirectory) {
-    // TODO is this the appropriate scope?
-    // also - do we want to make this a directory install location?
-    let location = new TemporaryInstallLocation();
-    location.isTemporary = true;
-
-    let addon = syncLoadManifestFromFile(aDirectory, location);
-    addon.isTemporary = true;
+    let addon = syncLoadManifestFromFile(aDirectory, TemporaryInstallLocation);
 
     let self = this;
-
-    this.getAddonByID(addon.id,
-                      function installTemporaryAddon_getAddonByID(oldAddon) {
+    XPIDatabase.getVisibleAddonForID(addon.id,
+      function installTemporaryAddon_getVisibleAddonForID(oldAddon) {
       if (oldAddon) {
-        if (oldAddon.isTemporary) {
+        if (oldAddon.location == KEY_APP_TEMPORARY) {
           throw new Error("Add-on with ID " + oldAddon.id + " is already "
                           + "temporarily installed");
         } else {
           logger.warn("Addon with ID " + oldAddon.id + " already installed,"
-                      + " disabling");
-          oldAddon.userDisabled = true;
+                      + " older version will be disabled");
+
+          XPIProvider.callBootstrapMethod(oldAddon, oldAddon._sourceBundle,
+                                          "shutdown",
+                                          BOOTSTRAP_REASONS.ADDON_UPGRADE);
+
+          // TODO not sure if this is needed, aped from the uninstall process
+          self.unloadBootstrapScope(addon.id);
+          flushStartupCache();
+
+          AddonManagerPrivate.callAddonListeners("onInstalled", createWrapper(addon));
         }
       }
 
-      if (!self.addon.bootstrap) {
-        throw new Error("Only restartless (bootstrap) add-ons are supported");
-      }
-
-      let file = self.addon._sourceBundle;
-      let wrapper = createWrapper(self.addon);
-
-      AddonManagerPrivate.callAddonListeners("onInstalling", wrapper);
-      XPIProvider.callBootstrapMethod(self.addon, file, "install",
-                                        BOOTSTRAP_REASONS.ADDON_INSTALL);
+      if (!addon.bootstrap) {
+        throw new Error("Only restartless (bootstrap) add-ons can be"
+                        + " temporarily installed");
+      }
+
+      let file = addon._sourceBundle;
+
+      AddonManagerPrivate.callAddonListeners("onInstalling", createWrapper(addon));
+      XPIProvider.callBootstrapMethod(addon, file, "install",
+                                      BOOTSTRAP_REASONS.ADDON_INSTALL);
+      logger.debug("Install of temporary addon in " + aDirectory.path + " completed.");
+      addon.visible = true;
+      addon.enabled = true;
+      addon.active = true;
+
+      addon = XPIDatabase.addAddonMetadata(addon, file.persistentDescriptor);
+
+      XPIStates.addAddon(addon);
+      XPIDatabase.saveChanges();
+
+      let wrapper = createWrapper(addon);
+      addon.state = AddonManager.STATE_INSTALLED;
       AddonManagerPrivate.callAddonListeners("onInstalled", wrapper);
 
-      logger.debug("Install of temporary addon in " + aDirectory.path + " completed.");
-      self.addon.visible = true;
-      self.addon.enabled = true;
-      self.addon.active = true;
-
-      self.addon = XPIDatabase.addAddonMetadata(self.addon, file.persistentDescriptor);
-
-      XPIStates.addAddon(self.addon);
-      // TODO
-      //self.addon.installDate = self.addon.updateDate;
-      XPIDatabase.saveChanges();
-
-      wrapper = createWrapper(self.addon);
-      self.state = AddonManager.STATE_INSTALLED;
       AddonManagerPrivate.callAddonListeners("onEnabling", wrapper, false);
-      XPIProvider.callBootstrapMethod(self.addon, file, "startup",
+      XPIProvider.callBootstrapMethod(addon, file, "startup",
                                       BOOTSTRAP_REASONS.ADDON_ENABLE);
       AddonManagerPrivate.callInstallListeners("onExternalInstall", null, wrapper);
     });
   },
 
   /**
    * Removes an AddonInstall from the list of active installs.
    *
@@ -4315,26 +4314,24 @@ this.XPIProvider = {
    * @param  aMultiprocessCompatible
    *         Boolean indicating whether the add-on is compatible with electrolysis.
    * @param  aRunInSafeMode
    *         Boolean indicating whether the add-on can run in safe mode.
    * @return a JavaScript scope
    */
   loadBootstrapScope: function XPI_loadBootstrapScope(aId, aFile, aVersion, aType,
                                                       aMultiprocessCompatible,
-                                                      aRunInSafeMode,
-                                                      isTemporary) {
+                                                      aRunInSafeMode) {
     // Mark the add-on as active for the crash reporter before loading
     this.bootstrappedAddons[aId] = {
       version: aVersion,
       type: aType,
       descriptor: aFile.persistentDescriptor,
       multiprocessCompatible: aMultiprocessCompatible,
       runInSafeMode: aRunInSafeMode,
-      isTemporary: isTemporary,
     };
     this.persistBootstrappedAddons();
     this.addAddonsToCrashReporter();
 
     // Locales only contain chrome and can't have bootstrap scripts
     if (aType == "locale") {
       this.bootstrapScopes[aId] = null;
       return;
@@ -4466,17 +4463,17 @@ this.XPIProvider = {
       Components.manager.addBootstrappedManifestLocation(aFile);
     }
 
     try {
       // Load the scope if it hasn't already been loaded
       if (!(aAddon.id in this.bootstrapScopes)) {
         this.loadBootstrapScope(aAddon.id, aFile, aAddon.version, aAddon.type,
                                 aAddon.multiprocessCompatible || false,
-                                runInSafeMode, aAddon.isTemporary);
+                                runInSafeMode);
       }
 
       // Nothing to call for locales
       if (aAddon.type == "locale")
         return;
 
       let method = undefined;
       try {
@@ -6345,17 +6342,16 @@ AddonInternal.prototype = {
   active: false,
   visible: false,
   userDisabled: false,
   appDisabled: false,
   softDisabled: false,
   sourceURI: null,
   releaseNotesURI: null,
   foreignInstall: false,
-  isTemporary: false,
 
   get selectedLocale() {
     if (this._selectedLocale)
       return this._selectedLocale;
     let locale = Locale.findClosestLocale(this.locales);
     this._selectedLocale = locale ? locale : this.defaultLocale;
     return this._selectedLocale;
   },
@@ -6673,17 +6669,17 @@ function AddonWrapper(aAddon) {
 
     return [objValue, false];
   }
 
   ["id", "syncGUID", "version", "isCompatible", "isPlatformCompatible",
    "providesUpdatesSecurely", "blocklistState", "blocklistURL", "appDisabled",
    "softDisabled", "skinnable", "size", "foreignInstall", "hasBinaryComponents",
    "strictCompatibility", "compatibilityOverrides", "updateURL",
-   "getDataDirectory", "multiprocessCompatible", "signedState", "isTemporary"].forEach(function(aProp) {
+   "getDataDirectory", "multiprocessCompatible", "signedState"].forEach(function(aProp) {
      this.__defineGetter__(aProp, function AddonWrapper_propertyGetter() {
        return aAddon[aProp];
      });
   }, this);
 
   this.__defineGetter__("type", () => getExternalType(aAddon.type));
 
   ["fullDescription", "developerComments", "eula", "supportURL",
@@ -6952,17 +6948,17 @@ function AddonWrapper(aAddon) {
       // If an add-on is pending uninstall then we ignore any other pending
       // operations
       return AddonManager.PENDING_UNINSTALL;
     }
 
     // Extensions have an intentional inconsistency between what the DB says is
     // enabled and what we say to the ouside world. so we need to cover up that
     // lie here as well.
-    if (aAddon.type != "experiment" && !aAddon.isTemporary) {
+    if (aAddon.type != "experiment") {
       if (aAddon.active && aAddon.disabled)
         pending |= AddonManager.PENDING_DISABLE;
       else if (!aAddon.active && !aAddon.disabled)
         pending |= AddonManager.PENDING_ENABLE;
     }
 
     if (aAddon.pendingUpgrade)
       pending |= AddonManager.PENDING_UPGRADE;
@@ -7060,16 +7056,20 @@ function AddonWrapper(aAddon) {
       if (!aAddon.userDisabled)
         aAddon.softDisabled = val;
     }
 
     return val;
   });
 
   this.__defineGetter__("hidden", function AddonWrapper_hidden() {
+    // FIXME temporary add-ons don't have _installLocation :/
+    if (aAddon.location == KEY_APP_TEMPORARY)
+      return false;
+
     return (aAddon._installLocation.name == KEY_APP_SYSTEM_DEFAULTS ||
             aAddon._installLocation.name == KEY_APP_SYSTEM_ADDONS);
   });
 
   this.isCompatibleWith = function AddonWrapper_isCompatiblewith(aAppVersion, aPlatformVersion) {
     return aAddon.isCompatibleWith(aAppVersion, aPlatformVersion);
   };
 
@@ -7315,24 +7315,16 @@ DirectoryInstallLocation.prototype = {
 
   /**
    * Gets the scope of this install location.
    */
   get scope() {
     return this._scope;
   },
 
-  get isTemporary() {
-    return this._temporaryAddon;
-  },
-
-  set isTemporary(isTemporary) {
-    this._temporaryAddon = isTemporary;
-  },
-
   /**
    * Gets an array of nsIFiles for add-ons installed in this location.
    */
   getAddonLocations: function() {
     let locations = new Map();
     for (let id in this._IDToFileMap) {
       locations.set(id, this._IDToFileMap[id].clone());
     }
@@ -7901,27 +7893,27 @@ Object.assign(SystemAddonInstallLocation
 
     this._saveAddonSet(state);
     this._nextDir = newDir;
   }),
 });
 
 /**
  * An object which identifies a directory install location for temporary
- * add-ons. The
- * location consists of a directory which contains the add-ons installed in the
- * location.
+ * add-ons.
  */
 const TemporaryInstallLocation = {
   locked: false,
-  name: "app-temporary":
-  scope: SCOPE_USER,
-  getAddonLocations(),
+  name: KEY_APP_TEMPORARY,
+  // FIXME some callers use _name, should they be fixed to use name instead?
+  _name: KEY_APP_TEMPORARY,
+  scope: AddonManager.SCOPE_TEMPORARY,
+  getAddonLocations: () => [],
   isLinkedAddon: () => false,
-  installAddon()
+  installAddon: () => {},
 }
 
 #ifdef XP_WIN
 /**
  * An object that identifies a registry install location for add-ons. The location
  * consists of a registry key which contains string values mapping ID to the
  * path where an add-on is installed
  *
--- a/toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
@@ -1377,41 +1377,88 @@ function run_test_28() {
     }));
    });
   });
 }
 
 // Tests that installing a temporary add-on ignores signature requirements
 // and does not persist beyond a restart
 function run_test_29() {
-  let TEST_ID = "unpacked-enabled@tests.mozilla.org";
+  let TEST_ID = "temporary@tests.mozilla.org";
 
-  writeInstallRDFToDir({
+  writeInstallRDFToXPI({
         id: TEST_ID,
-    version: "1.0",
-    bootstrap: true,
-    unpack: true,
-    targetApplications: [{
-          id: "xpcshell@tests.mozilla.org",
-      minVersion: "1",
-      maxVersion: "1"
+        version: "1.0",
+        bootstrap: true,
+        targetApplications: [{
+                id: "xpcshell@tests.mozilla.org",
+          minVersion: "1",
+          maxVersion: "1"
         }],
-    name: "Unpacked, Enabled",
-  }, profileDir, null, "extraFile.js");
+        name: "Packed, Enabled",
+  }, profileDir);
+
+  let packed_addon = profileDir.clone();
+  packed_addon.append(TEST_ID + ".xpi");
+
+  installAllFiles([packed_addon], function() {
+
+    AddonManager.getAddonByID(TEST_ID, function(b1) {
+      do_check_neq(b1, null);
+      do_check_false(b1.userDisabled);
+      do_check_eq(b1.version, "1.0");
+      do_check_true(b1.isActive);
+      do_check_eq(b1.pendingOperations, AddonManager.PENDING_NONE);
+    });
 
-  let unpacked_addon = profileDir.clone();
-  unpacked_addon.append(TEST_ID);
+    writeInstallRDFToDir({
+          id: TEST_ID,
+      version: "2.0",
+      bootstrap: true,
+      unpack: true,
+      targetApplications: [{
+            id: "xpcshell@tests.mozilla.org",
+        minVersion: "1",
+        maxVersion: "1"
+          }],
+      name: "Unpacked, Enabled",
+    }, profileDir, null, "extraFile.js");
+
+    let unpacked_addon = profileDir.clone();
+    unpacked_addon.append(TEST_ID);
+
+    AddonManager.installTemporaryAddon(FileUtils.File(unpacked_addon.path));
 
-  AddonManager.installTemporaryAddon(FileUtils.File(unpacked_addon.path));
-  AddonManager.getAddonByID(TEST_ID, function(b1) {
-    do_check_neq(b1, null);
-    do_check_false(b1.userDisabled);
-    do_check_eq(b1.version, "1.0");
-    do_check_true(b1.isActive);
-    do_check_eq(b1.pendingOperations, AddonManager.PENDING_NONE);
+    // attempting to install another temporary add-on with the same ID
+    // throws an Error
+    try {
+      AddonManager.installTemporaryAddon(FileUtils.File(unpacked_addon.path));
+    }
+    catch (e) {
+      do_check_true(e instanceof Error);
+    }
+
+    AddonManager.getAddonByID(TEST_ID, function(tempAddon) {
+      do_check_neq(tempAddon, null);
+      do_check_false(tempAddon.userDisabled);
+      do_check_eq(tempAddon.version, "2.0");
+      do_check_true(tempAddon.isActive);
+      do_check_eq(tempAddon.pendingOperations, AddonManager.PENDING_NONE);
+    });
 
+    // after restarting, we expect the temporary add-on to be disabled
+    // and the normal add-on with the same name to be re-enabled
     restartManager();
+    AddonManager.getAddonByID(TEST_ID, function(addon) {
 
-    BootstrapMonitor.checkAddonNotInstalled(TEST_ID, "1.0");
+      // FIXME 2.0 is returned here, should it be?
+      if (addon.version == "1.0") {
+        do_check_neq(addon, null);
+        do_check_false(addon.userDisabled);
+        do_check_eq(addon.version, "1.0");
+        do_check_true(addon.isActive);
+        do_check_eq(addon.pendingOperations, AddonManager.PENDING_NONE);
+      }
 
-    do_check_bootstrappedPref(do_test_finished);
+      do_check_bootstrappedPref(do_test_finished);
+    });
   });
 }