Files.app: Fix flakiness in chaining directory

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 // If directory files changes too often, don't rescan directory more than once
 // per specified interval
 var SIMULTANEOUS_RESCAN_INTERVAL = 1000;
 // Used for operations that require almost instant rescan.
@@ skipping 14 matching lines @@
                         metadataCache, volumeManager) {
   this.fileListSelection_ = singleSelection ?
       new cr.ui.ListSingleSelectionModel() : new cr.ui.ListSelectionModel();
 
   this.runningScan_ = null;
   this.pendingScan_ = null;
   this.rescanTime_ = null;
   this.scanFailures_ = 0;
   this.changeDirectorySequence_ = 0;
 
+  this.directoryChangeQueue_ = new AsyncUtil.Queue();
+
   this.fileFilter_ = fileFilter;
   this.fileFilter_.addEventListener('changed',
                                     this.onFilterChanged_.bind(this));
 
   this.currentFileListContext_ = new FileListContext(
       fileFilter, metadataCache);
   this.currentDirContents_ =
       DirectoryContents.createForDirectory(this.currentFileListContext_, null);
 
   this.metadataCache_ = metadataCache;
@@ skipping 226 matching lines @@
  * a single refresh.
  * @param {number} delay Delay in ms after which the rescan will be performed.
  */
 DirectoryModel.prototype.scheduleRescan = function(delay) {
   if (this.rescanTime_) {
     if (this.rescanTime_ <= Date.now() + delay)
       return;
     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);
 };
 
 /**
  * Cancel a rescan on timeout if it is scheduled.
  * @private
  */
 DirectoryModel.prototype.clearRescanTimeout_ = function() {
   this.rescanTime_ = null;
   if (this.rescanTimeoutId_) {
     clearTimeout(this.rescanTimeoutId_);
@@ skipping 12 matching lines @@
 DirectoryModel.prototype.rescan = function() {
   this.clearRescanTimeout_();
   if (this.runningScan_) {
     this.pendingRescan_ = true;
     return;
   }
 
   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);
 
   this.scan_(dirContents,
              successCallback, function() {}, function() {}, function() {});
 };
 
 /**
  * Run scan on the current DirectoryContents. The active fileList is cleared and
  * the entries are added directly.
  *
  * This should be used when changing directory or initiating a new search.
  *
  * @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;
   this.clearRescanTimeout_();
 
   if (this.pendingScan_)
     this.pendingScan_ = false;
 
   if (this.runningScan_) {
     if (this.runningScan_.isScanning())
       this.runningScan_.cancelScan();
     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.
   cr.dispatchSimpleEvent(this, 'scan-started');
   var fileList = this.getFileList();
   fileList.splice(0, fileList.length);
   this.scan_(this.currentDirContents_,
              onDone, onFailed, onUpdated, onCancelled);
 };
 
@@ skipping 284 matching lines @@
  *     be opened.
  * @param {function()=} opt_callback Executed if the directory loads
  *     successfully.
  */
 DirectoryModel.prototype.changeDirectoryEntry = function(
     dirEntry, opt_callback) {
   // Increment the sequence value.
   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);
+          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.
+          // 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');
           event.previousDirEntry = previousDirEntry;
           event.newDirEntry = dirEntry;
           this.dispatchEvent(event);
         }.bind(this));
-      }.bind(this, this.changeDirectorySequence_));
+  }.bind(this, this.changeDirectorySequence_));
 };
 
 /**
  * Activates the given directry.
  * This method:
  *  - Changes the current directory, if the given directory is the current
  *    directory.
  *  - Clears the selection, if the given directory is the current directory.
  *
  * @param {DirectoryEntry|Object} dirEntry The entry of the new directory to
@@ skipping 187 matching lines @@
 DirectoryModel.prototype.search = function(query,
                                            onSearchRescan,
                                            onClearSearch) {
   this.clearSearch_();
   var currentDirEntry = this.getCurrentDirEntry();
   if (!currentDirEntry) {
     // Not yet initialized. Do nothing.
     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;
+    }
 
-  this.onSearchCompleted_ = onSearchRescan;
-  this.onClearSearch_ = onClearSearch;
-  this.addEventListener('scan-completed', this.onSearchCompleted_);
-  this.clearAndScan_(newDirContents);
+    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,
+                       sequence,
+                       callback);
+  }.bind(this, this.changeDirectorySequence_));
 };
 
 /**
  * In case the search was active, remove listeners and send notifications on
  * its canceling.
  * @private
  */
 DirectoryModel.prototype.clearSearch_ = function() {
   if (!this.isSearching())
     return;
 
   if (this.onSearchCompleted_) {
     this.removeEventListener('scan-completed', this.onSearchCompleted_);
     this.onSearchCompleted_ = null;
   }
 
   if (this.onClearSearch_) {
     this.onClearSearch_();
     this.onClearSearch_ = null;
   }
 };