Bug 1358846: Part 5 - Clean up some path manipulation code. r=rhelmer
☠☠ backed out by ebad7f51280d ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Sun, 23 Apr 2017 10:30:39 -0700
changeset 358053 26825f1e33dd2ef48bef737d97d9ba9af700417f
parent 358052 98b57ff82a5477e3f0c0f880c0a72cc115cdf9af
child 358054 7dcb80a051a3c6d70561b2965795cdc886b6028b
push id31808
push usercbook@mozilla.com
push dateFri, 12 May 2017 12:37:49 +0000
treeherdermozilla-central@030c0a7c8781 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhelmer
bugs1358846
milestone55.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 1358846: Part 5 - Clean up some path manipulation code. r=rhelmer MozReview-Commit-ID: ArgurF3QVSw
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -607,18 +607,17 @@ SafeInstallOperation.prototype = {
 
   _installFile(aFile, aTargetDirectory, aCopy) {
     let oldFile = aCopy ? null : aFile.clone();
     let newFile = aFile.clone();
     try {
       if (aCopy) {
         newFile.copyTo(aTargetDirectory, null);
         // copyTo does not update the nsIFile with the new.
-        newFile = aTargetDirectory.clone();
-        newFile.append(aFile.leafName);
+        newFile = getFile(aFile.leafName, aTargetDirectory);
         // Windows roaming profiles won't properly sync directories if a new file
         // has an older lastModifiedTime than a previous file, so update.
         newFile.lastModifiedTime = Date.now();
       } else {
         newFile.moveTo(aTargetDirectory, null);
       }
     } catch (e) {
       logger.error("Failed to " + (aCopy ? "copy" : "move") + " file " + aFile.path +
@@ -630,18 +629,17 @@ SafeInstallOperation.prototype = {
 
   _installDirectory(aDirectory, aTargetDirectory, aCopy) {
     if (aDirectory.contains(aTargetDirectory)) {
       let err = new Error(`Not installing ${aDirectory} into its own descendent ${aTargetDirectory}`);
       logger.error(err);
       throw err;
     }
 
-    let newDir = aTargetDirectory.clone();
-    newDir.append(aDirectory.leafName);
+    let newDir = getFile(aDirectory.leafName, aTargetDirectory);
     try {
       newDir.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
     } catch (e) {
       logger.error("Failed to create directory " + newDir.path, e);
       throw e;
     }
     this._createdDirs.push(newDir);
 
@@ -772,18 +770,17 @@ SafeInstallOperation.prototype = {
    * state
    */
   rollback() {
     while (this._installedFiles.length > 0) {
       let move = this._installedFiles.pop();
       if (move.isMoveTo) {
         move.newFile.moveTo(move.oldDir.parent, move.oldDir.leafName);
       } else if (move.newFile.isDirectory() && !move.newFile.isSymlink()) {
-        let oldDir = move.oldFile.parent.clone();
-        oldDir.append(move.oldFile.leafName);
+        let oldDir = getFile(move.oldFile.leafName, move.oldFile.parent);
         oldDir.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
       } else if (!move.oldFile) {
         // No old file means this was a copied file
         move.newFile.remove(true);
       } else {
         move.newFile.moveTo(move.oldFile.parent, null);
       }
     }
@@ -985,18 +982,17 @@ function createAddonDetails(id, aAddon) 
  */
 function getExternalType(aType) {
   if (aType in TYPE_ALIASES)
     return TYPE_ALIASES[aType];
   return aType;
 }
 
 function getManifestFileForDir(aDir) {
-  let file = aDir.clone();
-  file.append(FILE_RDF_MANIFEST);
+  let file = getFile(FILE_RDF_MANIFEST, aDir);
   if (file.exists() && file.isFile())
     return file;
   file.leafName = FILE_WEB_MANIFEST;
   if (file.exists() && file.isFile())
     return file;
   return null;
 }
 
@@ -1560,33 +1556,30 @@ var loadManifestFromDir = Task.async(fun
     bis.init(fis, 4096);
     try {
       var addon = yield loadManifestFromRDF(aUri, bis);
     } finally {
       bis.close();
       fis.close();
     }
 
-    let iconFile = aDir.clone();
-    iconFile.append("icon.png");
+    let iconFile = getFile("icon.png", aDir);
 
     if (iconFile.exists()) {
       addon.icons[32] = "icon.png";
       addon.icons[48] = "icon.png";
     }
 
-    let icon64File = aDir.clone();
-    icon64File.append("icon64.png");
+    let icon64File = getFile("icon64.png", aDir);
 
     if (icon64File.exists()) {
       addon.icons[64] = "icon64.png";
     }
 
-    let file = aDir.clone();
-    file.append("chrome.manifest");
+    let file = getFile("chrome.manifest", aDir);
     let chromeManifest = ChromeManifestParser.parseSync(Services.io.newFileURI(file));
     addon.hasBinaryComponents = ChromeManifestParser.hasType(chromeManifest,
                                                              "binary-component");
     return addon;
   }
 
   let file = getManifestFileForDir(aDir);
   if (!file) {
@@ -3827,18 +3820,17 @@ this.XPIProvider = {
             // The file check later will spot the removal and cleanup the database
             continue;
           }
         }
 
         aManifests[location.name][id] = null;
         let existingAddonID = id;
 
-        let jsonfile = stagingDir.clone();
-        jsonfile.append(id + ".json");
+        let jsonfile = getFile(`${id}.json`, stagingDir);
         // Assume this was a foreign install if there is no cached metadata file
         let foreignInstall = !jsonfile.exists();
         let addon;
 
         try {
           addon = syncLoadManifestFromFile(stageDirEntry, location);
         } catch (e) {
           logger.error("Unable to read add-on manifest from " + stageDirEntry.path, e);
@@ -5566,18 +5558,17 @@ this.XPIProvider = {
     let wasPending = aAddon.pendingUninstall;
 
     if (makePending) {
       // We create an empty directory in the staging directory to indicate
       // that an uninstall is necessary on next startup. Temporary add-ons are
       // automatically uninstalled on shutdown anyway so there is no need to
       // do this for them.
       if (aAddon._installLocation.name != KEY_APP_TEMPORARY) {
-        let stage = aAddon._installLocation.getStagingDir();
-        stage.append(aAddon.id);
+        let stage = getFile(aAddon.id, aAddon._installLocation.getStagingDir());
         if (!stage.exists())
           stage.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
       }
 
       XPIDatabase.setAddonProperties(aAddon, {
         pendingUninstall: true
       });
       Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
@@ -5847,18 +5838,17 @@ class AddonInstall {
       this.state = AddonManager.STATE_CANCELLED;
       XPIProvider.removeActiveInstall(this);
       AddonManagerPrivate.callInstallListeners("onDownloadCancelled",
                                                this.listeners, this.wrapper);
       this.removeTemporaryFile();
       break;
     case AddonManager.STATE_INSTALLED:
       logger.debug("Cancelling install of " + this.addon.id);
-      let xpi = this.installLocation.getStagingDir();
-      xpi.append(this.addon.id + ".xpi");
+      let xpi = getFile(`${this.addon.id}.xpi`, this.installLocation.getStagingDir());
       flushJarCache(xpi);
       this.installLocation.cleanStagingDir([this.addon.id, this.addon.id + ".xpi",
                                             this.addon.id + ".json"]);
       this.state = AddonManager.STATE_CANCELLED;
       XPIProvider.removeActiveInstall(this);
 
       if (this.existingAddon) {
         delete this.existingAddon.pendingUpgrade;
@@ -6169,18 +6159,17 @@ class AddonInstall {
     Task.spawn((function*() {
       let installedUnpacked = 0;
 
       yield this.installLocation.requestStagingDir();
 
       // remove any previously staged files
       yield this.unstageInstall(stagedAddon);
 
-      stagedAddon.append(this.addon.id);
-      stagedAddon.leafName = this.addon.id + ".xpi";
+      stagedAddon.append(`${this.addon.id}.xpi`);
 
       installedUnpacked = yield this.stageInstall(requiresRestart, stagedAddon, isUpgrade);
 
       if (requiresRestart) {
         this.state = AddonManager.STATE_INSTALLED;
         AddonManagerPrivate.callInstallListeners("onInstallEnded",
                                                  this.listeners, this.wrapper,
                                                  this.addon.wrapper);
@@ -6342,83 +6331,72 @@ class AddonInstall {
 
       return installedUnpacked;
     }).bind(this));
   }
 
   /**
    * Removes any previously staged upgrade.
    */
-  unstageInstall(stagedAddon) {
-    return Task.spawn((function*() {
-      let stagedJSON = stagedAddon.clone();
-      let removedAddon = stagedAddon.clone();
-
-      stagedJSON.append(this.addon.id + ".json");
-
-      if (stagedJSON.exists()) {
-        stagedJSON.remove(true);
-      }
-
-      removedAddon.append(this.addon.id);
-      yield removeAsync(removedAddon);
-      removedAddon.leafName = this.addon.id + ".xpi";
-      yield removeAsync(removedAddon);
-    }).bind(this));
+  async unstageInstall(stagedAddon) {
+    let stagedJSON = getFile(`${this.addon.id}.json`, stagedAddon);
+    if (stagedJSON.exists()) {
+      stagedJSON.remove(true);
+    }
+
+    await removeAsync(getFile(this.addon.id, stagedAddon));
+
+    await removeAsync(getFile(`${this.addon.id}.xpi`, stagedAddon));
   }
 
   /**
     * Postone a pending update, until restart or until the add-on resumes.
     *
     * @param {Function} resumeFn - a function for the add-on to run
     *                                    when resuming.
     */
-  postpone(resumeFn) {
-    return Task.spawn((function*() {
-      this.state = AddonManager.STATE_POSTPONED;
-
-      let stagingDir = this.installLocation.getStagingDir();
-      let stagedAddon = stagingDir.clone();
-
-      yield this.installLocation.requestStagingDir();
-      yield this.unstageInstall(stagedAddon);
-
-      stagedAddon.append(this.addon.id);
-      stagedAddon.leafName = this.addon.id + ".xpi";
-
-      yield this.stageInstall(true, stagedAddon, true);
-
-      AddonManagerPrivate.callInstallListeners("onInstallPostponed",
-                                               this.listeners, this.wrapper)
-
-      // upgrade has been staged for restart, provide a way for it to call the
-      // resume function.
-      let callback = AddonManagerPrivate.getUpgradeListener(this.addon.id);
-      if (callback) {
-        callback({
-          version: this.version,
-          install: () => {
-            switch (this.state) {
-            case AddonManager.STATE_POSTPONED:
-              if (resumeFn) {
-                resumeFn();
-              }
-              break;
-            default:
-              logger.warn(`${this.addon.id} cannot resume postponed upgrade from state (${this.state})`);
-              break;
+  async postpone(resumeFn) {
+    this.state = AddonManager.STATE_POSTPONED;
+
+    let stagingDir = this.installLocation.getStagingDir();
+
+    await this.installLocation.requestStagingDir();
+    await this.unstageInstall(stagingDir);
+
+    let stagedAddon = getFile(`${this.addon.id}.xpi`, stagingDir);
+
+    await this.stageInstall(true, stagedAddon, true);
+
+    AddonManagerPrivate.callInstallListeners("onInstallPostponed",
+                                             this.listeners, this.wrapper)
+
+    // upgrade has been staged for restart, provide a way for it to call the
+    // resume function.
+    let callback = AddonManagerPrivate.getUpgradeListener(this.addon.id);
+    if (callback) {
+      callback({
+        version: this.version,
+        install: () => {
+          switch (this.state) {
+          case AddonManager.STATE_POSTPONED:
+            if (resumeFn) {
+              resumeFn();
             }
-          },
-        });
-      }
-      // Release the staging directory lock, but since the staging dir is populated
-      // it will not be removed until resumed or installed by restart.
-      // See also cleanStagingDir()
-      this.installLocation.releaseStagingDir();
-    }).bind(this));
+            break;
+          default:
+            logger.warn(`${this.addon.id} cannot resume postponed upgrade from state (${this.state})`);
+            break;
+          }
+        },
+      });
+    }
+    // Release the staging directory lock, but since the staging dir is populated
+    // it will not be removed until resumed or installed by restart.
+    // See also cleanStagingDir()
+    this.installLocation.releaseStagingDir();
   }
 }
 
 class LocalAddonInstall extends AddonInstall {
   /**
    * Initialises this install to be an install from a local file.
    *
    * @returns Promise
@@ -8571,19 +8549,17 @@ class MutableDirectoryInstallLocation ex
 
   /**
    * Gets the staging directory to put add-ons that are pending install and
    * uninstall into.
    *
    * @return an nsIFile
    */
   getStagingDir() {
-    let dir = this._directory.clone();
-    dir.append(DIR_STAGE);
-    return dir;
+    return getFile(DIR_STAGE, this._directory);
   }
 
   requestStagingDir() {
     this._stagingDirLock++;
 
     if (this._stagingDirPromise)
       return this._stagingDirPromise;
 
@@ -8615,18 +8591,17 @@ class MutableDirectoryInstallLocation ex
    * @param  aLeafNames
    *         An array of file or directory to remove from the directory, the
    *         array may be empty
    */
   cleanStagingDir(aLeafNames = []) {
     let dir = this.getStagingDir();
 
     for (let name of aLeafNames) {
-      let file = dir.clone();
-      file.append(name);
+      let file = getFile(name, dir);
       recursiveRemove(file);
     }
 
     if (this._stagingDirLock > 0)
       return;
 
     let dirEntries = dir.directoryEntries.QueryInterface(Ci.nsIDirectoryEnumerator);
     try {
@@ -8649,18 +8624,17 @@ class MutableDirectoryInstallLocation ex
    * Returns a directory that is normally on the same filesystem as the rest of
    * the install location and can be used for temporarily storing files during
    * safe move operations. Calling this method will delete the existing trash
    * directory and its contents.
    *
    * @return an nsIFile
    */
   getTrashDir() {
-    let trashDir = this._directory.clone();
-    trashDir.append(DIR_TRASH);
+    let trashDir = getFile(DIR_TRASH, this._directory);
     let trashDirExists = trashDir.exists();
     try {
       if (trashDirExists)
         recursiveRemove(trashDir);
       trashDirExists = false;
     } catch (e) {
       logger.warn("Failed to remove trash directory", e);
     }
@@ -8691,24 +8665,21 @@ class MutableDirectoryInstallLocation ex
    * @return an nsIFile indicating where the add-on was installed to
    */
   installAddon({ id, source, existingAddonID, action = "move" }) {
     let trashDir = this.getTrashDir();
 
     let transaction = new SafeInstallOperation();
 
     let moveOldAddon = aId => {
-      let file = this._directory.clone();
-      file.append(aId);
-
+      let file = getFile(aId, this._directory);
       if (file.exists())
         transaction.moveUnder(file, trashDir);
 
-      file = this._directory.clone();
-      file.append(aId + ".xpi");
+      file = getFile(`${aId}.xpi`, this._directory);
       if (file.exists()) {
         flushJarCache(file);
         transaction.moveUnder(file, trashDir);
       }
     }
 
     // If any of these operations fails the finally block will clean up the
     // temporary directory
@@ -8727,18 +8698,17 @@ class MutableDirectoryInstallLocation ex
             KEY_PROFILEDIR, ["extension-data", existingAddonID], false, true
           );
 
           if (oldDataDir.exists()) {
             let newDataDir = FileUtils.getDir(
               KEY_PROFILEDIR, ["extension-data", id], false, true
             );
             if (newDataDir.exists()) {
-              let trashData = trashDir.clone();
-              trashData.append("data-directory");
+              let trashData = getFile("data-directory", trashDir);
               transaction.moveUnder(newDataDir, trashData);
             }
 
             transaction.moveTo(oldDataDir, newDataDir);
           }
         }
       }
 
@@ -8799,18 +8769,17 @@ class MutableDirectoryInstallLocation ex
   uninstallAddon(aId) {
     let file = this._IDToFileMap[aId];
     if (!file) {
       logger.warn("Attempted to remove " + aId + " from " +
            this._name + " but it was already gone");
       return;
     }
 
-    file = this._directory.clone();
-    file.append(aId);
+    file = getFile(aId, this._directory);
     if (!file.exists())
       file.leafName += ".xpi";
 
     if (!file.exists()) {
       logger.warn("Attempted to remove " + aId + " from " +
            this._name + " but it was already gone");
 
       delete this._IDToFileMap[aId];
@@ -8864,18 +8833,17 @@ class SystemAddonInstallLocation extends
   constructor(aName, aDirectory, aScope, aResetSet) {
     let addonSet = SystemAddonInstallLocation._loadAddonSet();
     let directory = null;
 
     // The system add-on update directory is stored in a pref.
     // Therefore, this is looked up before calling the
     // constructor on the superclass.
     if (addonSet.directory) {
-      directory = aDirectory.clone();
-      directory.append(addonSet.directory);
+      directory = getFile(addonSet.directory, aDirectory);
       logger.info("SystemAddonInstallLocation scanning directory " + directory.path);
     } else {
       logger.info("SystemAddonInstallLocation directory is missing");
     }
 
     super(aName, directory, aScope);
 
     this._addonSet = addonSet;
@@ -8897,32 +8865,29 @@ class SystemAddonInstallLocation extends
    * uninstall into.
    *
    * @return {nsIFile} - staging directory for system add-on upgrades.
    */
   getStagingDir() {
     this._addonSet = SystemAddonInstallLocation._loadAddonSet();
     let dir = null;
     if (this._addonSet.directory) {
-      this._directory = this._baseDir.clone();
-      this._directory.append(this._addonSet.directory);
-      dir = this._directory.clone();
-      dir.append(DIR_STAGE);
+      this._directory = getFile(this._addonSet.directory, this._baseDir);
+      dir = getFile(DIR_STAGE, this._directory);
     } else {
       logger.info("SystemAddonInstallLocation directory is missing");
     }
 
     return dir;
   }
 
   requestStagingDir() {
     this._addonSet = SystemAddonInstallLocation._loadAddonSet();
     if (this._addonSet.directory) {
-      this._directory = this._baseDir.clone();
-      this._directory.append(this._addonSet.directory);
+      this._directory = getFile(this._addonSet.directory, this._baseDir);
     }
     return super.requestStagingDir();
   }
 
   /**
    * Reads the current set of system add-ons
    */
   static _loadAddonSet() {
@@ -9238,18 +9203,17 @@ class SystemAddonInstallLocation extends
    * Returns a directory that is normally on the same filesystem as the rest of
    * the install location and can be used for temporarily storing files during
    * safe move operations. Calling this method will delete the existing trash
    * directory and its contents.
    *
    * @return an nsIFile
    */
   getTrashDir() {
-    let trashDir = this._directory.clone();
-    trashDir.append(DIR_TRASH);
+    let trashDir = getFile(DIR_TRASH, this._directory);
     let trashDirExists = trashDir.exists();
     try {
       if (trashDirExists)
         recursiveRemove(trashDir);
       trashDirExists = false;
     } catch (e) {
       logger.warn("Failed to remove trash directory", e);
     }
@@ -9285,18 +9249,17 @@ class SystemAddonInstallLocation extends
       // the install because of it.
       try {
         recursiveRemove(trashDir);
       } catch (e) {
         logger.warn("Failed to remove trash directory when installing " + id, e);
       }
     }
 
-    let newFile = this._directory.clone();
-    newFile.append(source.leafName);
+    let newFile = getFile(source.leafName, this._directory);
 
     try {
       newFile.lastModifiedTime = Date.now();
     } catch (e) {
       logger.warn("failed to set lastModifiedTime on " + newFile.path, e);
     }
     this._IDToFileMap[id] = newFile;
     XPIProvider._addURIMapping(id, newFile);