Fix bug #6609 (Fast file replacement operations can be missed)

src/file/FileUtils.js

@@ -45,8 +45,8 @@ define(function (require, exports, module) {
      * Asynchronously reads a file as UTF-8 encoded text.
      * @param {!File} file File to read
      * @return {$.Promise} a jQuery promise that will be resolved with the 
-     *  file's text content plus its timestamp, or rejected with a FileSystemError if
-     *  the file can not be read.
+     *  file's text content plus its timestamp, or rejected with a FileSystemError string
+     *  constant if the file can not be read.
      */
     function readAsText(file) {
         var result = new $.Deferred();
@@ -77,7 +77,7 @@ define(function (require, exports, module) {
      *      errors---which can be triggered if the actual file contents differ from 
      *      the FileSystem's last-known contents---should be ignored.
      * @return {$.Promise} a jQuery promise that will be resolved when
-     * file writing completes, or rejected with a FileSystemError.
+     * file writing completes, or rejected with a FileSystemError string constant.
      */
     function writeText(file, text, allowBlindWrite) {
         var result = new $.Deferred(),

src/filesystem/FileSystem.js

@@ -46,6 +46,11 @@
  *          *  a File - the contents of the file have changed, and should be reloaded.
  *          *  a Directory - an immediate child of the directory has been added, removed,
  *             or renamed/moved. Not triggered for "grandchildren".
+ *               - If the added & removed arguments are null, we don't know what was added/removed:
+ *                 clients should assume the whole subtree may have changed.
+ *               - If the added & removed arguments are 0-length, there's no net change in the set
+ *                 of files but a file may have been replaced: clients should assume the contents
+ *                 of any immediate child file may have changed.
  *          *  null - a 'wholesale' change happened, and you should assume everything may
  *             have changed.
  *          For changes made externally, there may be a significant delay before a "change" event
@@ -777,9 +782,11 @@ define(function (require, exports, module) {
             } else {
                 this._handleDirectoryChange(entry, function (added, removed) {
                     entry._stat = stat;
-                    if (!(added && added.length === 0 && removed && removed.length === 0)) {
-                        this._fireChangeEvent(entry, added, removed);
-                    }
+
+                    // We send a change even if added & removed are both zero-length. Something may still have changed,
+                    // e.g. a file may have been quickly removed & re-added before we got a chance to reread the directory
+                    // listing.
+                    this._fireChangeEvent(entry, added, removed);
                 }.bind(this));
             }
         }

test/spec/FileSystem-test.js

@@ -1311,6 +1311,8 @@ define(function (require, exports, module) {
                 });
             });
         });
+
+
         describe("External change events", function () {
             var _model,
                 changedEntry,
@@ -1454,6 +1456,44 @@ define(function (require, exports, module) {
                     expect(removedEntries[0]).toBe(oldfile);
                 });
             });
+
+            it("should fire change event after rapid delete-add pair", function () {
+                var dirname = "/subdir/",
+                    filename = "/subdir/file3.txt",
+                    dir,
+                    newfile;
+
+                runs(function () {
+                    // Delay watcher change notifications so that the FS doesn't get a chance to
+                    // read the directory contents in between the deletion and the re-creation
+                    MockFileSystemImpl.when("change", "/subdir/", delay(100));
+
+                    dir = fileSystem.getDirectoryForPath(dirname);
+
+                    dir.getContents(function () {
+                        _model.unlink(filename);
+                    });
+
+                    // Normally we'd get a change event here, but due to our delay the FS doesn't
+                    // know of the change yet and thus has no reason to trigger an event
+                    expect(changeDone).toBe(false);
+
+                    dir.getContents(function () {
+                        _model.writeFile(filename, "new file content");
+                    });
+                });
+
+                waitsFor(function () { return changeDone; }, "external change event");
+
+                runs(function () {
+                    // We should still receive a change event, but the dir-contents diff shows no changes
+                    // since it didn't happen in between the delete and the create - so we expect the added
+                    // & removed lists to both be empty.
+                    expect(changedEntry).toBe(dir);
+                    expect(addedEntries.length).toBe(0);
+                    expect(removedEntries.length).toBe(0);
+                });
+            });
         });
     });
 });

test/spec/MockFileSystemImpl.js

@@ -229,7 +229,8 @@ define(function (require, exports, module) {
 
         $(_model).on("change", function (event, path) {
             if (_changeCallback) {
-                _changeCallback(path, _model.stat(path));
+                var cb = _getCallback("change", path, _changeCallback);
+                cb(path, _model.stat(path));
             }
         });
 
@@ -252,7 +253,8 @@ define(function (require, exports, module) {
      * Add callback hooks to be used when specific methods are called with a
      * specific path.
      *
-     * @param {string} method The name of the method
+     * @param {string} method The name of the method. The special name "change"
+     *          may be used to hook the "change" event handler as well.
      * @param {string} path The path that must be matched
      * @param {function} getCallback A function that has one parameter and
      *           must return a callback function.