FilesApp do not show confirm delete dialog if using trash

Bug: 953310
Change-Id: I04762056fb288807d9dc7a8fce717f331d43784a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504750Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822057}

ui/file_manager/externs/background/file_operation_manager.js

@@ -49,10 +49,19 @@ class FileOperationManager extends EventTarget {
    */
   paste(sourceEntries, targetEntry, isMove, opt_taskId) {}
 
+  /**
+   * Returns true if all entries will use trash for delete.
+   *
+   * @param {!VolumeManager} volumeManager
+   * @param {!Array<!Entry>} entries The entries.
+   * @return {boolean}
+   */
+  willUseTrash(volumeManager, entries) {}
+
   /**
    * Schedules the files deletion.
    *
-   * @param {Array<Entry>} entries The entries.
+   * @param {!Array<!Entry>} entries The entries.
    */
   deleteEntries(entries) {}
 

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

@@ -387,10 +387,22 @@ class FileOperationManagerImpl {
     nextTask.run(onEntryChanged, onTaskProgress, onTaskSuccess, onTaskError);
   }
 
+  /**
+   * Returns true if all entries will use trash for delete.
+   *
+   * @param {!VolumeManager} volumeManager
+   * @param {!Array<!Entry>} entries The entries.
+   * @return {boolean}
+   */
+  willUseTrash(volumeManager, entries) {
+    return entries.every(
+        entry => this.trash_.shouldMoveToTrash(volumeManager, entry));
+  }
+
   /**
    * Schedules the files deletion.
    *
-   * @param {Array<Entry>} entries The entries.
+   * @param {!Array<!Entry>} entries The entries.
    */
   deleteEntries(entries) {
     const task =

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

@@ -93,6 +93,7 @@ class MockFileOperationManager extends cr.EventTarget {
 
   hasQueuedTasks() {}
   filterSameDirectoryEntry() {}
+  willUseTrash() {}
   deleteEntries() {}
   restoreDeleted() {}
   zipSelection() {}

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

@@ -84,9 +84,8 @@ class Trash {
    * @param {!Entry} entry The entry to remove.
    * @return {?TrashConfig} Valid TrashConfig if item should be moved to trash,
    *     else null if item should be permanently deleted.
-   * @private
    */
-  shouldMoveToTrash_(volumeManager, entry) {
+  shouldMoveToTrash(volumeManager, entry) {
     const info = volumeManager.getLocationInfo(entry);
     if (!loadTimeData.getBoolean('FILES_TRASH_ENABLED') || !info) {
       return null;
@@ -118,7 +117,7 @@ class Trash {
    */
   removeFileOrDirectory(volumeManager, entry, permanentlyDelete) {
     if (!permanentlyDelete) {
-      const config = this.shouldMoveToTrash_(volumeManager, entry);
+      const config = this.shouldMoveToTrash(volumeManager, entry);
       if (config) {
         return this.trashFileOrDirectory_(entry, config);
       }

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

@@ -162,8 +162,6 @@ async function testMyFilesTrash(done) {
 /**
  * Test that Downloads has its own /Downloads/.Trash since it is a separate
  * mount on a device and we don't want move to trash to be a copy operation.
- *
- * @suppress {accessControls} Access shouldMoveToTrash_().
  */
 async function testDownloadsHasOwnTrash(done) {
   const trash = new Trash();
@@ -197,14 +195,14 @@ async function testDownloadsHasOwnTrash(done) {
 
   // Delete /Downloads/.Trash/files/file2.
   const file2Trashed = fs.entries['/Downloads/.Trash/files/file2'];
-  assertFalse(!!trash.shouldMoveToTrash_(volumeManager, file2Trashed));
+  assertFalse(!!trash.shouldMoveToTrash(volumeManager, file2Trashed));
   await trash.removeFileOrDirectory(
       volumeManager, file2Trashed, deletePermanently);
   assertEquals(11, Object.keys(fs.entries).length);
 
   // Delete /Downloads/.Trash.
   const downloadsTrash = fs.entries['/Downloads/.Trash'];
-  assertFalse(!!trash.shouldMoveToTrash_(volumeManager, downloadsTrash));
+  assertFalse(!!trash.shouldMoveToTrash(volumeManager, downloadsTrash));
   await trash.removeFileOrDirectory(
       volumeManager, downloadsTrash, deletePermanently);
   assertEquals(7, Object.keys(fs.entries).length);
@@ -214,8 +212,6 @@ async function testDownloadsHasOwnTrash(done) {
 
 /**
  * Test crostini trash in .local/share/Trash.
- *
- * @suppress {accessControls} Access shouldMoveToTrash_().
  */
 async function testCrostiniTrash(done) {
   const trash = new Trash();
@@ -252,14 +248,14 @@ async function testCrostiniTrash(done) {
   // Move /file2 to trash, then delete /.local/share/Trash/files/file2.
   await trash.removeFileOrDirectory(volumeManager, file2, deletePermanently);
   const file2Trashed = fs.entries['/.local/share/Trash/files/file2'];
-  assertFalse(!!trash.shouldMoveToTrash_(volumeManager, file2Trashed));
+  assertFalse(!!trash.shouldMoveToTrash(volumeManager, file2Trashed));
   await trash.removeFileOrDirectory(
       volumeManager, file2Trashed, deletePermanently);
   assertEquals(8, Object.keys(fs.entries).length);
 
   // Delete /.local/share/Trash.
   const crostiniTrash = fs.entries['/.local/share/Trash'];
-  assertFalse(!!trash.shouldMoveToTrash_(volumeManager, crostiniTrash));
+  assertFalse(!!trash.shouldMoveToTrash(volumeManager, crostiniTrash));
   await trash.removeFileOrDirectory(
       volumeManager, crostiniTrash, deletePermanently);
   assertEquals(4, Object.keys(fs.entries).length);

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

@@ -1112,6 +1112,13 @@ CommandHandler.COMMANDS_['delete'] = new class extends Command {
       return;
     }
 
+    // We show undo toast rather than dialog for entries which will use trash.
+    if (fileManager.fileOperationManager.willUseTrash(
+            fileManager.volumeManager, entries)) {
+      fileManager.fileOperationManager.deleteEntries(entries);
+      return;
+    }
+
     const message = entries.length === 1 ?
         strf('GALLERY_CONFIRM_DELETE_ONE', entries[0].name) :
         strf('GALLERY_CONFIRM_DELETE_SOME', entries.length);

ui/file_manager/integration_tests/file_manager/trash.js

@@ -6,6 +6,7 @@
 
 /**
  * Delete files in MyFiles and ensure they are moved to /.Trash.
+ * Then delete items from /.Trash/files and /.Trash/info, then delete /.Trash.
  */
 testcase.trashMoveToTrash = async () => {
   const appId = await setupAndWaitUntilReady(
@@ -15,12 +16,8 @@ testcase.trashMoveToTrash = async () => {
   await remoteCall.waitAndClickElement(
       appId, '#file-list [file-name="hello.txt"]');
 
-  // Delete item and confirm delete.
+  // Delete item and wait for it to be removed (no dialog).
   await remoteCall.waitAndClickElement(appId, '#delete-button');
-  await remoteCall.waitAndClickElement(
-      appId, '.files-confirm-dialog .cr-dialog-ok');
-
-  // Wait for completion of file deletion.
   await remoteCall.waitForElementLost(
       appId, '#file-list [file-name="hello.txt"]');
 
@@ -42,15 +39,49 @@ testcase.trashMoveToTrash = async () => {
   // Navigate to /My files/Downloads/.Trash/files.
   await navigateWithDirectoryTree(appId, '/My files/Downloads/.Trash/files');
 
-  // Ensure hello.txt exists.
-  await remoteCall.waitForElement(appId, '#file-list [file-name="hello.txt"]');
+  // Select hello.txt.
+  await remoteCall.waitAndClickElement(
+      appId, '#file-list [file-name="hello.txt"]');
 
-  // Navigate to /My files/.Trash/files.
+  // Delete item and confirm delete (dialog shown).
+  await remoteCall.waitAndClickElement(appId, '#delete-button');
+  await remoteCall.waitAndClickElement(
+      appId, '.files-confirm-dialog .cr-dialog-ok');
+
+  // Wait for completion of file deletion.
+  await remoteCall.waitForElementLost(
+      appId, '#file-list [file-name="hello.txt"]');
+
+  // Navigate to /My files/Downloads/.Trash/info.
   await navigateWithDirectoryTree(appId, '/My files/Downloads/.Trash/info');
 
-  // Ensure hello.txt.trashinfo exists.
-  await remoteCall.waitForElement(
+  // Select hello.txt.trashinfo.
+  await remoteCall.waitAndClickElement(
+      appId, '#file-list [file-name="hello.txt.trashinfo"]');
+
+  // Delete item and confirm delete (dialog shown).
+  await remoteCall.waitAndClickElement(appId, '#delete-button');
+  await remoteCall.waitAndClickElement(
+      appId, '.files-confirm-dialog .cr-dialog-ok');
+
+  // Wait for completion of file deletion.
+  await remoteCall.waitForElementLost(
       appId, '#file-list [file-name="hello.txt.trashinfo"]');
+
+  // Navigate to /My files/Downloads.
+  await navigateWithDirectoryTree(appId, '/My files/Downloads');
+
+  // Select .Trash.
+  await remoteCall.waitAndClickElement(
+      appId, '#file-list [file-name=".Trash"]');
+
+  // Delete item and confirm delete (dialog shown).
+  await remoteCall.waitAndClickElement(appId, '#delete-button');
+  await remoteCall.waitAndClickElement(
+      appId, '.files-confirm-dialog .cr-dialog-ok');
+
+  // Wait for completion of file deletion.
+  await remoteCall.waitForElementLost(appId, '#file-list [file-name=".Trash"]');
 };
 
 /**
@@ -64,12 +95,8 @@ testcase.trashRestore = async () => {
   await remoteCall.waitAndClickElement(
       appId, '#file-list [file-name="hello.txt"]');
 
-  // Delete item and confirm delete.
+  // Delete item and wait for it to be removed (no dialog).
   await remoteCall.waitAndClickElement(appId, '#delete-button');
-  await remoteCall.waitAndClickElement(
-      appId, '.files-confirm-dialog .cr-dialog-ok');
-
-  // Wait for file to be removed from list.
   await remoteCall.waitForElementLost(
       appId, '#file-list [file-name="hello.txt"]');