Bug 858723 - Fix _fdmsg test in async OS.File. r=froydnj

toolkit/components/osfile/osfile_async_front.jsm

@@ -237,7 +237,7 @@ File.prototype = {
    * @rejects {OS.File.Error}
    */
   close: function close() {
-    if (this._fdmsg) {
+    if (this._fdmsg != null) {
       let msg = this._fdmsg;
       this._fdmsg = null;
       return this._closeResult =
@@ -254,9 +254,6 @@ File.prototype = {
    * @rejects {OS.File.Error}
    */
   stat: function stat() {
-    if (!this._fdmsg) {
-      return Promise.reject(OSError.closed("accessing file"));
-    }
     return Scheduler.post("File_prototype_stat", [this._fdmsg], this).then(
       File.Info.fromMsg
     );

toolkit/components/osfile/osfile_async_worker.js

@@ -162,10 +162,13 @@ if (this.Components) {
       * the file exists, it executes |f| within the |this| set
       * to the corresponding file. Otherwise, it throws an error.
       */
-     let withFile = function withFile(id, f) {
+     let withFile = function withFile(id, f, ignoreAbsent) {
        let file = OpenedFiles.get(id);
        if (file == null) {
-         throw new Error("Could not find File");
+         if (!ignoreAbsent) {
+           throw OS.File.Error.closed("accessing file");
+         }
+         return undefined;
        }
        return f.call(file);
      };
@@ -175,9 +178,9 @@ if (this.Components) {
        let file = OpenedDirectoryIterators.get(fd);
        if (file == null) {
          if (!ignoreAbsent) {
-           throw new Error("Could not find Directory");
+           throw OS.File.Error.closed("accessing directory");
          }
-         return;
+         return undefined;
        }
        if (!(file instanceof File.DirectoryIterator)) {
          throw new Error("file is not a directory iterator " + file.__proto__.toSource());

toolkit/components/osfile/osfile_win_allthreads.jsm

@@ -148,7 +148,7 @@ if (typeof Components != "undefined") {
    */
   Object.defineProperty(OSError.prototype, "becauseClosed", {
     get: function becauseClosed() {
-      return this.winLastError == exports.OS.Constants.Win.INVALID_HANDLE_VALUE;
+      return this.winLastError == exports.OS.Constants.Win.ERROR_INVALID_HANDLE;
     }
   });
 
@@ -336,7 +336,7 @@ if (typeof Components != "undefined") {
 
   // Special constructors that need to be defined on all threads
   OSError.closed = function closed(operation) {
-    return new OSError(operation, exports.OS.Constants.Win.INVALID_HANDLE_VALUE);
+    return new OSError(operation, exports.OS.Constants.Win.ERROR_INVALID_HANDLE);
   };
 
   OSError.exists = function exists(operation) {

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

@@ -695,6 +695,7 @@ let test_iter = maketest("iter", function iter(test) {
       }
       return null;
     });
+    yield iterator.close();
 
     // Ensuring that we find new files if they appear
     let file = yield OS.File.open(temporary_file_name, { write: true } );
@@ -815,45 +816,60 @@ let test_system_shutdown = maketest("system_shutdown", function system_shutdown(
     // Save original DEBUG value.
     let originalDebug = OS.Shared.DEBUG;
     // Create a console listener.
-    function getConsoleListener(resource) {
+    function inDebugTest(resource, f) {
-      let consoleListener = {
+      return Task.spawn(function task() {
+        let originalDebug = OS.Shared.DEBUG;
+        OS.Shared.TEST = true;
+        OS.Shared.DEBUG = true;
+        let waitObservation = Promise.defer();
+        let listener = {
           observe: function (aMessage) {
+            test.info("Waiting for a console message mentioning resource " + resource);
             // Ignore unexpected messages.
             if (!(aMessage instanceof Components.interfaces.nsIConsoleMessage)) {
+              test.info("Not a console message");
               return;
             }
             if (aMessage.message.indexOf("TEST OS Controller WARNING") < 0) {
+              test.info("Not a warning");
               return;
             }
             test.ok(aMessage.message.indexOf("WARNING: File descriptors leaks " +
-            "detected.") >= 0, "File descriptors leaks are logged correctly.");
+              "detected.") >= 0, "Noticing file descriptors leaks, as expected.");
-          test.ok(aMessage.message.indexOf(resource) >= 0,
+            let found = aMessage.message.indexOf(resource) >= 0;
-            "Leaked resource is correctly listed in the log.");
+            if (found) {
-          toggleDebugTest(false, resource, consoleListener);
+              test.ok(true, "Leaked resource is correctly listed in the log.");
+              setTimeout(function() { waitObservation.resolve(); });
+            } else {
+              test.info("This log didn't list the expected resource: " + resource + "\ngot " + aMessage.message);
+            }
           }
         };
-      return consoleListener;
+        Services.console.registerListener(listener);
-    }
+        f();
-    // Set/Unset testing flags and Services.console listener.
+        yield waitObservation.promise;
-    function toggleDebugTest(startDebug, resource, consoleListener) {
+        Services.console.unregisterListener(listener);
-      Services.console[startDebug ? "registerListener" : "unregisterListener"](
+        OS.Shared.DEBUG = originalDebug;
-        consoleListener || getConsoleListener(resource));
+        OS.Shared.TEST = false;
-      OS.Shared.TEST = startDebug;
+        test.info("Unregistered listener for resource " + resource);
-      // If pref is false, restore DEBUG to its original.
+      });
-      OS.Shared.DEBUG = startDebug || originalDebug;
     }
 
     let currentDir = yield OS.File.getCurrentDirectory();
+    test.info("Testing for leaks of directory iterator " + currentDir);
     let iterator = new OS.File.DirectoryIterator(currentDir);
-    toggleDebugTest(true, currentDir);
+    yield inDebugTest(currentDir, function() {
       Services.obs.notifyObservers(null, "test.osfile.web-workers-shutdown",
         null);
+    });
     yield iterator.close();
 
+    test.info("Testing for leaks of file " + EXISTING_FILE);
     let openedFile = yield OS.File.open(EXISTING_FILE);
-    toggleDebugTest(true, EXISTING_FILE);
+    yield inDebugTest(EXISTING_FILE, function() {
       Services.obs.notifyObservers(null, "test.osfile.web-workers-shutdown",
         null);
+    });
     yield openedFile.close();
   });
 });
@@ -916,11 +932,11 @@ let test_duration = maketest("duration", function duration(test) {
     let backupDuration = ARBITRARY_BASE_DURATION;
     // Testing duration of OS.File.copy.
     yield OS.File.copy(pathSource, copyFile, copyOptions);
-    test.ok(copyOptions.outExecutionDuration >= backupDuration);
+    test.ok(copyOptions.outExecutionDuration >= backupDuration, "duration has increased 1");
 
     backupDuration = copyOptions.outExecutionDuration;
     yield OS.File.remove(copyFile, copyOptions);
-    test.ok(copyOptions.outExecutionDuration >= backupDuration);
+    test.ok(copyOptions.outExecutionDuration >= backupDuration, "duration has increased 2");
 
     // Trying an operation where options are cloned.
     // Options structure passed to a OS.File writeAtomic method.
@@ -931,10 +947,14 @@ let test_duration = maketest("duration", function duration(test) {
       tmpPath: tmpPath
     };
     backupDuration = writeAtomicOptions.outExecutionDuration;
+
     yield OS.File.writeAtomic(pathDest, contents, writeAtomicOptions);
-    test.ok(copyOptions.outExecutionDuration >= backupDuration);
+    test.ok(copyOptions.outExecutionDuration >= backupDuration, "duration has increased 3");
     OS.File.remove(pathDest);
 
+    Services.prefs.setBoolPref("toolkit.osfile.log", true);
+    OS.Shared.TEST = true;
+
     // Testing an operation that doesn't take arguments at all
     let file = yield OS.File.open(pathSource);
     yield file.stat();

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

@@ -0,0 +1,48 @@
+"use strict";
+
+Components.utils.import("resource://gre/modules/osfile.jsm");
+Components.utils.import("resource://gre/modules/Task.jsm");
+
+function run_test() {
+  do_test_pending();
+  run_next_test();
+}
+
+add_task(function test_closed() {
+  OS.Shared.DEBUG = true;
+  let currentDir = yield OS.File.getCurrentDirectory();
+  do_print("Open a file, ensure that we can call stat()");
+  let path = OS.Path.join(currentDir, "test_osfile_closed.js");
+  let file = yield OS.File.open(path);
+  yield file.stat();
+  do_check_true(true);
+
+  yield file.close();
+
+  do_print("Ensure that we cannot stat() on closed file");
+  let exn;
+  try {
+    yield file.stat();
+  } catch (ex) {
+    exn = ex;
+  }
+  do_print("Ensure that this raises the correct error");
+  do_check_true(!!exn);
+  do_check_true(exn instanceof OS.File.Error);
+  do_check_true(exn.becauseClosed);
+
+  do_print("Ensure that we cannot read() on closed file");
+  exn = null;
+  try {
+    yield file.read();
+  } catch (ex) {
+    exn = ex;
+  }
+  do_print("Ensure that this raises the correct error");
+  do_check_true(!!exn);
+  do_check_true(exn instanceof OS.File.Error);
+  do_check_true(exn.becauseClosed);
+
+});
+
+add_task(do_test_finished);

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

@@ -2,6 +2,7 @@
 head =
 tail =
 
+[test_osfile_closed.js]
 [test_path.js]
 [test_osfile_async.js]
 [test_profiledir.js]