Bug 897023 - [OS.File] Be smarter with functions that can fail with becauseAbsent/becauseExists. r=Yoric
☠☠ backed out by d4685e3d278e ☠ ☠
authorAvinash Kundaliya <avinash@avinash.com.np>
Mon, 11 Nov 2013 09:02:13 -0500
changeset 154345 3232989e79e717ef0bee0bfbb4ddd338c4c3d736
parent 154344 2c85e4d1d6786a94d94acc706b67d1c33e7aecd9
child 154346 a3ea4025d34036f57600407826060fc3bf9b137e
push id3453
push userryanvm@gmail.com
push dateMon, 11 Nov 2013 14:04:31 +0000
treeherderfx-team@427ec5ff2318 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs897023
milestone28.0a1
Bug 897023 - [OS.File] Be smarter with functions that can fail with becauseAbsent/becauseExists. r=Yoric
toolkit/components/osfile/modules/osfile_unix_front.jsm
toolkit/components/osfile/modules/osfile_win_front.jsm
toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
toolkit/components/osfile/tests/xpcshell/test_makeDir.js
toolkit/components/osfile/tests/xpcshell/test_removeEmptyDir.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
@@ -332,23 +332,24 @@
        }
      };
 
      /**
       * Remove an empty directory.
       *
       * @param {string} path The name of the directory to remove.
       * @param {*=} options Additional options.
-      *   - {bool} ignoreAbsent If |true|, do not fail if the
-      *     directory does not exist yet.
+      *   - {bool} ignoreAbsent If |false|, throw an error if the directory
+      *     does not exist. |true| by default
       */
      File.removeEmptyDir = function removeEmptyDir(path, options = {}) {
        let result = UnixFile.rmdir(path);
        if (result == -1) {
-         if (options.ignoreAbsent && ctypes.errno == Const.ENOENT) {
+         if ((!("ignoreAbsent" in options) || options.ignoreAbsent) &&
+             ctypes.errno == Const.ENOENT) {
            return;
          }
          throw new File.Error("removeEmptyDir");
        }
      };
 
      /**
       * Default mode for opening directories.
@@ -361,27 +362,29 @@
       * @param {string} path The name of the directory.
       * @param {*=} options Additional options. This
       * implementation interprets the following fields:
       *
       * - {number} unixMode If specified, a file creation mode,
       * as per libc function |mkdir|. If unspecified, dirs are
       * created with a default mode of 0700 (dir is private to
       * the user, the user can read, write and execute).
-      * - {bool} ignoreExisting If |true|, do not fail if the
-      * directory already exists.
+      * - {bool} ignoreExisting If |false|, throw error if the directory
+      * already exists. |true| by default
       */
      File.makeDir = function makeDir(path, options = {}) {
        let omode = options.unixMode !== undefined ? options.unixMode : DEFAULT_UNIX_MODE_DIR;
        let result = UnixFile.mkdir(path, omode);
-       if (result != -1 ||
-           options.ignoreExisting && ctypes.errno == Const.EEXIST) {
-        return;
+       if (result == -1) {
+         if ((!("ignoreExisting" in options) || options.ignoreExisting) &&
+             ctypes.errno == Const.EEXIST) {
+           return;
+         }
+         throw new File.Error("makeDir");
        }
-       throw new File.Error("makeDir");
      };
 
      /**
       * Copy a file to a destination.
       *
       * @param {string} sourcePath The platform-specific path at which
       * the file may currently be found.
       * @param {string} destPath The platform-specific path at which the
--- a/toolkit/components/osfile/modules/osfile_win_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_win_front.jsm
@@ -405,23 +405,23 @@
        throw new File.Error("remove");
      };
 
      /**
       * Remove an empty directory.
       *
       * @param {string} path The name of the directory to remove.
       * @param {*=} options Additional options.
-      *   - {bool} ignoreAbsent If |true|, do not fail if the
-      *     directory does not exist yet.
+      *   - {bool} ignoreAbsent If |false|, throw an error if the directory
+      *     does not exist. |true| by default
       */
      File.removeEmptyDir = function removeEmptyDir(path, options = {}) {
        let result = WinFile.RemoveDirectory(path);
        if (!result) {
-         if (options.ignoreAbsent &&
+         if ((!("ignoreAbsent" in options) || options.ignoreAbsent) &&
              ctypes.winLastError == Const.ERROR_FILE_NOT_FOUND) {
            return;
          }
          throw new File.Error("removeEmptyDir");
        }
      };
 
      /**
@@ -430,28 +430,29 @@
       * @param {string} path The name of the directory.
       * @param {*=} options Additional options. This
       * implementation interprets the following fields:
       *
       * - {C pointer} winSecurity If specified, security attributes
       * as per winapi function |CreateDirectory|. If unspecified,
       * use the default security descriptor, inherited from the
       * parent directory.
-      * - {bool} ignoreExisting If |true|, do not fail if the
-      * directory already exists.
+      * - {bool} ignoreExisting If |false|, throw an error if the directory
+      * already exists. |true| by default
       */
      File.makeDir = function makeDir(path, options = {}) {
        let security = options.winSecurity || null;
        let result = WinFile.CreateDirectory(path, security);
-       if (result ||
-           options.ignoreExisting &&
-           ctypes.winLastError == Const.ERROR_ALREADY_EXISTS) {
-        return;
+       if (!result) {
+         if ((!("ignoreExisting" in options) || options.ignoreExisting) &&
+             ctypes.winLastError == Const.ERROR_ALREADY_EXISTS) {
+           return;
+         }
+         throw new File.Error("makeDir");
        }
-       throw new File.Error("makeDir");
      };
 
      /**
       * Copy a file to a destination.
       *
       * @param {string} sourcePath The platform-specific path at which
       * the file may currently be found.
       * @param {string} destPath The platform-specific path at which the
--- a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
+++ b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
@@ -152,17 +152,16 @@ let test = maketest("Main", function mai
     yield test_constants();
     yield test_path();
     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_mkdir();
     yield test_iter();
     yield test_exists();
     yield test_debug_test();
     yield test_duration();
     info("Test is over");
     SimpleTest.finish();
   });
 });
@@ -425,78 +424,16 @@ let test_position = maketest("position",
       }
     } finally {
       yield file.close();
     }
   });
 });
 
 /**
- * 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});
-
-    // Remove an absent directory with ignoreAbsent
-    yield OS.File.removeEmptyDir(DIRNAME, {ignoreAbsent: true});
-    test.ok(true, "Removing an absent directory with ignoreAbsent succeeds");
-
-    // Remove an absent directory without ignoreAbsent
-    try {
-      yield OS.File.removeEmptyDir(DIRNAME);
-      test.fail("Removing an absent directory without ignoreAbsent should have failed");
-    } catch (err) {
-      test.ok(err, "Removing an absent directory without ignoreAbsent throws the right error");
-      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");
-    }
-
-    // Creating a directory (should succeed)
-    test.ok(true, "Creating a directory");
-    yield OS.File.makeDir(DIRNAME);
-    let stat = yield OS.File.stat(DIRNAME);
-    test.ok(stat.isDir, "I have effectively created a directory");
-
-    // Creating a directory with ignoreExisting (should succeed)
-    try {
-      yield OS.File.makeDir(DIRNAME, {ignoreExisting: true});
-      test.ok(true, "Creating a directory with ignoreExisting succeeds");
-    } catch(err) {
-      test.ok(false, "Creating a directory with ignoreExisting fails");
-    }
-
-    // Creating a directory (should fail)
-    try {
-      yield OS.File.makeDir(DIRNAME);
-      test.fail("Creating over an existing directory should have failed");
-    } catch (err) {
-      test.ok(err, "Creating over an existing directory throws the right error");
-      test.ok(err instanceof OS.File.Error, "Error is an OS.File.Error");
-      test.ok(err.becauseExists, "Error mentions that the file already exists");
-    }
-
-    // Remove a directory and check the result
-    yield OS.File.removeEmptyDir(DIRNAME);
-    test.ok(true, "Removing empty directory suceeded");
-    try {
-      yield OS.File.stat(DIRNAME);
-      test.fail("Removing directory should have failed");
-    } catch (err) {
-      test.ok(err, "Directory was effectively removed");
-      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.prototype.{DirectoryIterator}
  */
 let test_iter = maketest("iter", function iter(test) {
   return Task.spawn(function() {
     let currentDir = yield OS.File.getCurrentDirectory();
 
     // Trivial walks through the directory
     test.info("Preparing iteration");
new file mode 100644
--- /dev/null
+++ b/toolkit/components/osfile/tests/xpcshell/test_makeDir.js
@@ -0,0 +1,56 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+do_register_cleanup(function() {
+  Services.prefs.setBoolPref("toolkit.osfile.log", false);
+});
+
+function run_test() {
+  Services.prefs.setBoolPref("toolkit.osfile.log", true);
+
+  run_next_test();
+}
+
+/**
+ * Test OS.File.makeDir
+ */
+add_task(function() {
+  // Set up profile. We create the directory in the profile, because the profile
+  // is removed after every test run.
+  do_get_profile();
+
+  let dir = OS.Path.join(OS.Constants.Path.profileDir, "directory");
+
+  // Sanity checking for the test
+  do_check_false((yield OS.File.exists(dir)));
+
+  // Make a directory
+  yield OS.File.makeDir(dir);
+
+  //check if the directory exists
+  yield OS.File.stat(dir);
+
+  // Make a directory that already exists
+  yield OS.File.makeDir(dir);
+
+  // Make a directory with ignoreExisting
+  yield OS.File.makeDir(dir, {ignoreExisting: true});
+
+  // Make a directory with ignoreExisting false
+  let exception = null;
+  try {
+    yield OS.File.makeDir(dir, {ignoreExisting: false});
+  } catch (ex) {
+    exception = ex;
+  }
+
+  do_check_true(!!exception);
+  do_check_true(exception instanceof OS.File.Error);
+  do_check_true(exception.becauseExists);
+});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/osfile/tests/xpcshell/test_removeEmptyDir.js
@@ -0,0 +1,55 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+do_register_cleanup(function() {
+  Services.prefs.setBoolPref("toolkit.osfile.log", false);
+});
+
+function run_test() {
+  Services.prefs.setBoolPref("toolkit.osfile.log", true);
+
+  run_next_test();
+}
+
+/**
+ * Test OS.File.removeEmptyDir
+ */
+add_task(function() {
+  // Set up profile. We create the directory in the profile, because the profile
+  // is removed after every test run.
+  do_get_profile();
+
+  let dir = OS.Path.join(OS.Constants.Path.profileDir, "directory");
+
+  // Sanity checking for the test
+  do_check_false((yield OS.File.exists(dir)));
+
+  // Remove non-existent directory
+  yield OS.File.removeEmptyDir(dir);
+
+  // Remove non-existent directory with ignoreAbsent
+  yield OS.File.removeEmptyDir(dir, {ignoreAbsent: true});
+
+  // Remove non-existent directory with ignoreAbsent false
+  let exception = null;
+  try {
+    yield OS.File.removeEmptyDir(dir, {ignoreAbsent: false});
+  } catch (ex) {
+    exception = ex;
+  }
+
+  do_check_true(!!exception);
+  do_check_true(exception instanceof OS.File.Error);
+  do_check_true(exception.becauseNoSuchFile);
+
+  // Remove empty directory
+  yield OS.File.makeDir(dir);
+  yield OS.File.removeEmptyDir(dir);
+  do_check_false((yield OS.File.exists(dir)));
+});
--- a/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ -5,16 +5,18 @@ tail =
 [test_osfile_closed.js]
 [test_path.js]
 [test_osfile_async.js]
 [test_osfile_async_append.js]
 [test_osfile_async_bytes.js]
 [test_osfile_async_copy.js]
 [test_osfile_async_flush.js]
 [test_osfile_async_setDates.js]
+[test_removeEmptyDir.js]
+[test_makeDir.js]
 [test_profiledir.js]
 [test_logging.js]
 [test_creationDate.js]
 [test_exception.js]
 [test_path_constants.js]
 [test_removeDir.js]
 [test_reset.js]
 [test_shutdown.js]