Commit 14a56c52 authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

Interleave file processing and iframe loading in chrome://media-app

Some tracing with performance.now() suggests this will shave about 300ms
off the critical path for loading a file.

We would previously wait for the window 'load' event which indicated
that the <iframe> subresource (and all *its* subresources) had fully
loaded. This was "safe" but too conservative. There's a tip in the spec:
https://html.spec.whatwg.org/multipage/web-messaging.html#posting-messages
which suggests to have the <iframe> send a message to its parent to
indicate that it's ready to process messages, which this CL does (saving
~100ms).

For the other ~200ms, the steps involved in handling the launch event to
open the file and determine related files are now allowed to run before
the iframe is ready to receive messages. Only the final postMessage that
triggers the load must wait for the iframe to be ready.

Sample trace for loading a file in a directory with 75 files after this
change that explains the "300ms" improve:

<privileged>
75~85 ms: JS starts (t=0 in privileged)
t+5ms: launch message received
t+80ms ms: waiting for iframe
t+200ms: iframe "ready" received
t+300ms: privileged 'load' event received <-- we used to start "launch" here.

<unprivileged>
75~85 ms: JS starts (t=0 in unprivileged)
t+50ms: iframe ready (app_main.js executed, app is still null)
t+180ms: load in iframe begins
t+570ms: load done [warm start]
t+1050ms: load done [cold start]

Covered by existing tests, which were hammered with some invocations of
  browser_tests --test-launcher-jobs=20 --gtest_filter='*MediaApp*' \
      --gtest_repeat=10
to flush out any flakes.

Bug: b/157949149
Change-Id: Ie1c3ca79bb9f7b5d323967ee653943ac7901afd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2227716Reviewed-by: default avatardstockwell <dstockwell@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774904}
parent a93cbf1b
...@@ -54,6 +54,15 @@ let currentDirectoryHandle = null; ...@@ -54,6 +54,15 @@ let currentDirectoryHandle = null;
/** A pipe through which we can send messages to the guest frame. */ /** A pipe through which we can send messages to the guest frame. */
const guestMessagePipe = new MessagePipe('chrome-untrusted://media-app'); const guestMessagePipe = new MessagePipe('chrome-untrusted://media-app');
/**
* Promise that resolves once the iframe is ready to receive messages. This is
* to allow initial file processing to run in parallel with the iframe load.
* @type {!Promise<undefined>}
*/
const iframeReady = new Promise(resolve => {
guestMessagePipe.registerHandler(Message.IFRAME_READY, resolve);
});
guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => { guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => {
let response = mediaAppPageHandler.openFeedbackDialog(); let response = mediaAppPageHandler.openFeedbackDialog();
if (response === null) { if (response === null) {
...@@ -284,6 +293,7 @@ async function sendSnapshotToGuest(snapshot) { ...@@ -284,6 +293,7 @@ async function sendSnapshotToGuest(snapshot) {
for (const fd of snapshot) { for (const fd of snapshot) {
fd.file = null; fd.file = null;
} }
await iframeReady;
await guestMessagePipe.sendMessage(Message.LOAD_FILES, loadFilesMessage); await guestMessagePipe.sendMessage(Message.LOAD_FILES, loadFilesMessage);
} }
...@@ -469,9 +479,10 @@ async function advance(direction) { ...@@ -469,9 +479,10 @@ async function advance(direction) {
await sendFilesToGuest(); await sendFilesToGuest();
} }
// Wait for 'load' (and not DOMContentLoaded) to ensure the subframe has been /**
// loaded and is ready to respond to postMessage. * Installs the handler for launch files, if window.launchQueue is available.
window.addEventListener('load', () => { */
function installLaunchHandler() {
if (!window.launchQueue) { if (!window.launchQueue) {
console.error('FileHandling API missing.'); console.error('FileHandling API missing.');
return; return;
...@@ -491,4 +502,6 @@ window.addEventListener('load', () => { ...@@ -491,4 +502,6 @@ window.addEventListener('load', () => {
const focusEntry = assertCast(params.files[1]); const focusEntry = assertCast(params.files[1]);
launchWithDirectory(directory, focusEntry); launchWithDirectory(directory, focusEntry);
}); });
}); }
installLaunchHandler();
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
*/ */
const Message = { const Message = {
DELETE_FILE: 'delete-file', DELETE_FILE: 'delete-file',
IFRAME_READY: 'iframe-ready',
LOAD_FILES: 'load-files', LOAD_FILES: 'load-files',
NAVIGATE: 'navigate', NAVIGATE: 'navigate',
OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog', OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog',
......
...@@ -145,6 +145,10 @@ parentMessagePipe.registerHandler(Message.LOAD_FILES, async (message) => { ...@@ -145,6 +145,10 @@ parentMessagePipe.registerHandler(Message.LOAD_FILES, async (message) => {
await loadFiles(new ReceivedFileList(filesMessage)); await loadFiles(new ReceivedFileList(filesMessage));
}); });
// As soon as the LOAD_FILES handler is installed, signal readiness to the
// parent frame (privileged context).
parentMessagePipe.sendMessage(Message.IFRAME_READY);
/** /**
* A delegate which exposes privileged WebUI functionality to the media * A delegate which exposes privileged WebUI functionality to the media
* app. * app.
......
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