Bug 1315407 Revise tracking of active AddonInstalls r=rhelmer
☠☠ backed out by 8b8c3c3b29c9 ☠ ☠
authorAndrew Swan <aswan@mozilla.com>
Thu, 10 Nov 2016 12:49:27 -0800
changeset 352049 95ef280ccc1bafebc7bdcbf47c399713195e5b62
parent 352048 df489872d561d35b4b416651d338784575de4911
child 352050 7f747c8f6c60d87ab282d79899e9f10655d8ec82
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhelmer
bugs1315407
milestone52.0a1
Bug 1315407 Revise tracking of active AddonInstalls r=rhelmer MozReview-Commit-ID: Is3RMjSN6Bw
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -2736,17 +2736,17 @@ this.XPIProvider = {
       XPIProvider.installLocationsByName[location.name] = location;
     }
 
     try {
       AddonManagerPrivate.recordTimestamp("XPI_startup_begin");
 
       logger.debug("startup");
       this.runPhase = XPI_STARTING;
-      this.installs = [];
+      this.installs = new Set();
       this.installLocations = [];
       this.installLocationsByName = {};
       // Hook for tests to detect when saving database at shutdown time fails
       this._shutdownError = null;
       // Clear this at startup for xpcshell test restarts
       this._telemetryDetails = {};
       // Register our details structure with AddonManager
       AddonManagerPrivate.setTelemetryDetails("XPI", this._telemetryDetails);
@@ -4197,23 +4197,17 @@ this.XPIProvider = {
 
   /**
    * Removes an AddonInstall from the list of active installs.
    *
    * @param  install
    *         The AddonInstall to remove
    */
   removeActiveInstall: function(aInstall) {
-    let where = this.installs.indexOf(aInstall);
-    if (where == -1) {
-      logger.warn("removeActiveInstall: could not find active install for "
-          + aInstall.sourceURI.spec);
-      return;
-    }
-    this.installs.splice(where, 1);
+    this.installs.delete(aInstall);
   },
 
   /**
    * Called to get an Addon with a particular ID.
    *
    * @param  aId
    *         The ID of the add-on to retrieve
    * @param  aCallback
@@ -4282,17 +4276,17 @@ this.XPIProvider = {
    * types.
    *
    * @param  aTypes
    *         An array of types or null to get all types
    * @param  aCallback
    *         A callback to pass the array of AddonInstalls to
    */
   getInstallsByTypes: function(aTypes, aCallback) {
-    let results = this.installs.slice(0);
+    let results = [...this.installs];
     if (aTypes) {
       results = results.filter(install => {
         return aTypes.includes(getExternalType(install.type));
       });
     }
 
     aCallback(results.map(install => install.wrapper));
   },
@@ -5401,16 +5395,18 @@ function AddonInstall(aInstallLocation, 
   this.listeners = [];
   this.icons = {};
   this.existingAddon = aExistingAddon;
   this.error = 0;
   this.window = aBrowser ? aBrowser.contentWindow : null;
 
   // Giving each instance of AddonInstall a reference to the logger.
   this.logger = logger;
+
+  XPIProvider.installs.add(this);
 }
 
 AddonInstall.prototype = {
   installLocation: null,
   wrapper: null,
   stream: null,
   crypto: null,
   originalHash: null,
@@ -5448,42 +5444,46 @@ AddonInstall.prototype = {
    */
   initStagedInstall: function(aManifest) {
     this.name = aManifest.name;
     this.type = aManifest.type;
     this.version = aManifest.version;
     this.icons = aManifest.icons;
     this.releaseNotesURI = aManifest.releaseNotesURI ?
                            NetUtil.newURI(aManifest.releaseNotesURI) :
-                           null
+                           null;
     this.sourceURI = aManifest.sourceURI ?
                      NetUtil.newURI(aManifest.sourceURI) :
                      null;
-    this.file = null;
+    this.file = this.sourceURI;
+    this._sourceBundle = this.sourceURI;
     this.addon = aManifest;
+    this.addon.sourceURI = this.sourceURI;
 
     this.state = AddonManager.STATE_INSTALLED;
 
-    XPIProvider.installs.push(this);
+    XPIProvider.installs.add(this);
+    return this;
   },
 
   /**
    * Initialises this install to be an install from a local file.
    *
    * @param  aCallback
    *         The callback to pass the initialised AddonInstall to
    */
   initLocalInstall: function(aCallback) {
     aCallback = makeSafe(aCallback);
     this.file = this.sourceURI.QueryInterface(Ci.nsIFileURL).file;
 
     if (!this.file.exists()) {
       logger.warn("XPI file " + this.file.path + " does not exist");
       this.state = AddonManager.STATE_DOWNLOAD_FAILED;
       this.error = AddonManager.ERROR_NETWORK_FAILURE;
+      XPIProvider.removeActiveInstall(this);
       aCallback(this);
       return;
     }
 
     this.state = AddonManager.STATE_DOWNLOADED;
     this.progress = this.file.fileSize;
     this.maxProgress = this.file.fileSize;
 
@@ -5492,30 +5492,32 @@ AddonInstall.prototype = {
                    createInstance(Ci.nsICryptoHash);
       try {
         crypto.initWithString(this.hash.algorithm);
       }
       catch (e) {
         logger.warn("Unknown hash algorithm '" + this.hash.algorithm + "' for addon " + this.sourceURI.spec, e);
         this.state = AddonManager.STATE_DOWNLOAD_FAILED;
         this.error = AddonManager.ERROR_INCORRECT_HASH;
+        XPIProvider.removeActiveInstall(this);
         aCallback(this);
         return;
       }
 
       let fis = Cc["@mozilla.org/network/file-input-stream;1"].
                 createInstance(Ci.nsIFileInputStream);
       fis.init(this.file, -1, -1, false);
       crypto.updateFromStream(fis, this.file.fileSize);
       let calculatedHash = getHashStringForCrypto(crypto);
       if (calculatedHash != this.hash.data) {
         logger.warn("File hash (" + calculatedHash + ") did not match provided hash (" +
              this.hash.data + ")");
         this.state = AddonManager.STATE_DOWNLOAD_FAILED;
         this.error = AddonManager.ERROR_INCORRECT_HASH;
+        XPIProvider.removeActiveInstall(this);
         aCallback(this);
         return;
       }
     }
 
     this.loadManifest(this.file).then(() => {
       XPIDatabase.getVisibleAddonForID(this.addon.id, aAddon => {
         this.existingAddon = aAddon;
@@ -5525,38 +5527,37 @@ AddonInstall.prototype = {
         this.addon.installDate = aAddon ? aAddon.installDate : this.addon.updateDate;
 
         if (!this.addon.isCompatible) {
           // TODO Should we send some event here?
           this.state = AddonManager.STATE_CHECKING;
           new UpdateChecker(this.addon, {
             onUpdateFinished: aAddon => {
               this.state = AddonManager.STATE_DOWNLOADED;
-              XPIProvider.installs.push(this);
               AddonManagerPrivate.callInstallListeners("onNewInstall",
                                                        this.listeners,
                                                        this.wrapper);
 
               aCallback(this);
             }
           }, AddonManager.UPDATE_WHEN_ADDON_INSTALLED);
         }
         else {
-          XPIProvider.installs.push(this);
           AddonManagerPrivate.callInstallListeners("onNewInstall",
                                                    this.listeners,
                                                    this.wrapper);
 
           aCallback(this);
         }
       });
     }, ([error, message]) => {
       logger.warn("Invalid XPI", message);
       this.state = AddonManager.STATE_DOWNLOAD_FAILED;
       this.error = error;
+      XPIProvider.removeActiveInstall(this);
       AddonManagerPrivate.callInstallListeners("onNewInstall",
                                                this.listeners,
                                                this.wrapper);
 
       aCallback(this);
     });
   },
 
@@ -5578,17 +5579,16 @@ AddonInstall.prototype = {
     this.state = AddonManager.STATE_AVAILABLE;
     this.name = aName;
     this.type = aType;
     this.version = aVersion;
     this.icons = aIcons;
     this.progress = 0;
     this.maxProgress = -1;
 
-    XPIProvider.installs.push(this);
     AddonManagerPrivate.callInstallListeners("onNewInstall", this.listeners,
                                              this.wrapper);
 
     makeSafe(aCallback)(this);
   },
 
   /**
    * Starts installation of this add-on from whatever state it is currently at
@@ -5608,17 +5608,16 @@ AddonInstall.prototype = {
     case AddonManager.STATE_INSTALL_FAILED:
     case AddonManager.STATE_CANCELLED:
       this.removeTemporaryFile();
       this.state = AddonManager.STATE_AVAILABLE;
       this.error = 0;
       this.progress = 0;
       this.maxProgress = -1;
       this.hash = this.originalHash;
-      XPIProvider.installs.push(this);
       this.startDownload();
       break;
     case AddonManager.STATE_POSTPONED:
       logger.debug(`Postponing install of ${this.addon.id}`);
       break;
     case AddonManager.STATE_DOWNLOADING:
     case AddonManager.STATE_CHECKING:
     case AddonManager.STATE_INSTALLING:
@@ -6666,16 +6665,17 @@ AddonInstall.createInstall = function(aC
   let url = Services.io.newFileURI(aFile);
 
   try {
     let install = new AddonInstall(aLocation, url);
     install.initLocalInstall(aCallback);
   }
   catch (e) {
     logger.error("Error creating install", e);
+    XPIProvider.removeActiveInstall(this);
     makeSafe(aCallback)(null);
   }
 };
 
 /**
  * Creates a new AddonInstall to download and install a URL.
  *
  * @param  aCallback