Files.app: Fix flakiness in chaining directory

ui/file_manager/file_manager/foreground/js/directory_model.js

Index: ui/file_manager/file_manager/foreground/js/directory_model.js
diff --git a/ui/file_manager/file_manager/foreground/js/directory_model.js b/ui/file_manager/file_manager/foreground/js/directory_model.js
index 810a12706b2273bb31ef6388f71004d72f55ed50..301ab4beb5cd79d2ed4ae8af42cbfc111ff2c81e 100644
--- a/ui/file_manager/file_manager/foreground/js/directory_model.js
+++ b/ui/file_manager/file_manager/foreground/js/directory_model.js
@@ -32,6 +32,8 @@ function DirectoryModel(singleSelection, fileFilter, fileWatcher,
   this.scanFailures_ = 0;
   this.changeDirectorySequence_ = 0;
 
+  this.directoryChangeQueue_ = new AsyncUtil.Queue();
+
   this.fileFilter_ = fileFilter;
   this.fileFilter_.addEventListener('changed',
                                     this.onFilterChanged_.bind(this));
@@ -278,8 +280,16 @@ DirectoryModel.prototype.scheduleRescan = function(delay) {
     clearTimeout(this.rescanTimeoutId_);
   }
 
+  var tracker = this.createDirectoryChangeTracker();

[hirono, 2014/06/02 09:18:03]

It looks tracker does not detect starting search, but
this.changeDirectorySequence_ do. Is it intended?

[yoshiki, 2014/06/02 11:37:11]

Good catch. search should be detected, since it changes DirectoryContents.

+  tracker.start();
+
   this.rescanTime_ = Date.now() + delay;
-  this.rescanTimeoutId_ = setTimeout(this.rescan.bind(this), delay);
+  this.rescanTimeoutId_ = setTimeout(function() {
+    this.rescanTimeoutId_ = null;
+    tracker.stop();
+    if (!tracker.hasChanged)
+      this.rescan();
+  }.bind(this), delay);
 };
 
 /**
@@ -312,8 +322,13 @@ DirectoryModel.prototype.rescan = function() {
   var dirContents = this.currentDirContents_.clone();
   dirContents.setFileList([]);
 
+  var tracker = this.createDirectoryChangeTracker();
+  tracker.start();
+
   var successCallback = (function() {
-    this.replaceDirectoryContents_(dirContents);
+    tracker.stop();
+    if (!tracker.hasChanged)
+      this.replaceDirectoryContents_(dirContents);
     cr.dispatchSimpleEvent(this, 'rescan-completed');
   }).bind(this);
 
@@ -329,11 +344,11 @@ DirectoryModel.prototype.rescan = function() {
  *
  * @param {DirectoryContentes} newDirContents New DirectoryContents instance to
  *     replace currentDirContents_.
- * @param {function()=} opt_callback Called on success.
+ * @param {function()} callback Callback.

[hirono, 2014/06/02 09:18:03]

Boolean is needed as an argument?

[yoshiki, 2014/06/02 11:37:11]

Done.

  * @private
  */
 DirectoryModel.prototype.clearAndScan_ = function(newDirContents,
-                                                  opt_callback) {
+                                                  callback) {
   if (this.currentDirContents_.isScanning())
     this.currentDirContents_.cancelScan();
   this.currentDirContents_ = newDirContents;
@@ -348,22 +363,46 @@ DirectoryModel.prototype.clearAndScan_ = function(newDirContents,
     this.runningScan_ = null;
   }
 
+  var sequence = this.changeDirectorySequence_;
+  var cancelled = false;
+
   var onDone = function() {
+    if (cancelled)
+      return;
+
     cr.dispatchSimpleEvent(this, 'scan-completed');
-    if (opt_callback)
-      opt_callback();
+    callback(true);
   }.bind(this);
 
   var onFailed = function() {
+    if (cancelled)
+      return;
+
     cr.dispatchSimpleEvent(this, 'scan-failed');
+    callback(false);
   }.bind(this);
 
   var onUpdated = function() {
+    if (cancelled)
+      return;
+
+    if (this.changeDirectorySequence_ !== sequence) {
+      cancelled = true;
+      cr.dispatchSimpleEvent(this, 'scan-cancelled');
+      callback(false);
+      return;
+    }
+
     cr.dispatchSimpleEvent(this, 'scan-updated');
   }.bind(this);
 
   var onCancelled = function() {
+    if (cancelled)
+      return;
+
+    cancelled = true;
     cr.dispatchSimpleEvent(this, 'scan-cancelled');
+    callback(false);
   }.bind(this);
 
   // Clear the table, and start scanning.
@@ -668,26 +707,35 @@ DirectoryModel.prototype.changeDirectoryEntry = function(
   this.changeDirectorySequence_++;
   this.clearSearch_();
 
-  var promise = new Promise(
-      function(onFulfilled, onRejected) {
-        this.fileWatcher_.changeWatchedDirectory(dirEntry, onFulfilled);
-      }.bind(this)).
-
-      then(function(sequence) {
-        return new Promise(function(onFulfilled, onRejected) {
-          if (this.changeDirectorySequence_ !== sequence)
+  this.directoryChangeQueue_.run(function(sequence, callback) {

[hirono, 2014/06/02 09:18:03]

Maybe more specific name is needed for callback, since having both opt_callback
and callback is confusing.

[yoshiki, 2014/06/02 11:37:11]

Done.

+    this.fileWatcher_.changeWatchedDirectory(
+        dirEntry,
+        function() {
+          if (this.changeDirectorySequence_ !== sequence) {
+            callback();
             return;
+          }
 
           var newDirectoryContents = this.createDirectoryContents_(
               this.currentFileListContext_, dirEntry, '');
-          if (!newDirectoryContents)
+          if (!newDirectoryContents) {
+            callback();
             return;
+          }
 
-          var previousDirEntry = this.currentDirContents_.getDirectoryEntry();
-          this.clearAndScan_(newDirectoryContents, opt_callback);
-
-          // For tests that open the dialog to empty directories, everything is
-          // loaded at this point.
+          var previousDirEntry =
+              this.currentDirContents_.getDirectoryEntry();
+          this.clearAndScan_(
+              newDirectoryContents,
+              function(result) {
+                // Calls the callback when successful.
+                if (result && opt_callback)
+                  opt_callback();
+                callback();
+              });
+
+          // For tests that open the dialog to empty directories, everything
+          // is loaded at this point.
           util.testSendMessage('directory-change-complete');
 
           var event = new Event('directory-changed');
@@ -695,7 +743,7 @@ DirectoryModel.prototype.changeDirectoryEntry = function(
           event.newDirEntry = dirEntry;
           this.dispatchEvent(event);
         }.bind(this));
-      }.bind(this, this.changeDirectorySequence_));
+  }.bind(this, this.changeDirectorySequence_));
 };
 
 /**
@@ -903,25 +951,41 @@ DirectoryModel.prototype.search = function(query,
     return;
   }
 
-  if (!(query || '').trimLeft()) {
-    if (this.isSearching()) {
-      var newDirContents = this.createDirectoryContents_(
-          this.currentFileListContext_,
-          currentDirEntry);
-      this.clearAndScan_(newDirContents);
+  this.changeDirectorySequence_++;
+  this.directoryChangeQueue_.run(function(sequence, callback) {
+    if (this.changeDirectorySequence_ !== sequence) {
+      callback();
+      return;
     }
-    return;
-  }
 
-  var newDirContents = this.createDirectoryContents_(
-      this.currentFileListContext_, currentDirEntry, query);
-  if (!newDirContents)
-    return;
+    if (!(query || '').trimLeft()) {
+      if (this.isSearching()) {
+        var newDirContents = this.createDirectoryContents_(
+            this.currentFileListContext_,
+            currentDirEntry);
+        this.clearAndScan_(newDirContents,
+                           sequence,
+                           callback);
+      } else {
+        callback();
+      }
+      return;
+    }
+
+    var newDirContents = this.createDirectoryContents_(
+        this.currentFileListContext_, currentDirEntry, query);
+    if (!newDirContents) {
+      callback();
+      return;
+    }
 
-  this.onSearchCompleted_ = onSearchRescan;
-  this.onClearSearch_ = onClearSearch;
-  this.addEventListener('scan-completed', this.onSearchCompleted_);
-  this.clearAndScan_(newDirContents);
+    this.onSearchCompleted_ = onSearchRescan;
+    this.onClearSearch_ = onClearSearch;
+    this.addEventListener('scan-completed', this.onSearchCompleted_);
+    this.clearAndScan_(newDirContents,
+                       sequence,
+                       callback);
+  }.bind(this, this.changeDirectorySequence_));
 };
 
 /**