Cloud import: enable retry upon failure with the affected items.

When backing up things via cloud import, they first get snapshotted locally
before being synced to drive. If there is not enough local space for the
snapshot, backup for the affected file fails. However, sync to drive is the
bottleneck to the whole operation, so backing up a sum of files whose size
exceeds the storage available locally might have a couple of them failing. This
shouldn't affect the user, though - so wait until drive sends a sync complete
signal, then retry backing up the failed items, as long as at least one file is
backed up successfully and at least one of them fails.

Cloud import / Infini backup: Enable retry upon failure with the affected items.

Bug: 354574
Test: manual + ./out/Release/browser_tests --gtest_filter=*MediaImportHandler*
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Id29217980f7d42e7137fdb00fa7f508e1ab0c85d
Reviewed-on: https://chromium-review.googlesource.com/789171
Commit-Queue: Marianne Thieffry <mariannet@google.com>
Reviewed-by: Steve McKay <smckay@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Keigo Oka <oka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520885}

chrome/app/chromeos_strings.grdp

@@ -647,6 +647,9 @@
   <message name="IDS_FILE_BROWSER_CLOUD_IMPORT_STATUS_DONE" desc="Cloud import backup is complete status.">
     <ph name="FILE_COUNT">$1<ex>5</ex></ph> photos backed up to <ph name="BEGIN_LINK">&lt;a is='action-link' class='destination-link'&gt;</ph>Google Drive<ph name="END_LINK">&lt;/a&gt;</ph>
   </message>
+  <message name="IDS_FILE_BROWSER_CLOUD_IMPORT_ERROR_ITEM" desc="Cloud import error message for a file it couldn't back up.">
+    Couldn't back up <ph name="FILE_NAME">$1<ex>photo.jpg</ex></ph>
+  </message>
   <message name="IDS_FILE_BROWSER_CLOUD_IMPORT_TOOLTIP_DONE" desc="Cloud import backup is complete tooltip.">
     <ph name="FILE_COUNT">$1<ex>5</ex></ph> photos backed up
   </message>

chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc

@@ -309,6 +309,8 @@ void AddStringsForCloudImport(base::DictionaryValue* dict) {
   SET_STRING("CLOUD_IMPORT_COMMAND", IDS_FILE_BROWSER_CLOUD_IMPORT_COMMAND);
   SET_STRING("CLOUD_IMPORT_CANCEL_COMMAND",
              IDS_FILE_BROWSER_CLOUD_IMPORT_CANCEL_COMMAND);
+  SET_STRING("CLOUD_IMPORT_ERROR_ITEM",
+             IDS_FILE_BROWSER_CLOUD_IMPORT_ERROR_ITEM);
 
   SET_STRING("CLOUD_IMPORT_STATUS_READY",
              IDS_FILE_BROWSER_CLOUD_IMPORT_STATUS_DONE);

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

@@ -79,10 +79,8 @@ function FileBrowserBackgroundImpl() {
    * @type {!importer.MediaImportHandler}
    */
   this.mediaImportHandler = new importer.MediaImportHandler(
-      this.progressCenter,
-      this.historyLoader,
-      this.dispositionChecker_,
-      this.tracker);
+      this.progressCenter, this.historyLoader, this.dispositionChecker_,
+      this.tracker, this.driveSyncHandler);
 
   /**
    * String assets.

ui/file_manager/file_manager/background/js/compiled_resources2.gyp

@@ -174,6 +174,7 @@
         '../../../externs/background/compiled_resources2.gyp:import_runner',
         '../../common/js/compiled_resources2.gyp:importer_common',
         '../../common/js/compiled_resources2.gyp:metrics',
+        'drive_sync_handler',
         'import_history',
         'media_scanner',
         'progress_center',

ui/file_manager/file_manager/background/js/media_import_handler.js

@@ -16,9 +16,11 @@ var importer = importer || {};
  * @param {!importer.HistoryLoader} historyLoader
  * @param {!importer.DispositionChecker.CheckerFunction} dispositionChecker
  * @param {!analytics.Tracker} tracker
+ * @param {!DriveSyncHandler} driveSyncHandler
  */
-importer.MediaImportHandler = function(progressCenter, historyLoader,
-                                       dispositionChecker, tracker) {
+importer.MediaImportHandler = function(
+    progressCenter, historyLoader, dispositionChecker, tracker,
+    driveSyncHandler) {
   /** @private {!ProgressCenter} */
   this.progressCenter_ = progressCenter;
 
@@ -44,6 +46,9 @@ importer.MediaImportHandler = function(progressCenter, historyLoader,
 
   /** @private {!importer.DispositionChecker.CheckerFunction} */
   this.getDisposition_ = dispositionChecker;
+
+  /** @private {!DriveSyncHandler} */
+  this.driveSyncHandler_ = driveSyncHandler;
 };
 
 // The name of the Drive property used to tag imported files.  Used to look up
@@ -56,22 +61,23 @@ importer.MediaImportHandler.IMPORTS_TAG_KEY = 'cloud-import';
 importer.MediaImportHandler.IMPORTS_TAG_VALUE = 'media';
 
 /** @override */
-importer.MediaImportHandler.prototype.importFromScanResult =
-    function(scanResult, destination, directoryPromise) {
+importer.MediaImportHandler.prototype.importFromScanResult = function(
+    scanResult, destination, directoryPromise) {
 
   var task = new importer.MediaImportHandler.ImportTask(
-      this.generateTaskId_(),
-      this.historyLoader_,
-      scanResult,
-      directoryPromise,
-      destination,
-      this.getDisposition_,
-      this.tracker_);
+      this.generateTaskId_(), this.historyLoader_, scanResult, directoryPromise,
+      destination, this.getDisposition_, this.tracker_);
 
   task.addObserver(this.onTaskProgress_.bind(this, task));
   task.addObserver(this.onFileImported_.bind(this, task));
 
+  // Schedule the task when it is initialized.
+  scanResult.whenFinal()
+      .then(task.initialize_.bind(task))
+      .then(function(scanResult, task) {
+        task.importEntries = scanResult.getFileEntries();
         this.queue_.queueTask(task);
+      }.bind(this, scanResult, task));
 
   return task;
 };
@@ -114,17 +120,41 @@ importer.MediaImportHandler.prototype.onTaskProgress_ =
       item.progressValue = task.processedBytes;
       item.state = ProgressItemState.PROGRESSING;
       break;
+
     case UpdateType.COMPLETE:
+      // Remove the event handler that gets attached for retries.
+      this.driveSyncHandler_.removeEventListener(
+          DriveSyncHandler.COMPLETED_EVENT, task.driveListener_);
+
+      if (task.failedEntries.length > 0 &&
+          task.failedEntries.length < task.importEntries.length) {
+        // If there are failed entries but at least one entry succeeded on the
+        // last run, there is a chance of more succeeding when retrying.
+        this.retryTaskFailedEntries_(task);
+      } else {
+        // Otherwise, finish progress bar.
+        // Display all errors.
+        var errorIdCounter = 0;
+        task.failedEntries.forEach(function(entry) {
+          var errorItem = new ProgressCenterItem();
+          errorItem.id = task.taskId_ + '-' + (errorIdCounter++);
+          errorItem.type = ProgressItemType.COPY;
+          errorItem.quiet = true;
+          errorItem.state = ProgressItemState.ERROR;
+          errorItem.message = strf('CLOUD_IMPORT_ERROR_ITEM', entry.name);
+          this.progressCenter_.updateItem(item);
+        }.bind(this));
+
+        // Complete progress bar.
         item.message = '';
         item.progressValue = item.progressMax;
         item.state = ProgressItemState.COMPLETED;
+
+        task.sendImportStats_();
+      }
+
       break;
-    case UpdateType.ERROR:
-      item.message =
-          strf('CLOUD_IMPORT_ITEMS_REMAINING', task.remainingFilesCount);
-      item.progressValue = task.processedBytes;
-      item.state = ProgressItemState.ERROR;
-      break;
+
     case UpdateType.CANCELED:
       item.message = '';
       item.state = ProgressItemState.CANCELED;
@@ -134,6 +164,25 @@ importer.MediaImportHandler.prototype.onTaskProgress_ =
   this.progressCenter_.updateItem(item);
 };
 
+/**
+ * Restarts a task with failed entries.
+ * @param {!importer.TaskQueue.Task} task
+ */
+importer.MediaImportHandler.prototype.retryTaskFailedEntries_ = function(task) {
+  // Reset the entry lists.
+  task.importEntries = task.failedEntries;
+  task.failedEntries = [];
+
+  // When Drive is done syncing, it will mark the synced files as eligible
+  // for cache eviction, enabling files that failed to be imported because
+  // of not having enough local space to succeed on retry.
+  task.driveListener_ = function(task) {
+    this.queue_.queueTask(task);
+  }.bind(this, task);
+  this.driveSyncHandler_.addEventListener(
+      DriveSyncHandler.COMPLETED_EVENT, task.driveListener_);
+};
+
 /**
  * Tags newly-imported files with a Drive property.
  * @param {!importer.TaskQueue.Task} task
@@ -236,18 +285,60 @@ importer.MediaImportHandler.ImportTask = function(
 
   /** @private {!importer.DispositionChecker.CheckerFunction} */
   this.getDisposition_ = dispositionChecker;
+
+  /**
+   * The entries to be imported.
+   * @private {!Array<!FileEntry>} */
+  this.importEntries_ = [];
+
+  /**
+   * The failed entries.
+   * @private {!Array<!FileEntry>} */
+  this.failedEntries_ = [];
+
+  /**
+   * A placeholder for identifying the appropriate retry function for a given
+   * task.
+   * @private {EventListenerType} */
+  this.driveListener_ = null;
 };
 
 /** @struct */
 importer.MediaImportHandler.ImportTask.prototype = {
   /** @return {number} Number of imported bytes */
-  get processedBytes() { return this.processedBytes_; },
+  get processedBytes() {
+    return this.processedBytes_;
+  },
 
   /** @return {number} Total number of bytes to import */
-  get totalBytes() { return this.totalBytes_; },
+  get totalBytes() {
+    return this.totalBytes_;
+  },
 
   /** @return {number} Number of files left to import */
-  get remainingFilesCount() { return this.remainingFilesCount_; }
+  get remainingFilesCount() {
+    return this.remainingFilesCount_;
+  },
+
+  /** @return {!Array<!FileEntry>} The files to be imported */
+  get importEntries() {
+    return this.importEntries_;
+  },
+
+  /** @param {!Array<!FileEntry>} entries The files to be imported */
+  set importEntries(entries) {
+    this.importEntries_ = entries.slice();
+  },
+
+  /** @return {!Array<!FileEntry>} The files that couldn't be imported */
+  get failedEntries() {
+    return this.failedEntries_;
+  },
+
+  /** @param {!Array<!FileEntry>} entries The files that couldn't be imported */
+  set failedEntries(entries) {
+    this.failedEntries_ = entries.slice();
+  },
 };
 
 /**
@@ -278,9 +369,7 @@ importer.MediaImportHandler.ImportTask.prototype.__proto__ =
 importer.MediaImportHandler.ImportTask.prototype.run = function() {
   // Wait for the scan to finish, then get the destination entry, then start the
   // import.
-  this.scanResult_.whenFinal()
-      .then(this.initialize_.bind(this))
-      .then(this.importScanEntries_.bind(this))
+  this.importScanEntries_()
       .then(this.markDuplicatesImported_.bind(this))
       .then(this.onSuccess_.bind(this))
       .catch(importer.getLogger().catcher('import-task-run'));
@@ -292,6 +381,10 @@ importer.MediaImportHandler.ImportTask.prototype.run = function() {
  */
 importer.MediaImportHandler.ImportTask.prototype.requestCancel = function() {
   this.canceled_ = true;
+  setTimeout(function() {
+    this.notify(importer.TaskQueue.UpdateType.CANCELED);
+    this.sendImportStats_();
+  }.bind(this));
   if (this.cancelCallback_) {
     // Reset the callback before calling it, as the callback might do anything
     // (including calling #requestCancel again).
@@ -306,7 +399,6 @@ 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);
 };
@@ -320,13 +412,10 @@ importer.MediaImportHandler.ImportTask.prototype.initialize_ = function() {
 importer.MediaImportHandler.ImportTask.prototype.importScanEntries_ =
     function() {
   var resolver = new importer.Resolver();
-  this.directoryPromise_.then(
-      function(destinationDirectory) {
+  this.directoryPromise_.then(function(destinationDirectory) {
     AsyncUtil.forEach(
-            this.scanResult_.getFileEntries(),
-            this.importOne_.bind(this, destinationDirectory),
-            resolver.resolve,
-            resolver);
+        this.importEntries_, this.importOne_.bind(this, destinationDirectory),
+        resolver.resolve, resolver);
   }.bind(this));
   return resolver.promise;
 };
@@ -363,21 +452,20 @@ importer.MediaImportHandler.ImportTask.prototype.markDuplicatesImported_ =
  * @param {function()} completionCallback Called after this operation is
  *     complete.
  * @param {!FileEntry} entry The entry to import.
+ * @param {number} index The index of the entry.
+ * @param {Array<!FileEntry>} all All the entries.
  * @private
  */
-importer.MediaImportHandler.ImportTask.prototype.importOne_ =
-    function(destinationDirectory, completionCallback, entry) {
+importer.MediaImportHandler.ImportTask.prototype.importOne_ = function(
+    destinationDirectory, completionCallback, entry, index, all) {
   if (this.canceled_) {
-    this.notify(importer.TaskQueue.UpdateType.CANCELED);
     this.tracker_.send(metrics.ImportEvents.IMPORT_CANCELLED);
-    this.sendImportStats_();
     return;
   }
 
-  this.getDisposition_(entry, importer.Destination.GOOGLE_DRIVE,
-                       importer.ScanMode.CONTENT)
-      .then(
-          (/**
+  this.getDisposition_(
+          entry, importer.Destination.GOOGLE_DRIVE, importer.ScanMode.CONTENT)
+      .then((/**
               * @param {!importer.Disposition} disposition The disposition
               *     of the entry. Either some sort of dupe, or an original.
               * @this {importer.MediaImportHandler.ImportTask}
@@ -391,12 +479,17 @@ importer.MediaImportHandler.ImportTask.prototype.importOne_ =
              }).bind(this))
       // Regardless of the result of this copy, push on to the next file.
       .then(completionCallback)
-      .catch(
+      .catch((
                  /** @param {*} error */
                  function(error) {
-            importer.getLogger().catcher('import-task-import-one')(error);
+                   importer.getLogger().catcher('import-task-import-one')(
+                       error);
+                   // TODO(oka): Retry copies only when failed due to
+                   // insufficient disk space. crbug.com/788692.
+                   this.failedEntries_.push(entry);
                    completionCallback();
-          });
+                 })
+                 .bind(this));
 };
 
 /**
@@ -461,10 +554,6 @@ importer.MediaImportHandler.ImportTask.prototype.copy_ =
   /** @this {importer.MediaImportHandler.ImportTask} */
   var onError = function(error) {
     this.cancelCallback_ = null;
-    // Log the bytes as processed in spite of the error.  This ensures
-    // completion of the progress bar.
-    this.processedBytes_ -= currentBytes;
-    this.processedBytes_ += entry.size;
     if (error.name === util.FileError.ABORT_ERR) {
       // Task cancellations result in the error callback being triggered with an
       // ABORT_ERR, but we want to ignore these errors.
@@ -539,7 +628,6 @@ importer.MediaImportHandler.ImportTask.prototype.markAsImported_ =
 /** @private */
 importer.MediaImportHandler.ImportTask.prototype.onSuccess_ = function() {
   this.notify(importer.TaskQueue.UpdateType.COMPLETE);
-  this.sendImportStats_();
 };
 
 /**

ui/file_manager/file_manager/background/js/media_import_handler_unittest.html

@@ -25,8 +25,10 @@
   <script src="../../common/js/progress_center_common.js"></script>
   <script src="../../common/js/test_tracker.js"></script>
 
+  <script src="drive_sync_handler.js"></script>
   <script src="entry_location_impl.js"></script>
   <script src="test_import_history.js"></script>
+  <script src="mock_drive_sync_handler.js"></script>
   <script src="mock_progress_center.js"></script>
   <script src="mock_media_scanner.js"></script>
   <script src="mock_volume_manager.js"></script>

ui/file_manager/file_manager/background/js/media_import_handler_unittest.js

@@ -32,6 +32,9 @@ var duplicateFinderFactory;
 /** @type {!Promise<!DirectoryEntry>} */
 var destinationFactory;
 
+/** @type {!MockDriveSyncHandler} */
+var driveSyncHandler;
+
 // Set up string assets.
 loadTimeData.data = {
   CLOUD_IMPORT_ITEMS_REMAINING: '',
@@ -90,14 +93,12 @@ function setUp() {
   destinationFileSystem = new MockFileSystem('googleDriveFilesystem');
   destinationFactory = Promise.resolve(destinationFileSystem.root);
   duplicateFinderFactory = new importer.TestDuplicateFinder.Factory();
+  driveSyncHandler = new MockDriveSyncHandler();
 
   mediaImporter = new importer.MediaImportHandler(
-      progressCenter,
-      importHistory,
-      function(entry, destination) {
+      progressCenter, importHistory, function(entry, destination) {
         return dispositionChecker(entry, destination);
-      },
-      new TestTracker());
+      }, new TestTracker(), driveSyncHandler);
 }
 
 function testImportMedia(callback) {
@@ -167,10 +168,8 @@ function testImportMedia_skipAndMarkDuplicatedFiles(callback) {
     return Promise.resolve(importer.Disposition.ORIGINAL);
   };
   mediaImporter = new importer.MediaImportHandler(
-      progressCenter,
-      importHistory,
-      dispositionChecker,
-      new TestTracker());
+      progressCenter, importHistory, dispositionChecker, new TestTracker(),
+      driveSyncHandler);
   var scanResult = new TestScanResult(media);
   var importTask = mediaImporter.importFromScanResult(
       scanResult,
@@ -564,7 +563,7 @@ function testImportWithErrors(callback) {
     }
   });
 
-  // Verify that the error didn't result in only 3 files being imported.
+  // Verify that the error didn't result in some files not being copied.
   reportPromise(
       whenImportDone.then(
         function() {

ui/file_manager/file_manager/background/js/mock_drive_sync_handler.js

+// Copyright 2017 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';
+
+/**
+ * Mock of DriveSyncHandler.
+ * @constructor
+ * @struct
+ */
+function MockDriveSyncHandler() {}
+
+MockDriveSyncHandler.prototype = {
+  __proto__: cr.EventTarget.prototype,
+};

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

@@ -222,10 +222,14 @@ TestScanResult.prototype.canceled = function() {
 
 /** @override */
 TestScanResult.prototype.getStatistics = function() {
+  duplicates = {};
+  duplicates[importer.Disposition.CONTENT_DUPLICATE] = 0;
+  duplicates[importer.Disposition.HISTORY_DUPLICATE] = 0;
+  duplicates[importer.Disposition.SCAN_DUPLICATE] = 0;
   return {
     scanDuration: this.scanDuration,
     newFileCount: this.fileEntries.length,
-    duplicates: {},
+    duplicates: duplicates,
     sizeBytes: this.totalBytes
   };
 };