Bug 928239 - Fix pump_userland fallback in OS.File.copy. r=yoric

toolkit/components/osfile/modules/osfile_unix_front.jsm

@@ -94,7 +94,7 @@
       * the end of the file has been reached.
       * @throws {OS.File.Error} In case of I/O error.
       */
-     File.prototype._read = function _read(buffer, nbytes, options) {
+     File.prototype._read = function _read(buffer, nbytes, options = {}) {
       // Populate the page cache with data from a file so the subsequent reads
       // from that file will not block on disk I/O.
        if (typeof(UnixFile.posix_fadvise) === 'function' &&
@@ -120,7 +120,7 @@
       * @return {number} The number of bytes effectively written.
       * @throws {OS.File.Error} In case of I/O error.
       */
-     File.prototype._write = function _write(buffer, nbytes, options) {
+     File.prototype._write = function _write(buffer, nbytes, options = {}) {
        return throw_on_negative("write",
          UnixFile.write(this.fd, buffer, nbytes)
        );
@@ -431,6 +431,9 @@
         * @option {number} bufSize A hint regarding the size of the
         * buffer to use for copying. The implementation may decide to
         * ignore this hint.
+        * @option {bool} unixUserland Will force the copy operation to be
+        * caried out in user land, instead of using optimized syscalls such
+        * as splice(2).
         *
         * @throws {OS.File.Error} In case of error.
         */
@@ -550,7 +553,11 @@
            } else {
              dest = File.open(destPath, {trunc:true});
            }
-           result = pump(source, dest, options);
+           if (options.unixUserland) {
+             result = pump_userland(source, dest, options);
+           } else {
+             result = pump(source, dest, options);
+           }
          } catch (x) {
            if (dest) {
              dest.close();

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

@@ -158,7 +158,6 @@ let test = maketest("Main", function main(test) {
     yield test_read_write();
     yield test_read_write_all();
     yield test_position();
-    yield test_copy();
     yield test_mkdir();
     yield test_iter();
     yield test_exists();
@@ -476,40 +475,6 @@ let test_position = maketest("position", function position(test) {
   });
 });
 
-/**
- * Test OS.File.prototype.{copy, move}
- */
-let test_copy = maketest("copy", function copy(test) {
-  return Task.spawn(function() {
-    let currentDir = yield OS.File.getCurrentDirectory();
-    let pathSource = OS.Path.join(currentDir, EXISTING_FILE);
-    let pathDest = OS.Path.join(OS.Constants.Path.tmpDir,
-      "osfile async test 2.tmp");
-    yield OS.File.copy(pathSource, pathDest);
-    test.info("Copy complete");
-    yield reference_compare_files(pathSource, pathDest, test);
-    test.info("First compare complete");
-
-    let pathDest2 = OS.Path.join(OS.Constants.Path.tmpDir,
-      "osfile async test 3.tmp");
-    yield OS.File.move(pathDest, pathDest2);
-    test.info("Move complete");
-    yield reference_compare_files(pathSource, pathDest2, test);
-    test.info("Second compare complete");
-    OS.File.remove(pathDest2);
-
-    try {
-      let field = yield OS.File.open(pathDest);
-      test.fail("I should not have been able to open " + pathDest);
-      file.close();
-    } catch (err) {
-      test.ok(err, "Could not open a file after it has been moved away " + err);
-      test.ok(err instanceof OS.File.Error, "Error is an OS.File.Error");
-      test.ok(err.becauseNoSuchFile, "Error mentions that the file does not exist");
-    }
-  });
-});
-
 /**
  * Test OS.File.{removeEmptyDir, makeDir}
  */

toolkit/components/osfile/tests/xpcshell/test_osfile_async_copy.js

@@ -0,0 +1,109 @@
+"use strict";
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/FileUtils.jsm");
+Components.utils.import("resource://gre/modules/NetUtil.jsm");
+Components.utils.import("resource://gre/modules/Promise.jsm");
+Components.utils.import("resource://gre/modules/Task.jsm");
+
+function run_test() {
+  do_test_pending();
+  run_next_test();
+}
+
+/**
+ * A file that we know exists and that can be used for reading.
+ */
+let EXISTING_FILE = "test_osfile_async_copy.js";
+
+/**
+ * Fetch asynchronously the contents of a file using xpcom.
+ *
+ * Used for comparing xpcom-based results to os.file-based results.
+ *
+ * @param {string} path The _absolute_ path to the file.
+ * @return {promise}
+ * @resolves {string} The contents of the file.
+ */
+let reference_fetch_file = function reference_fetch_file(path) {
+  let promise = Promise.defer();
+  let file = new FileUtils.File(path);
+  NetUtil.asyncFetch(file,
+    function(stream, status) {
+      if (!Components.isSuccessCode(status)) {
+        promise.reject(status);
+        return;
+      }
+      let result, reject;
+      try {
+        result = NetUtil.readInputStreamToString(stream, stream.available());
+      } catch (x) {
+        reject = x;
+      }
+      stream.close();
+      if (reject) {
+        promise.reject(reject);
+      } else {
+        promise.resolve(result);
+      }
+  });
+  return promise.promise;
+};
+
+/**
+ * Compare asynchronously the contents two files using xpcom.
+ *
+ * Used for comparing xpcom-based results to os.file-based results.
+ *
+ * @param {string} a The _absolute_ path to the first file.
+ * @param {string} b The _absolute_ path to the second file.
+ *
+ * @resolves {null}
+ */
+let reference_compare_files = function reference_compare_files(a, b) {
+  let a_contents = yield reference_fetch_file(a);
+  let b_contents = yield reference_fetch_file(b);
+  // Not using do_check_eq to avoid dumping the whole file to the log.
+  // It is OK to === compare here, as both variables contain a string.
+  do_check_true(a_contents === b_contents);
+};
+
+/**
+ * Test to ensure that OS.File.copy works.
+ */
+function test_copymove(options = {}) {
+  let source = OS.Path.join((yield OS.File.getCurrentDirectory()),
+                            EXISTING_FILE);
+  let dest = OS.Path.join(OS.Constants.Path.tmpDir,
+                          "test_osfile_async_copy_dest.tmp");
+  let dest2 = OS.Path.join(OS.Constants.Path.tmpDir,
+                           "test_osfile_async_copy_dest2.tmp");
+  try {
+    // 1. Test copy.
+    yield OS.File.copy(source, dest, options);
+    yield reference_compare_files(source, dest);
+    // 2. Test subsequent move.
+    yield OS.File.move(dest, dest2);
+    yield reference_compare_files(source, dest2);
+    // 3. Check that the moved file was really moved.
+    do_check_eq((yield OS.File.exists(dest)), false);
+  } finally {
+    try {
+      yield OS.File.remove(dest);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore
+    }
+    try {
+      yield OS.File.remove(dest2);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // ignore
+    }
+  }
+}
+
+// Regular copy test.
+add_task(test_copymove);
+// Userland copy test.
+add_task(test_copymove.bind(null, {unixUserland: true}));
+
+add_task(do_test_finished);

toolkit/components/osfile/tests/xpcshell/xpcshell.ini

@@ -6,6 +6,7 @@ tail =
 [test_path.js]
 [test_osfile_async.js]
 [test_osfile_async_bytes.js]
+[test_osfile_async_copy.js]
 [test_profiledir.js]
 [test_logging.js]
 [test_creationDate.js]