Update scanning to scan files in small batches (well batches of 1)...this...

Update scanning to scan files in small batches (well batches of 1)...this fixes an issue where scanning appeared to be frozen for long periods of time.

This change also sets the stage for making Scans cancelable...which we'll want to do when a user changes directory...and even allow them to explicitly cancel a scan from the details panel.

TEST=MediaScannerTest

TBR=hirono  // for mock_media_scanner.js

Review URL: https://codereview.chromium.org/1038433002

Cr-Commit-Position: refs/heads/master@{#322099}

ui/file_manager/file_manager/background/js/media_scanner.js

@@ -12,11 +12,22 @@ importer.MediaScanner = function() {};
 /**
  * Initiates scanning.
  *
- * @param {!Array.<!Entry>} entries Must be non-empty.
+ * @param {!DirectoryEntry} directory
  * @return {!importer.ScanResult} ScanResult object representing the scan
  *     job both while in-progress and when completed.
  */
-importer.MediaScanner.prototype.scan;
+importer.MediaScanner.prototype.scanDirectory;
+
+/**
+ * Initiates scanning.
+ *
+ * @param {!Array<!FileEntry>} entries Must be non-empty, and all entires
+ *     must be of a supported media type. Individually supplied files
+ *     are not subject to deduplication.
+ * @return {!importer.ScanResult} ScanResult object representing the scan
+ *     job for the explicitly supplied entries.
+ */
+importer.MediaScanner.prototype.scanFiles;
 
 /**
  * Adds an observer, which will be notified on scan events.
@@ -57,7 +68,7 @@ importer.DefaultMediaScanner =
     return new importer.DefaultScanResult(hashGenerator);
   };
 
-  /** @private {!Array.<!importer.ScanObserver>} */
+  /** @private {!Array<!importer.ScanObserver>} */
   this.observers_ = [];
 
   /**
@@ -90,11 +101,35 @@ importer.DefaultMediaScanner.prototype.removeObserver = function(observer) {
 };
 
 /** @override */
-importer.DefaultMediaScanner.prototype.scan = function(entries) {
-  if (entries.length == 0) {
-    throw new Error('Cannot scan empty list of entries.');
-  }
+importer.DefaultMediaScanner.prototype.scanDirectory = function(directory) {
+  var scan = this.createScanResult_();
+  var watcher = this.watcherFactory_(
+      /** @this {importer.DefaultMediaScanner} */
+      function() {
+        scan.invalidateScan();
+        this.notify_(importer.ScanEvent.INVALIDATED, scan);
+      }.bind(this));
 
+  this.crawlDirectory_(directory, watcher)
+      .then(this.scanMediaFiles_.bind(this, scan))
+      .then(scan.resolve)
+      .catch(scan.reject);
+
+  scan.whenFinal()
+      .then(
+          /** @this {importer.DefaultMediaScanner} */
+          function() {
+            this.notify_(importer.ScanEvent.FINALIZED, scan);
+          }.bind(this));
+
+  return scan;
+};
+
+/** @override */
+importer.DefaultMediaScanner.prototype.scanFiles = function(entries) {
+  if (entries.length === 0) {
+    throw new Error('Cannot scan empty list.');
+  }
   var scan = this.createScanResult_();
   var watcher = this.watcherFactory_(
       /** @this {importer.DefaultMediaScanner} */
@@ -103,8 +138,7 @@ importer.DefaultMediaScanner.prototype.scan = function(entries) {
         this.notify_(importer.ScanEvent.INVALIDATED, scan);
       }.bind(this));
 
-  var scanPromises = entries.map(
-      this.scanEntry_.bind(this, scan, watcher));
+  var scanPromises = entries.map(this.onUniqueFileFound_.bind(this, scan));
 
   Promise.all(scanPromises)
       .then(scan.resolve)
@@ -120,6 +154,40 @@ importer.DefaultMediaScanner.prototype.scan = function(entries) {
   return scan;
 };
 
+/** @const {number} */
+importer.DefaultMediaScanner.SCAN_BATCH_SIZE = 1;
+
+/**
+ * @param {!importer.DefaultScanResult} scan
+ * @param  {!Array<!FileEntry>} entries
+ * @return {!Promise} Resolves when scanning is finished.
+ * @private
+ */
+importer.DefaultMediaScanner.prototype.scanMediaFiles_ =
+    function(scan, entries) {
+  var handleFileEntry = this.onFileEntryFound_.bind(this, scan);
+
+  /**
+   * @param {number} begin The beginning offset in the list of entries
+   *     to process.
+   * @return {!Promise}
+   */
+  var scanChunk = function(begin) {
+    // the second arg to slice is an exclusive end index, so we +1 batch size.
+    var end = begin + importer.DefaultMediaScanner.SCAN_BATCH_SIZE + 1;
+    return Promise.all(
+        entries.slice(begin, end).map(handleFileEntry))
+        .then(
+            function() {
+              if (end < entries.length) {
+                return scanChunk(end);
+              }
+            });
+  };
+
+  return scanChunk(0);
+};
+
 /**
  * Notifies all listeners at some point in the near future.
  *
@@ -136,55 +204,21 @@ importer.DefaultMediaScanner.prototype.notify_ = function(event, result) {
 };
 
 /**
- * Resolves the entry by either:
- * a) recursing on it (when a directory)
- * b) adding it to the results (when a media type file)
- * c) ignoring it, if neither a or b
+ * Finds all files media files beneath directory AND adds directory
+ * watchers for each encountered directory.
  *
- * @param {!importer.DefaultScanResult} scan
+ * @param {!DirectoryEntry} directory
  * @param {!importer.DirectoryWatcher} watcher
- * @param {!Entry} entry
- *
- * @return {!Promise}
+ * @return {!Promise<!Array<!FileEntry>>}
  * @private
  */
-importer.DefaultMediaScanner.prototype.scanEntry_ =
-    function(scan, watcher, entry) {
-
-  if (entry.isDirectory) {
-    return this.scanDirectory_(
-      scan,
-      watcher,
-      /** @type {!DirectoryEntry} */ (entry));
-  }
-
-  // Since this entry is by client code (and presumably the user)
-  // we add it directly (skipping over the history dupe check).
-  return this.onUniqueFileFound_(scan, /** @type {!FileEntry} */ (entry));
-};
-
-/**
- * Finds all files beneath directory.
- *
- * @param {!importer.DefaultScanResult} scan
- * @param {!importer.DirectoryWatcher} watcher
- * @param {!DirectoryEntry} entry
- * @return {!Promise}
- * @private
- */
-importer.DefaultMediaScanner.prototype.scanDirectory_ =
-    function(scan, watcher, entry) {
-  // Collect promises for all files being added to results.
-  // The directory scan promise can't resolve until all
-  // file entries are completely promised.
-  var promises = [];
+importer.DefaultMediaScanner.prototype.crawlDirectory_ =
+    function(directory, watcher) {
+  var mediaFiles = [];
 
   return fileOperationUtil.findEntriesRecursively(
-      entry,
-      /**
-       * @param  {!Entry} entry
-       * @this {importer.DefaultMediaScanner}
-       */
+      directory,
+      /** @param  {!Entry} entry */
       function(entry) {
         if (watcher.triggered) {
           return;
@@ -195,14 +229,14 @@ importer.DefaultMediaScanner.prototype.scanDirectory_ =
           // function findEntriesRecursively does that. So we
           // just watch the directory for modifications, and that's it.
           watcher.addDirectory(/** @type {!DirectoryEntry} */(entry));
-          return;
+        } else if (importer.isEligibleType(entry)) {
+          mediaFiles.push(/** @type {!FileEntry} */ (entry));
         }
-
-        promises.push(
-            this.onFileEntryFound_(scan, /** @type {!FileEntry} */(entry)));
-
-      }.bind(this))
-      .then(Promise.all.bind(Promise, promises));
+      })
+      .then(
+          function() {
+            return mediaFiles;
+          });
 };
 
 /**
@@ -343,7 +377,7 @@ importer.DefaultScanResult = function(hashGenerator) {
 
   /**
    * List of file entries found while scanning.
-   * @private {!Array.<!FileEntry>}
+   * @private {!Array<!FileEntry>}
    */
   this.fileEntries_ = [];
 

ui/file_manager/file_manager/background/js/media_scanner_unittest.js

@@ -58,7 +58,7 @@ function setUp() {
 function testEmptySourceList() {
   assertThrows(
     function() {
-      scanner.scan([]);
+      scanner.scanFiles([]);
     });
 }
 
@@ -76,7 +76,7 @@ function testIsScanning(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                var results = scanner.scan([root]);
+                var results = scanner.scanDirectory(root);
                 assertFalse(results.isFinal());
               }),
       callback);
@@ -99,7 +99,7 @@ function testObserverNotifiedOnScanFinish(callback) {
             // We kick this off first so we can capture the result for
             // use in an assert. Promises ensure the scan won't finish
             // until after our funciton is fully processed.
-            var result = scanner.scan([root]);
+            var result = scanner.scanDirectory(root);
             scanner.addObserver(
                 function(eventType, scanResult) {
                   assertEquals(importer.ScanEvent.FINALIZED, eventType);
@@ -113,6 +113,34 @@ function testObserverNotifiedOnScanFinish(callback) {
           });
 }
 
+/**
+ * Verifies that scanFiles slurps up all specified files.
+ */
+function testScanFiles(callback) {
+  var filenames = [
+    'foo',
+    'foo.jpg',
+    'bar.gif',
+    'baz.avi'
+  ];
+  var expectedFiles = [
+    '/testScanFiles/foo.jpg',
+    '/testScanFiles/bar.gif',
+    '/testScanFiles/baz.avi'
+  ];
+  reportPromise(
+      makeTestFileSystemRoot('testScanFiles')
+          .then(populateDir.bind(null, filenames))
+          .then(fileOperationUtil.gatherEntriesRecursively)
+          .then(
+              /** @param {!Array<!FileEntry>} files */
+              function(files) {
+                return scanner.scanFiles(files).whenFinal();
+              })
+          .then(assertResults.bind(null, expectedFiles)),
+      callback);
+}
+
 /**
  * Verifies that scanning a simple single-level directory structure works.
  */
@@ -130,7 +158,7 @@ function testEmptyScanResults(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                return scanner.scan([root]).whenFinal();
+                return scanner.scanDirectory(root).whenFinal();
               })
           .then(assertResults.bind(null, [])),
       callback);
@@ -162,7 +190,7 @@ function testSingleLevel(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                return scanner.scan([root]).whenFinal();
+                return scanner.scanDirectory(root).whenFinal();
               })
           .then(assertResults.bind(null, expectedFiles)),
       callback);
@@ -209,7 +237,7 @@ function testIgnoresPreviousImports(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                return scanner.scan([root]).whenFinal();
+                return scanner.scanDirectory(root).whenFinal();
               })
           .then(assertResults.bind(null, expectedFiles)),
       callback);
@@ -220,23 +248,23 @@ function testMultiLevel(callback) {
     'foo.jpg',
     'bar',
     [
-      'foo.0',
+      'dir1',
       'bar.0.jpg'
     ],
     [
-      'foo.1',
+      'dir2',
       'bar.1.gif',
       [
-        'foo.1.0',
+        'dir3',
         'bar.1.0.avi'
       ]
     ]
   ];
   var expectedFiles = [
     '/testMultiLevel/foo.jpg',
-    '/testMultiLevel/foo.0/bar.0.jpg',
-    '/testMultiLevel/foo.1/bar.1.gif',
-    '/testMultiLevel/foo.1/foo.1.0/bar.1.0.avi'
+    '/testMultiLevel/dir1/bar.0.jpg',
+    '/testMultiLevel/dir2/bar.1.gif',
+    '/testMultiLevel/dir2/dir3/bar.1.0.avi'
   ];
 
   reportPromise(
@@ -248,53 +276,7 @@ function testMultiLevel(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                return scanner.scan([root]).whenFinal();
-              })
-          .then(assertResults.bind(null, expectedFiles)),
-      callback);
-}
-
-function testMultipleDirectories(callback) {
-  var filenames = [
-    'foo',
-    'bar',
-    [
-      'foo.0',
-      'bar.0.jpg'
-    ],
-    [
-      'foo.1',
-      'bar.1.jpg',
-    ]
-  ];
-  // Expected file paths from the scan.  We're scanning the two subdirectories
-  // only.
-  var expectedFiles = [
-    '/testMultipleDirectories/foo.0/bar.0.jpg',
-    '/testMultipleDirectories/foo.1/bar.1.jpg'
-  ];
-
-  var getDirectory = function(root, dirname) {
-    return new Promise(function(resolve, reject) {
-      root.getDirectory(
-          dirname, {create: false}, resolve, reject);
-    });
-  };
-
-  reportPromise(
-      makeTestFileSystemRoot('testMultipleDirectories')
-          .then(populateDir.bind(null, filenames))
-          .then(
-              /**
-               * Scans the directories.
-               * @param {!DirectoryEntry} root
-               */
-              function(root) {
-                return Promise.all(['foo.0', 'foo.1'].map(
-                    getDirectory.bind(null, root))).then(
-                        function(directories) {
-                          return scanner.scan(directories).whenFinal();
-                        });
+                return scanner.scanDirectory(root).whenFinal();
               })
           .then(assertResults.bind(null, expectedFiles)),
       callback);
@@ -302,47 +284,39 @@ function testMultipleDirectories(callback) {
 
 function testDedupesFilesInScanResult(callback) {
   var filenames = [
+    'foo.jpg',
+    'bar.jpg',
     [
-      'a',
+      'dir1',
       'foo.jpg',
       'bar.jpg'
     ],
     [
-      'b',
+      'dir2',
       'foo.jpg',
       'bar.jpg',
-      'wee.jpg'
+      [
+        'dir3',
+        'foo.jpg',
+        'bar.jpg'
+      ]
     ]
   ];
-  // Expected file paths from the scan.  We're scanning the two subdirectories
-  // only.
   var expectedFiles = [
-    '/testDedupesFiles/a/foo.jpg',
-    '/testDedupesFiles/a/bar.jpg',
-    '/testDedupesFiles/b/wee.jpg'
+    '/testDedupesFilesInScanResult/foo.jpg',
+    '/testDedupesFilesInScanResult/bar.jpg'
   ];
 
-  var getDirectory = function(root, dirname) {
-    return new Promise(function(resolve, reject) {
-      root.getDirectory(
-          dirname, {create: false}, resolve, reject);
-    });
-  };
-
   reportPromise(
-      makeTestFileSystemRoot('testDedupesFiles')
+      makeTestFileSystemRoot('testDedupesFilesInScanResult')
           .then(populateDir.bind(null, filenames))
           .then(
               /**
-               * Scans the directories.
+               * Scans the directory.
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                return Promise.all(['a', 'b'].map(
-                    getDirectory.bind(null, root))).then(
-                        function(directories) {
-                          return scanner.scan(directories).whenFinal();
-                        });
+                return scanner.scanDirectory(root).whenFinal();
               })
           .then(assertResults.bind(null, expectedFiles)),
       callback);
@@ -361,7 +335,7 @@ function testInvalidation(callback) {
                * @param {!DirectoryEntry} root
                */
               function(root) {
-                scan = scanner.scan([root]);
+                scan = scanner.scanDirectory(root);
                 watcher.callback();
                 return invalidatePromise;
               }),

ui/file_manager/file_manager/background/js/mock_media_scanner.js

@@ -46,7 +46,16 @@ TestMediaScanner.prototype.removeObserver = function(observer) {
 };
 
 /** @override */
-TestMediaScanner.prototype.scan = function(entries) {
+TestMediaScanner.prototype.scanDirectory = function(directory) {
+  var scan = new TestScanResult(this.fileEntries);
+  scan.totalBytes = this.totalBytes;
+  scan.scanDuration = this.scanDuration;
+  this.scans_.push(scan);
+  return scan;
+};
+
+/** @override */
+TestMediaScanner.prototype.scanFiles = function(entries) {
   var scan = new TestScanResult(this.fileEntries);
   scan.totalBytes = this.totalBytes;
   scan.scanDuration = this.scanDuration;

ui/file_manager/file_manager/foreground/js/import_controller.js

@@ -879,7 +879,7 @@ importer.ScanManager.prototype.isActiveScan = function(scan) {
 importer.ScanManager.prototype.getSelectionScan = function(entries) {
   console.assert(!this.selectionScan_,
       'Cannot create new selection scan with another in the cache.');
-  this.selectionScan_ = this.scanner_.scan(entries);
+  this.selectionScan_ = this.scanner_.scanFiles(entries);
   return this.selectionScan_;
 };
 
@@ -897,7 +897,8 @@ importer.ScanManager.prototype.getCurrentDirectoryScan = function() {
   var url = directory.toURL();
   var scan = this.directoryScans_[url];
   if (!scan) {
-    scan = this.scanner_.scan([directory]);
+    scan = this.scanner_.scanDirectory(
+        /** @type {!DirectoryEntry} */ (directory));
     this.directoryScans_[url] = scan;
   }