Bug 1649602, remove OS.File from PageThumbs.jsm r=Gijs,barret

Differential Revision: https://phabricator.services.mozilla.com/D101066

toolkit/components/promiseworker/PromiseWorker.jsm

@@ -102,6 +102,10 @@ const EXCEPTION_CONSTRUCTORS = {
     result.stack = error.stack;
     return result;
   },
+  DOMException(error) {
+    let result = new DOMException(error.message, error.name);
+    return result;
+  },
 };
 
 /**

toolkit/components/promiseworker/worker/PromiseWorker.js

@@ -172,6 +172,16 @@ AbstractWorker.prototype = {
       } else {
         this.postMessage({ ok: result, id, durationMs });
       }
+    } else if (exn.constructor.name == "DOMException") {
+      // We can receive instances of DOMExceptions with file I/O.
+      // DOMExceptions are not yet serializable (Bug 1561357) and must be
+      // handled differently, as they only have a name and message
+      this.log("Sending back DOM exception", exn.constructor.name);
+      let error = {
+        exn: exn.constructor.name,
+        message: exn.message,
+      };
+      this.postMessage({ fail: error, id, durationMs });
     } else if (exn.constructor.name in EXCEPTION_NAMES) {
       // Rather than letting the DOM mechanism [de]serialize built-in
       // JS errors, which loses lots of information (in particular,

toolkit/components/thumbnails/PageThumbs.jsm

@@ -28,7 +28,6 @@ const { XPCOMUtils } = ChromeUtils.import(
 const { BasePromiseWorker } = ChromeUtils.import(
   "resource://gre/modules/PromiseWorker.jsm"
 );
-const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["FileReader"]);
 
@@ -619,9 +618,7 @@ var PageThumbsStorage = {
       aData,
       {
         tmpPath: path + ".tmp",
-        bytes: aData.byteLength,
-        noOverwrite: aNoOverwrite,
-        flush: false /* thumbnails do not require the level of guarantee provided by flush*/,
+        mode: aNoOverwrite ? "create" : "overwrite",
       },
     ];
     return PageThumbsWorker.post(
@@ -733,11 +730,11 @@ var PageThumbsStorage = {
   },
 
   /**
-   * For functions that take a noOverwrite option, OS.File throws an error if
+   * For functions that take a noOverwrite option, IOUtils throws an error if
    * the target file exists and noOverwrite is true.  We don't consider that an
    * error, and we don't want such errors propagated.
    *
-   * @param {aNoOverwrite} The noOverwrite option used in the OS.File operation.
+   * @param {aNoOverwrite} The noOverwrite option used in the IOUtils operation.
    *
    * @return {function} A function that should be passed as the second argument
    * to then() (the `onError` argument).
@@ -746,8 +743,8 @@ var PageThumbsStorage = {
     return function onError(err) {
       if (
         !aNoOverwrite ||
-        !(err instanceof OS.File.Error) ||
-        !err.becauseExists
+        !(err instanceof DOMException) ||
+        err.name !== "TypeMismatchError"
       ) {
         throw err;
       }
@@ -803,12 +800,12 @@ var PageThumbsStorageMigrator = {
    * Used for testing. Default argument is good for all non-testing uses.
    */
   migrateToVersion3: function Migrator_migrateToVersion3(
-    local = OS.Constants.Path.localProfileDir,
-    roaming = OS.Constants.Path.profileDir
+    local = Services.dirsvc.get("ProfLD", Ci.nsIFile).path,
+    roaming = Services.dirsvc.get("ProfD", Ci.nsIFile).path
   ) {
     PageThumbsWorker.post("moveOrDeleteAllThumbnails", [
-      OS.Path.join(roaming, THUMBNAIL_DIRECTORY),
-      OS.Path.join(local, THUMBNAIL_DIRECTORY),
+      PathUtils.join(roaming, THUMBNAIL_DIRECTORY),
+      PathUtils.join(local, THUMBNAIL_DIRECTORY),
     ]);
   },
 };
@@ -881,6 +878,3 @@ var PageThumbsExpiration = {
 var PageThumbsWorker = new BasePromiseWorker(
   "resource://gre/modules/PageThumbsWorker.js"
 );
-// As the PageThumbsWorker performs I/O, we can receive instances of
-// OS.File.Error, so we need to install a decoder.
-PageThumbsWorker.ExceptionHandlers["OS.File.Error"] = OS.File.Error.fromMsg;

toolkit/components/thumbnails/PageThumbsWorker.js

@@ -6,20 +6,14 @@
 
 /**
  * A worker dedicated for the I/O component of PageThumbs storage.
- *
- * Do not rely on the API of this worker. In a future version, it might be
- * fully replaced by a OS.File global I/O worker.
  */
 
 "use strict";
 
-importScripts("resource://gre/modules/osfile.jsm");
+importScripts("resource://gre/modules/workers/require.js");
 
 var PromiseWorker = require("resource://gre/modules/workers/PromiseWorker.js");
 
-var File = OS.File;
-var Type = OS.Shared.Type;
-
 var worker = new PromiseWorker.AbstractWorker();
 worker.dispatch = function(method, args = []) {
   return Agent[method](...args);
@@ -39,14 +33,14 @@ self.addEventListener("unhandledrejection", function(error) {
 var Agent = {
   // Checks if the specified file exists and has an age less than as
   // specifed (in seconds).
-  isFileRecent: function Agent_isFileRecent(path, maxAge) {
+  async isFileRecent(path, maxAge) {
     try {
-      let stat = OS.File.stat(path);
+      let stat = await IOUtils.stat(path);
       let maxDate = new Date();
       maxDate.setSeconds(maxDate.getSeconds() - maxAge);
-      return stat.lastModificationDate > maxDate;
+      return stat.lastModified > maxDate;
     } catch (ex) {
-      if (!(ex instanceof OS.File.Error)) {
+      if (!(ex instanceof DOMException)) {
         throw ex;
       }
       // file doesn't exist (or can't be stat'd) - must be stale.
@@ -54,25 +48,21 @@ var Agent = {
     }
   },
 
-  remove: function Agent_removeFile(path) {
+  async remove(path) {
     try {
-      OS.File.remove(path);
+      await IOUtils.remove(path);
       return true;
     } catch (e) {
       return false;
     }
   },
 
-  expireFilesInDirectory: function Agent_expireFilesInDirectory(
-    path,
-    filesToKeep,
-    minChunkSize
-  ) {
-    let entries = this.getFileEntriesInDirectory(path, filesToKeep);
+  async expireFilesInDirectory(path, filesToKeep, minChunkSize) {
+    let entries = await this.getFileEntriesInDirectory(path, filesToKeep);
     let limit = Math.max(minChunkSize, Math.round(entries.length / 2));
 
     for (let entry of entries) {
-      this.remove(entry.path);
+      await this.remove(entry);
 
       // Check if we reached the limit of files to remove.
       if (--limit <= 0) {
@@ -83,59 +73,45 @@ var Agent = {
     return true;
   },
 
-  getFileEntriesInDirectory: function Agent_getFileEntriesInDirectory(
-    path,
-    skipFiles
-  ) {
-    let iter = new OS.File.DirectoryIterator(path);
-    try {
-      if (!iter.exists()) {
-        return [];
-      }
+  async getFileEntriesInDirectory(path, skipFiles) {
+    let children = await IOUtils.getChildren(path);
+    let skip = new Set(skipFiles);
 
-      let skip = new Set(skipFiles);
-
-      let entries = [];
-      for (let entry of iter) {
-        if (!entry.isDir && !entry.isSymLink && !skip.has(entry.name)) {
-          entries.push(entry);
-        }
+    let entries = [];
+    for (let entry of children) {
+      let stat = await IOUtils.stat(entry);
+      if (stat.type === "regular" && !skip.has(PathUtils.filename(entry))) {
+        entries.push(entry);
       }
-      return entries;
-    } finally {
-      iter.close();
     }
+    return entries;
   },
 
-  moveOrDeleteAllThumbnails: function Agent_moveOrDeleteAllThumbnails(
-    pathFrom,
-    pathTo
-  ) {
-    OS.File.makeDir(pathTo, { ignoreExisting: true });
+  async moveOrDeleteAllThumbnails(pathFrom, pathTo) {
+    await IOUtils.makeDirectory(pathTo);
     if (pathFrom == pathTo) {
       return true;
     }
-    let iter = new OS.File.DirectoryIterator(pathFrom);
-    if (iter.exists()) {
-      for (let entry of iter) {
-        if (entry.isDir || entry.isSymLink) {
-          continue;
-        }
+    let children = await IOUtils.getChildren(pathFrom);
+    for (let entry of children) {
+      let stat = await IOUtils.stat(entry);
+      if (stat.type !== "regular") {
+        continue;
+      }
 
-        let from = OS.Path.join(pathFrom, entry.name);
-        let to = OS.Path.join(pathTo, entry.name);
+      let fileName = PathUtils.filename(entry);
+      let from = PathUtils.join(pathFrom, fileName);
+      let to = PathUtils.join(pathTo, fileName);
 
-        try {
-          OS.File.move(from, to, { noOverwrite: true, noCopy: true });
-        } catch (e) {
-          OS.File.remove(from);
-        }
+      try {
+        await IOUtils.move(from, to, { noOverwrite: true });
+      } catch (e) {
+        await IOUtils.remove(from);
       }
     }
-    iter.close();
 
     try {
-      OS.File.removeEmptyDir(pathFrom);
+      await IOUtils.remove(pathFrom, { recursive: true });
     } catch (e) {
       // This could fail if there's something in
       // the folder we're not permitted to remove.
@@ -144,40 +120,36 @@ var Agent = {
     return true;
   },
 
-  writeAtomic: function Agent_writeAtomic(path, buffer, options) {
-    return File.writeAtomic(path, buffer, options);
+  writeAtomic(path, buffer, options) {
+    return IOUtils.write(path, buffer, options);
   },
 
-  makeDir: function Agent_makeDir(path, options) {
-    return File.makeDir(path, options);
+  makeDir(path, options) {
+    return IOUtils.makeDirectory(path, options);
   },
 
-  copy: function Agent_copy(source, dest, options) {
-    return File.copy(source, dest, options);
+  copy(source, dest, options) {
+    return IOUtils.copy(source, dest, options);
   },
 
-  wipe: function Agent_wipe(path) {
-    let iterator = new File.DirectoryIterator(path);
-    try {
-      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
-          //  can happen under Windows);
-          // - file has been removed by the user or another process.
-        }
+  async wipe(path) {
+    let children = await IOUtils.getChildren(path);
+    for (let entry of children) {
+      try {
+        await IOUtils.remove(entry);
+      } 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
+        //  can happen under Windows);
+        // - file has been removed by the user or another process.
       }
-    } finally {
-      iterator.close();
     }
   },
 
-  exists: function Agent_exists(path) {
-    return File.exists(path);
+  exists(path) {
+    return IOUtils.exists(path);
   },
 };