Move content deduplication into the scan process.

ui/file_manager/file_manager/background/js/duplicate_finder.js

Index: ui/file_manager/file_manager/background/js/duplicate_finder.js
diff --git a/ui/file_manager/file_manager/background/js/duplicate_finder.js b/ui/file_manager/file_manager/background/js/duplicate_finder.js
index ce797e677d92e339a6db3c1902567c4335feb48f..61d4a2ceda255e4b978e7bf8da9d4ed7664dcf62 100644
--- a/ui/file_manager/file_manager/background/js/duplicate_finder.js
+++ b/ui/file_manager/file_manager/background/js/duplicate_finder.js
@@ -6,63 +6,27 @@
 var importer = importer || {};
 
 /**
- * Interface for import deduplicators.  A duplicate finder is linked to an
- * import destination, and will check whether files already exist in that import
- * destination.
- * @interface
- */
-importer.DuplicateFinder = function() {};
-
-/**
- * Checks whether the given file already exists in the import destination.
- * @param {!FileEntry} entry The file entry to check.
- * @return {!Promise<boolean>}
- */
-importer.DuplicateFinder.prototype.checkDuplicate;
-
-/**
- * A factory for producing duplicate finders.
- * @interface
- */
-importer.DuplicateFinder.Factory = function() {};
-
-/** @return {!importer.DuplicateFinder} */
-importer.DuplicateFinder.Factory.prototype.create;
-
-/**
  * A duplicate finder for Google Drive.
  *
  * @constructor
- * @implements {importer.DuplicateFinder}
  * @struct
+ *
+ * @param {!analytics.Tracker} tracker
  */
-importer.DriveDuplicateFinder = function() {
-  /** @private {Promise<string>} */
-  this.driveIdPromise_ = null;
+importer.DriveDuplicateFinder = function(tracker) {
 
-  /**
-   * Aggregate time spent computing content hashes (in ms).
-   * @private {number}
-   */
-  this.computeHashTime_ = 0;
+  /** @private {!analytics.Tracker} */
+  this.tracker_ = tracker;
 
-  /**
-   * Aggregate time spent performing content hash searches (in ms).
-   * @private {number}
-   */
-  this.searchHashTime_ = 0;
+  /** @private {Promise<string>} */
+  this.driveIdPromise_ = null;
 };
 
 /**
- * @typedef {{
- *   computeHashTime: number,
- *   searchHashTime: number
- * }}
+ * @param {!FileEntry} entry
+ * @return {!Promise<boolean>}
  */
-importer.DriveDuplicateFinder.Statistics;
-
-/** @override */
-importer.DriveDuplicateFinder.prototype.checkDuplicate = function(entry) {
+importer.DriveDuplicateFinder.prototype.isDuplicate = function(entry) {
   return this.computeHash_(entry)
       .then(this.findByHash_.bind(this))
       .then(
@@ -87,16 +51,26 @@ importer.DriveDuplicateFinder.prototype.computeHash_ = function(entry) {
         var startTime = new Date().getTime();
         chrome.fileManagerPrivate.computeChecksum(
             entry.toURL(),
-            /** @param {string} result The content hash. */
+            /**
+             * @param {string} result The content hash.
+             * @this {importer.DriveDuplicateFinder}
+             */
             function(result) {
-              var endTime = new Date().getTime();
-              this.searchHashTime_ += endTime - startTime;
+              var elapsedTime = new Date().getTime() - startTime;
+              // Send the timing to GA only if it is sorta exceptionally long.
+              // A one second, CPU intensive operation, is pretty long.
+              if (elapsedTime >= 5000) {

[mtomasz, 2015/03/05 04:36:52]

nit: Can we move to a constant?

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                this.tracker_.sendTiming(
+                   metrics.Categories.ACQUISITION,
+                   metrics.timing.Variables.COMPUTE_HASH,
+                   elapsedTime);
+              }
               if (chrome.runtime.lastError) {
                 reject(chrome.runtime.lastError);
               } else {
                 resolve(result);
               }
-            });
+            }.bind(this));
       }.bind(this));
 };
 
@@ -152,8 +126,14 @@ importer.DriveDuplicateFinder.prototype.searchFilesByHash_ =
              * @this {importer.DriveDuplicateFinder}
              */
             function(urls) {
-              var endTime = new Date().getTime();
-              this.searchHashTime_ += endTime - startTime;
+              var elapsedTime = new Date().getTime() - startTime;
+              // Send the timing to GA only if it is sorta exceptionally long.
+              if (elapsedTime >= 1000) {

[mtomasz, 2015/03/05 04:36:52]

nit: Constant?

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                this.tracker_.sendTiming(
+                   metrics.Categories.ACQUISITION,
+                   metrics.timing.Variables.SEARCH_BY_HASH,
+                   elapsedTime);
+              }
               if (chrome.runtime.lastError) {
                 reject(chrome.runtime.lastError);
               } else {
@@ -163,21 +143,107 @@ importer.DriveDuplicateFinder.prototype.searchFilesByHash_ =
       }.bind(this));
 };
 
-/** @return {!importer.DriveDuplicateFinder.Statistics} */
-importer.DriveDuplicateFinder.prototype.getStatistics = function() {
-  return {
-    computeHashTime: this.computeHashTime_,
-    searchHashTime: this.searchHashTime_
-  };
+/**
+ * A class that aggregates history/content-dupe checking
+ * into a single "Disposition" value. Should now be the
+ * primary source for duplicate checking (with the exception
+ * of in-scan deduplication, where duplicate results that
+ * are within the scan are ignored).
+ *
+ * @constructor
+ *
+ * @param {!importer.HistoryLoader} historyLoader
+ * @param {!importer.DriveDuplicateFinder} contentMatcher
+ */
+importer.DispositionChecker = function(historyLoader, contentMatcher) {
+  /** @private {!importer.HistoryLoader} */
+  this.historyLoader_ = historyLoader;
+
+  /** @private {!importer.DriveDuplicateFinder} */
+  this.contentMatcher_ = contentMatcher;
 };
 
 /**
- * @constructor
- * @implements {importer.DuplicateFinder.Factory}
+ * @param {!FileEntry} entry
+ * @param {!importer.Destination} destination
+ * @return {!Promise<!importer.Disposition>}
+ */
+importer.DispositionChecker.prototype.getDisposition =
+    function(entry, destination) {
+  if (destination !== importer.Destination.GOOGLE_DRIVE) {
+    throw new Error('Unsupported destination: ' + destination);

[mtomasz, 2015/03/05 04:36:52]

nit: assert or return Promise.reject('Unsupported...')?

[Steve McKay, 2015/03/05 21:39:03]

Done.

+  }
+
+  return new Promise(
+      /** @this {importer.DispositionChecker} */
+      function(resolve, reject) {
+        this.hasHistoryDuplicate_(entry, destination)
+              .then(

[mtomasz, 2015/03/05 04:36:52]

nit: indent

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                  /** @this {importer.DispositionChecker} */
+                  function(duplicate) {

[Ben Kwa, 2015/03/05 20:05:10]

nit: param needs jsdoc

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                    if (duplicate) {
+                      resolve(importer.Disposition.HISTORY_DUPLICATE);
+                    } else {
+                      this.contentMatcher_.isDuplicate(entry)
+                          .then(
+                              function(duplicate) {

[Ben Kwa, 2015/03/05 20:05:10]

nit: param needs jsdoc

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                                if (duplicate) {
+                                  resolve(
+                                      importer.Disposition.CONTENT_DUPLICATE);
+                                } else {
+                                  resolve(importer.Disposition.ORIGINAL);
+                                }
+                              });
+                    }
+                  }.bind(this));
+            }.bind(this));
+};
+
+/**
+ * @param {!FileEntry} entry
+ * @param {!importer.Destination} destination
+ * @return {!Promise.<boolean>} True if there is a history-entry-duplicate
+ *     for the file.
+ * @private
  */
-importer.DriveDuplicateFinder.Factory = function() {};
+importer.DispositionChecker.prototype.hasHistoryDuplicate_ =
+    function(entry, destination) {
+  return this.historyLoader_.getHistory()
+      .then(
+          /**
+           * @param {!importer.ImportHistory} history
+           * @return {!Promise}
+           * @this {importer.DefaultMediaScanner}
+           */
+          function(history) {
+            return Promise.all([
+              history.wasCopied(entry, destination),
+              history.wasImported(entry, destination)
+            ]).then(
+                /**
+                 * @param {!Array<boolean>} results
+                 * @return {!Promise}
+                 * @this {importer.DefaultMediaScanner}
+                 */
+                function(results) {

[Ben Kwa, 2015/03/05 20:05:10]

nit: This function doesn't need a @this or a bind.

[Steve McKay, 2015/03/05 21:39:03]

Done.

+                  return results[0] || results[1];
+                }.bind(this));
+          }.bind(this));
+};
 
-/** @override */
-importer.DriveDuplicateFinder.Factory.prototype.create = function() {
-  return new importer.DriveDuplicateFinder();
+/**
+ * Factory for a function that returns an entry's disposition.
+ *
+ * @param {!importer.HistoryLoader} historyLoader
+ * @param {!analytics.Tracker} tracker
+ *
+ * @return {function(!FileEntry, !importer.Destination):
+ *     !Promise<importer.Disposition>}

[mtomasz, 2015/03/05 04:36:52]

nit: Shall it be !importer.Disposition?

[Steve McKay, 2015/03/05 21:39:03]

Done.

+ */
+importer.DispositionChecker.createChecker =
+    function(historyLoader, tracker) {
+  var checker = new importer.DispositionChecker(
+      historyLoader,
+      new importer.DriveDuplicateFinder(tracker));
+  return checker.getDisposition.bind(checker);
 };