Fix naming issue when renaming files inside My files

Change DirectoryModel to "unwrap" VolumeEntry when it's selected via
file list. This fixes the issue where after selecting a VolumeEntry any
operation in the private API would fail because it wouldn't be able to
send a VolumeEntry to private API, because it requires a real Entry.

This fixes the user issue of trying to rename and always getting "Use
shorter name" error, even with a short name.

Update FakeEntry to be an implementation of FilesAppDirEntry, since all
users were using as directory, also update some functions to typing to
use FilesAppDirEntry instead of FakeEntry which is more correct.

Test: browser_tests --gtest_filter="*/myFilesFolderRename"
Bug: 889636
Change-Id: Ieb75ffbb811a91e4742fb7a49a3badfd129376ad
Reviewed-on: https://chromium-review.googlesource.com/c/1276067Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599136}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -682,6 +682,7 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
                       TestCase("hideSearchButton"),
                       TestCase("myFilesDisplaysAndOpensEntries"),
                       TestCase("directoryTreeRefresh"),
+                      TestCase("myFilesFolderRename"),
                       TestCase("myFilesUpdatesChildren")));
 
 WRAPPED_INSTANTIATE_TEST_CASE_P(

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

@@ -65,12 +65,12 @@ function VolumeInfoImpl(
     this.fakeEntries_[VolumeManagerCommon.RootType.DRIVE_OFFLINE] =
         new FakeEntry(
             str('DRIVE_OFFLINE_COLLECTION_LABEL'),
-            VolumeManagerCommon.RootType.DRIVE_OFFLINE, true);
+            VolumeManagerCommon.RootType.DRIVE_OFFLINE);
 
     this.fakeEntries_[VolumeManagerCommon.RootType.DRIVE_SHARED_WITH_ME] =
         new FakeEntry(
             str('DRIVE_SHARED_WITH_ME_COLLECTION_LABEL'),
-            VolumeManagerCommon.RootType.DRIVE_SHARED_WITH_ME, true);
+            VolumeManagerCommon.RootType.DRIVE_SHARED_WITH_ME);
   }
 
   // Note: This represents if the mounting of the volume is successfully done

ui/file_manager/file_manager/common/js/files_app_entry_types.js

@@ -487,17 +487,16 @@ class VolumeEntry {
  * FakeEntry is used for entries that used only for UI, that weren't generated
  * by FileSystem API, like Drive, Downloads or Provided.
  *
- * @implements FilesAppEntry
+ * @implements FilesAppDirEntry
  */
 class FakeEntry {
   /**
    * @param {string} label Translated text to be displayed to user.
    * @param {!VolumeManagerCommon.RootType} rootType Root type of this entry.
-   * @param {boolean} isDirectory Is this entry a directory-like entry?
    * @param {chrome.fileManagerPrivate.SourceRestriction=} opt_sourceRestriction
    *    used on Recents to filter the source of recent files/directories.
    */
-  constructor(label, rootType, isDirectory, opt_sourceRestriction) {
+  constructor(label, rootType, opt_sourceRestriction) {
     /**
      * @public {string} label: Label to be used when displaying to user, it
      *      should be already translated. */
@@ -509,13 +508,11 @@ class FakeEntry {
     /** @public {!VolumeManagerCommon.RootType} */
     this.rootType = rootType;
 
-    /**
-     * @public {boolean} true if this entry represents a Directory-like entry.
-     */
-    this.isDirectory = isDirectory;
+    /** @public {boolean} true FakeEntry are always directory-like. */
+    this.isDirectory = true;
 
-    /** @public {boolean} true if this entry represents a File-like entry. */
-    this.isFile = !this.isDirectory;
+    /** @public {boolean} false FakeEntry are always directory-like. */
+    this.isFile = false;
 
     /**
      * @public {chrome.fileManagerPrivate.SourceRestriction|undefined} It's used
@@ -564,4 +561,13 @@ class FakeEntry {
   get isNativeType() {
     return false;
   }
+
+  /**
+   * @return {!StaticReader} Returns a reader compatible with
+   * DirectoryEntry.createReader (from Web Standards) that reads 0 entries.
+   * @override
+   */
+  createReader() {
+    return new StaticReader([]);
+  }
 }

ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js

@@ -328,8 +328,7 @@ function testEntryListAddEntrySetsPrefix() {
  * Test FakeEntry, which is only static data.
  */
 function testFakeEntry(testReportCallback) {
-  let fakeEntry =
-      new FakeEntry('label', VolumeManagerCommon.RootType.CROSTINI, true);
+  let fakeEntry = new FakeEntry('label', VolumeManagerCommon.RootType.CROSTINI);
 
   assertEquals(undefined, fakeEntry.sourceRestriction);
   assertEquals('FakeEntry', fakeEntry.type_name);
@@ -342,15 +341,12 @@ function testFakeEntry(testReportCallback) {
   assertTrue(fakeEntry.isDirectory);
   assertFalse(fakeEntry.isFile);
 
-  // Check the isDirectory and sourceRestriction constructor args.
+  // Check sourceRestriction constructor args.
   const kSourceRestriction =
       /** @type{chrome.fileManagerPrivate.SourceRestriction} */ ('fake');
   fakeEntry = new FakeEntry(
-      'label', VolumeManagerCommon.RootType.CROSTINI, false,
-      kSourceRestriction);
+      'label', VolumeManagerCommon.RootType.CROSTINI, kSourceRestriction);
   assertEquals(kSourceRestriction, fakeEntry.sourceRestriction);
-  assertFalse(fakeEntry.isDirectory);
-  assertTrue(fakeEntry.isFile);
 
   let callCounter = 0;
 

ui/file_manager/file_manager/foreground/js/directory_model.js

@@ -875,7 +875,7 @@ DirectoryModel.prototype.onRenameEntry = function(
     // new one.
     if (util.isSameEntry(oldEntry, this.getCurrentDirEntry())) {
       this.changeDirectoryEntry(
-          /** @type {!DirectoryEntry|!FakeEntry} */ (newEntry));
+          /** @type {!DirectoryEntry|!FilesAppDirEntry} */ (newEntry));
     }
 
     // Replace the old item with the new item. oldEntry instance itself may
@@ -960,13 +960,18 @@ DirectoryModel.prototype.updateAndSelectNewDirectory = function(newDirectory) {
  * activateDirectoryEntry instead of this, which is higher-level function and
  * cares about the selection.
  *
- * @param {!DirectoryEntry|!FakeEntry} dirEntry The entry of the new directory
- *     to be opened.
+ * @param {!DirectoryEntry|!FilesAppDirEntry} dirEntry The entry of the new
+ *     directory to be opened.
  * @param {function()=} opt_callback Executed if the directory loads
  *     successfully.
  */
 DirectoryModel.prototype.changeDirectoryEntry = function(
     dirEntry, opt_callback) {
+  // If it's a VolumeEntry which wraps an actual entry, we should use the
+  // unwrapped entry.
+  if (dirEntry instanceof VolumeEntry)
+    dirEntry = assert(dirEntry.rootEntry);
+
   // Increment the sequence value.
   this.changeDirectorySequence_++;
   this.clearSearch_();
@@ -1082,8 +1087,8 @@ DirectoryModel.prototype.onVolumeChanged_ = function(volumeInfo) {
  *    directory.
  *  - Clears the selection, if the given directory is the current directory.
  *
- * @param {!DirectoryEntry|!FakeEntry} dirEntry The entry of the new directory
- *     to be opened.
+ * @param {!DirectoryEntry|!FilesAppDirEntry} dirEntry The entry of the new
+ *     directory to be opened.
  * @param {function()=} opt_callback Executed if the directory loads
  *     successfully.
  */

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

@@ -1207,7 +1207,7 @@ FileManager.prototype = /** @struct */ {
                 str('RECENT_ROOT_LABEL'), NavigationModelItemType.RECENT,
                 new FakeEntry(
                     str('RECENT_ROOT_LABEL'),
-                    VolumeManagerCommon.RootType.RECENT, true,
+                    VolumeManagerCommon.RootType.RECENT,
                     this.getSourceRestriction_())) :
             null,
         this.commandLineFlags_['disable-my-files-navigation']);
@@ -1232,7 +1232,7 @@ FileManager.prototype = /** @struct */ {
               str('LINUX_FILES_ROOT_LABEL'), NavigationModelItemType.CROSTINI,
               new FakeEntry(
                   str('LINUX_FILES_ROOT_LABEL'),
-                  VolumeManagerCommon.RootType.CROSTINI, true)) :
+                  VolumeManagerCommon.RootType.CROSTINI)) :
           null;
 
       // Redraw the tree even if not enabled.  This is required for testing.

ui/file_manager/file_manager/foreground/js/file_transfer_controller.js

@@ -167,7 +167,7 @@ function FileTransferController(
       queryRequiredElement('command#cut', this.document_));
 
   /**
-   * @private {DirectoryEntry|FakeEntry}
+   * @private {DirectoryEntry|FilesAppDirEntry}
    */
   this.destinationEntry_ = null;
 

ui/file_manager/file_manager/foreground/js/file_watcher.js

@@ -69,8 +69,8 @@ FileWatcher.prototype.onDirectoryChanged_ = function(event) {
  * Changes the watched directory. In case of a fake entry, the watch is
  * just released, since there is no reason to track a fake directory.
  *
- * @param {!DirectoryEntry|!FakeEntry} entry Directory entry to be tracked, or
- *     the fake entry.
+ * @param {!DirectoryEntry|!FilesAppEntry} entry Directory entry to be tracked,
+ *     or the fake entry.
  * @return {!Promise}
  */
 FileWatcher.prototype.changeWatchedDirectory = function(entry) {

ui/file_manager/integration_tests/file_manager/my_files.js

@@ -325,3 +325,107 @@ testcase.myFilesUpdatesChildren = function() {
     },
   ]);
 };
+
+/**
+ * Check naming a folder after navigating inside MyFiles using file list (RHS).
+ * crbug.com/889636.
+ */
+testcase.myFilesFolderRename = function() {
+  let appId;
+  const textInput = '#file-list .table-row[renaming] input.rename';
+  StepsRunner.run([
+    // Open Files app on local Downloads.
+    function() {
+      setupAndWaitUntilReady(
+          null, RootPath.DOWNLOADS, this.next, [ENTRIES.photos], []);
+    },
+    // Select "My files" folder via directory tree.
+    function(result) {
+      appId = result.windowId;
+      const myFilesQuery = '#directory-tree [entry-label="My files"]';
+      const isDriveQuery = false;
+      remoteCall.callRemoteTestUtil(
+          'selectInDirectoryTree', appId, [myFilesQuery, isDriveQuery],
+          this.next);
+    },
+    // Wait for Downloads to load.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'selectInDirectoryTree failed');
+      const expectedRows = [
+        ['Downloads', '--', 'Folder'],
+        ['Play files', '--', 'Folder'],
+        ['Linux files', '--', 'Folder'],
+      ];
+      remoteCall
+          .waitForFiles(
+              appId, expectedRows,
+              {ignoreFileSize: true, ignoreLastModifiedTime: true})
+          .then(this.next);
+    },
+    // Select Downloads via file list.
+    function() {
+      const downloads = ['Downloads'];
+      remoteCall.callRemoteTestUtil('selectFile', appId, downloads)
+          .then(result => {
+            chrome.test.assertTrue(!!result, 'selectFile failed');
+            this.next();
+          });
+    },
+    // Open Downloads via file list.
+    function() {
+      const fileListItem = '#file-list .table-row';
+      const key = [fileListItem, 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key).then(this.next);
+    },
+    // Wait for Downloads to load.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      remoteCall.waitForFiles(appId, [ENTRIES.photos.getExpectedRow()])
+          .then(this.next);
+    },
+    // Select photos via file list.
+    function() {
+      const folder = ['photos'];
+      remoteCall.callRemoteTestUtil('selectFile', appId, folder)
+          .then(result => {
+            chrome.test.assertTrue(!!result, 'selectFile failed');
+            this.next();
+          });
+    },
+    // Press Ctrl+Enter for start renaming.
+    function() {
+      const fileListItem = '#file-list .table-row';
+      const key = [fileListItem, 'Enter', true, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key).then(this.next);
+    },
+    // Wait for input for renaming to appear.
+    function(result) {
+      chrome.test.assertTrue(result, 'fakeKeyDown ctrl+Enter failed');
+      // Check: the renaming text input should be shown in the file list.
+      remoteCall.waitForElement(appId, textInput).then(this.next);
+    },
+    // Type new name.
+    function() {
+      remoteCall.callRemoteTestUtil('inputText', appId, [textInput, 'new name'])
+          .then(this.next);
+    },
+    // Send Enter key to the text input.
+    function() {
+      const key = [textInput, 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key).then(this.next);
+    },
+    // Wait for new name to appear on the file list.
+    function(result) {
+      chrome.test.assertTrue(result, 'fakeKeyDown failed');
+      const expectedRows = [['new name', '--', 'Folder', '']];
+      remoteCall
+          .waitForFiles(
+              appId, expectedRows,
+              {ignoreFileSize: true, ignoreLastModifiedTime: true})
+          .then(this.next);
+    },
+    function() {
+      checkIfNoErrorsOccured(this.next);
+    },
+  ]);
+};