Bug 928239 - Fix pump_userland fallback in OS.File.copy. r=yoric
authorNils Maier <maierman@web.de>
Mon, 21 Oct 2013 06:50:00 +0100
changeset 166710 ec99b2269e95ada1153d72fc4ce7ef75e37fccfc
parent 166709 b9c7369eb6159db08f0239e934e4a9b1f581af89
child 166711 af63dffeb63f8f196668e5b81fa4ebc6bd843302
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs928239
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 928239 - Fix pump_userland fallback in OS.File.copy. r=yoric
toolkit/components/osfile/modules/osfile_unix_front.jsm
toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
toolkit/components/osfile/tests/xpcshell/test_osfile_async_copy.js
toolkit/components/osfile/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/osfile/modules/osfile_unix_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_unix_front.jsm
@@ -89,17 +89,17 @@
       * of bytes unread in the file.
       * @param {*=} options Additional options for reading. Ignored in
       * this implementation.
       *
       * @return {number} The number of bytes effectively read. If zero,
       * 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' &&
            (options.sequential || !("sequential" in options))) {
          UnixFile.posix_fadvise(this.fd, 0, nbytes,
           OS.Constants.libc.POSIX_FADV_SEQUENTIAL);
        }
        return throw_on_negative("read",
@@ -115,17 +115,17 @@
       * @param {number} nbytes The number of bytes to write. It must not
       * exceed the size of |buffer| in bytes.
       * @param {*=} options Additional options for writing. Ignored in
       * this implementation.
       *
       * @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)
        );
      };
 
      /**
       * Return the current position in the file.
       */
@@ -426,16 +426,19 @@
         * @param {*=} options An object which may contain the following fields:
         *
         * @option {number} nbytes The maximal number of bytes to
         * copy. If unspecified, copy everything from the current
         * position.
         * @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.
         */
        let pump;
 
        // A buffer used by |pump_userland|
        let pump_buffer = null;
 
@@ -545,17 +548,21 @@
          let result;
          try {
            source = File.open(sourcePath);
            if (options.noOverwrite) {
              dest = File.open(destPath, {create:true});
            } 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();
            }
            if (source) {
              source.close();
            }
            throw x;
--- a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
+++ b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
@@ -153,17 +153,16 @@ let test = maketest("Main", function mai
     yield test_path();
     yield test_open();
     yield test_stat();
     yield test_debug();
     yield test_info_features_detect();
     yield test_read_write();
     yield test_read_write_all();
     yield test_position();
-    yield test_copy();
     yield test_mkdir();
     yield test_iter();
     yield test_exists();
     yield test_debug_test();
     yield test_system_shutdown();
     yield test_duration();
     info("Test is over");
     SimpleTest.finish();
@@ -472,50 +471,16 @@ let test_position = maketest("position",
       }
     } finally {
       yield file.close();
     }
   });
 });
 
 /**
- * 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}
  */
 let test_mkdir = maketest("mkdir", function mkdir(test) {
   return Task.spawn(function() {
     const DIRNAME = "test_dir.tmp";
 
     // Cleanup
     yield OS.File.removeEmptyDir(DIRNAME, {ignoreAbsent: true});
new file mode 100644
--- /dev/null
+++ b/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);
--- a/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ -1,15 +1,16 @@
 [DEFAULT]
 head =
 tail =
 
 [test_osfile_closed.js]
 [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]
 [test_exception.js]
 [test_path_constants.js]
 [test_removeDir.js]
 [test_unique.js]