Bug 938704 - Make OS.File support modern iterators. r=florian,Yoric

MozReview-Commit-ID: 8F1DtgakxM3

--HG--
extra : rebase_source : 05c42a3236ad55356a9149d8107e4a569fc06cd0

browser/components/sessionstore/SessionWorker.js

@@ -372,7 +372,7 @@ var Agent = {
       if (!iterator.exists()) {
         return;
       }
-      for (let entry in iterator) {
+      for (let entry of iterator) {
         if (entry.isDir) {
           continue;
         }

devtools/client/shared/file-watcher-worker.js

@@ -14,7 +14,7 @@ function gatherFiles(path, fileRegex) {
   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")) {

toolkit/components/osfile/modules/osfile_async_front.jsm

@@ -1245,64 +1245,45 @@ var DirectoryIterator = function DirectoryIterator(path, options) {
    * @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() {
+  [Symbol.asyncIterator]() {
-    return this;
-  },
-  __iterator__() {
     return this;
   },
 
-  // Once close() is called, _itmsg should reject with a
+  _itmsg: null,
-  // 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;
-  },
 
   /**
    * Determine whether the directory exists.
    *
    * @resolves {boolean}
    */
-  exists: function exists() {
+  async exists() {
-    return this._itmsg.then(
+    if (this._isClosed) {
-      function onSuccess(iterator) {
+      return Promise.resolve(false);
-        return Scheduler.post("DirectoryIterator_prototype_exists", [iterator]);
+    }
-      }
+    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}
+   * @resolves By definition of the async iterator protocol, either
-   * @rejects {StopIteration} If all entries have already been visited.
+   * `{value: {File.Entry}, done: false}` if there is an unvisited entry
+   * in the directory, or `{value: undefined, done: true}`, otherwise.
    */
-  next: function next() {
+  async next() {
-    let self = this;
+    if (this._isClosed) {
-    let promise = this._itmsg;
+      return {value: undefined, done: true};
-
+    }
-    // Get the iterator, call _next
+    return this._next(await this._itmsg);
-    promise = promise.then(
-      function withIterator(iterator) {
-        return self._next(iterator);
-      });
-
-    return promise;
   },
   /**
    * Get several entries at once.
@@ -1312,20 +1293,13 @@ DirectoryIterator.prototype = {
    * @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;
+    let iterator = await this._itmsg;
-    promise = promise.then(
+    let array = await Scheduler.post("DirectoryIterator_prototype_nextBatch", [iterator, size]);
-      function withIterator(iterator) {
+    return array.map(DirectoryIterator.Entry.fromMsg);
-        return Scheduler.post("DirectoryIterator_prototype_nextBatch", [iterator, size]);
-      });
-    promise = promise.then(
-      function withEntries(array) {
-        return array.map(DirectoryIterator.Entry.fromMsg);
-      });
-    return promise;
   },
   /**
    * Apply a function to all elements of the directory sequentially.
@@ -1342,82 +1316,51 @@ DirectoryIterator.prototype = {
    * @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;
+    let iterator = await this._itmsg;
-
+    while (true) {
-    // Grab iterator
+      if (this._isClosed) {
-    let promise = this._itmsg.then(
+        return undefined;
-      function(aIterator) {
-        iterator = aIterator;
       }
-    );
+      let {value, done} = await this._next(iterator);
-
+      if (done) {
-    // Then iterate
+        return undefined;
-    let loop = function loop() {
-      if (self._isClosed) {
-        return Promise.resolve();
       }
-      return self._next(iterator).then(
+      await cb(value, position++, this);
-        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);
   },
   /**
    * Auxiliary method: fetch the next item
    *
-   * @rejects {StopIteration} If all entries have already been visited
+   * @resolves `{value: undefined, done: true}` If all entries have already
-   * or the iterator has been closed.
+   * 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 {value, done} = await Scheduler.post("DirectoryIterator_prototype_next", [iterator]);
-    let promise = Scheduler.post("DirectoryIterator_prototype_next", [iterator]);
+    if (done) {
-    promise = promise.then(
+      this.close();
-      DirectoryIterator.Entry.fromMsg,
+      return {value: undefined, done: true};
-      function onReject(reason) {
+    }
-        if (reason == StopIteration) {
+    return {value: DirectoryIterator.Entry.fromMsg(value), done: false};
-          self.close();
-          throw StopIteration;
-        }
-        throw reason;
-      });
-    return promise;
   },
   /**
    * Close the iterator
    */
-  close: function close() {
+  async close() {
     if (this._isClosed) {
-      return Promise.resolve();
+      return undefined;
     }
     this._isClosed = true;
-    let self = this;
+    let iterator = this._itmsg;
-    return this._itmsg.then(
+    this._itmsg = null;
-      function withIterator(iterator) {
+    return Scheduler.post("DirectoryIterator_prototype_close", [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]);
-      }
-    );
   }
 };
 

toolkit/components/osfile/modules/osfile_async_worker.js

@@ -356,14 +356,12 @@ if (this.Components) {
    DirectoryIterator_prototype_next: function next(dir) {
      return withDir(dir,
        function do_next() {
-         try {
+         let {value, done} = this.next();
-           return File.DirectoryIterator.Entry.toMsg(this.next());
+         if (done) {
-         } catch (x) {
+           OpenedDirectoryIterators.remove(dir);
-           if (x == StopIteration) {
+           return {value: undefined, done: true};
-             OpenedDirectoryIterators.remove(dir);
-           }
-           throw x;
          }
+         return {value: File.DirectoryIterator.Entry.toMsg(value), done: false};
        }, false);
    },
    DirectoryIterator_prototype_nextBatch: function nextBatch(dir, size) {

toolkit/components/osfile/modules/osfile_shared_front.jsm

@@ -190,9 +190,9 @@ AbstractFile.AbstractIterator = function AbstractIterator() {
 };
 AbstractFile.AbstractIterator.prototype = {
   /**
-   * Allow iterating with |for|
+   * Allow iterating with |for-of|
    */
-  __iterator__: function __iterator__() {
+  [Symbol.iterator]() {
     return this;
   },
   /**
@@ -207,7 +207,7 @@ AbstractFile.AbstractIterator.prototype = {
    */
   forEach: function forEach(cb) {
     let index = 0;
-    for (let entry in this) {
+    for (let entry of this) {
       cb(entry, index++, this);
     }
   },
@@ -225,7 +225,7 @@ AbstractFile.AbstractIterator.prototype = {
   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;
@@ -503,7 +503,7 @@ AbstractFile.removeRecursive = function(path, options = {}) {
   }
 
   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

toolkit/components/osfile/modules/osfile_unix_front.jsm

@@ -732,16 +732,16 @@
       *
       * Skip special directories "." and "..".
       *
-      * @return {File.Entry} The next entry in the directory.
+      * @return By definition of the iterator protocol, either
-      * @throws {StopIteration} Once all files in the directory have been
+      * `{value: {File.Entry}, done: false}` if there is an unvisited entry
-      * encountered.
+      * 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();
@@ -764,10 +764,13 @@
            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};
      };
 
      /**

toolkit/components/osfile/modules/osfile_win_front.jsm

@@ -811,9 +811,9 @@
       *
       * Skip special directories "." and "..".
       *
-      * @return {File.Entry} The next entry in the directory.
+      * @return By definition of the iterator protocol, either
-      * @throws {StopIteration} Once all files in the directory have been
+      * `{value: {File.Entry}, done: false}` if there is an unvisited entry
-      * encountered.
+      * 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
@@ -824,9 +824,12 @@
            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() {

toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js

@@ -215,7 +215,7 @@ function test_iter_dir() {
   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);
 
@@ -266,7 +266,7 @@ function test_iter_dir() {
   // 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();

toolkit/components/search/nsSearchService.js

@@ -2973,12 +2973,9 @@ SearchService.prototype = {
                                                    { winPattern: "*.xml" });
       try {
         // Add dir to distDirs if it contains any files.
-        await checkForSyncCompletion(iterator.next());
+        let {done} = await checkForSyncCompletion(iterator.next());
-        distDirs.push(dir);
+        if (!done) {
-      } catch (ex) {
+          distDirs.push(dir);
-        // Catch for StopIteration exception.
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
         }
       } finally {
         iterator.close();
@@ -2998,12 +2995,9 @@ SearchService.prototype = {
                                                    { winPattern: "*.xml" });
       try {
         // Add dir to otherDirs if it contains any files.
-        await checkForSyncCompletion(iterator.next());
+        let {done} = await checkForSyncCompletion(iterator.next());
-        otherDirs.push(dir);
+        if (!done) {
-      } catch (ex) {
+          otherDirs.push(dir);
-        // Catch for StopIteration exception.
-        if (ex.result == Cr.NS_ERROR_ALREADY_INITIALIZED) {
-          throw ex;
         }
       } finally {
         iterator.close();

toolkit/components/thumbnails/PageThumbsWorker.js

@@ -89,7 +89,7 @@ var Agent = {
       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);
         }
@@ -108,7 +108,7 @@ var Agent = {
     }
     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;
         }
@@ -153,7 +153,7 @@ var Agent = {
   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) {

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -6567,8 +6567,11 @@ class SystemAddonInstallLocation extends MutableDirectoryInstallLocation {
     }
 
     try {
-      for (let promise in iterator) {
+      for (;;) {
-        let entry = await promise;
+        let {value: entry, done} = await iterator.next();
+        if (done) {
+          break;
+        }
 
         // Skip the directory currently in use
         if (this._directory && this._directory.path == entry.path) {
@@ -6581,16 +6584,16 @@ class SystemAddonInstallLocation extends MutableDirectoryInstallLocation {
         }
 
         if (entry.isDir) {
-           await OS.File.removeDir(entry.path, {
+          await OS.File.removeDir(entry.path, {
-             ignoreAbsent: true,
+            ignoreAbsent: true,
-             ignorePermissions: true,
+            ignorePermissions: true,
-           });
+          });
-         } else {
+        } else {
-           await OS.File.remove(entry.path, {
+          await OS.File.remove(entry.path, {
-             ignoreAbsent: true,
+            ignoreAbsent: true,
-           });
+          });
-         }
+        }
-       }
+      }
 
     } catch (e) {
       logger.error("Failed to clean updated system add-ons directories.", e);