Bug 938704 - Make OS.File support modern iterators. r?Yoric,florian draft
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 19 Aug 2017 15:04:13 +0900
changeset 650079 b88aaf9c10da9a96c0a794a19e0c9b37da18311e
parent 649541 5ca5691372cb432ec1fa4693ca608a30858226de
child 650458 cc9f65b881f0ad286c934374a6333f88abd1f82d
push id75257
push userVYV03354@nifty.ne.jp
push dateMon, 21 Aug 2017 18:08:05 +0000
reviewersYoric, florian
bugs938704
milestone57.0a1
Bug 938704 - Make OS.File support modern iterators. r?Yoric,florian MozReview-Commit-ID: 8F1DtgakxM3
browser/components/sessionstore/SessionWorker.js
devtools/client/shared/file-watcher-worker.js
toolkit/components/osfile/modules/osfile_async_front.jsm
toolkit/components/osfile/modules/osfile_async_worker.js
toolkit/components/osfile/modules/osfile_shared_front.jsm
toolkit/components/osfile/modules/osfile_unix_front.jsm
toolkit/components/osfile/modules/osfile_win_front.jsm
toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js
toolkit/components/search/nsSearchService.js
toolkit/components/thumbnails/PageThumbsWorker.js
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/browser/components/sessionstore/SessionWorker.js
+++ b/browser/components/sessionstore/SessionWorker.js
@@ -367,17 +367,17 @@ var Agent = {
 
     let exn = null;
 
     let iterator = new File.DirectoryIterator(path);
     try {
       if (!iterator.exists()) {
         return;
       }
-      for (let entry in iterator) {
+      for (let entry of iterator) {
         if (entry.isDir) {
           continue;
         }
         if (!prefix || entry.name.startsWith(prefix)) {
           try {
             File.remove(entry.path);
           } catch (ex) {
             // Don't stop immediately
--- a/devtools/client/shared/file-watcher-worker.js
+++ b/devtools/client/shared/file-watcher-worker.js
@@ -9,17 +9,17 @@ importScripts("resource://gre/modules/os
 
 const modifiedTimes = new Map();
 
 function gatherFiles(path, fileRegex) {
   let files = [];
   const iterator = new OS.File.DirectoryIterator(path);
 
   try {
-    for (let child in iterator) {
+    for (let child of iterator) {
       // Don't descend into test directories. Saves us some time and
       // there's no reason to.
       if (child.isDir && !child.path.endsWith("/test")) {
         files = files.concat(gatherFiles(child.path, fileRegex));
       } else if (child.path.match(fileRegex)) {
         let info;
         try {
           info = OS.File.stat(child.path);
--- a/toolkit/components/osfile/modules/osfile_async_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_async_front.jsm
@@ -1255,189 +1255,132 @@ File.GET_DEBUG = function GET_DEBUG() {
  */
 var DirectoryIterator = function DirectoryIterator(path, options) {
   /**
    * Open the iterator on the worker thread
    *
    * @type {Promise}
    * @resolves {*} A message accepted by the methods of DirectoryIterator
    * in the worker thread
-   * @rejects {StopIteration} If all entries have already been visited
-   * or the iterator has been closed.
    */
-  this.__itmsg = Scheduler.post(
+  this._itmsg = Scheduler.post(
     "new_DirectoryIterator", [Type.path.toMsg(path), options],
     path
   );
   this._isClosed = false;
 };
 DirectoryIterator.prototype = {
-  iterator() {
-    return this;
-  },
-  __iterator__() {
+  [Symbol.asyncIterator]() {
     return this;
   },
 
-  // Once close() is called, _itmsg should reject with a
-  // StopIteration. However, we don't want to create the promise until
-  // it's needed because it might never be used. In that case, we
-  // would get a warning on the console.
-  get _itmsg() {
-    if (!this.__itmsg) {
-      this.__itmsg = Promise.reject(StopIteration);
-    }
-    return this.__itmsg;
-  },
+  _itmsg: null,
 
   /**
    * Determine whether the directory exists.
    *
    * @resolves {boolean}
    */
-  exists: function exists() {
-    return this._itmsg.then(
-      function onSuccess(iterator) {
-        return Scheduler.post("DirectoryIterator_prototype_exists", [iterator]);
-      }
-    );
+  async exists() {
+    if (this._isClosed) {
+      return Promise.resolve(false);
+    }
+    let iterator = await this._itmsg;
+    return Scheduler.post("DirectoryIterator_prototype_exists", [iterator]);
   },
   /**
    * Get the next entry in the directory.
    *
    * @return {Promise}
-   * @resolves {OS.File.Entry}
-   * @rejects {StopIteration} If all entries have already been visited.
+   * @resolves By definition of the async iterator protocol, either
+   * `{value: {File.Entry}, done: false}` if there is an unvisited entry
+   * in the directory, or `{value: undefined, done: true}`, otherwise.
    */
-  next: function next() {
-    let self = this;
-    let promise = this._itmsg;
-
-    // Get the iterator, call _next
-    promise = promise.then(
-      function withIterator(iterator) {
-        return self._next(iterator);
-      });
-
-    return promise;
+  async next() {
+    if (this._isClosed) {
+      return {value: undefined, done: true};
+    }
+    return this._next(await this._itmsg);
   },
   /**
    * Get several entries at once.
    *
    * @param {number=} length If specified, the number of entries
    * to return. If unspecified, return all remaining entries.
    * @return {Promise}
    * @resolves {Array} An array containing the |length| next entries.
    */
-  nextBatch: function nextBatch(size) {
+  async nextBatch(size) {
     if (this._isClosed) {
-      return Promise.resolve([]);
+      return [];
     }
-    let promise = this._itmsg;
-    promise = promise.then(
-      function withIterator(iterator) {
-        return Scheduler.post("DirectoryIterator_prototype_nextBatch", [iterator, size]);
-      });
-    promise = promise.then(
-      function withEntries(array) {
-        return array.map(DirectoryIterator.Entry.fromMsg);
-      });
-    return promise;
+    let iterator = await this._itmsg;
+    let array = await Scheduler.post("DirectoryIterator_prototype_nextBatch", [iterator, size]);
+    return array.map(DirectoryIterator.Entry.fromMsg);
   },
   /**
    * Apply a function to all elements of the directory sequentially.
    *
    * @param {Function} cb This function will be applied to all entries
    * of the directory. It receives as arguments
    *  - the OS.File.Entry corresponding to the entry;
    *  - the index of the entry in the enumeration;
    *  - the iterator itself - return |iterator.close()| to stop the loop.
    *
    * If the callback returns a promise, iteration waits until the
    * promise is resolved before proceeding.
    *
    * @return {Promise} A promise resolved once the loop has reached
    * its end.
    */
-  forEach: function forEach(cb, options) {
+  async forEach(cb, options) {
     if (this._isClosed) {
-      return Promise.resolve();
+      return undefined;
     }
-
-    let self = this;
     let position = 0;
-    let iterator;
-
-    // Grab iterator
-    let promise = this._itmsg.then(
-      function(aIterator) {
-        iterator = aIterator;
+    let iterator = await this._itmsg;
+    while (true) {
+      if (this._isClosed) {
+        return undefined;
       }
-    );
-
-    // Then iterate
-    let loop = function loop() {
-      if (self._isClosed) {
-        return Promise.resolve();
+      let {value, done} = await this._next(iterator);
+      if (done) {
+        return undefined;
       }
-      return self._next(iterator).then(
-        function onSuccess(value) {
-          return Promise.resolve(cb(value, position++, self)).then(loop);
-        },
-        function onFailure(reason) {
-          if (reason == StopIteration) {
-            return;
-          }
-          throw reason;
-        }
-      );
-    };
-
-    return promise.then(loop);
+      await cb(value, position++, this);
+    }
   },
   /**
    * Auxiliary method: fetch the next item
    *
-   * @rejects {StopIteration} If all entries have already been visited
-   * or the iterator has been closed.
+   * @resolves `{value: undefined, done: true}` If all entries have already
+   * been visited or the iterator has been closed.
    */
-  _next: function _next(iterator) {
+  async _next(iterator) {
     if (this._isClosed) {
-      return this._itmsg;
+      return {value: undefined, done: true};
     }
-    let self = this;
-    let promise = Scheduler.post("DirectoryIterator_prototype_next", [iterator]);
-    promise = promise.then(
-      DirectoryIterator.Entry.fromMsg,
-      function onReject(reason) {
-        if (reason == StopIteration) {
-          self.close();
-          throw StopIteration;
-        }
-        throw reason;
-      });
-    return promise;
+    let {value, done} = await Scheduler.post("DirectoryIterator_prototype_next", [iterator]);
+    if (done) {
+      this.close();
+      return {value: undefined, done: true};
+    }
+    return {value: DirectoryIterator.Entry.fromMsg(value), done: false};
   },
   /**
    * Close the iterator
    */
-  close: function close() {
+  async close() {
     if (this._isClosed) {
-      return Promise.resolve();
+      return undefined;
     }
     this._isClosed = true;
-    let self = this;
-    return this._itmsg.then(
-      function withIterator(iterator) {
-        // Set __itmsg to null so that the _itmsg getter returns a
-        // rejected StopIteration promise if it's ever used.
-        self.__itmsg = null;
-        return Scheduler.post("DirectoryIterator_prototype_close", [iterator]);
-      }
-    );
+    let iterator = this._itmsg;
+    this._itmsg = null;
+    return Scheduler.post("DirectoryIterator_prototype_close", [iterator]);
   }
 };
 
 DirectoryIterator.Entry = function Entry(value) {
   return value;
 };
 DirectoryIterator.Entry.prototype = Object.create(SysAll.AbstractEntry.prototype);
 
--- a/toolkit/components/osfile/modules/osfile_async_worker.js
+++ b/toolkit/components/osfile/modules/osfile_async_worker.js
@@ -355,24 +355,22 @@ if (this.Components) {
        function do_flush() {
          return this.flush();
        });
    },
    // Methods of OS.File.DirectoryIterator
    DirectoryIterator_prototype_next: function next(dir) {
      return withDir(dir,
        function do_next() {
-         try {
-           return File.DirectoryIterator.Entry.toMsg(this.next());
-         } catch (x) {
-           if (x == StopIteration) {
-             OpenedDirectoryIterators.remove(dir);
-           }
-           throw x;
+         let {value, done} = this.next();
+         if (done) {
+           OpenedDirectoryIterators.remove(dir);
+           return {value: undefined, done: true};
          }
+         return {value: File.DirectoryIterator.Entry.toMsg(value), done: false};
        }, false);
    },
    DirectoryIterator_prototype_nextBatch: function nextBatch(dir, size) {
      return withDir(dir,
        function do_nextBatch() {
          let result;
          try {
            result = this.nextBatch(size);
--- a/toolkit/components/osfile/modules/osfile_shared_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_shared_front.jsm
@@ -185,34 +185,34 @@ AbstractFile.openUnique = function openU
 
 /**
  * Code shared by iterators.
  */
 AbstractFile.AbstractIterator = function AbstractIterator() {
 };
 AbstractFile.AbstractIterator.prototype = {
   /**
-   * Allow iterating with |for|
+   * Allow iterating with |for-of|
    */
-  __iterator__: function __iterator__() {
+  [Symbol.iterator]() {
     return this;
   },
   /**
    * Apply a function to all elements of the directory sequentially.
    *
    * @param {Function} cb This function will be applied to all entries
    * of the directory. It receives as arguments
    *  - the OS.File.Entry corresponding to the entry;
    *  - the index of the entry in the enumeration;
    *  - the iterator itself - calling |close| on the iterator stops
    *   the loop.
    */
   forEach: function forEach(cb) {
     let index = 0;
-    for (let entry in this) {
+    for (let entry of this) {
       cb(entry, index++, this);
     }
   },
   /**
    * Return several entries at once.
    *
    * Entries are returned in the same order as a walk with |forEach| or
    * |for(...)|.
@@ -220,17 +220,17 @@ AbstractFile.AbstractIterator.prototype 
    * @param {number=} length If specified, the number of entries
    * to return. If unspecified, return all remaining entries.
    * @return {Array} An array containing the next |length| entries, or
    * less if the iteration contains less than |length| entries left.
    */
   nextBatch: function nextBatch(length) {
     let array = [];
     let i = 0;
-    for (let entry in this) {
+    for (let entry of this) {
       array.push(entry);
       if (++i >= length) {
         return array;
       }
     }
     return array;
   }
 };
@@ -498,17 +498,17 @@ AbstractFile.removeRecursive = function(
   let iterator = new OS.File.DirectoryIterator(path);
   if (!iterator.exists()) {
     if (!("ignoreAbsent" in options) || options.ignoreAbsent) {
       return;
     }
   }
 
   try {
-    for (let entry in iterator) {
+    for (let entry of iterator) {
       if (entry.isDir) {
         if (entry.isLink) {
           // Unlike Unix symlinks, NTFS junctions or NTFS symlinks to
           // directories are directories themselves. OS.File.remove()
           // will not work for them.
           OS.File.removeEmptyDir(entry.path, options);
         } else {
           // Normal directories.
--- a/toolkit/components/osfile/modules/osfile_unix_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_unix_front.jsm
@@ -748,26 +748,26 @@
      File.DirectoryIterator.prototype = Object.create(exports.OS.Shared.AbstractFile.AbstractIterator.prototype);
 
      /**
       * Return the next entry in the directory, if any such entry is
       * available.
       *
       * Skip special directories "." and "..".
       *
-      * @return {File.Entry} The next entry in the directory.
-      * @throws {StopIteration} Once all files in the directory have been
-      * encountered.
+      * @return By definition of the iterator protocol, either
+      * `{value: {File.Entry}, done: false}` if there is an unvisited entry
+      * in the directory, or `{value: undefined, done: true}`, otherwise.
       */
      File.DirectoryIterator.prototype.next = function next() {
        if (!this._exists) {
          throw File.Error.noSuchFile("DirectoryIterator.prototype.next", this._path);
        }
        if (this._closed) {
-         throw StopIteration;
+         return {value: undefined, done: true};
        }
        for (let entry = UnixFile.readdir(this._dir);
             entry != null && !entry.isNull();
             entry = UnixFile.readdir(this._dir)) {
          let contents = entry.contents;
          let name = contents.d_name.readString();
          if (name == "." || name == "..") {
            continue;
@@ -780,20 +780,23 @@
            throw_on_negative("lstat", UnixFile.lstat(path, gStatDataPtr), this._path);
            isDir = (gStatData.st_mode & Const.S_IFMT) == Const.S_IFDIR;
            isSymLink = (gStatData.st_mode & Const.S_IFMT) == Const.S_IFLNK;
          } else {
            isDir = contents.d_type == Const.DT_DIR;
            isSymLink = contents.d_type == Const.DT_LNK;
          }
 
-         return new File.DirectoryIterator.Entry(isDir, isSymLink, name, this._path);
+         return {
+           value: new File.DirectoryIterator.Entry(isDir, isSymLink, name, this._path),
+           done: false
+         };
        }
        this.close();
-       throw StopIteration;
+       return {value: undefined, done: true};
      };
 
      /**
       * Close the iterator and recover all resources.
       * You should call this once you have finished iterating on a directory.
       */
      File.DirectoryIterator.prototype.close = function close() {
        if (this._closed) return;
--- a/toolkit/components/osfile/modules/osfile_win_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_win_front.jsm
@@ -826,32 +826,35 @@
      },
 
      /**
       * Return the next entry in the directory, if any such entry is
       * available.
       *
       * Skip special directories "." and "..".
       *
-      * @return {File.Entry} The next entry in the directory.
-      * @throws {StopIteration} Once all files in the directory have been
-      * encountered.
+      * @return By definition of the iterator protocol, either
+      * `{value: {File.Entry}, done: false}` if there is an unvisited entry
+      * in the directory, or `{value: undefined, done: true}`, otherwise.
       */
      File.DirectoryIterator.prototype.next = function next() {
          // FIXME: If we start supporting "\\?\"-prefixed paths, do not forget
          // that "." and ".." are absolutely normal file names if _path starts
          // with such prefix
          for (let entry = this._next(); entry != null; entry = this._next()) {
            let name = entry.cFileName.readString();
            if (name == "." || name == "..") {
              continue;
            }
-           return new File.DirectoryIterator.Entry(entry, this._path);
+           return {
+             value: new File.DirectoryIterator.Entry(entry, this._path),
+             done: false
+           };
          }
-         throw StopIteration;
+         return {value: undefined, done: true};
      };
 
      File.DirectoryIterator.prototype.close = function close() {
        if (this._closed) {
          return;
        }
        this._closed = true;
        if (this._handle) {
--- a/toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js
+++ b/toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js
@@ -210,17 +210,17 @@ function test_iter_dir() {
   let tmp_file = OS.File.open(tmp_file_name, {write: true, trunc: true});
   tmp_file.close();
 
   let parent = OS.File.getCurrentDirectory();
   info("test_iter_dir: directory " + parent);
   let iterator = new OS.File.DirectoryIterator(parent);
   info("test_iter_dir: iterator created");
   let encountered_tmp_file = false;
-  for (let entry in iterator) {
+  for (let entry of iterator) {
     // Checking that |name| can be decoded properly
     info("test_iter_dir: encountering entry " + entry.name);
 
     if (entry.name == tmp_file_name) {
       encountered_tmp_file = true;
       isnot(entry.isDir, "test_iter_dir: The temporary file is not a directory");
       isnot(entry.isSymLink, "test_iter_dir: The temporary file is not a link");
     }
@@ -261,17 +261,17 @@ function test_iter_dir() {
   ok(encountered_tmp_file, "test_iter_dir: We have found the temporary file");
 
   info("test_iter_dir: Cleaning up");
   iterator.close();
 
   // Testing nextBatch()
   iterator = new OS.File.DirectoryIterator(parent);
   let allentries = [];
-  for (let x in iterator) {
+  for (let x of iterator) {
     allentries.push(x);
   }
   iterator.close();
 
   ok(allentries.length >= 14, "test_iter_dir: Meta-check: the test directory should contain at least 14 items");
 
   iterator = new OS.File.DirectoryIterator(parent);
   let firstten = iterator.nextBatch(10);
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -2968,22 +2968,19 @@ SearchService.prototype = {
       locations = {hasMoreElements: () => false};
     }
     while (locations.hasMoreElements()) {
       let dir = locations.getNext().QueryInterface(Ci.nsIFile);
       let iterator = new OS.File.DirectoryIterator(dir.path,
                                                    { winPattern: "*.xml" });
       try {
         // Add dir to distDirs if it contains any files.
-        await checkForSyncCompletion(iterator.next());
-        distDirs.push(dir);
-      } catch (ex) {
-        // Catch for StopIteration exception.
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
+        let {done} = await checkForSyncCompletion(iterator.next());
+        if (!done) {
+          distDirs.push(dir);
         }
       } finally {
         iterator.close();
       }
     }
 
     // Add the non-empty directories of NS_APP_SEARCH_DIR_LIST to
     // otherDirs...
@@ -2993,22 +2990,19 @@ SearchService.prototype = {
     while (locations.hasMoreElements()) {
       let dir = locations.getNext().QueryInterface(Ci.nsIFile);
       if (cache.engines && dir.equals(userSearchDir))
         continue;
       let iterator = new OS.File.DirectoryIterator(dir.path,
                                                    { winPattern: "*.xml" });
       try {
         // Add dir to otherDirs if it contains any files.
-        await checkForSyncCompletion(iterator.next());
-        otherDirs.push(dir);
-      } catch (ex) {
-        // Catch for StopIteration exception.
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
+        let {done} = await checkForSyncCompletion(iterator.next());
+        if (!done) {
+          otherDirs.push(dir);
         }
       } finally {
         iterator.close();
       }
     }
 
     let hasModifiedDir = async function(aList) {
       let modifiedDir = false;
--- a/toolkit/components/thumbnails/PageThumbsWorker.js
+++ b/toolkit/components/thumbnails/PageThumbsWorker.js
@@ -84,17 +84,17 @@ var Agent = {
     try {
       if (!iter.exists()) {
         return [];
       }
 
       let skip = new Set(skipFiles);
 
       let entries = [];
-      for (let entry in iter) {
+      for (let entry of iter) {
         if (!entry.isDir && !entry.isSymLink && !skip.has(entry.name)) {
           entries.push(entry);
         }
       }
       return entries;
     } finally {
       iter.close();
     }
@@ -103,17 +103,17 @@ var Agent = {
   moveOrDeleteAllThumbnails:
   function Agent_moveOrDeleteAllThumbnails(pathFrom, pathTo) {
     OS.File.makeDir(pathTo, {ignoreExisting: true});
     if (pathFrom == pathTo) {
       return true;
     }
     let iter = new OS.File.DirectoryIterator(pathFrom);
     if (iter.exists()) {
-      for (let entry in iter) {
+      for (let entry of iter) {
         if (entry.isDir || entry.isSymLink) {
           continue;
         }
 
 
         let from = OS.Path.join(pathFrom, entry.name);
         let to = OS.Path.join(pathTo, entry.name);
 
@@ -148,17 +148,17 @@ var Agent = {
 
   copy: function Agent_copy(source, dest, options) {
     return File.copy(source, dest, options);
   },
 
   wipe: function Agent_wipe(path) {
     let iterator = new File.DirectoryIterator(path);
     try {
-      for (let entry in iterator) {
+      for (let entry of iterator) {
         try {
           File.remove(entry.path);
         } catch (ex) {
           // If a file cannot be removed, we should still continue.
           // This can happen at least for any of the following reasons:
           // - access denied;
           // - file has been removed recently during a previous wipe
           //  and the file system has not flushed that yet (yes, this
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -6562,40 +6562,43 @@ class SystemAddonInstallLocation extends
     try {
       iterator = new OS.File.DirectoryIterator(this._baseDir.path);
     } catch (e) {
       logger.error("Failed to clean updated system add-ons directories.", e);
       return;
     }
 
     try {
-      for (let promise in iterator) {
-        let entry = await promise;
+      for (;;) {
+        let {value: entry, done} = await iterator.next();
+        if (done) {
+          break;
+        }
 
         // Skip the directory currently in use
         if (this._directory && this._directory.path == entry.path) {
           continue;
         }
 
         // Skip the next directory
         if (this._nextDir && this._nextDir.path == entry.path) {
           continue;
         }
 
         if (entry.isDir) {
-           await OS.File.removeDir(entry.path, {
-             ignoreAbsent: true,
-             ignorePermissions: true,
-           });
-         } else {
-           await OS.File.remove(entry.path, {
-             ignoreAbsent: true,
-           });
-         }
-       }
+          await OS.File.removeDir(entry.path, {
+            ignoreAbsent: true,
+            ignorePermissions: true,
+          });
+        } else {
+          await OS.File.remove(entry.path, {
+            ignoreAbsent: true,
+          });
+        }
+      }
 
     } catch (e) {
       logger.error("Failed to clean updated system add-ons directories.", e);
     } finally {
       iterator.close();
     }
   }