Bug 680802: Installing a new add-on at the same time as installing a new version of the app which uses new properties from install.rdf will fail. r=robstrong

toolkit/mozapps/extensions/XPIProvider.jsm

@@ -1971,12 +1971,24 @@ var XPIProvider = {
         aManifests[aLocation.name][id] = null;
         let existingAddonID = id;
 
-        // Check for a cached AddonInternal for this add-on, it may contain
-        // updated compatibility information
         let jsonfile = stagingDir.clone();
         jsonfile.append(id + ".json");
+
+        try {
+          aManifests[aLocation.name][id] = loadManifestFromFile(stageDirEntry);
+        }
+        catch (e) {
+          ERROR("Unable to read add-on manifest from " + stageDirEntry.path, e);
+          // This add-on can't be installed so just remove it now
+          seenFiles.push(stageDirEntry.leafName);
+          seenFiles.push(jsonfile.leafName);
+          continue;
+        }
+
+        // Check for a cached metadata for this add-on, it may contain updated
+        // compatibility information
         if (jsonfile.exists()) {
-          LOG("Found updated manifest for " + id + " in " + aLocation.name);
+          LOG("Found updated metadata for " + id + " in " + aLocation.name);
           let fis = Cc["@mozilla.org/network/file-input-stream;1"].
                        createInstance(Ci.nsIFileInputStream);
           let json = Cc["@mozilla.org/dom/json;1"].
@@ -1984,13 +1996,14 @@ var XPIProvider = {
 
           try {
             fis.init(jsonfile, -1, 0, 0);
-            let addonObj = json.decodeFromStream(fis, jsonfile.fileSize);
-            aManifests[aLocation.name][id] = new AddonInternal();
-            aManifests[aLocation.name][id].fromJSON(addonObj);
-            existingAddonID = aManifests[aLocation.name][id].existingAddonID || id;
+            let metadata = json.decodeFromStream(fis, jsonfile.fileSize);
+            aManifests[aLocation.name][id].importMetadata(metadata);
           }
           catch (e) {
-            ERROR("Unable to read add-on manifest from " + jsonfile.path, e);
+            // If some data can't be recovered from the cached metadata then it
+            // is unlikely to be a problem big enough to justify throwing away
+            // the install, just log and error and continue
+            ERROR("Unable to read metadata from " + jsonfile.path, e);
           }
           finally {
             fis.close();
@@ -1998,19 +2011,7 @@ var XPIProvider = {
         }
         seenFiles.push(jsonfile.leafName);
 
-        // If there was no cached AddonInternal then load it directly
-        if (!aManifests[aLocation.name][id]) {
-          try {
-            aManifests[aLocation.name][id] = loadManifestFromFile(stageDirEntry);
-            existingAddonID = aManifests[aLocation.name][id].existingAddonID || id;
-          }
-          catch (e) {
-            ERROR("Unable to read add-on manifest from " + stageDirEntry.path, e);
-            // This add-on can't be installed so just remove it now
-            seenFiles.push(stageDirEntry.leafName);
-            continue;
-          }
-        }
+        existingAddonID = aManifests[aLocation.name][id].existingAddonID || id;
 
         var oldBootstrap = null;
         LOG("Processing install of " + id + " in " + aLocation.name);
@@ -6913,15 +6914,25 @@ AddonInternal.prototype = {
   },
 
   /**
-   * fromJSON should be called to set the properties of this AddonInternal to
-   * those from the passed in object. It is essentially the inverse of toJSON.
+   * When an add-on install is pending its metadata will be cached in a file.
+   * This method reads particular properties of that metadata that may be newer
+   * than that in the install manifest, like compatibility information.
    *
    * @param  aObj
-   *         A JS object containing properties to be set on this AddonInternal
+   *         A JS object containing the cached metadata
    */
-  fromJSON: function(aObj) {
-    for (let prop in aObj)
-      this[prop] = aObj[prop];
+  importMetadata: function(aObj) {
+    ["targetApplications", "userDisabled", "softDisabled", "existingAddonID",
+     "sourceURI", "releaseNotesURI", "installDate", "updateDate",
+     "applyBackgroundUpdates"].forEach(function(aProp) {
+      if (!(aProp in aObj))
+        return;
+
+      this[aProp] = aObj[aProp];
+    }, this);
+
+    // Compatibility info may have changed so update appDisabled
+    this.appDisabled = !isUsableAddon(this);
   }
 };
 

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

@@ -114,6 +114,34 @@ function run_test_1() {
       Services.prefs.setIntPref("extensions.databaseSchema", 1);
       db.close();
 
+      let jsonfile = gProfD.clone();
+      jsonfile.append("extensions");
+      jsonfile.append("staged");
+      jsonfile.append("addon3@tests.mozilla.org.json");
+      do_check_true(jsonfile.exists());
+
+      // Remove an unnecessary property from the cached manifest
+      let fis = AM_Cc["@mozilla.org/network/file-input-stream;1"].
+                   createInstance(AM_Ci.nsIFileInputStream);
+      let json = AM_Cc["@mozilla.org/dom/json;1"].
+                 createInstance(AM_Ci.nsIJSON);
+      fis.init(jsonfile, -1, 0, 0);
+      let addonObj = json.decodeFromStream(fis, jsonfile.fileSize);
+      fis.close();
+      delete addonObj.optionsType;
+
+      let stream = AM_Cc["@mozilla.org/network/file-output-stream;1"].
+                   createInstance(AM_Ci.nsIFileOutputStream);
+      let converter = AM_Cc["@mozilla.org/intl/converter-output-stream;1"].
+                      createInstance(AM_Ci.nsIConverterOutputStream);
+      stream.init(jsonfile, FileUtils.MODE_WRONLY | FileUtils.MODE_CREATE |
+                            FileUtils.MODE_TRUNCATE, FileUtils.PERMS_FILE,
+                            0);
+      converter.init(stream, "UTF-8", 0, 0x0000);
+      converter.writeString(JSON.stringify(addonObj));
+      converter.close();
+      stream.close();
+
       startupManager(false);
 
       AddonManager.getAddonsByIDs(["addon1@tests.mozilla.org",
@@ -221,6 +249,34 @@ function run_test_2() {
       Services.prefs.setIntPref("extensions.databaseSchema", 1);
       db.close();
 
+      let jsonfile = gProfD.clone();
+      jsonfile.append("extensions");
+      jsonfile.append("staged");
+      jsonfile.append("addon3@tests.mozilla.org.json");
+      do_check_true(jsonfile.exists());
+
+      // Remove an unnecessary property from the cached manifest
+      let fis = AM_Cc["@mozilla.org/network/file-input-stream;1"].
+                   createInstance(AM_Ci.nsIFileInputStream);
+      let json = AM_Cc["@mozilla.org/dom/json;1"].
+                 createInstance(AM_Ci.nsIJSON);
+      fis.init(jsonfile, -1, 0, 0);
+      let addonObj = json.decodeFromStream(fis, jsonfile.fileSize);
+      fis.close();
+      delete addonObj.optionsType;
+
+      let stream = AM_Cc["@mozilla.org/network/file-output-stream;1"].
+                   createInstance(AM_Ci.nsIFileOutputStream);
+      let converter = AM_Cc["@mozilla.org/intl/converter-output-stream;1"].
+                      createInstance(AM_Ci.nsIConverterOutputStream);
+      stream.init(jsonfile, FileUtils.MODE_WRONLY | FileUtils.MODE_CREATE |
+                            FileUtils.MODE_TRUNCATE, FileUtils.PERMS_FILE,
+                            0);
+      converter.init(stream, "UTF-8", 0, 0x0000);
+      converter.writeString(JSON.stringify(addonObj));
+      converter.close();
+      stream.close();
+
       gAppInfo.version = "2";
       startupManager(true);