Commit c7067b96 authored by David's avatar David Committed by Commit Bot

Enable fast file loads for chrome://media-app.

This cl changes the way we load files such that
1. we load a single file, after this users can interact with the app.
2. we asynchronously load every other related file in that directory,
will update the app when it is done.

Notably:
* adds a way to register observers to AbstractFileList
* adds a way to add files and notify observers to ReceivedFileList
* adds IPC to 'load-extra-files'
* adds `processOtherFilesInDirectory` & `globalLaunchNumber` which
keeps loads in sync

Existing tests provide a lot of coverage for the new codepaths, however
there are edge cases such as loading the first file then doing one of
the following
1. invoking launchable IPC i.e. rename
2. invoking non launchable IPC i.e. delete / save
3. start an edit (or another action) changing the UI

This cl adds tests for 1 & 2.
in the case of 1, we have a token `globalLaunchNumber` to make sure
only the most recent load is fulfilled.
in the case of 2, we load files as normal as the previous focus file is
now removed from the file system.

3. is handled internally in cl/317047655

Bug: b/158043802, 996088
Change-Id: I23e05efa1ffb9c720a116f63e06ca8565dcc7539
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249266
Commit-Queue: David Lei <dlei@google.com>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780696}
parent c152bbd2
...@@ -137,6 +137,20 @@ content::EvalJsResult WaitForImageAlt(content::WebContents* web_ui, ...@@ -137,6 +137,20 @@ content::EvalJsResult WaitForImageAlt(content::WebContents* web_ui,
web_ui, base::ReplaceStringPlaceholders(kScript, {alt}, nullptr)); web_ui, base::ReplaceStringPlaceholders(kScript, {alt}, nullptr));
} }
// Waits for the "shownav" attribute to show up in the MediaApp's current
// handler. Also checks the panel isn't open indicating an edit is not in
// progress. This prevents trying to traverse a directory before other files are
// available / while editing.
content::EvalJsResult WaitForNavigable(content::WebContents* web_ui) {
constexpr char kScript[] = R"(
(async () => {
await waitForNode(':not([panelopen])[shownav]');
})();
)";
return MediaAppUiBrowserTest::EvalJsInAppFrame(web_ui, kScript);
}
void TouchFileSync(const base::FilePath& path, const base::Time& time) { void TouchFileSync(const base::FilePath& path, const base::Time& time) {
base::ScopedAllowBlockingForTesting allow_blocking; base::ScopedAllowBlockingForTesting allow_blocking;
EXPECT_TRUE(base::TouchFile(path, time, time)); EXPECT_TRUE(base::TouchFile(path, time, time));
...@@ -305,6 +319,11 @@ IN_PROC_BROWSER_TEST_P(MediaAppIntegrationWithFilesAppTest, ...@@ -305,6 +319,11 @@ IN_PROC_BROWSER_TEST_P(MediaAppIntegrationWithFilesAppTest,
EXPECT_EQ("640x480", WaitForImageAlt(web_ui, kFileJpeg640x480)); EXPECT_EQ("640x480", WaitForImageAlt(web_ui, kFileJpeg640x480));
// We load the first file when the app launches, other files in the working
// directory are loaded afterwards. Wait for the second load to occur
// indicated by being able to navigate.
WaitForNavigable(web_ui);
// Navigate to the next file in the directory. // Navigate to the next file in the directory.
EXPECT_EQ(true, ExecuteScript(web_ui, "advance(1)")); EXPECT_EQ(true, ExecuteScript(web_ui, "advance(1)"));
EXPECT_EQ("800x600", WaitForImageAlt(web_ui, kFilePng800x600)); EXPECT_EQ("800x600", WaitForImageAlt(web_ui, kFilePng800x600));
......
...@@ -31,6 +31,17 @@ const currentFiles = []; ...@@ -31,6 +31,17 @@ const currentFiles = [];
*/ */
let entryIndex = -1; let entryIndex = -1;
/**
* Keeps track of the current launch (i.e. call to `launchWithDirectory`) .
* Since file loading can be deferred i.e. we can load the first focused file
* and start using the app then load other files in `loadOtherRelatedFiles()` we
* need to make sure `loadOtherRelatedFiles` gets aborted if it is out of date
* i.e. in interleaved launches.
*
* @type {number}
*/
let globalLaunchNumber = -1;
/** /**
* Reference to the directory handle that contains the first file in the most * Reference to the directory handle that contains the first file in the most
* recent launch event. * recent launch event.
...@@ -289,19 +300,30 @@ async function refreshFile(fd) { ...@@ -289,19 +300,30 @@ async function refreshFile(fd) {
* @return {!Promise<undefined>} * @return {!Promise<undefined>}
*/ */
async function sendFilesToGuest() { async function sendFilesToGuest() {
return sendSnapshotToGuest([...currentFiles]); // Shallow copy. return sendSnapshotToGuest(
[...currentFiles], globalLaunchNumber); // Shallow copy.
} }
/** /**
* Loads the provided file list into the guest without making any file writable. * Loads the provided file list into the guest without making any file writable.
* Note: code paths can defer loads i.e. `launchWithDirectory()` increment
* `globalLaunchNumber` to ensure their deferred load is still relevant when it
* finishes processing. Other code paths that call `sendSnapshotToGuest()` don't
* have to.
* @param {!Array<!FileDescriptor>} snapshot * @param {!Array<!FileDescriptor>} snapshot
* @param {number} localLaunchNumber
* @param {boolean=} extraFiles
* @return {!Promise<undefined>} * @return {!Promise<undefined>}
*/ */
async function sendSnapshotToGuest(snapshot) { async function sendSnapshotToGuest(
snapshot, localLaunchNumber, extraFiles = false) {
// On first launch, files are opened to determine navigation candidates. Don't // On first launch, files are opened to determine navigation candidates. Don't
// reopen in that case. Otherwise, attempt to reopen here. Some files may be // reopen in that case. Otherwise, attempt to reopen here. Some files may be
// assigned null, e.g., if they have been moved to a different folder. // assigned null, e.g., if they have been moved to a different folder.
await Promise.all(snapshot.map(refreshFile)); await Promise.all(snapshot.map(refreshFile));
if (localLaunchNumber !== globalLaunchNumber) {
return;
}
/** @type {!LoadFilesMessage} */ /** @type {!LoadFilesMessage} */
const loadFilesMessage = { const loadFilesMessage = {
...@@ -321,7 +343,12 @@ async function sendSnapshotToGuest(snapshot) { ...@@ -321,7 +343,12 @@ async function sendSnapshotToGuest(snapshot) {
fd.file = null; fd.file = null;
} }
await iframeReady; await iframeReady;
await guestMessagePipe.sendMessage(Message.LOAD_FILES, loadFilesMessage); if (extraFiles) {
await guestMessagePipe.sendMessage(
Message.LOAD_EXTRA_FILES, loadFilesMessage);
} else {
await guestMessagePipe.sendMessage(Message.LOAD_FILES, loadFilesMessage);
}
} }
/** /**
...@@ -431,19 +458,44 @@ function isFileRelated(focusFile, siblingFile) { ...@@ -431,19 +458,44 @@ function isFileRelated(focusFile, siblingFile) {
} }
/** /**
* Changes the working directory and initializes file iteration according to * Enum like return value of `processOtherFilesInDirectory()`.
* the type of the opened file. * @enum {number}
*/
const ProcessOtherFilesResult = {
// Newer load in progress, can abort loading these files.
ABORT: -2,
// The focusFile is missing, treat this as a normal load.
FOCUS_FILE_MISSING: -1,
// The focusFile is present, load these files as extra files.
FOCUS_FILE_RELEVANT: 0,
};
/**
* Loads related files the working directory to initialize file iteration
* according to the type of the opened file. If `globalLaunchNumber` changes
* (i.e. another launch occurs), this will abort early and not change
* `currentFiles`.
* @param {!FileSystemDirectoryHandle} directory * @param {!FileSystemDirectoryHandle} directory
* @param {?File} focusFile * @param {?File} focusFile
* @param {number} localLaunchNumber
* @return {!Promise<!ProcessOtherFilesResult>}
*/ */
async function setCurrentDirectory(directory, focusFile) { async function processOtherFilesInDirectory(
directory, focusFile, localLaunchNumber) {
if (!focusFile || !focusFile.name) { if (!focusFile || !focusFile.name) {
return; return ProcessOtherFilesResult.ABORT;
} }
/** @type {!Array<!FileDescriptor>} */
const relatedFiles = [];
// TODO(b/158149714): Clear out old tokens as well? Care needs to be taken to // TODO(b/158149714): Clear out old tokens as well? Care needs to be taken to
// ensure any file currently open with unsaved changes can still be saved. // ensure any file currently open with unsaved changes can still be saved.
currentFiles.length = 0;
for await (const /** !FileSystemHandle */ handle of directory.getEntries()) { for await (const /** !FileSystemHandle */ handle of directory.getEntries()) {
if (localLaunchNumber !== globalLaunchNumber) {
// Abort, another more up to date launch in progress.
return ProcessOtherFilesResult.ABORT;
}
if (!handle.isFile) { if (!handle.isFile) {
continue; continue;
} }
...@@ -459,17 +511,22 @@ async function setCurrentDirectory(directory, focusFile) { ...@@ -459,17 +511,22 @@ async function setCurrentDirectory(directory, focusFile) {
// Only allow traversal of related file types. // Only allow traversal of related file types.
if (entry && isFileRelated(focusFile, entry.file)) { if (entry && isFileRelated(focusFile, entry.file)) {
currentFiles.push({ relatedFiles.push({
token: generateToken(entry.handle), token: generateToken(entry.handle),
file: entry.file, file: entry.file,
handle: entry.handle, handle: entry.handle,
}); });
} }
} }
if (localLaunchNumber !== globalLaunchNumber) {
return ProcessOtherFilesResult.ABORT;
}
// Iteration order is not guaranteed using `directory.getEntries()`, so we // Iteration order is not guaranteed using `directory.getEntries()`, so we
// sort it afterwards by modification time to ensure a consistent and logical // sort it afterwards by modification time to ensure a consistent and logical
// order. More recent (i.e. higher timestamp) files should appear first. // order. More recent (i.e. higher timestamp) files should appear first.
currentFiles.sort((a, b) => { relatedFiles.sort((a, b) => {
// Sort null files last if they racily appear. // Sort null files last if they racily appear.
if (!a.file && !b.file) { if (!a.file && !b.file) {
return 0; return 0;
...@@ -480,9 +537,68 @@ async function setCurrentDirectory(directory, focusFile) { ...@@ -480,9 +537,68 @@ async function setCurrentDirectory(directory, focusFile) {
} }
return b.file.lastModified - a.file.lastModified; return b.file.lastModified - a.file.lastModified;
}); });
const name = focusFile.name; const name = focusFile.name;
entryIndex = currentFiles.findIndex(i => !!i.file && i.file.name === name); const focusIndex =
relatedFiles.findIndex(i => !!i.file && i.file.name === name);
entryIndex = 0;
if (focusIndex === -1) {
// The focus file is no longer there i.e. might have been deleted, should be
// missing form `currentFiles` as well.
currentFiles.push(...relatedFiles);
return ProcessOtherFilesResult.FOCUS_FILE_MISSING;
} else {
// Rotate the sorted files so focusIndex becomes index 0 such that we have
// [focus file, ...files larger, ...files smaller].
currentFiles.push(...relatedFiles.slice(focusIndex + 1));
currentFiles.push(...relatedFiles.slice(0, focusIndex));
return ProcessOtherFilesResult.FOCUS_FILE_RELEVANT;
}
}
/**
* Loads related files in the working directory and sends them to the guest. If
* the focus file (currentFiles[0]) is no longer relevant i.e. is has been
* deleted, we load files as usual.
* @param {!FileSystemDirectoryHandle} directory
* @param {?File} focusFile
* @param {?FileSystemFileHandle} focusHandle
* @param {number} localLaunchNumber
*/
async function loadOtherRelatedFiles(
directory, focusFile, focusHandle, localLaunchNumber) {
const processResult = await processOtherFilesInDirectory(
directory, focusFile, localLaunchNumber);
if (localLaunchNumber !== globalLaunchNumber ||
processResult === ProcessOtherFilesResult.ABORT) {
return;
}
const shallowCopy = [...currentFiles];
if (processResult === ProcessOtherFilesResult.FOCUS_FILE_RELEVANT) {
shallowCopy.shift();
await sendSnapshotToGuest(shallowCopy, localLaunchNumber, true);
} else {
// If the focus file is no longer relevant, load files as normal.
await sendSnapshotToGuest(shallowCopy, localLaunchNumber);
}
}
/**
* Sets state for the files opened in the current directory.
* @param {!FileSystemDirectoryHandle} directory
* @param {!{file: !File, handle: !FileSystemFileHandle}} focusFile
*/
function setCurrentDirectory(directory, focusFile) {
// Load currentFiles into the guest.
currentFiles.length = 0;
currentFiles.push({
token: generateToken(focusFile.handle),
file: focusFile.file,
handle: focusFile.handle,
});
currentDirectoryHandle = directory; currentDirectoryHandle = directory;
entryIndex = 0;
} }
/** /**
...@@ -492,18 +608,25 @@ async function setCurrentDirectory(directory, focusFile) { ...@@ -492,18 +608,25 @@ async function setCurrentDirectory(directory, focusFile) {
* @param {!FileSystemHandle} handle * @param {!FileSystemHandle} handle
*/ */
async function launchWithDirectory(directory, handle) { async function launchWithDirectory(directory, handle) {
const localLaunchNumber = ++globalLaunchNumber;
let asFile; let asFile;
try { try {
asFile = await getFileFromHandle(handle); asFile = await getFileFromHandle(handle);
} catch (/** @type {!DOMException} */ e) { } catch (/** @type {!DOMException} */ e) {
console.warn(`${handle.name}: ${e.message}`); console.warn(`${handle.name}: ${e.message}`);
sendSnapshotToGuest([{token: -1, file: null, handle, error: e.name}]); sendSnapshotToGuest(
[{token: -1, file: null, handle, error: e.name}], localLaunchNumber);
return; return;
} }
await setCurrentDirectory(directory, asFile.file);
// Load currentFiles into the guest. // Load currentFiles into the guest.
await sendFilesToGuest(); setCurrentDirectory(directory, asFile);
await sendSnapshotToGuest([...currentFiles], localLaunchNumber);
// The app is operable with the first file now.
// Process other files in directory.
await loadOtherRelatedFiles(
directory, asFile.file, asFile.handle, localLaunchNumber);
} }
/** /**
......
...@@ -105,6 +105,11 @@ mediaApp.AbstractFileList.prototype.loadNext = function() {}; ...@@ -105,6 +105,11 @@ mediaApp.AbstractFileList.prototype.loadNext = function() {};
* @return {!Promise<undefined>} * @return {!Promise<undefined>}
*/ */
mediaApp.AbstractFileList.prototype.loadPrev = function() {}; mediaApp.AbstractFileList.prototype.loadPrev = function() {};
/**
* @param {function(!mediaApp.AbstractFileList): void} observer invoked when the
* size or contents of the file list changes.
*/
mediaApp.AbstractFileList.prototype.addObserver = function(observer) {};
/** /**
* The delegate which exposes open source privileged WebUi functions to * The delegate which exposes open source privileged WebUi functions to
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
const Message = { const Message = {
DELETE_FILE: 'delete-file', DELETE_FILE: 'delete-file',
IFRAME_READY: 'iframe-ready', IFRAME_READY: 'iframe-ready',
LOAD_EXTRA_FILES: 'load-extra-files',
LOAD_FILES: 'load-files', LOAD_FILES: 'load-files',
NAVIGATE: 'navigate', NAVIGATE: 'navigate',
OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog', OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog',
......
...@@ -62,6 +62,13 @@ class ReceivedFile { ...@@ -62,6 +62,13 @@ class ReceivedFile {
} }
} }
/**
* Source of truth for what files are loaded in the app and writable. This can
* be appended to via `ReceivedFileList.addFiles()`.
* @type {?ReceivedFileList}
*/
let lastLoadedReceivedFileList = null;
/** /**
* A file list consisting of all files received from the parent. Exposes the * A file list consisting of all files received from the parent. Exposes the
* currently writable file and all other readable files in the current * currently writable file and all other readable files in the current
...@@ -87,6 +94,8 @@ class ReceivedFileList { ...@@ -87,6 +94,8 @@ class ReceivedFileList {
this.files = files.map(f => new ReceivedFile(f)); this.files = files.map(f => new ReceivedFile(f));
/** @type {number} */ /** @type {number} */
this.writableFileIndex = 0; this.writableFileIndex = 0;
/** @type {!Array<function(!mediaApp.AbstractFileList): void>} */
this.observers = [];
} }
/** @override */ /** @override */
...@@ -96,6 +105,7 @@ class ReceivedFileList { ...@@ -96,6 +105,7 @@ class ReceivedFileList {
/** /**
* Returns the file which is currently writable or null if there isn't one. * Returns the file which is currently writable or null if there isn't one.
* @override
* @return {?mediaApp.AbstractFile} * @return {?mediaApp.AbstractFile}
*/ */
getCurrentlyWritable() { getCurrentlyWritable() {
...@@ -104,6 +114,7 @@ class ReceivedFileList { ...@@ -104,6 +114,7 @@ class ReceivedFileList {
/** /**
* Loads in the next file in the list as a writable. * Loads in the next file in the list as a writable.
* @override
* @return {!Promise<undefined>} * @return {!Promise<undefined>}
*/ */
async loadNext() { async loadNext() {
...@@ -116,16 +127,44 @@ class ReceivedFileList { ...@@ -116,16 +127,44 @@ class ReceivedFileList {
/** /**
* Loads in the previous file in the list as a writable. * Loads in the previous file in the list as a writable.
* @override
* @return {!Promise<undefined>} * @return {!Promise<undefined>}
*/ */
async loadPrev() { async loadPrev() {
await parentMessagePipe.sendMessage(Message.NAVIGATE, {direction: -1}); await parentMessagePipe.sendMessage(Message.NAVIGATE, {direction: -1});
} }
/** @override */
addObserver(observer) {
this.observers.push(observer);
}
/** @param {!Array<!ReceivedFile>} files */
addFiles(files) {
if (files.length === 0) {
return;
}
this.files = [...this.files, ...files];
this.length = this.files.length;
// Call observers with the new underlying files.
this.observers.map(o => o(this));
}
} }
parentMessagePipe.registerHandler(Message.LOAD_FILES, async (message) => { parentMessagePipe.registerHandler(Message.LOAD_FILES, async (message) => {
const filesMessage = /** @type {!LoadFilesMessage} */ (message); const filesMessage = /** @type {!LoadFilesMessage} */ (message);
await loadFiles(new ReceivedFileList(filesMessage)); lastLoadedReceivedFileList = new ReceivedFileList(filesMessage);
await loadFiles(lastLoadedReceivedFileList);
});
// Load extra files by appending to the current `ReceivedFileList`.
parentMessagePipe.registerHandler(Message.LOAD_EXTRA_FILES, async (message) => {
if (!lastLoadedReceivedFileList) {
return;
}
const extraFilesMessage = /** @type {!LoadFilesMessage} */ (message);
const newFiles = extraFilesMessage.files.map(f => new ReceivedFile(f));
lastLoadedReceivedFileList.addFiles(newFiles);
}); });
// As soon as the LOAD_FILES handler is installed, signal readiness to the // As soon as the LOAD_FILES handler is installed, signal readiness to the
......
...@@ -16,17 +16,17 @@ ...@@ -16,17 +16,17 @@
*/ */
let ModuleHandler; let ModuleHandler;
/** @type{ModuleHandler} */ /** @type {ModuleHandler} */
const createVideoChild = async (blobSrc) => { const createVideoChild = async (blobSrc) => {
const video = const video =
/** @type{HTMLVideoElement} */ (document.createElement('video')); /** @type {HTMLVideoElement} */ (document.createElement('video'));
video.src = blobSrc; video.src = blobSrc;
return video; return video;
}; };
/** @type{ModuleHandler} */ /** @type {ModuleHandler} */
const createImgChild = async (blobSrc, altText) => { const createImgChild = async (blobSrc, altText) => {
const img = /** @type{!HTMLImageElement} */ (document.createElement('img')); const img = /** @type {!HTMLImageElement} */ (document.createElement('img'));
img.src = blobSrc; img.src = blobSrc;
img.alt = altText; img.alt = altText;
try { try {
...@@ -46,9 +46,15 @@ const createImgChild = async (blobSrc, altText) => { ...@@ -46,9 +46,15 @@ const createImgChild = async (blobSrc, altText) => {
class BacklightApp extends HTMLElement { class BacklightApp extends HTMLElement {
constructor() { constructor() {
super(); super();
/** @type {?HTMLElement} */
this.currentHandler = /** @type {HTMLElement} */ (
document.createElement('backlight-media-handler'));
this.appendChild(this.currentHandler);
this.currentMedia = this.currentMedia =
/** @type{!HTMLElement} */ (document.createElement('img')); /** @type {!HTMLElement} */ (document.createElement('img'));
this.appendChild(this.currentMedia); this.appendChild(this.currentMedia);
/** @type {?mediaApp.AbstractFileList} */
this.files;
} }
/** @override */ /** @override */
...@@ -68,13 +74,42 @@ class BacklightApp extends HTMLElement { ...@@ -68,13 +74,42 @@ class BacklightApp extends HTMLElement {
// state) at a time. // state) at a time.
this.replaceChild(child, this.currentMedia); this.replaceChild(child, this.currentMedia);
this.currentMedia = child; this.currentMedia = child;
// Loads a new handler each time a new media is loaded. Note: in actual
// implementation we cache our handler instances and early exit if we load
// the same media type.
const newHandler = /** @type {HTMLElement} */ (
document.createElement('backlight-media-handler'));
this.replaceChild(newHandler, this.currentHandler);
this.currentHandler = newHandler;
this.files = files;
files.addObserver((f) => this.onNewFiles(f));
} }
/** @override */ /** @override */
setDelegate(delegate) {} setDelegate(delegate) {}
/** @param {!mediaApp.AbstractFileList} files */
onNewFiles(files) {
if (files !== this.files) {
return;
}
if (!this.currentHandler) {
return;
}
// Toggle 'shownav' indicating the navigation buttons are available.
this.currentHandler.toggleAttribute('shownav', files.length > 1);
}
} }
window.customElements.define('backlight-app', BacklightApp); window.customElements.define('backlight-app', BacklightApp);
// Element mimicking the image/video handler which is the parent of the
// `navigation-overlay`.
class BacklightMediaHandler extends HTMLElement {}
window.customElements.define('backlight-media-handler', BacklightMediaHandler);
class VideoContainer extends HTMLElement {} class VideoContainer extends HTMLElement {}
window.customElements.define('backlight-video-container', VideoContainer); window.customElements.define('backlight-video-container', VideoContainer);
......
...@@ -205,9 +205,10 @@ class FakeFileSystemDirectoryHandle extends FakeFileSystemHandle { ...@@ -205,9 +205,10 @@ class FakeFileSystemDirectoryHandle extends FakeFileSystemHandle {
async getFile(name, options) { async getFile(name, options) {
const fileHandle = this.files.find(f => f.name === name); const fileHandle = this.files.find(f => f.name === name);
if (!fileHandle && options.create === true) { if (!fileHandle && options.create === true) {
// Simulate creating a new file. // Simulate creating a new file, assume it is an image. This is needed for
const newFileHandle = new FakeFileSystemFileHandle(); // renaming files to ensure it has the right mime type, the real
newFileHandle.name = name; // implementation copies the mime type from the binary.
const newFileHandle = new FakeFileSystemFileHandle(name, 'image/png');
this.files.push(newFileHandle); this.files.push(newFileHandle);
return Promise.resolve(newFileHandle); return Promise.resolve(newFileHandle);
} }
...@@ -361,6 +362,17 @@ function createNamedError(name, msg) { ...@@ -361,6 +362,17 @@ function createNamedError(name, msg) {
return error; return error;
} }
/**
* @param {!FileSystemDirectoryHandle} directory
* @param {!File} file
*/
async function loadFilesWithoutSendingToGuest(directory, file) {
const handle = await directory.getFile(file.name);
globalLaunchNumber++;
setCurrentDirectory(directory, {file, handle});
await processOtherFilesInDirectory(directory, file, globalLaunchNumber);
}
/** /**
* Checks that the `currentFiles` array maintained by launch.js has the same * Checks that the `currentFiles` array maintained by launch.js has the same
* sequence of files as `expectedFiles`. * sequence of files as `expectedFiles`.
...@@ -410,13 +422,13 @@ function assertMatchErrorStack(stackTrace, regexLines, opt_message) { ...@@ -410,13 +422,13 @@ function assertMatchErrorStack(stackTrace, regexLines, opt_message) {
/** /**
* Returns the files loaded in the most recent call to `loadFiles()`. * Returns the files loaded in the most recent call to `loadFiles()`.
* @return {Promise<?ReceivedFileList>} * @return {Promise<?Array<!ReceivedFile>>}
*/ */
async function getLoadedFiles() { async function getLoadedFiles() {
const response = /** @type {LastLoadedFilesResponse} */ ( const response = /** @type {LastLoadedFilesResponse} */ (
await guestMessagePipe.sendMessage('get-last-loaded-files')); await guestMessagePipe.sendMessage('get-last-loaded-files'));
if (response.fileList) { if (response.fileList) {
return response.fileList.files; return response.fileList;
} }
return null; return null;
} }
...@@ -429,3 +441,52 @@ async function getLoadedFiles() { ...@@ -429,3 +441,52 @@ async function getLoadedFiles() {
function simulateLosingAccessToDirectory() { function simulateLosingAccessToDirectory() {
currentDirectoryHandle = null; currentDirectoryHandle = null;
} }
/**
* @param {!FakeFileSystemDirectoryHandle} directory
*/
function launchWithFocusFile(directory) {
const focusFile = {
handle: directory.files[0],
file: directory.files[0].getFileSync()
};
globalLaunchNumber++;
setCurrentDirectory(directory, focusFile);
return focusFile;
}
/**
* @param {!FakeFileSystemDirectoryHandle} directory
* @param {number} totalFiles
*/
async function assertSingleFileLaunch(directory, totalFiles) {
chai.assert.equal(1, currentFiles.length);
await sendFilesToGuest();
const loadedFiles = await getLoadedFiles();
// The untrusted context only loads the first file.
chai.assert.equal(1, loadedFiles.length);
// All files are in the `FileSystemDirectoryHandle`.
chai.assert.equal(totalFiles, directory.files.length);
}
/**
* Check files loaded in the trusted context `currentFiles` against the working
* directory and the untrusted context.
* @param {!FakeFileSystemDirectoryHandle} directory
* @param {!Array<string>} fileNames
* @param {?string} testCase
*/
async function assertFilesLoaded(directory, fileNames, testCase) {
chai.assert.equal(fileNames.length, directory.files.length);
chai.assert.equal(fileNames.length, currentFiles.length);
const loadedFiles = /** @type {!Array<!File>} */ (await getLoadedFiles());
chai.assert.equal(fileNames.length, loadedFiles.length);
// Check `currentFiles` in the trusted context matches up with files sent
// to guest.
assertFilenamesToBe(fileNames.join(), testCase);
assertFilesToBe(loadedFiles, testCase);
}
...@@ -27,7 +27,9 @@ let TestMessageRunTestCase; ...@@ -27,7 +27,9 @@ let TestMessageRunTestCase;
/** /**
* Return type of `get-last-loaded-files` used to spy on the files sent to the * Return type of `get-last-loaded-files` used to spy on the files sent to the
* guest app using `loadFiles()`. * guest app using `loadFiles()`. We pass `ReceivedFileList.files` since passing
* @typedef {{fileList: ?{files: !ReceivedFileList}}} * `ReceivedFileList` through different contexts prunes methods and fails due to
* observers.
* @typedef {{fileList: ?Array<ReceivedFile>}}
*/ */
let LastLoadedFilesResponse; let LastLoadedFilesResponse;
...@@ -154,8 +154,10 @@ function installTestHandlers() { ...@@ -154,8 +154,10 @@ function installTestHandlers() {
}); });
parentMessagePipe.registerHandler('get-last-loaded-files', () => { parentMessagePipe.registerHandler('get-last-loaded-files', () => {
// Note: the `ReceivedFileList` has methods stripped since it gets sent
// over a pipe so just send the underlying files.
return /** @type {LastLoadedFilesResponse} */ ( return /** @type {LastLoadedFilesResponse} */ (
{fileList: lastReceivedFileList}); {fileList: lastReceivedFileList.files});
}); });
// Log errors, rather than send them to console.error. This allows the error // Log errors, rather than send them to console.error. This allows the error
......
...@@ -119,6 +119,16 @@ async function createTestImageFile( ...@@ -119,6 +119,16 @@ async function createTestImageFile(
return new File([blob], name, {type: 'image/png', lastModified}); return new File([blob], name, {type: 'image/png', lastModified});
} }
/**
* @param {!Array<string>} filenames
* @return {!Array<!File>}
*/
async function createMultipleImageFiles(filenames) {
const filePromise = name => createTestImageFile(1, 1, `${name}.png`);
const files = await Promise.all(filenames.map(filePromise));
return files;
}
// Tests that chrome://media-app is allowed to frame // Tests that chrome://media-app is allowed to frame
// chrome-untrusted://media-app. The URL is set in the html. If that URL can't // chrome-untrusted://media-app. The URL is set in the html. If that URL can't
// load, test this fails like JS ERROR: "Refused to frame '...' because it // load, test this fails like JS ERROR: "Refused to frame '...' because it
...@@ -156,11 +166,117 @@ TEST_F('MediaAppUIBrowserTest', 'LaunchFile', async () => { ...@@ -156,11 +166,117 @@ TEST_F('MediaAppUIBrowserTest', 'LaunchFile', async () => {
testDone(); testDone();
}); });
// Tests that a regular launch for multiple images succeeds, and the files get // Tests that we can launch the MediaApp with the selected (first) file,
// interact with it by invoking IPC (deletion) that doesn't re-launch the
// MediaApp i.e. doesn't call `launchWithDirectory`, then the rest of the files
// in the current directory are loaded in.
TEST_F('MediaAppUIBrowserTest', 'NonLaunchableIpcAfterFastLoad', async () => {
const files =
await createMultipleImageFiles(['file1', 'file2', 'file3', 'file4']);
const directory = await createMockTestDirectory(files);
// Emulate steps in `launchWithDirectory()` by launching with the first
// file.
const focusFile = launchWithFocusFile(directory);
await assertSingleFileLaunch(directory, files.length);
// Invoke Deletion IPC that doesn't relaunch the app.
const messageDelete = {deleteLastFile: true};
testResponse = await guestMessagePipe.sendMessage('test', messageDelete);
assertEquals(
'deleteOriginalFile resolved success', testResponse.testQueryResult);
// File removed from `FileSystemDirectoryHandle` internal state.
assertEquals(3, directory.files.length);
// Deletion results reloading the app with `currentFiles`, in this case
// nothing.
const lastLoadedFiles = await getLoadedFiles();
assertEquals(0, lastLoadedFiles.length);
// Load all other files in the `FileSystemDirectoryHandle`.
await loadOtherRelatedFiles(directory, focusFile.file, focusFile.handle, 0);
await assertFilesLoaded(
directory, ['file2.png', 'file3.png', 'file4.png'],
'fast files: check files after deletion');
testDone();
});
// Tests that we can launch the MediaApp with the selected (first) file,
// interact with it by invoking IPC (rename) that re-launches the
// MediaApp (calls `launchWithDirectory`), then the rest of the files
// in the current directory are loaded in.
TEST_F('MediaAppUIBrowserTest', 'ReLaunchableIpcAfterFastLoad', async () => {
const files =
await createMultipleImageFiles(['file1', 'file2', 'file3', 'file4']);
const directory = await createMockTestDirectory(files);
// Emulate steps in `launchWithDirectory()` by launching with the first
// file.
const focusFile = launchWithFocusFile(directory);
// `globalLaunchNumber` starts at -1, ensure first launch increments it.
assertEquals(0, globalLaunchNumber);
await assertSingleFileLaunch(directory, files.length);
// Invoke Rename IPC that relaunches the app, this calls
// `launchWithDirectory()` which increments globalLaunchNumber.
const messageRename = {renameLastFile: 'new_file_name.png'};
testResponse = await guestMessagePipe.sendMessage('test', messageRename);
assertEquals(
testResponse.testQueryResult, 'renameOriginalFile resolved success');
// Ensure rename relaunches the app incremented the `globalLaunchNumber`.
assertEquals(1, globalLaunchNumber);
// The renamed file (also the focused file) is at the end of the
// `FileSystemDirectoryHandle.files` since it was deleted and a new file
// with the same contents and a different name was created.
assertEquals(directory.files[3].name, 'new_file_name.png');
// The call to `launchWithDirectory()` from rename relaunching the app loads
// other files into `currentFiles`.
await assertFilesLoaded(
directory, ['new_file_name.png', 'file2.png', 'file3.png', 'file4.png'],
'fast files: check files after renaming');
const currentFilesAfterRenameLaunch = [...currentFiles];
const loadedFilesAfterRename = await getLoadedFiles();
// Try to load with previous launch number, has no effect as it is aborted
// early due to different launch numbers.
const previousLaunchNumber = 0;
await loadOtherRelatedFiles(
directory, focusFile.file, focusFile.handle, previousLaunchNumber);
// Ensure same files as before the call to `loadOtherRelatedFiles()` by for
// equality equality.
currentFilesAfterRenameLaunch.map(
(fd, index) => assertEquals(
fd, currentFiles[index],
`Equality check for file ${
JSON.stringify(fd)} in currentFiles filed`));
// Focus file stays index 0.
lastLoadedFiles = await getLoadedFiles();
assertEquals('new_file_name.png', lastLoadedFiles[0].name);
assertEquals(loadedFilesAfterRename[0].name, lastLoadedFiles[0].name);
// Focus file in the `FileSystemDirectoryHandle` is at index 3.
assertEquals(directory.files[3].name, lastLoadedFiles[0].name);
testDone();
});
// Tests that a regular
// launch for multiple images succeeds, and the files get
// distinct token mappings. // distinct token mappings.
TEST_F('MediaAppUIBrowserTest', 'MultipleFileHaveTokens', async () => { TEST_F('MediaAppUIBrowserTest', 'MultipleFilesHaveTokens', async () => {
const directory = await launchWithFiles( const directory = await launchWithFiles([
[await createTestImageFile(), await createTestImageFile()]); await createTestImageFile(1, 1, 'file1.png'),
await createTestImageFile(1, 1, 'file2.png')
]);
assertEquals(currentFiles.length, 2); assertEquals(currentFiles.length, 2);
assertGE(currentFiles[0].token, 0); assertGE(currentFiles[0].token, 0);
...@@ -175,8 +291,7 @@ TEST_F('MediaAppUIBrowserTest', 'MultipleFileHaveTokens', async () => { ...@@ -175,8 +291,7 @@ TEST_F('MediaAppUIBrowserTest', 'MultipleFileHaveTokens', async () => {
// Tests that a launch with multiple files selected in the files app loads only // Tests that a launch with multiple files selected in the files app loads only
// the files selected. // the files selected.
TEST_F('MediaAppUIBrowserTest', 'MultipleSelectionLaunch', async () => { TEST_F('MediaAppUIBrowserTest', 'MultipleSelectionLaunch', async () => {
const filePromise = name => createTestImageFile(1, 1, `${name}.png`); const directoryContents = await createMultipleImageFiles([0, 1, 2, 3]);
const directoryContents = await Promise.all([0, 1, 2, 3].map(filePromise));
const selectedIndexes = [1, 3]; const selectedIndexes = [1, 3];
const directory = await launchWithFiles(directoryContents, selectedIndexes); const directory = await launchWithFiles(directoryContents, selectedIndexes);
...@@ -706,26 +821,31 @@ TEST_F('MediaAppUIBrowserTest', 'RelatedFiles', async () => { ...@@ -706,26 +821,31 @@ TEST_F('MediaAppUIBrowserTest', 'RelatedFiles', async () => {
const directory = await createMockTestDirectory(testFiles); const directory = await createMockTestDirectory(testFiles);
const [mkv, jpg, txt, gif, webm, other, ext, html] = directory.getFilesSync(); const [mkv, jpg, txt, gif, webm, other, ext, html] = directory.getFilesSync();
const imageAndVideoFiles = [mkv, jpg, gif, webm]; const imageAndVideoFiles = [mkv, jpg, gif, webm];
// These files all have a last modified time of 0 so the order they end up in
// is the order they are added i.e. `matroska.mkv, jaypeg.jpg, jiff.gif,
// world.webm`. When a file is loaded it becomes the "focus file" and files
// get rotated around like such that we get `currentFiles = [focus file,
// ...larger files, ...smaller files]`.
await setCurrentDirectory(directory, mkv); await loadFilesWithoutSendingToGuest(directory, mkv);
assertFilesToBe(imageAndVideoFiles, 'mkv'); assertFilesToBe(imageAndVideoFiles, 'mkv');
await setCurrentDirectory(directory, jpg); await loadFilesWithoutSendingToGuest(directory, jpg);
assertFilesToBe(imageAndVideoFiles, 'jpg'); assertFilenamesToBe('jaypeg.jpg,jiff.gif,world.webm,matroska.mkv', 'jpg');
await setCurrentDirectory(directory, gif); await loadFilesWithoutSendingToGuest(directory, gif);
assertFilesToBe(imageAndVideoFiles, 'gif'); assertFilenamesToBe('jiff.gif,world.webm,matroska.mkv,jaypeg.jpg', 'gif');
await setCurrentDirectory(directory, webm); await loadFilesWithoutSendingToGuest(directory, webm);
assertFilesToBe(imageAndVideoFiles, 'webm'); assertFilenamesToBe('world.webm,matroska.mkv,jaypeg.jpg,jiff.gif', 'webm');
await setCurrentDirectory(directory, txt); await loadFilesWithoutSendingToGuest(directory, txt);
assertFilesToBe([txt, other], 'txt'); assertFilesToBe([txt, other], 'txt');
await setCurrentDirectory(directory, html); await loadFilesWithoutSendingToGuest(directory, html);
assertFilesToBe([html], 'html'); assertFilesToBe([html], 'html');
await setCurrentDirectory(directory, ext); await loadFilesWithoutSendingToGuest(directory, ext);
assertFilesToBe([ext], 'ext'); assertFilesToBe([ext], 'ext');
testDone(); testDone();
......
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