Pass in a populated file list to media-app.

At the moment media-app receives a file list with only 1 item in it.
This CL updates the launch code to send over all files in the current
directory if the app was launched via the files app.

This is part of a larger set of CLs to expose more directory methods and
data to the guest frame.

Bug: 996088
Change-Id: I485bbe143fcc953c8cd189ded30d0cf23f337fbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108453Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Zain Afzal <zafzal@google.com>
Cr-Commit-Position: refs/heads/master@{#753464}

chromeos/components/media_app_ui/BUILD.gn

@@ -94,6 +94,11 @@ js_type_check("closure_compile_tests") {
                     "jscomp_error=strictCheckTypes",
                     "jscomp_error=reportUnknownTypes",
                     "language_in=ECMASCRIPT_2018",
+
+                    # TODO(crbug/1048973): Remove these when the mojo bindings
+                    # js is updated to pass a closure compile check.
+                    "hide_warnings_for=mojo/public/js/",
+                    "hide_warnings_for=chromeos/components/media_app_ui/media_app_ui.mojom-lite-for-compile.js",
                   ]
   deps = [
     ":test_driver_api_js",
@@ -129,6 +134,7 @@ js_library("test_driver_js") {
   sources = [ "test/driver.js" ]
   deps = [
     ":test_driver_api_js",
+    "//chromeos/components/media_app_ui/resources/js:launch",
     "//chromeos/components/media_app_ui/resources/js:message_pipe",
     "//ui/webui/resources/js:assert",
   ]

chromeos/components/media_app_ui/resources/js/BUILD.gn

@@ -49,7 +49,7 @@ js_library("launch") {
   deps = [
     ":message_pipe",
     ":message_types",
-    "//chromeos/components/media_app_ui:mojo_bindings_js_library_for_compile",
+    ":mojo_api_bootstrap",
   ]
 }
 

chromeos/components/media_app_ui/resources/js/launch.js

@@ -2,10 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+/** @typedef {{token: number, file: !File, handle: !FileSystemFileHandle}} */
+let FileDescriptor;
+
 /**
  * Array of entries available in the current directory.
  *
- * @type {Array<{file: !File, handle: !FileSystemFileHandle}>}
+ * @type {!Array<!FileDescriptor>}
  */
 const currentFiles = [];
 
@@ -25,9 +28,9 @@ let fileToken = 0;
 
 /**
  * The file currently writable.
- * @type {?FileSystemFileHandle}
+ * @type {?FileDescriptor}
  */
-let currentlyWritableFileHandle = null;
+let currentlyWritableFile = null;
 
 /**
  * Reference to the directory handle that contains the first file in the most
@@ -49,10 +52,10 @@ guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => {
 
 guestMessagePipe.registerHandler(Message.OVERWRITE_FILE, async (message) => {
   const overwrite = /** @type{OverwriteFileMessage} */ (message);
-  if (!currentlyWritableFileHandle || overwrite.token !== fileToken) {
+  if (!currentlyWritableFile || overwrite.token !== fileToken) {
     throw new Error('File not current.');
   }
-  const writer = await currentlyWritableFileHandle.createWritable();
+  const writer = await currentlyWritableFile.handle.createWritable();
   await writer.write(overwrite.blob);
   await writer.truncate(overwrite.blob.size);
   await writer.close();
@@ -62,7 +65,7 @@ guestMessagePipe.registerHandler(Message.OVERWRITE_FILE, async (message) => {
 guestMessagePipe.registerHandler(Message.DELETE_FILE, async (message) => {
   const deleteMsg = /** @type{DeleteFileMessage} */ (message);
 
-  if (!currentlyWritableFileHandle || deleteMsg.token !== fileToken) {
+  if (!currentlyWritableFile || deleteMsg.token !== fileToken) {
     throw new Error('File not current for delete.');
   }
 
@@ -71,7 +74,7 @@ guestMessagePipe.registerHandler(Message.DELETE_FILE, async (message) => {
   }
 
   // Get the name from the file reference. Handles file renames.
-  const currentFilename = (await currentlyWritableFileHandle.getFile()).name;
+  const currentFilename = (await currentlyWritableFile.handle.getFile()).name;
 
   // Check the file to be deleted exists in the directory handle. Prevents
   // deleting the wrong file / deleting a file that doesn't exist (this isn't
@@ -80,7 +83,7 @@ guestMessagePipe.registerHandler(Message.DELETE_FILE, async (message) => {
   const fileHandle = await currentDirectoryHandle.getFile(currentFilename);
 
   const isSameFileHandle =
-      await fileHandle.isSameEntry(currentlyWritableFileHandle);
+      await fileHandle.isSameEntry(currentlyWritableFile.handle);
   if (!isSameFileHandle) {
     return {deleteResult: DeleteResult.FILE_MOVED};
   }
@@ -89,40 +92,44 @@ guestMessagePipe.registerHandler(Message.DELETE_FILE, async (message) => {
   return {deleteResult: DeleteResult.SUCCESS};
 });
 
-/**
- * Loads a file in the guest.
- *
- * @param {?File} file
- * @param {!FileSystemFileHandle} handle
- */
-function loadFile(file, handle) {
-  const token = ++fileToken;
-  currentlyWritableFileHandle = handle;
-  guestMessagePipe.sendMessage(Message.LOAD_FILE, {token, file});
+/** Loads the current file list into the guest. */
+function sendFilesToGuest() {
+  // Before sending to guest ensure writableFileIndex is set to be writable,
+  // also clear the old token.
+  if (currentlyWritableFile) {
+    currentlyWritableFile.token = -1;
+  }
+  currentlyWritableFile = currentFiles[entryIndex];
+  currentlyWritableFile.token = ++fileToken;
+
+  /** @type {!LoadFilesMessage} */
+  const loadFilesMessage = {
+    writableFileIndex: entryIndex,
+    // Handle can't be passed through a message pipe.
+    files: currentFiles.map(fd => ({token: fd.token, file: fd.file}))
+  };
+  guestMessagePipe.sendMessage(Message.LOAD_FILES, loadFilesMessage);
 }
 
 /**
- * Loads a file from a handle received via the fileHandling API.
- *
- * @param {?FileSystemHandle} handle
- * @return {Promise<?File>}
+ * Gets a file from a handle received via the fileHandling API.
+ * @param {?FileSystemHandle} fileSystemHandle
+ * @return {Promise<?{file: !File, handle: !FileSystemFileHandle}>}
  */
-async function loadFileFromHandle(handle) {
-  if (!handle || !handle.isFile) {
+async function getFileFromHandle(fileSystemHandle) {
+  if (!fileSystemHandle || !fileSystemHandle.isFile) {
     return null;
   }
-
-  const fileHandle = /** @type{!FileSystemFileHandle} */ (handle);
-  const file = await fileHandle.getFile();
-  loadFile(file, fileHandle);
-  return file;
+  const handle = /** @type{!FileSystemFileHandle} */ (fileSystemHandle);
+  const file = await handle.getFile();
+  return {file, handle};
 }
 
 /**
  * Changes the working directory and initializes file iteration according to
  * the type of the opened file.
  *
- * @param {FileSystemDirectoryHandle} directory
+ * @param {!FileSystemDirectoryHandle} directory
  * @param {?File} focusFile
  */
 async function setCurrentDirectory(directory, focusFile) {
@@ -131,19 +138,32 @@ async function setCurrentDirectory(directory, focusFile) {
   }
   currentFiles.length = 0;
   for await (const /** !FileSystemHandle */ handle of directory.getEntries()) {
-    if (!handle.isFile) {
+    const asFile = await getFileFromHandle(handle);
+    if (!asFile) {
       continue;
     }
-    const fileHandle = /** @type{FileSystemFileHandle} */ (handle);
-    const file = await fileHandle.getFile();
 
     // Only allow traversal of matching mime types.
-    if (file.type === focusFile.type) {
-      currentFiles.push({file, handle: fileHandle});
+    if (asFile.file.type === focusFile.type) {
+      currentFiles.push({token: -1, file: asFile.file, handle: asFile.handle});
     }
   }
   entryIndex = currentFiles.findIndex(i => i.file.name == focusFile.name);
   currentDirectoryHandle = directory;
+  sendFilesToGuest();
+}
+
+/**
+ * Launch the media app with the files in the provided directory.
+ * @param {!FileSystemDirectoryHandle} directory
+ * @param {?FileSystemHandle} initialFileEntry
+ */
+async function launchWithDirectory(directory, initialFileEntry) {
+  const asFile = await getFileFromHandle(initialFileEntry);
+  await setCurrentDirectory(directory, asFile.file);
+
+  // Load currentFiles into the guest.
+  sendFilesToGuest();
 }
 
 /**
@@ -159,8 +179,8 @@ async function advance(direction) {
   if (entryIndex < 0) {
     entryIndex += currentFiles.length;
   }
-  const entry = currentFiles[entryIndex];
-  loadFile(entry.file, entry.handle);
+
+  sendFilesToGuest();
 }
 
 document.getElementById('prev-container')
@@ -181,9 +201,8 @@ window.addEventListener('load', () => {
       console.error('Invalid launch: files[0] is not a directory: ', params);
       return;
     }
-
-    const directory = /** @type{FileSystemDirectoryHandle} */ (params.files[0]);
-    loadFileFromHandle(params.files[1])
-        .then(file => setCurrentDirectory(directory, file));
+    const directory =
+        /** @type{!FileSystemDirectoryHandle} */ (params.files[0]);
+    launchWithDirectory(directory, params.files[1]);
   });
 });

chromeos/components/media_app_ui/resources/js/message_types.js

@@ -13,7 +13,7 @@
  */
 const Message = {
   DELETE_FILE: 'delete-file',
-  LOAD_FILE: 'load-file',
+  LOAD_FILES: 'load-files',
   OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog',
   OVERWRITE_FILE: 'overwrite-file',
 };
@@ -33,8 +33,13 @@ let DeleteFileMessage;
 /** @typedef {{ deleteResult: DeleteResult }}  */
 let DeleteFileResponse;
 
-/** @typedef {{token: number, file: !File}} */
-let OpenFileMessage;
+/**
+ * @typedef {{
+ *    writableFileIndex: number,
+ *    files: !Array<{token: number, file: !File}>
+ * }}
+ */
+let LoadFilesMessage;
 
 /** @typedef {{token: number, blob: !Blob}} */
 let OverwriteFileMessage;

chromeos/components/media_app_ui/resources/js/receiver.js

@@ -5,19 +5,6 @@
 /** A pipe through which we can send messages to the parent frame. */
 const parentMessagePipe = new MessagePipe('chrome://media-app', window.parent);
 
-/** @implements mediaApp.AbstractFileList */
-class SingleArrayBufferFileList {
-  /** @param {!mediaApp.AbstractFile} file */
-  constructor(file) {
-    this.file = file;
-    this.length = 1;
-  }
-  /** @override */
-  item(index) {
-    return index === 0 ? this.file : null;
-  }
-}
-
 /**
  * A file received from the privileged context.
  * @implements {mediaApp.AbstractFile}
@@ -72,9 +59,49 @@ class ReceivedFile {
   }
 }
 
-parentMessagePipe.registerHandler(Message.LOAD_FILE, (message) => {
-  const fileMessage = /** @type{!OpenFileMessage} */ (message);
-  loadFile(fileMessage.token, fileMessage.file);
+/**
+ * A file list consisting of all files received from the parent. Exposes the
+ * currently writable file and all other readable files in the current
+ * directory.
+ * @implements mediaApp.AbstractFileList
+ */
+class ReceivedFileList {
+  /** @param {!LoadFilesMessage} filesMessage */
+  constructor(filesMessage) {
+    // We make sure the 0th item in the list is the writable one so we
+    // don't break older versions of the media app which uses item(0) instead
+    // of getCurrentlyWritable()
+    // TODO(b/151880563): remove this.
+    let {files, writableFileIndex} = filesMessage;
+    while (writableFileIndex > 0) {
+      files.push(files.shift());
+      writableFileIndex--;
+    }
+
+    this.length = files.length;
+    /** @type {!Array<!ReceivedFile>} */
+    this.files = files.map(f => new ReceivedFile(f.file, f.token));
+    /** @type {number} */
+    this.writableFileIndex = 0;
+  }
+
+  /** @override */
+  item(index) {
+    return this.files[index] || null;
+  }
+
+  /**
+   * Returns the file which is currently writable or null if there isn't one.
+   * @return {?mediaApp.AbstractFile}
+   */
+  getCurrentlyWritable() {
+    return this.item(this.writableFileIndex);
+  }
+}
+
+parentMessagePipe.registerHandler(Message.LOAD_FILES, (message) => {
+  const filesMessage = /** @type{!LoadFilesMessage} */ (message);
+  loadFiles(new ReceivedFileList(filesMessage));
 })
 
 /**
@@ -101,22 +128,16 @@ function getApp() {
 }
 
 /**
- * Loads files associated with a message received from the host.
- * @param {number} token
- * @param {!File} file
- * @return {!Promise<!ReceivedFile>} The received file (for testing).
+ * Loads a file list into the media app.
+ * @param {!ReceivedFileList} fileList
  */
-async function loadFile(token, file) {
-  const receivedFile = new ReceivedFile(file, token);
-  const fileList = new SingleArrayBufferFileList(receivedFile);
-
+async function loadFiles(fileList) {
   const app = getApp();
   if (app) {
     await app.loadFiles(fileList);
   } else {
     window.customLaunchData = {files: fileList};
   }
-  return receivedFile;
 }
 
 /**

chromeos/components/media_app_ui/test/driver.js

@@ -158,3 +158,15 @@ function createMockTestDirectory() {
   directory.addFileHandleForTest(new FakeFileSystemFileHandle());
   return directory;
 }
+
+/**
+ * Helper to send a single file to the guest.
+ * @param {!File} file
+ * @param {!FileSystemFileHandle} handle
+ */
+function loadFile(file, handle) {
+  currentFiles.length = 0;
+  currentFiles.push({token: -1, file, handle});
+  entryIndex = 0;
+  sendFilesToGuest();
+}

chromeos/components/media_app_ui/test/driver_api.js

@@ -16,6 +16,3 @@ var TestMessageResponseData;
  * }}
  */
 var TestMessageQueryData;
-
-/** @type {MessagePipe} */
-var guestMessagePipe;

chromeos/components/media_app_ui/test/guest_query_receiver.js

@@ -3,10 +3,10 @@
 // found in the LICENSE file.
 
 /**
- * The last file loaded into the guest, updated via a spy on loadFile().
- * @type {?Promise<!ReceivedFile>}
+ * The last file list loaded into the guest, updated via a spy on loadFiles().
+ * @type {?ReceivedFileList}
  */
-let lastReceivedFile = null;
+let lastReceivedFileList = null;
 
 /**
  * Acts on received TestMessageQueryData.
@@ -32,13 +32,12 @@ async function runTestQuery(data) {
     }
   } else if (data.overwriteLastFile) {
     const testBlob = new Blob([data.overwriteLastFile]);
-    const ensureLoaded = await lastReceivedFile;
-    await ensureLoaded.overwriteOriginal(testBlob);
+    await lastReceivedFileList.item(0).overwriteOriginal(testBlob);
     result = 'overwriteOriginal resolved';
   } else if (data.deleteLastFile) {
     try {
-      const ensureLoaded = await lastReceivedFile;
-      const deleteResult = await ensureLoaded.deleteOriginalFile();
+      const deleteResult =
+          await lastReceivedFileList.item(0).deleteOriginalFile();
       if (deleteResult === DeleteResult.FILE_MOVED) {
         result = 'deleteOriginalFile resolved file moved';
       } else {
@@ -65,15 +64,15 @@ function installTestHandlers() {
     throw Error('This is an error');
   });
 
-  // Log errors, rather than send them to console.error.
+  // Log errors, rather than sending them to console.error.
   parentMessagePipe.logClientError = error =>
       console.log(JSON.stringify(error));
 
   // Install spies.
-  const realLoadFile = loadFile;
-  loadFile = async (/** number */ token, /** !File */ file) => {
-    lastReceivedFile = realLoadFile(token, file);
-    return lastReceivedFile;
+  const realLoadFiles = loadFiles;
+  loadFiles = async (/** !ReceivedFileList */ fileList) => {
+    lastReceivedFileList = fileList;
+    realLoadFiles(fileList);
   }
 }
 

chromeos/components/media_app_ui/test/media_app_ui_browsertest.js

@@ -200,7 +200,7 @@ TEST_F('MediaAppUIBrowserTest', 'DeleteOriginalIPC', async () => {
   // Simulate steps taken to load a file via a launch event.
   const firstFile = directory.files[0];
   loadFile(await createTestImageFile(), firstFile);
-  setCurrentDirectory(directory, firstFile);
+  currentDirectoryHandle = directory;
   let testResponse;
 
   // Nothing should be deleted initially.
@@ -217,7 +217,7 @@ TEST_F('MediaAppUIBrowserTest', 'DeleteOriginalIPC', async () => {
   // File removed from `DirectoryHandle` internal state.
   assertEquals(directory.files.length, 0);
 
-  // Even though the name is the same, the new file shouldn't shouldn't
+  // Even though the name is the same, the new file shouldn't
   // be deleted as it has a different `FileHandle` reference.
   directory.addFileHandleForTest(new FakeFileSystemFileHandle());