Commit 596ad176 authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

Handle multi selection launches in chrome://media-app

The launch handler follows the approach of `setCurrentDirectory`, but
it is much simpler. There's no need to obtain the File in order to get
the MIME type, since we assume all selected files are "related". (The
Files app does not offer the media app as a viable launch task if all
selected files do not match the file handler type declaration).

Update and consolidate the test helpers used to trigger launch. Expose
the "real" launchConsumer and use it for existing tests as well for
extra test coverage.

Bug: b/159107125
Change-Id: I2c0239b58ba9ae0dfe472b600af086a941c828ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247922Reviewed-by: default avatardstockwell <dstockwell@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779651}
parent 2adbb514
...@@ -506,6 +506,28 @@ async function launchWithDirectory(directory, handle) { ...@@ -506,6 +506,28 @@ async function launchWithDirectory(directory, handle) {
await sendFilesToGuest(); await sendFilesToGuest();
} }
/**
* Launch the media app with the selected files.
* @param {!FileSystemDirectoryHandle} directory
* @param {!Array<?FileSystemHandle>} handles
*/
async function launchWithMultipleSelection(directory, handles) {
currentFiles.length = 0;
for (const handle of handles) {
if (handle && handle.isFile) {
const fileHandle = /** @type{!FileSystemFileHandle} */ (handle);
currentFiles.push({
token: generateToken(fileHandle),
file: null, // Just let sendSnapshotToGuest() "refresh" it.
handle: fileHandle,
});
}
}
entryIndex = currentFiles.length > 0 ? 0 : -1;
currentDirectoryHandle = directory;
await sendFilesToGuest();
}
/** /**
* Advance to another file. * Advance to another file.
* *
...@@ -524,6 +546,40 @@ async function advance(direction) { ...@@ -524,6 +546,40 @@ async function advance(direction) {
await sendFilesToGuest(); await sendFilesToGuest();
} }
/**
* The launchQueue consumer. This returns a promise to help tests, but the file
* handling API will ignore it.
* @param {LaunchParams} params
* @return {!Promise<undefined>}
*/
function launchConsumer(params) {
// The MediaApp sets `include_launch_directory = true` in its SystemAppInfo
// struct compiled into Chrome. That means files[0] is guaranteed to be a
// directory, with remaining launch files following it. Validate that this is
// true and abort the launch if is is not.
if (!params || !params.files || params.files.length < 2) {
console.error('Invalid launch (missing files): ', params);
return Promise.resolve();
}
if (!assertCast(params.files[0]).isDirectory) {
console.error('Invalid launch: files[0] is not a directory: ', params);
return Promise.resolve();
}
const directory =
/** @type{!FileSystemDirectoryHandle} */ (params.files[0]);
// With a single file selected, launch with all files in the directory as
// navigation candidates. Otherwise, launch with all selected files (except
// the launch directory itself) as navigation candidates.
if (params.files.length === 2) {
const focusEntry = assertCast(params.files[1]);
return launchWithDirectory(directory, focusEntry);
} else {
return launchWithMultipleSelection(directory, params.files.slice(1));
}
}
/** /**
* Installs the handler for launch files, if window.launchQueue is available. * Installs the handler for launch files, if window.launchQueue is available.
*/ */
...@@ -532,21 +588,7 @@ function installLaunchHandler() { ...@@ -532,21 +588,7 @@ function installLaunchHandler() {
console.error('FileHandling API missing.'); console.error('FileHandling API missing.');
return; return;
} }
window.launchQueue.setConsumer(params => { window.launchQueue.setConsumer(launchConsumer);
if (!params || !params.files || params.files.length < 2) {
console.error('Invalid launch (missing files): ', params);
return;
}
if (!assertCast(params.files[0]).isDirectory) {
console.error('Invalid launch: files[0] is not a directory: ', params);
return;
}
const directory =
/** @type{!FileSystemDirectoryHandle} */ (params.files[0]);
const focusEntry = assertCast(params.files[1]);
launchWithDirectory(directory, focusEntry);
});
} }
installLaunchHandler(); installLaunchHandler();
......
...@@ -293,17 +293,36 @@ async function loadMultipleFiles(files) { ...@@ -293,17 +293,36 @@ async function loadMultipleFiles(files) {
} }
/** /**
* Helper to "launch" with the given handles, by populating a fake directory, * Creates a mock LaunchParams object from the provided `files`.
* then launching with the first file in the provided array as the focus file. * @param {!Array<FileSystemHandle>} files
* @param {!Array<!FakeFileSystemFileHandle>} handles * @return {LaunchParams}
*/
function handlesToLaunchParams(files) {
return /** @type{LaunchParams} */ ({files});
}
/**
* Helper to "launch" with the given `directoryContents`. Populates a fake
* directory containing those handles, then launches the app. The focus file is
* either the first file in `multiSelectionFiles`, or the first directory entry.
* @param {!Array<!FakeFileSystemFileHandle>} directoryContents
* @param {!Array<!FakeFileSystemFileHandle>=} multiSelectionFiles If provided,
* holds additional files selected in the files app at launch time.
* @return {!Promise<FakeFileSystemDirectoryHandle>} * @return {!Promise<FakeFileSystemDirectoryHandle>}
*/ */
async function launchWithHandles(handles) { async function launchWithHandles(directoryContents, multiSelectionFiles = []) {
/** @type {?FakeFileSystemFileHandle} */
let focusFile = multiSelectionFiles[0];
if (!focusFile) {
focusFile = directoryContents[0];
}
multiSelectionFiles = multiSelectionFiles.slice(1);
const directory = new FakeFileSystemDirectoryHandle(); const directory = new FakeFileSystemDirectoryHandle();
for (const handle of handles) { for (const handle of directoryContents) {
directory.addFileHandleForTest(handle); directory.addFileHandleForTest(handle);
} }
await launchWithDirectory(directory, handles[0]); const files = [directory, focusFile, ...multiSelectionFiles];
await launchConsumer(handlesToLaunchParams(files));
return directory; return directory;
} }
...@@ -320,10 +339,14 @@ function fileToFileHandle(file) { ...@@ -320,10 +339,14 @@ function fileToFileHandle(file) {
/** /**
* Helper to invoke launchWithHandles after wrapping `files` in fake handles. * Helper to invoke launchWithHandles after wrapping `files` in fake handles.
* @param {!Array<!File>} files * @param {!Array<!File>} files
* @param {!Array<!number>=} selectedIndexes
* @return {!Promise<FakeFileSystemDirectoryHandle>} * @return {!Promise<FakeFileSystemDirectoryHandle>}
*/ */
async function launchWithFiles(files) { async function launchWithFiles(files, selectedIndexes = []) {
return launchWithHandles(files.map(fileToFileHandle)); const fileHandles = files.map(fileToFileHandle);
const selection =
selectedIndexes.map((/** @type {number} */ i) => fileHandles[i]);
return launchWithHandles(fileHandles, selection);
} }
/** /**
...@@ -345,9 +368,18 @@ function createNamedError(name, msg) { ...@@ -345,9 +368,18 @@ function createNamedError(name, msg) {
* @param {?string} testCase * @param {?string} testCase
*/ */
function assertFilesToBe(expectedFiles, testCase) { function assertFilesToBe(expectedFiles, testCase) {
return assertFilenamesToBe(expectedFiles.map(f => f.name).join(), testCase);
}
/**
* Checks that the `currentFiles` array maintained by launch.js has the same
* sequence of filenames as `expectedFilenames`.
* @param {string} expectedFilenames
* @param {?string} testCase
*/
function assertFilenamesToBe(expectedFilenames, testCase) {
// Use filenames as an approximation of file uniqueness. // Use filenames as an approximation of file uniqueness.
const currentFilenames = currentFiles.map(d => d.handle.name).join(); const currentFilenames = currentFiles.map(d => d.handle.name).join();
const expectedFilenames = expectedFiles.map(f => f.name).join();
chai.assert.equal( chai.assert.equal(
currentFilenames, expectedFilenames, currentFilenames, expectedFilenames,
`Expected '${expectedFilenames}' but got '${currentFilenames}'` + `Expected '${expectedFilenames}' but got '${currentFilenames}'` +
......
...@@ -172,6 +172,19 @@ TEST_F('MediaAppUIBrowserTest', 'MultipleFileHaveTokens', async () => { ...@@ -172,6 +172,19 @@ TEST_F('MediaAppUIBrowserTest', 'MultipleFileHaveTokens', async () => {
testDone(); testDone();
}); });
// Tests that a launch with multiple files selected in the files app loads only
// the files selected.
TEST_F('MediaAppUIBrowserTest', 'MultipleSelectionLaunch', async () => {
const filePromise = name => createTestImageFile(1, 1, `${name}.png`);
const directoryContents = await Promise.all([0, 1, 2, 3].map(filePromise));
const selectedIndexes = [1, 3];
const directory = await launchWithFiles(directoryContents, selectedIndexes);
assertFilenamesToBe('1.png,3.png');
testDone();
});
// Tests that we show error UX when trying to launch an unopenable file. // Tests that we show error UX when trying to launch an unopenable file.
TEST_F('MediaAppUIBrowserTest', 'LaunchUnopenableFile', async () => { TEST_F('MediaAppUIBrowserTest', 'LaunchUnopenableFile', async () => {
const mockFileHandle = const mockFileHandle =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment