Directly write ZIP file to destination when it's not on Drive or MTP.

Existing code always wrote zip file to temporary filesystem and then
moved to the destination, as a workaround for crbug.com/785086.
It is not needed for local volumes like Downloads.
Requesting temporary file system in the guest mode caused runtime error.
We can avoid that error by this because there is no Drive volume in the
guest mode.

Bug: 834675
Test: manually verified by zipping files in Drive, Downloads and in guest mode
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If920f49564309a43ddcf5ca02ee8aa6baa3e6482
Reviewed-on: https://chromium-review.googlesource.com/1027394
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553985}

chrome/browser/resources/chromeos/zip_archiver/js/app.js

@@ -582,8 +582,9 @@ unpacker.app = {
   /**
    * Creates a new compressor and compresses entries.
    * @param {!Object} launchData
+   * @param {boolean} useTemporaryDirectory
    */
-  onLaunchedWithPack: function(launchData) {
+  onLaunchedWithPack: function(launchData, useTemporaryDirectory) {
     unpacker.app.mountProcessCounter++;
 
     // Create a promise to load the NaCL module.
@@ -599,7 +600,7 @@ unpacker.app = {
         .then(function(stringData) {
           var compressor = new unpacker.Compressor(
               /** @type {!Object} */ (unpacker.app.naclModule),
-              launchData.items);
+              launchData.items, useTemporaryDirectory);
 
           var compressorId = compressor.getCompressorId();
           unpacker.app.compressors[compressorId] = compressor;
@@ -876,7 +877,9 @@ unpacker.app = {
     }
 
     if (launchData.id === 'pack')
-      unpacker.app.onLaunchedWithPack(launchData);
+      unpacker.app.onLaunchedWithPack(launchData, false);
+    else if (launchData.id === 'pack_using_tmp')
+      unpacker.app.onLaunchedWithPack(launchData, true);
     else
       unpacker.app.onLaunchedWithUnpack(launchData, opt_onSuccess, opt_onError);
   },

chrome/browser/resources/chromeos/zip_archiver/js/compressor.js

@@ -13,8 +13,12 @@
  * @constructor
  * @param {!Object} naclModule The nacl module.
  * @param {!Array} items The items to be packed.
+ * @param {boolean} useTemporaryDirectory Whether to use the temporary
+ *     filesystem as work directory for creating a ZIP file. This is used for
+ *     filesystem that degrades performance with frequent write operations,
+ *     like online storages.
  */
-unpacker.Compressor = function(naclModule, items) {
+unpacker.Compressor = function(naclModule, items, useTemporaryDirectory) {
   /**
    * @private {Object}
    * @const
@@ -103,6 +107,12 @@ unpacker.Compressor = function(naclModule, items) {
    * @type {number}
    */
   this.processedSize_ = 0;
+
+  /**
+   * Whether to write in-progress zip file in temporary location or not.
+   * @type {boolean}
+   */
+  this.useTemporaryDirectory_ = useTemporaryDirectory;
 };
 
 /**
@@ -213,8 +223,14 @@ unpacker.Compressor.prototype.getArchiveFile_ = function() {
           this.onErrorInternal_();
         });
   };
+  var getWorkRootPromise;
+  if (this.useTemporaryDirectory_) {
+    getWorkRootPromise = this.getTemporaryRootEntry_();
+  } else {
+    getWorkRootPromise = this.getParentEntry_();
+  }
 
-  this.getTemporaryRootEntry_().then(saveZipFile).catch((domException) => {
+  getWorkRootPromise.then(saveZipFile).catch((domException) => {
     console.error(domException);
     this.onErrorInternal_();
   });
@@ -566,35 +582,32 @@ unpacker.Compressor.prototype.onAddToArchiveDone_ = function() {
 
 /**
  * Moves the temporary file to actual destination folder.
+ * @param {function()} onFinished called when file move is finished.
  */
-unpacker.Compressor.prototype.moveZipFileToActualDestination = function() {
+unpacker.Compressor.prototype.moveZipFileToActualDestination = function(
+    onFinished) {
   var suggestedName = this.getArchiveName_();
-  var moveZipFileToParentDir = (rootEntry) => {
+  const moveZipFileToParentDir = (rootEntry) => {
     // If parent directory of currently selected files is available then we
     // deduplicate |suggestedName| and save the zip file.
-    if (!rootEntry) {
-      console.error('rootEntry of selected files is undefined');
-      this.onErrorInternal_();
-      return;
-    }
+    if (!rootEntry)
+      return Promise.reject('rootEntry of selected files is undefined');
 
-    fileOperationUtils.deduplicateFileName(suggestedName, rootEntry)
-        .then((newName) => {
+    return fileOperationUtils.deduplicateFileName(suggestedName, rootEntry)
+        .then((newName) => new Promise((resolve, reject) => {
                 this.archiveFileEntry_.moveTo(
-              rootEntry, newName, function() {},
-              (error) => {
-                console.error('Failed to move the file to destination.');
-                this.onErrorInternal_();
-              });
-        })
-        .catch((error) => {
-          console.error(error);
-          this.onErrorInternal_();
+                    rootEntry, newName, resolve, (error) => {
+                      reject('Failed to move the file to destination.');
                     });
+              }));
   };
   this.getParentEntry_()
       .then(moveZipFileToParentDir)
-      .catch(this.onErrorInternal_.bind(this));
+      .then(onFinished)
+      .catch((error) => {
+        this.onErrorInternal_();
+        console.error(error);
+      });
 };
 
 /**
@@ -685,8 +698,10 @@ unpacker.Compressor.prototype.processMessage = function(data, operation) {
       break;
 
     case unpacker.request.Operation.CLOSE_ARCHIVE_DONE:
-      this.moveZipFileToActualDestination();
       this.sendReleaseCompressor();
+      if (this.useTemporaryDirectory_)
+        this.moveZipFileToActualDestination(() => this.onCloseArchiveDone_());
+      else
         this.onCloseArchiveDone_();
       break;
 

chrome/browser/resources/chromeos/zip_archiver/manifest.json

@@ -39,6 +39,11 @@
       "types": ["*"],
       "include_directories": true,
       "verb": "pack_with"
+    },
+    "pack_using_tmp": {
+      "types": ["*"],
+      "include_directories": true,
+      "verb": "pack_with"
     }
   },
   "icons": {

ui/file_manager/file_manager/foreground/js/file_manager_commands.js

@@ -1432,7 +1432,12 @@ CommandHandler.COMMANDS_['zip-selection'] = /** @type {Command} */ ({
     if (CommandHandler.IS_ZIP_ARCHIVER_PACKER_ENABLED_) {
       fileManager.taskController.getFileTasks()
           .then(function(tasks) {
+            if (fileManager.directoryModel.isOnDrive() ||
+                fileManager.directoryModel.isOnMTP()) {
+              tasks.execute(FileTasks.ZIP_ARCHIVER_ZIP_USING_TMP_TASK_ID);
+            } else {
               tasks.execute(FileTasks.ZIP_ARCHIVER_ZIP_TASK_ID);
+            }
           })
           .catch(function(error) {
             if (error)

ui/file_manager/file_manager/foreground/js/file_tasks.js

@@ -115,6 +115,15 @@ FileTasks.ZIP_ARCHIVER_UNZIP_TASK_ID =
 FileTasks.ZIP_ARCHIVER_ZIP_TASK_ID =
     'dmboannefpncccogfdikhmhpmdnddgoe|app|pack';
 
+/**
+ * The task id of zip action of Zip Archiver app, using temporary dir as workdir
+ * @const
+ * @type {string}
+ */
+FileTasks.ZIP_ARCHIVER_ZIP_USING_TMP_TASK_ID =
+    'dmboannefpncccogfdikhmhpmdnddgoe|app|pack_using_tmp';
+
+
 /**
  * Available tasks in task menu button.
  * @enum {string}
@@ -174,7 +183,8 @@ FileTasks.create = function(
       // Filters out Pack with Zip Archiver task because it will be accessible
       // via 'Zip selection' context menu button
       taskItems = taskItems.filter(function(item) {
-        return item.taskId !== FileTasks.ZIP_ARCHIVER_ZIP_TASK_ID;
+        return item.taskId !== FileTasks.ZIP_ARCHIVER_ZIP_TASK_ID &&
+            item.taskId !== FileTasks.ZIP_ARCHIVER_ZIP_USING_TMP_TASK_ID;
       });
 
       // Filters out Unpack with Zip Archiver task if switch is not enabled.