Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Unified Diff: ui/file_manager/file_manager/background/js/media_import_handler.js

Issue 980603003: Move content deduplication into the scan process. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Don't report error count when zero, and don't report an explicit END to import...since that is impl… Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/file_manager/file_manager/background/js/media_import_handler.js
diff --git a/ui/file_manager/file_manager/background/js/media_import_handler.js b/ui/file_manager/file_manager/background/js/media_import_handler.js
index 055c5f3355162fdc7013358400ce0dfb3d5117c8..6f58e9c56aa820385201e024aaf33a0c919ae08f 100644
--- a/ui/file_manager/file_manager/background/js/media_import_handler.js
+++ b/ui/file_manager/file_manager/background/js/media_import_handler.js
@@ -32,11 +32,10 @@ importer.ImportRunner.prototype.importFromScanResult;
*
* @param {!ProgressCenter} progressCenter
* @param {!importer.HistoryLoader} historyLoader
- * @param {!importer.DuplicateFinder.Factory} duplicateFinderFactory
* @param {!analytics.Tracker} tracker
*/
importer.MediaImportHandler =
mtomasz 2015/03/05 04:36:53 nit: Will it fit in one line? If not, then we brea
Steve McKay 2015/03/05 21:39:03 Done.
- function(progressCenter, historyLoader, duplicateFinderFactory, tracker) {
+ function(progressCenter, historyLoader, tracker) {
/** @private {!ProgressCenter} */
this.progressCenter_ = progressCenter;
@@ -54,9 +53,6 @@ importer.MediaImportHandler =
chrome.power.releaseKeepAwake();
});
- /** @private {!importer.DuplicateFinder.Factory} */
- this.duplicateFinderFactory_ = duplicateFinderFactory;
-
/** @private {!analytics.Tracker} */
this.tracker_ = tracker;
@@ -73,7 +69,6 @@ importer.MediaImportHandler.prototype.importFromScanResult =
this.historyLoader_,
scanResult,
directoryPromise,
- this.duplicateFinderFactory_.create(),
destination,
this.tracker_);
@@ -155,8 +150,6 @@ importer.MediaImportHandler.prototype.onTaskProgress_ =
* @param {!importer.HistoryLoader} historyLoader
* @param {!importer.ScanResult} scanResult
* @param {!Promise<!DirectoryEntry>} directoryPromise
- * @param {!importer.DuplicateFinder} duplicateFinder A duplicate-finder linked
- * to the import destination, that will be used to deduplicate imports.
* @param {!importer.Destination} destination The logical destination.
* @param {!analytics.Tracker} tracker
*/
@@ -165,7 +158,6 @@ importer.MediaImportHandler.ImportTask = function(
historyLoader,
scanResult,
directoryPromise,
- duplicateFinder,
destination,
tracker) {
@@ -179,9 +171,6 @@ importer.MediaImportHandler.ImportTask = function(
/** @private {!Promise<!DirectoryEntry>} */
this.directoryPromise_ = directoryPromise;
- /** @private {!importer.DuplicateFinder} */
- this.deduplicator_ = duplicateFinder;
-
/** @private {!importer.ScanResult} */
this.scanResult_ = scanResult;
@@ -206,9 +195,6 @@ importer.MediaImportHandler.ImportTask = function(
/** @private {boolean} Indicates whether this task was canceled. */
this.canceled_ = false;
- /** @private {number} Number of files deduped by content dedupe. */
- this.dedupeCount_ = 0;
-
/** @private {number} */
this.errorCount_ = 0;
};
@@ -277,14 +263,11 @@ importer.MediaImportHandler.ImportTask.prototype.requestCancel = function() {
/** @private */
importer.MediaImportHandler.ImportTask.prototype.initialize_ = function() {
var stats = this.scanResult_.getStatistics();
-
this.remainingFilesCount_ = stats.newFileCount;
this.totalBytes_ = stats.sizeBytes;
this.notify(importer.TaskQueue.UpdateType.PROGRESS);
this.tracker_.send(metrics.ImportEvents.STARTED);
- this.tracker_.send(metrics.ImportEvents.HISTORY_DEDUPE_COUNT
- .value(stats.duplicateFileCount));
};
/**
@@ -316,27 +299,11 @@ importer.MediaImportHandler.ImportTask.prototype.importOne_ =
function(destinationDirectory, completionCallback, entry) {
if (this.canceled_) {
this.notify(importer.TaskQueue.UpdateType.CANCELED);
- this.tracker_.send(metrics.ImportEvents.CANCELLED);
- this.sendImportStats_();
+ this.sendImportStats_(false);
return;
}
- this.deduplicator_.checkDuplicate(entry)
- .then(
- /** @param {boolean} isDuplicate */
- function(isDuplicate) {
- if (isDuplicate) {
- // If the given file is a duplicate, don't import it again. Just
- // update the progress indicator.
- this.dedupeCount_++;
- this.markAsImported_(entry);
- this.processedBytes_ += entry.size;
- this.notify(importer.TaskQueue.UpdateType.PROGRESS);
- return Promise.resolve();
- } else {
- return this.copy_(entry, destinationDirectory);
- }
- }.bind(this))
+ this.copy_(entry, destinationDirectory)
// Regardless of the result of this copy, push on to the next file.
.then(completionCallback)
.catch(
@@ -479,37 +446,58 @@ importer.MediaImportHandler.ImportTask.prototype.markAsImported_ =
/** @private */
importer.MediaImportHandler.ImportTask.prototype.onSuccess_ = function() {
this.notify(importer.TaskQueue.UpdateType.COMPLETE);
- this.tracker_.send(metrics.ImportEvents.ENDED);
- this.sendImportStats_();
+ this.sendImportStats_(true);
mtomasz 2015/03/05 04:36:53 nit: enum or /* arg_name */ for this true? It's im
Steve McKay 2015/03/05 21:39:03 Done.
};
/**
* Sends import statistics to analytics.
+ *
+ * @param {boolean} completedSuccessfully True if task
Ben Kwa 2015/03/05 20:05:10 suggestion: You could reuse the TaskQueue.UpdateTy
Steve McKay 2015/03/05 21:39:03 Done.
+ * wasn't cancelled or err'd out.
*/
-importer.MediaImportHandler.ImportTask.prototype.sendImportStats_ = function() {
- this.tracker_.send(
- metrics.ImportEvents.CONTENT_DEDUPE_COUNT
- .value(this.dedupeCount_));
- // TODO(kenobi): Send correct import byte counts.
- var importFileCount = this.scanResult_.getStatistics().newFileCount -
- (this.dedupeCount_ + this.remainingFilesCount_);
+importer.MediaImportHandler.ImportTask.prototype.sendImportStats_ =
+ function(completedSuccessfully) {
+ var scanStats = this.scanResult_.getStatistics();
+
+ if (!completedSuccessfully) {
+ this.tracker_.send(metrics.ImportEvents.CANCELLED);
+ } else {
+ // Ideally we'd report bytes imported for cancelled imports too.
+ // We just don't have the information needed to do this accurately.
Ben Kwa 2015/03/05 20:05:10 The processedBytes_ counter should be a fairly acc
Steve McKay 2015/03/05 21:39:03 Perfect!
+ var megaBytes = Math.floor(scanStats.sizeBytes / (1024 * 1024));
+ this.tracker_.send(
Ben Kwa 2015/03/05 20:05:10 We also need to do this.tracker_.send(metrics.Impo
Steve McKay 2015/03/05 21:39:03 Removed that as there are plenty of surrogate even
+ metrics.ImportEvents.MEGABYTES_IMPORTED.value(megaBytes));
+ }
+
+ // FYI, if the task was cancelled we substract the remaining files
+ // from the number of files in the scanStats.
+ var importFileCount = scanStats.newFileCount - this.remainingFilesCount_;
this.tracker_.send(
- metrics.ImportEvents.FILE_COUNT
- .value(importFileCount));
-
- this.tracker_.send(metrics.ImportEvents.ERROR.value(this.errorCount_));
-
- // Send aggregate deduplication timings, to avoid flooding analytics with one
- // timing per file.
- var deduplicatorStats = this.deduplicator_.getStatistics();
- this.tracker_.sendTiming(
- metrics.Categories.ACQUISITION,
- metrics.timing.Variables.COMPUTE_HASH,
- deduplicatorStats.computeHashTime,
- 'In Place');
- this.tracker_.sendTiming(
- metrics.Categories.ACQUISITION,
- metrics.timing.Variables.SEARCH_BY_HASH,
- deduplicatorStats.searchHashTime);
+ metrics.ImportEvents.FILES_IMPORTED.value(importFileCount));
+
+ if (this.errorCount_ > 0) {
+ this.tracker_.send(metrics.ImportEvents.ERRORS.value(this.errorCount_));
+ }
+
+ // Finally we want to report on the number of duplicates
+ // that were identified during scanning.
+ var totalDeduped = 0;
+ Object.keys(scanStats.duplicates).forEach(
+ /**
+ * @param {!importer.Disposition} disposition
+ * @this {importer.MediaImportHandler.ImportTask}
+ */
+ function(disposition) {
+ var count = scanStats.duplicates[disposition];
+ totalDeduped += count;
+ this.tracker_.send(
+ metrics.ImportEvents.FILES_DEDUPLICATED
+ .label(disposition)
+ .value(count));
+ }.bind(this));
+ this.tracker_.send(
+ metrics.ImportEvents.FILES_DEDUPLICATED
+ .label('all-duplicates')
+ .value(totalDeduped));
};

Powered by Google App Engine
This is Rietveld 408576698