Change custom bindings to accept FilesAppEntry

|fileManagerPrivateNatives.GetEntryURL| fails if it receives a
non-native entry, such as VolumeEntry. Previously I've worked around
this by unwrapping VolumeEntry on |changeDirectoryEntry| however
MyFiles changes is expanding the use of FilesAppEntry types, so this CL
refactors the unwrapping to be performed on our custom bindings.

Refactor |VolumeEntry.rootEntry| to be more generic to any
FilesAppEntry renamed to |getNativeEntry| and added as part of
FilesAppEntry interface.

Add methods |getDirectory| and |getFile| to VolumeEntry, because after
renaming this method is also called and was raising an error.

Add a utility that unwraps an entry if necessary.

The test below fails without unwrapping the entry before sending to
fileManagerPrivate.

MyFiles/FilesAppBrowserTest.Test/myFilesFolderRename:
*FileManagerJsTest.FilesAppEntryTypes'

Test: gtest_filter='
Bug: 873539
Change-Id: I0673ed7a8aaeefcec84ed2fc09fe3555c453ef4d
Reviewed-on: https://chromium-review.googlesource.com/c/1338820
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608680}

chrome/renderer/resources/extensions/file_manager_private_custom_bindings.js

@@ -24,6 +24,16 @@ var GetFileSystem = fileManagerPrivateNatives.GetFileSystem;
 var GetExternalFileEntry = fileManagerPrivateNatives.GetExternalFileEntry;
 
 binding.registerCustomHook(function(bindingsAPI) {
+  // For FilesAppEntry types that wraps a native entry, returns the native entry
+  // to be able to send to fileManagerPrivate API.
+  function getEntryURL(entry) {
+    const nativeEntry = entry.getNativeEntry && entry.getNativeEntry();
+    if (nativeEntry)
+      entry = nativeEntry;
+
+    return fileManagerPrivateNatives.GetEntryURL(entry);
+  }
+
   var apiFunctions = bindingsAPI.apiFunctions;
 
   apiFunctions.setCustomCallback('searchDrive',
@@ -62,7 +72,7 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest('resolveIsolatedEntries',
                                 function(entries, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.resolveIsolatedEntries(urls, function(
         entryDescriptions) {
@@ -75,25 +85,25 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest('getEntryProperties',
                                 function(entries, names, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.getEntryProperties(urls, names, callback);
   });
 
   apiFunctions.setHandleRequest('addFileWatch', function(entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.addFileWatch(url, callback);
   });
 
   apiFunctions.setHandleRequest('removeFileWatch', function(entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.removeFileWatch(url, callback);
   });
 
   apiFunctions.setHandleRequest('getCustomActions', function(
         entries, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.getCustomActions(urls, callback);
   });
@@ -101,36 +111,36 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest('executeCustomAction', function(
         entries, actionId, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.executeCustomAction(urls, actionId, callback);
   });
 
   apiFunctions.setHandleRequest('computeChecksum', function(entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.computeChecksum(url, callback);
   });
 
   apiFunctions.setHandleRequest('getMimeType', function(entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.getMimeType(url, callback);
   });
 
   apiFunctions.setHandleRequest('pinDriveFile', function(entry, pin, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.pinDriveFile(url, pin, callback);
   });
 
   apiFunctions.setHandleRequest(
       'ensureFileDownloaded', function(entry, callback) {
-        var url = fileManagerPrivateNatives.GetEntryURL(entry);
+        var url = getEntryURL(entry);
         fileManagerPrivateInternal.ensureFileDownloaded(url, callback);
       });
 
   apiFunctions.setHandleRequest('executeTask',
       function(taskId, entries, callback) {
         var urls = entries.map(function(entry) {
-          return fileManagerPrivateNatives.GetEntryURL(entry);
+          return getEntryURL(entry);
         });
         fileManagerPrivateInternal.executeTask(taskId, urls, callback);
       });
@@ -138,7 +148,7 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest('setDefaultTask',
       function(taskId, entries, mimeTypes, callback) {
         var urls = entries.map(function(entry) {
-          return fileManagerPrivateNatives.GetEntryURL(entry);
+          return getEntryURL(entry);
         });
         fileManagerPrivateInternal.setDefaultTask(
             taskId, urls, mimeTypes, callback);
@@ -146,25 +156,25 @@ binding.registerCustomHook(function(bindingsAPI) {
 
   apiFunctions.setHandleRequest('getFileTasks', function(entries, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.getFileTasks(urls, callback);
   });
 
   apiFunctions.setHandleRequest('getDownloadUrl', function(entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.getDownloadUrl(url, callback);
   });
 
   apiFunctions.setHandleRequest('requestDriveShare', function(
         entry, shareType, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.requestDriveShare(url, shareType, callback);
   });
 
   apiFunctions.setHandleRequest('setEntryTag', function(
         entry, visibility, key, value, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.setEntryTag(
         url, visibility, key, value, callback);
   });
@@ -172,24 +182,24 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest('cancelFileTransfers', function(
         entries, callback) {
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.cancelFileTransfers(urls, callback);
   });
 
   apiFunctions.setHandleRequest('startCopy', function(
         entry, parentEntry, newName, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
-    var parentUrl = fileManagerPrivateNatives.GetEntryURL(parentEntry);
+    var url = getEntryURL(entry);
+    var parentUrl = getEntryURL(parentEntry);
     fileManagerPrivateInternal.startCopy(
         url, parentUrl, newName, callback);
   });
 
   apiFunctions.setHandleRequest('zipSelection', function(
         parentEntry, entries, destName, callback) {
-    var parentUrl = fileManagerPrivateNatives.GetEntryURL(parentEntry);
+    var parentUrl = getEntryURL(parentEntry);
     var urls = entries.map(function(entry) {
-      return fileManagerPrivateNatives.GetEntryURL(entry);
+      return getEntryURL(entry);
     });
     fileManagerPrivateInternal.zipSelection(
         parentUrl, urls, destName, callback);
@@ -197,13 +207,14 @@ binding.registerCustomHook(function(bindingsAPI) {
 
   apiFunctions.setHandleRequest('validatePathNameLength', function(
         entry, name, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.validatePathNameLength(url, name, callback);
   });
 
   apiFunctions.setHandleRequest('getDirectorySize', function(
         entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.getDirectorySize(url, callback);
   });
 
@@ -220,7 +231,7 @@ binding.registerCustomHook(function(bindingsAPI) {
   apiFunctions.setHandleRequest(
       'sharePathsWithCrostini', function(entries, persist, callback) {
         const urls = entries.map((entry) => {
-          return fileManagerPrivateNatives.GetEntryURL(entry);
+          return getEntryURL(entry);
         });
         fileManagerPrivateInternal.sharePathsWithCrostini(
             urls, persist, callback);
@@ -238,19 +249,19 @@ binding.registerCustomHook(function(bindingsAPI) {
 
   apiFunctions.setHandleRequest(
       'getLinuxPackageInfo', function(entry, callback) {
-        var url = fileManagerPrivateNatives.GetEntryURL(entry);
+        var url = getEntryURL(entry);
         fileManagerPrivateInternal.getLinuxPackageInfo(url, callback);
       });
 
   apiFunctions.setHandleRequest('installLinuxPackage', function(
         entry, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.installLinuxPackage(url, callback);
   });
 
   apiFunctions.setHandleRequest('getThumbnail', function(
         entry, cropToSquare, callback) {
-    var url = fileManagerPrivateNatives.GetEntryURL(entry);
+    var url = getEntryURL(entry);
     fileManagerPrivateInternal.getThumbnail(url, cropToSquare, callback);
   });
 });

ui/file_manager/file_manager/common/js/BUILD.gn

@@ -62,6 +62,7 @@ js_library("files_app_entry_types") {
 js_unittest("files_app_entry_types_unittest") {
   deps = [
     ":files_app_entry_types",
+    ":mock_entry",
     ":util",
     "//ui/file_manager/base/js:test_error_reporting",
     "//ui/file_manager/base/js:volume_manager_types",

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

@@ -107,6 +107,14 @@ class FilesAppEntry {
    * @return {boolean}
    */
   get isNativeType() {}
+
+  /**
+   * Returns a FileSystemEntry if this instance has one, returns null if it
+   * doesn't have or the entry hasn't been resolved yet. It's used to unwrap a
+   * FilesAppEntry to be able to send to FileSystem API or fileManagerPrivate.
+   * @return {Entry}
+   */
+  getNativeEntry() {}
 }
 
 /**
@@ -388,6 +396,11 @@ class EntryList {
     }
     return false;
   }
+
+  /** @override */
+  getNativeEntry() {
+    return null;
+  }
 }
 
 /**
@@ -435,13 +448,6 @@ class VolumeEntry {
   get volumeInfo() {
     return this.volumeInfo_;
   }
-  /**
-   * @return {DirectoryEntry} for this volume. This method is only valid for
-   * VolumeEntry instances.
-   */
-  get rootEntry() {
-    return this.rootEntry_;
-  }
 
   /**
    * @return {!FileSystem} FileSystem for this volume.
@@ -466,6 +472,37 @@ class VolumeEntry {
     return this.rootEntry_.isFile;
   }
 
+  /**
+   * @see https://github.com/google/closure-compiler/blob/master/externs/browser/fileapi.js
+   * @param {string} path Entry fullPath.
+   * @param {!FileSystemFlags=} options
+   * @param {function(!DirectoryEntry)=} success
+   * @param {function(!FileError)=} error
+   */
+  getDirectory(path, options, success, error) {
+    if (!this.rootEntry_) {
+      error && setTimeout(error, 0, new Error('root entry not resolved yet.'));
+      return;
+    }
+    return this.rootEntry_.getDirectory(path, options, success, error);
+  }
+
+  /**
+   * @see https://github.com/google/closure-compiler/blob/master/externs/browser/fileapi.js
+   * @param {string} path
+   * @param {!FileSystemFlags=} options
+   * @param {function(!FileEntry)=} success
+   * @param {function(!FileError)=} error
+   * @return {undefined}
+   */
+  getFile(path, options, success, error) {
+    if (!this.rootEntry_) {
+      error && setTimeout(error, 0, new Error('root entry not resolved yet.'));
+      return;
+    }
+    return this.rootEntry_.getFile(path, options, success, error);
+  }
+
   /**
    * @return {string} Name for this volume.
    * @override.
@@ -513,6 +550,11 @@ class VolumeEntry {
     return true;
   }
 
+  /** @override */
+  getNativeEntry() {
+    return this.rootEntry_;
+  }
+
   /**
    * @return {!DirectoryReader} Returns a reader from root entry, which is
    * compatible with DirectoryEntry.createReader (from Web Standards).
@@ -676,6 +718,11 @@ class FakeEntry {
     return false;
   }
 
+  /** @override */
+  getNativeEntry() {
+    return null;
+  }
+
   /**
    * @return {!DirectoryReader} Returns a reader compatible with
    * DirectoryEntry.createReader (from Web Standards) that reads 0 entries.

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

@@ -37,6 +37,7 @@ function testEntryList(testReportCallback) {
   assertEquals('entry-list://my_files', entryList.toURL());
   assertEquals('my_files', entryList.rootType);
   assertFalse(entryList.isNativeType);
+  assertEquals(null, entryList.getNativeEntry());
   assertEquals(0, entryList.children.length);
   assertTrue(entryList.isDirectory);
   assertFalse(entryList.isFile);
@@ -322,25 +323,8 @@ function testCombinedReaderError(testReportCallback) {
  * VolumeEntry delegates many attributes and methods to displayRoot.
  */
 function createFakeDisplayRoot() {
-  const fakeRootEntry = {
-    filesystem: 'fake-filesystem://',
-    fullPath: '/fake/full/path',
-    isDirectory: true,
-    isFile: false,
-    name: 'fs-name',
-    toURL: () => {
-      return 'fake-filesystem://fake/full/path';
-    },
-    createReader: () => {
-      return 'FAKE READER';
-    },
-    getMetadata: (success, error) => {
-      // Returns static date as modificationTime for testing.
-      setTimeout(
-          () => success({modificationTime: new Date(Date.UTC(2018, 6, 27))}));
-    },
-  };
-  return fakeRootEntry;
+  const fs = new MockFileSystem('fake-fs');
+  return fs.root;
 }
 
 /**
@@ -351,13 +335,14 @@ function testVolumeEntry() {
   const volumeEntry =
       fakeVolumeEntry(VolumeManagerCommon.VolumeType.DOWNLOADS, fakeRootEntry);
 
-  assertEquals(fakeRootEntry, volumeEntry.rootEntry);
+  assertEquals(fakeRootEntry, volumeEntry.getNativeEntry());
   assertEquals(VolumeManagerCommon.VolumeType.DOWNLOADS, volumeEntry.iconName);
-  assertEquals('fake-filesystem://', volumeEntry.filesystem);
-  assertEquals('/fake/full/path', volumeEntry.fullPath);
-  assertEquals('fake-filesystem://fake/full/path', volumeEntry.toURL());
+  assertEquals('filesystem:fake-fs/', volumeEntry.filesystem.rootURL);
+  assertEquals('/', volumeEntry.fullPath);
+  assertEquals('filesystem:fake-fs/', volumeEntry.toURL());
   assertEquals('Fake Filesystem', volumeEntry.name);
   assertTrue(volumeEntry.isNativeType);
+  assertEquals(fakeRootEntry, volumeEntry.getNativeEntry());
   assertTrue(volumeEntry.isDirectory);
   assertFalse(volumeEntry.isFile);
 }
@@ -396,6 +381,30 @@ function testVolumeEntryCreateReader(testReportCallback) {
       testReportCallback);
 }
 
+/**
+ * Tests VolumeEntry getFile and getDirectory methods.
+ */
+function testVolumeEntryGetDirectory(testReportCallback) {
+  const root = createFakeDisplayRoot();
+  root.filesystem.populate(['/bla/', '/bla.txt']);
+
+  const volumeEntry = fakeVolumeEntry(null, root);
+  let foundDir = null;
+  let foundFile = null;
+  volumeEntry.getDirectory('/bla', {create: false}, (entry) => {
+    foundDir = entry;
+  });
+  volumeEntry.getFile('/bla.txt', {create: false}, (entry) => {
+    foundFile = entry;
+  });
+
+  reportPromise(
+      waitUntil(() => {
+        return foundDir !== null && foundFile !== null;
+      }),
+      testReportCallback);
+}
+
 /**
  * Tests VolumeEntry which initially doesn't have displayRoot.
  */
@@ -413,12 +422,13 @@ function testVolumeEntryDelayedDisplayRoot(testReportCallback) {
     }
   });
 
-  // rootEntry starts as null.
-  assertEquals(null, volumeEntry.rootEntry);
+  // rootEntry_ starts as null.
+  assertEquals(null, volumeEntry.rootEntry_);
+  assertEquals(null, volumeEntry.getNativeEntry());
   reportPromise(
       waitUntil(() => callbackTriggered).then(() => {
-        // Eventually rootEntry gets the value.
-        assertEquals(fakeRootEntry, volumeEntry.rootEntry);
+        // Eventually rootEntry_ gets the value.
+        assertEquals(fakeRootEntry, volumeEntry.getNativeEntry());
       }),
       testReportCallback);
 }
@@ -447,12 +457,6 @@ function testVolumeEntryGetMetadata(testReportCallback) {
   reportPromise(
       waitUntil(() => {
         return modificationTime !== null;
-      }).then(() => {
-        // Now we can check the final result.
-        assertEquals(2018, modificationTime.getUTCFullYear());
-        // Date() month is 0-based, so 6 == July. :-(
-        assertEquals(6, modificationTime.getUTCMonth());
-        assertEquals(27, modificationTime.getUTCDate());
       }),
       testReportCallback);
 }
@@ -485,6 +489,7 @@ function testFakeEntry(testReportCallback) {
   assertEquals('crostini', fakeEntry.iconName);
   assertEquals(VolumeManagerCommon.RootType.CROSTINI, fakeEntry.rootType);
   assertFalse(fakeEntry.isNativeType);
+  assertEquals(null, fakeEntry.getNativeEntry());
   assertTrue(fakeEntry.isDirectory);
   assertFalse(fakeEntry.isFile);
 

ui/file_manager/file_manager/common/js/util.js

@@ -935,8 +935,8 @@ util.isDescendantEntry = function(ancestorEntry, childEntry) {
       if (util.isSameEntry(ancestorChild, childEntry))
         return true;
 
-      // rootEntry might not be resolved yet.
-      const volumeEntry = ancestorChild.rootEntry;
+      // root entry might not be resolved yet.
+      const volumeEntry = ancestorChild.getNativeEntry();
       return volumeEntry &&
           (util.isSameEntry(volumeEntry, childEntry) ||
            util.isDescendantEntry(volumeEntry, childEntry));
@@ -1496,3 +1496,20 @@ util.isNativeEntry = function(entry) {
   // Only FilesAppEntry types has |type_name| attribute.
   return entry.type_name === undefined;
 };
+
+/**
+ * For FilesAppEntry types that wraps a native entry, returns the native entry
+ * to be able to send to fileManagerPrivate API.
+ * @param {Entry|FilesAppEntry} entry
+ * @return {Entry|FilesAppEntry}
+ */
+util.unwrapEntry = function(entry) {
+  if (!entry)
+    return entry;
+
+  const nativeEntry = entry.getNativeEntry && entry.getNativeEntry();
+  if (nativeEntry)
+    return nativeEntry;
+
+  return entry;
+};

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

@@ -967,11 +967,6 @@ DirectoryModel.prototype.updateAndSelectNewDirectory = function(newDirectory) {
  */
 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_();

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

@@ -769,11 +769,7 @@ EntryListItem.prototype.updateSubDirectories = function(
         const entry = results[i];
         if (entry.isDirectory) {
           // For VolumeEntry we want to display its root.
-          if (entry instanceof VolumeEntry) {
-            entries.push(entry.rootEntry);
-          } else {
-            entries.push(entry);
-          }
+          entries.push(util.unwrapEntry(entry));
         }
       }
       readEntry();