Bug 1462483: Part 2 - Cleanup directory iteration code. r?aswan
MozReview-Commit-ID: 3VzaVPsD8VT
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -397,54 +397,38 @@ var AddonTestUtils = {
let {value} = Object.getOwnPropertyDescriptor(FileTestUtils,
"_globalTemporaryDirectory");
if (value) {
ignoreEntries.add(value.leafName);
}
}
// Check that the temporary directory is empty
- var dirEntries = this.tempDir.directoryEntries
- .QueryInterface(Ci.nsIDirectoryEnumerator);
var entries = [];
- while (dirEntries.hasMoreElements()) {
- let {leafName} = dirEntries.nextFile;
+ for (let {leafName} of this.iterDirectory(this.tempDir)) {
if (!ignoreEntries.has(leafName)) {
entries.push(leafName);
}
}
if (entries.length)
throw new Error(`Found unexpected files in temporary directory: ${entries.join(", ")}`);
- dirEntries.close();
-
try {
appDirForAddons.remove(true);
} catch (ex) {
testScope.info(`Got exception removing addon app dir: ${ex}`);
}
// ensure no leftover files in the system addon upgrade location
let featuresDir = this.profileDir.clone();
featuresDir.append("features");
// upgrade directories will be in UUID folders under features/
- let systemAddonDirs = [];
- if (featuresDir.exists()) {
- let featuresDirEntries = featuresDir.directoryEntries
- .QueryInterface(Ci.nsIDirectoryEnumerator);
- while (featuresDirEntries.hasMoreElements()) {
- let entry = featuresDirEntries.getNext();
- entry.QueryInterface(Ci.nsIFile);
- systemAddonDirs.push(entry);
- }
-
- systemAddonDirs.map(dir => {
- dir.append("stage");
- pathShouldntExist(dir);
- });
+ for (let dir of this.iterDirectory(featuresDir)) {
+ dir.append("stage");
+ pathShouldntExist(dir);
}
// ensure no leftover files in the user addon location
let testDir = this.profileDir.clone();
testDir.append("extensions");
testDir.append("trash");
pathShouldntExist(testDir);
@@ -470,16 +454,43 @@ var AddonTestUtils = {
this.tempDir.remove(true);
} catch (e) {
Cu.reportError(e);
}
});
},
/**
+ * Iterates over the entries in a given directory.
+ *
+ * Fails silently if the given directory does not exist.
+ *
+ * @param {nsIFile} dir
+ * Directory to iterate.
+ */
+ * iterDirectory(dir) {
+ let dirEnum;
+ try {
+ dirEnum = dir.directoryEntries;
+ let file;
+ while ((file = dirEnum.nextFile)) {
+ yield file;
+ }
+ } catch (e) {
+ if (dir.exists()) {
+ Cu.reportError(e);
+ }
+ } finally {
+ if (dirEnum) {
+ dirEnum.close();
+ }
+ }
+ },
+
+ /**
* Creates a new HttpServer for testing, and begins listening on the
* specified port. Automatically shuts down the server when the test
* unit ends.
*
* @param {object} [options = {}]
* The options object.
* @param {integer} [options.port = -1]
* The port to listen on. If omitted, listen on a random
@@ -1205,21 +1216,18 @@ var AddonTestUtils = {
* @param {nsIFile} ext A file pointing to either the packed extension or its unpacked directory.
* @param {number} time The time to which we set the lastModifiedTime of the extension
*
* @deprecated Please use promiseSetExtensionModifiedTime instead
*/
setExtensionModifiedTime(ext, time) {
ext.lastModifiedTime = time;
if (ext.isDirectory()) {
- let entries = ext.directoryEntries
- .QueryInterface(Ci.nsIDirectoryEnumerator);
- while (entries.hasMoreElements())
- this.setExtensionModifiedTime(entries.nextFile, time);
- entries.close();
+ for (let file of this.iterDirectory(ext))
+ this.setExtensionModifiedTime(file, time);
}
},
async promiseSetExtensionModifiedTime(path, time) {
await OS.File.setDates(path, time, time);
let iterator = new OS.File.DirectoryIterator(path);
try {
--- a/toolkit/mozapps/extensions/internal/XPIInstall.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIInstall.jsm
@@ -83,31 +83,32 @@ const PREF_DISTRO_ADDONS_PERMS =
const PREF_INSTALL_REQUIRESECUREORIGIN = "extensions.install.requireSecureOrigin";
const PREF_PENDING_OPERATIONS = "extensions.pendingOperations";
const PREF_SYSTEM_ADDON_UPDATE_URL = "extensions.systemAddon.update.url";
const PREF_XPI_ENABLED = "xpinstall.enabled";
const PREF_XPI_DIRECT_WHITELISTED = "xpinstall.whitelist.directRequest";
const PREF_XPI_FILE_WHITELISTED = "xpinstall.whitelist.fileRequest";
const PREF_XPI_WHITELIST_REQUIRED = "xpinstall.whitelist.required";
-/* globals BOOTSTRAP_REASONS, KEY_APP_SYSTEM_ADDONS, KEY_APP_SYSTEM_DEFAULTS, PREF_BRANCH_INSTALLED_ADDON, PREF_SYSTEM_ADDON_SET, TEMPORARY_ADDON_SUFFIX, SIGNED_TYPES, TOOLKIT_ID, XPI_PERMISSION, XPIStates, getExternalType, isTheme, isWebExtension */
+/* globals BOOTSTRAP_REASONS, KEY_APP_SYSTEM_ADDONS, KEY_APP_SYSTEM_DEFAULTS, PREF_BRANCH_INSTALLED_ADDON, PREF_SYSTEM_ADDON_SET, TEMPORARY_ADDON_SUFFIX, SIGNED_TYPES, TOOLKIT_ID, XPI_PERMISSION, XPIStates, getExternalType, isTheme, isWebExtension, iterDirectory */
const XPI_INTERNAL_SYMBOLS = [
"BOOTSTRAP_REASONS",
"KEY_APP_SYSTEM_ADDONS",
"KEY_APP_SYSTEM_DEFAULTS",
"PREF_BRANCH_INSTALLED_ADDON",
"PREF_SYSTEM_ADDON_SET",
"SIGNED_TYPES",
"TEMPORARY_ADDON_SUFFIX",
"TOOLKIT_ID",
"XPI_PERMISSION",
"XPIStates",
"getExternalType",
"isTheme",
"isWebExtension",
+ "iterDirectory",
];
for (let name of XPI_INTERNAL_SYMBOLS) {
XPCOMUtils.defineLazyGetter(this, name, () => XPIInternal[name]);
}
/**
* Returns a nsIFile instance for the given path, relative to the given
@@ -1100,17 +1101,17 @@ function recursiveRemove(aFile) {
throw e;
}
}
// Use a snapshot of the directory contents to avoid possible issues with
// iterating over a directory while removing files from it (the YAFFS2
// embedded filesystem has this issue, see bug 772238), and to remove
// normal files before their resource forks on OSX (see bug 733436).
- let entries = getDirectoryEntries(aFile, true);
+ let entries = Array.from(iterDirectory(aFile));
entries.forEach(recursiveRemove);
try {
aFile.remove(true);
} catch (e) {
logger.error("Failed to remove empty directory " + aFile.path, e);
throw e;
}
@@ -1276,54 +1277,16 @@ SafeInstallOperation.prototype = {
}
}
while (this._createdDirs.length > 0)
recursiveRemove(this._createdDirs.pop());
}
};
-/**
- * Gets a snapshot of directory entries.
- *
- * @param {nsIFile} aDir
- * Directory to look at
- * @param {boolean} aSortEntries
- * True to sort entries by filename
- * @returns {nsIFile[]}
- * An files in the directory, or an empty array if aDir is not a
- * readable directory.
- */
-function getDirectoryEntries(aDir, aSortEntries) {
- let dirEnum;
- try {
- dirEnum = aDir.directoryEntries.QueryInterface(Ci.nsIDirectoryEnumerator);
- let entries = [];
- while (dirEnum.hasMoreElements())
- entries.push(dirEnum.nextFile);
-
- if (aSortEntries) {
- entries.sort(function(a, b) {
- return a.path > b.path ? -1 : 1;
- });
- }
-
- return entries;
- } catch (e) {
- if (aDir.exists()) {
- logger.warn("Can't iterate directory " + aDir.path, e);
- }
- return [];
- } finally {
- if (dirEnum) {
- dirEnum.close();
- }
- }
-}
-
function getHashStringForCrypto(aCrypto) {
// return the two-digit hexadecimal code for a byte
let toHexString = charCode => ("0" + charCode.toString(16)).slice(-2);
// convert the binary hash data to a hex string.
let binary = aCrypto.finish(false);
let hash = Array.from(binary, c => toHexString(c.charCodeAt(0)));
return hash.join("").toLowerCase();
@@ -2846,23 +2809,19 @@ class DirectoryInstallLocation {
for (let name of aLeafNames) {
let file = getFile(name, dir);
recursiveRemove(file);
}
if (this._stagingDirLock > 0)
return;
- let dirEntries = dir.directoryEntries.QueryInterface(Ci.nsIDirectoryEnumerator);
- try {
- if (dirEntries.nextFile)
- return;
- } finally {
- dirEntries.close();
- }
+ // eslint-disable-next-line no-unused-vars
+ for (let file of iterDirectory(dir))
+ return;
try {
setFilePermissions(dir, FileUtils.PERMS_DIRECTORY);
dir.remove(false);
} catch (e) {
logger.warn("Failed to remove staging dir", e);
// Failing to remove the staging directory is ignorable
}
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -411,45 +411,35 @@ function getURIForResourceInFile(aFile,
*/
function buildJarURI(aJarfile, aPath) {
let uri = Services.io.newFileURI(aJarfile);
uri = "jar:" + uri.spec + "!/" + aPath;
return Services.io.newURI(uri);
}
/**
- * Gets a snapshot of directory entries.
+ * Iterates over the entries in a given directory.
+ *
+ * Fails silently if the given directory does not exist.
*
- * @param {nsIURI} aDir
- * Directory to look at
- * @param {boolean} aSortEntries
- * True to sort entries by filename
- * @returns {Array<nsIFile>}
- * An array of nsIFile, or an empty array if aDir is not a readable directory
+ * @param {nsIFile} aDir
+ * Directory to iterate.
*/
-function getDirectoryEntries(aDir, aSortEntries) {
+function* iterDirectory(aDir) {
let dirEnum;
try {
- dirEnum = aDir.directoryEntries.QueryInterface(Ci.nsIDirectoryEnumerator);
- let entries = [];
- while (dirEnum.hasMoreElements())
- entries.push(dirEnum.nextFile);
-
- if (aSortEntries) {
- entries.sort(function(a, b) {
- return a.path > b.path ? -1 : 1;
- });
+ dirEnum = aDir.directoryEntries;
+ let file;
+ while ((file = dirEnum.nextFile)) {
+ yield file;
}
-
- return entries;
} catch (e) {
if (aDir.exists()) {
- logger.warn("Can't iterate directory " + aDir.path, e);
+ logger.warn(`Can't iterate directory ${aDir.path}`, e);
}
- return [];
} finally {
if (dirEnum) {
dirEnum.close();
}
}
}
/**
@@ -990,18 +980,17 @@ class DirectoryLocation extends XPIState
if (!this.dir) {
return addons;
}
this.initialized = true;
// Use a snapshot of the directory contents to avoid possible issues with
// iterating over a directory while removing files from it (the YAFFS2
// embedded filesystem has this issue, see bug 772238).
- let entries = getDirectoryEntries(this.dir);
- for (let entry of entries) {
+ for (let entry of Array.from(iterDirectory(this.dir))) {
let id = entry.leafName;
if (id == DIR_STAGE || id == DIR_TRASH)
continue;
let isFile = id.toLowerCase().endsWith(".xpi");
if (isFile) {
id = id.substring(0, id.length - 4);
}
@@ -2497,68 +2486,61 @@ var XPIProvider = {
* See checkForChanges
* @returns {boolean}
* True if any new add-ons were installed
*/
installDistributionAddons(aManifests, aAppChanged) {
let distroDir;
try {
distroDir = FileUtils.getDir(KEY_APP_DISTRIBUTION, [DIR_EXTENSIONS]);
- if (!distroDir.isDirectory())
- return false;
} catch (e) {
return false;
}
let changed = false;
let profileLocation = XPIStates.getLocation(KEY_APP_PROFILE);
- let entries = distroDir.directoryEntries
- .QueryInterface(Ci.nsIDirectoryEnumerator);
- let entry;
- while ((entry = entries.nextFile)) {
- let id = entry.leafName;
+ for (let file of iterDirectory(distroDir)) {
+ let id = file.leafName;
if (id.endsWith(".xpi")) {
id = id.slice(0, -4);
} else {
- logger.debug("Ignoring distribution add-on that isn't an XPI: " + entry.path);
+ logger.debug(`Ignoring distribution add-on that isn't an XPI: ${file.path}`);
continue;
}
if (!gIDTest.test(id)) {
logger.debug("Ignoring distribution add-on whose name is not a valid add-on ID: " +
- entry.path);
+ file.path);
continue;
}
/* If this is not an upgrade and we've already handled this extension
* just continue */
if (!aAppChanged && Services.prefs.prefHasUserValue(PREF_BRANCH_INSTALLED_ADDON + id)) {
continue;
}
try {
- let addon = awaitPromise(XPIInstall.installDistributionAddon(id, entry, profileLocation));
+ let addon = awaitPromise(XPIInstall.installDistributionAddon(id, file, profileLocation));
if (addon) {
// aManifests may contain a copy of a newly installed add-on's manifest
// and we'll have overwritten that so instead cache our install manifest
// which will later be put into the database in processFileChanges
if (!(KEY_APP_PROFILE in aManifests))
aManifests[KEY_APP_PROFILE] = {};
aManifests[KEY_APP_PROFILE][id] = addon;
changed = true;
}
} catch (e) {
- logger.error(`Failed to install distribution add-on ${entry.path}`, e);
+ logger.error(`Failed to install distribution add-on ${file.path}`, e);
}
}
- entries.close();
-
return changed;
},
getNewDistroAddons() {
let addons = this.newDistroAddons;
this.newDistroAddons = null;
return addons;
},
@@ -3012,16 +2994,17 @@ var XPIInternal = {
XPIStates,
XPI_PERMISSION,
awaitPromise,
canRunInSafeMode,
getExternalType,
getURIForResourceInFile,
isTheme,
isWebExtension,
+ iterDirectory,
};
var addonTypes = [
new AddonManagerPrivate.AddonType("extension", URI_EXTENSION_STRINGS,
"type.extension.name",
AddonManager.VIEW_TYPE_LIST, 4000,
AddonManager.TYPE_SUPPORTS_UNDO_RESTARTLESS_UNINSTALL),
new AddonManagerPrivate.AddonType("theme", URI_EXTENSION_STRINGS,