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 364855 95ef280ccc1bafebc7bdcbf47c399713195e5b62
parent 364854 df489872d561d35b4b416651d338784575de4911
child 364856 7f747c8f6c60d87ab282d79899e9f10655d8ec82
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhelmer
bugs1315407
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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