From 495f9d67cad20777987bf8a403eca7025d5f075e Mon Sep 17 00:00:00 2001 From: Blink WPT Bot Date: Wed, 6 Oct 2021 15:55:32 +0000 Subject: [PATCH] Bug 1710539 [wpt PR 28950] - [WPT] Introduce `RemoteContext.execute_script()` and add basic BFCache tests + helpers, a=testonly Automatic update from web-platform-tests [WPT] Introduce `RemoteContext.execute_script()` and add basic BFCache tests + helpers (#28950) This PR adds `RemoteContext.execute_script()` and its documentation in `/common/dispatcher/`. This is based on with `execute_script()`-related parts of RFCs 88/89/91: - https://github.com/web-platform-tests/rfcs/pull/88 - https://github.com/web-platform-tests/rfcs/pull/89 - https://github.com/web-platform-tests/rfcs/pull/91 and addresses comments: - https://github.com/web-platform-tests/rfcs/pull/86#issuecomment-881681930 - https://github.com/web-platform-tests/wpt/pull/28950#discussion_r669395488 plus additional clarifications around navigation, minus `testdriver` integration (so this PR is implemented using `send()`/`receive()` in `/common/dispatcher/`), minus https://github.com/web-platform-tests/rfcs/pull/90 (so this PR leaves `send()`/`receive()` as-is). This PR also adds back-forward cache WPTs (basic event firing tests), as well as BFCache-specific helpers, based on `RemoteContext.execute_script()`. Design doc: https://docs.google.com/document/d/1p3G-qNYMTHf5LU9hykaXcYtJ0k3wYOwcdVKGeps6EkU/edit?usp=sharing Bug: 1107415 Change-Id: I034f9f5376dc3f9f32ca0b936dbd06e458c9160b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2885636 Reviewed-by: Rakina Zata Amni Reviewed-by: Arthur Sonzogni Commit-Queue: Hiroshige Hayashizaki Cr-Commit-Position: refs/heads/main@{#927566} Co-authored-by: Hiroshige Hayashizaki -- wpt-commits: 2947a57c382cd0886906a8cbb6bad702d70a7976 wpt-pr: 28950 --- .../tests/common/dispatcher/README.md | 201 ++++++++++++++++-- .../tests/common/dispatcher/dispatcher.js | 102 +++++++++ .../back-forward-cache/README.md | 52 +++++ .../back-forward-cache/events.html | 44 ++++ .../resources/event-recorder.js | 54 +++++ .../resources/executor.html | 52 +++++ .../resources/helper.sub.js | 130 +++++++++++ 7 files changed, 623 insertions(+), 12 deletions(-) create mode 100644 testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/README.md create mode 100644 testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/events.html create mode 100644 testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/event-recorder.js create mode 100644 testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/executor.html create mode 100644 testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js diff --git a/testing/web-platform/tests/common/dispatcher/README.md b/testing/web-platform/tests/common/dispatcher/README.md index befbb19ae9116..9058ebc4b6461 100644 --- a/testing/web-platform/tests/common/dispatcher/README.md +++ b/testing/web-platform/tests/common/dispatcher/README.md @@ -1,4 +1,172 @@ -# Message passing API +# `RemoteContext`: API for script execution in another context + +`RemoteContext` in `/common/dispatcher/dispatcher.js` provides an interface to +execute JavaScript in another global object (page or worker, the "executor"), +based on: + +- [WPT RFC 88: context IDs from uuid searchParams in URL](https://github.com/web-platform-tests/rfcs/pull/88), +- [WPT RFC 89: execute_script](https://github.com/web-platform-tests/rfcs/pull/89) and +- [WPT RFC 91: RemoteContext](https://github.com/web-platform-tests/rfcs/pull/91). + +Tests can send arbitrary javascript to executors to evaluate in its global +object, like: + +``` +// injector.html +const argOnLocalContext = ...; + +async function execute() { + window.open('executor.html?uuid=' + uuid); + const ctx = new RemoteContext(uuid); + await ctx.execute_script( + (arg) => functionOnRemoteContext(arg), + [argOnLocalContext]); +}; +``` + +and on executor: + +``` +// executor.html +function functionOnRemoteContext(arg) { ... } + +const uuid = new URLSearchParams(window.location.search).get('uuid'); +const executor = new Executor(uuid); +``` + +For concrete examples, see +[events.html](../../html/browsers/browsing-the-web/back-forward-cache/events.html) +and +[executor.html](../../html/browsers/browsing-the-web/back-forward-cache/resources/executor.html) +in back-forward cache tests. +Note that executor files under `/common/dispatcher/` are NOT for +`RemoteContext.execute_script()`. + +This is universal and avoids introducing many specific `XXX-helper.html` +resources. +Moreover, tests are easier to read, because the whole logic of the test can be +defined in a single file. + +## `new RemoteContext(uuid)` + +- `uuid` is a UUID string that identifies the remote context and should match + with the `uuid` parameter of the URL of the remote context. +- Callers should create the remote context outside this constructor (e.g. + `window.open('executor.html?uuid=' + uuid)`). + +## `RemoteContext.execute_script(fn, args)` + +- `fn` is a JavaScript function to execute on the remote context, which is + converted to a string using `toString()` and sent to the remote context. +- `args` is null or an array of arguments to pass to the function on the + remote context. Arguments are passed as JSON. +- If the return value of `fn` when executed in the remote context is a promise, + the promise returned by `execute_script` resolves to the resolved value of + that promise. Otherwise the `execute_script` promise resolves to the return + value of `fn`. + +Note that `fn` is evaluated on the remote context (`executor.html` in the +example above), while `args` are evaluated on the caller context +(`injector.html`) and then passed to the remote context. + +## Return value of injected functions and `execute_script()` + +If the return value of the injected function when executed in the remote +context is a promise, the promise returned by `execute_script` resolves to the +resolved value of that promise. Otherwise the `execute_script` promise resolves +to the return value of the function. + +When the return value of an injected script is a Promise, it should be resolved +before any navigation starts on the remote context. For example, it shouldn't +be resolved after navigating out and navigating back to the page again. +It's fine to create a Promise to be resolved after navigations, if it's not the +return value of the injected function. + +## Calling timing of `execute_script()` + +When `RemoteContext.execute_script()` is called when the remote context is not +active (for example before it is created, before navigation to the page, or +during the page is in back-forward cache), the injected script is evaluated +after the remote context becomes active. + +`RemoteContext.execute_script()` calls should be serialized by always waiting +for the returned promise to be resolved. +So it's a good practice to always write `await ctx.execute_script(...)`. + +## Evaluation timing of injected functions + +The script injected by `RemoteContext.execute_script()` can be evaluated any +time during the remote context is active. +For example, even before DOMContentLoaded events or even during navigation. +It's the responsibility of test-specific code/helpers to ensure evaluation +timing constraints (which can be also test-specific), if any needed. + +### Ensuring evaluation timing around page load + +For example, to ensure that injected functions (`mainFunction` below) are +evaluated after the first `pageshow` event, we can use pure JavaScript code +like below: + +``` +// executor.html +window.pageShowPromise = new Promise(resolve => + window.addEventListener('pageshow', resolve, {once: true})); + + +// injector.html +const waitForPageShow = async () => { + while (!window.pageShowPromise) { + await new Promise(resolve => setTimeout(resolve, 100)); + } + await window.pageShowPromise; +}; + +await ctx.execute(waitForPageShow); +await ctx.execute(mainFunction); +``` + +### Ensuring evaluation timing around navigation out/unloading + +It can be important to ensure there are no injected functions nor code behind +`RemoteContext` (such as Fetch APIs accessing server-side stash) running after +navigation is initiated, for example in the case of back-forward cache testing. + +To ensure this, + +- Do not call the next `RemoteContext.execute()` for the remote context after + triggering the navigation, until we are sure that the remote context is not + active (e.g. after we confirm that the new page is loaded). +- Call `Executor.suspend(callback)` synchronously within the injected script. + This suspends executor-related code, and calls `callback` when it is ready + to start navigation. + +The code on the injector side would be like: + +``` +// injector.html +await ctx.execute_script(() => { + executor.suspend(() => { + location.href = 'new-url.html'; + }); +}); +``` + +## Future Work: Possible integration with `test_driver` + +Currently `RemoteContext` is implemented by JavaScript and WPT-server-side +stash, and not integrated with `test_driver` nor `testharness`. +There is a proposal of `test_driver`-integrated version (see the RFCs listed +above). + +The API semantics and guidelines in this document are designed to be applicable +to both the current stash-based `RemoteContext` and `test_driver`-based +version, and thus the tests using `RemoteContext` will be migrated with minimum +modifications (mostly in `/common/dispatcher/dispatcher.js` and executors), for +example in a +[draft CL](https://chromium-review.googlesource.com/c/chromium/src/+/3082215/). + + +# `send()`/`receive()` Message passing APIs `dispatcher.js` (and its server-side backend `dispatcher.py`) provides a universal queue-based message passing API. @@ -17,17 +185,26 @@ listen, before sending the first message (but still need to wait for the resolution of the promise returned by `send()` to ensure the order between `send()`s). -# Executor framework +## Executors -The message passing API can be used for sending arbitrary javascript to be -evaluated in another page or worker (the "executor"). +Similar to `RemoteContext.execute_script()`, `send()`/`receive()` can be used +for sending arbitrary javascript to be evaluated in another page or worker. -`executor.html` (as a Document), `executor-worker.js` (as a Web Worker), and -`executor-service-worker.js` (as a Service Worker) are examples of executors. -Tests can send arbitrary javascript to these executors to evaluate in its -execution context. +- `executor.html` (as a Document), +- `executor-worker.js` (as a Web Worker), and +- `executor-service-worker.js` (as a Service Worker) -This is universal and avoids introducing many specific `XXX-helper.html` -resources. -Moreover, tests are easier to read, because the whole logic of the test can be -defined in a single file. +are examples of executors. +Note that these executors are NOT compatible with +`RemoteContext.execute_script()`. + +## Future Work + +`send()`, `receive()` and the executors below are kept for COEP/COOP tests. + +For remote script execution, new tests should use +`RemoteContext.execute_script()` instead. + +For message passing, +[WPT RFC 90](https://github.com/web-platform-tests/rfcs/pull/90) is still under +discussion. diff --git a/testing/web-platform/tests/common/dispatcher/dispatcher.js b/testing/web-platform/tests/common/dispatcher/dispatcher.js index 8350933e80139..b789c6e50c2cc 100644 --- a/testing/web-platform/tests/common/dispatcher/dispatcher.js +++ b/testing/web-platform/tests/common/dispatcher/dispatcher.js @@ -85,3 +85,105 @@ const showRequestHeaders = function(origin, uuid) { const cacheableShowRequestHeaders = function(origin, uuid) { return origin + dispatcher_path + `?uuid=${uuid}&cacheable&show-headers`; } + +// This script requires +// - `/common/utils.js` for `token()`. + +// Represents a remote executor. For more detailed explanation see `README.md`. +class RemoteContext { + // `uuid` is a UUID string that identifies the remote context and should + // match with the `uuid` parameter of the URL of the remote context. + constructor(uuid) { + this.context_id = uuid; + } + + // Evaluates the script `expr` on the executor. + // - If `expr` is evaluated to a Promise that is resolved with a value: + // `execute_script()` returns a Promise resolved with the value. + // - If `expr` is evaluated to a non-Promise value: + // `execute_script()` returns a Promise resolved with the value. + // - If `expr` throws an error or is evaluated to a Promise that is rejected: + // `execute_script()` returns a rejected Promise with the error's + // `message`. + // Note that currently the type of error (e.g. DOMException) is not + // preserved. + // The values should be able to be serialized by JSON.stringify(). + async execute_script(fn, args) { + const receiver = token(); + await this.send({receiver: receiver, fn: fn.toString(), args: args}); + const response = JSON.parse(await receive(receiver)); + if (response.status === 'success') { + return response.value; + } + + // exception + throw new Error(response.value); + } + + async send(msg) { + return await send(this.context_id, JSON.stringify(msg)); + } +}; + +class Executor { + constructor(uuid) { + this.uuid = uuid; + + // If `suspend_callback` is not `null`, the executor should be suspended + // when there are no ongoing tasks. + this.suspend_callback = null; + + this.execute(); + } + + // Wait until there are no ongoing tasks nor fetch requests for polling + // tasks, and then suspend the executor and call `callback()`. + // Navigation from the executor page should be triggered inside `callback()`, + // to avoid conflict with in-flight fetch requests. + suspend(callback) { + this.suspend_callback = callback; + } + + resume() { + } + + async execute() { + while(true) { + if (this.suspend_callback !== null) { + this.suspend_callback(); + this.suspend_callback = null; + // Wait for `resume()` to be called. + await new Promise(resolve => this.resume = resolve); + + // Workaround for https://crbug.com/1244230. + // Without this workaround, the executor is resumed and the fetch + // request to poll the next task is initiated synchronously from + // pageshow event after the page restored from BFCache, and the fetch + // request promise is never resolved (and thus the test results in + // timeout) due to https://crbug.com/1244230. The root cause is not yet + // known, but setTimeout() with 0ms causes the resume triggered on + // another task and seems to resolve the issue. + await new Promise(resolve => setTimeout(resolve, 0)); + + continue; + } + + const task = JSON.parse(await receive(this.uuid)); + + let response; + try { + const value = await eval(task.fn).apply(null, task.args); + response = JSON.stringify({ + status: 'success', + value: value + }); + } catch(e) { + response = JSON.stringify({ + status: 'exception', + value: e.message + }); + } + await send(task.receiver, response); + } + } +} diff --git a/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/README.md b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/README.md new file mode 100644 index 0000000000000..5f10361d5cffd --- /dev/null +++ b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/README.md @@ -0,0 +1,52 @@ +# How to write back-forward cache tests + +In the back-forward cache tests, the main test HTML usually: + +1. Opens new executor Windows using `window.open()` + `noopener` option, + because less isolated Windows (e.g. iframes and `window.open()` without + `noopener` option) are often not eligible for back-forward cache (e.g. + in Chromium). +2. Injects scripts to the executor Windows and receives the results via + `RemoteContext.execute_script()` by + [/common/dispatcher](../../../../common/dispatcher/README.md). + Follow the semantics and guideline described there. + +Back-forward cache specific helpers are in: + +- [resources/executor.html](resources/executor.html): + The BFCache-specific executor and contains helpers for executors. +- [resources/helper.sub.js](resources/helper.sub.js): + Helpers for main test HTMLs. + +We must ensure that injected scripts are evaluated only after page load +(more precisely, the first `pageshow` event) and not during navigation, +to prevent unexpected interference between injected scripts, in-flight fetch +requests behind `RemoteContext.execute_script()`, navigation and back-forward +cache. To ensure this, + +- Call `await remoteContext.execute_script(waitForPageShow)` before any + other scripts are injected to the remote context, and +- Call `prepareNavigation(callback)` synchronously from the script injected + by `RemoteContext.execute_script()`, and trigger navigation on or after the + callback is called. + +In typical A-B-A scenarios (where we navigate from Page A to Page B and then +navigate back to Page A, assuming Page A is (or isn't) in BFCache), + +- Call `prepareNavigation()` on the executor, and then navigate to B, and then + navigate back to Page A. +- Call `assert_bfcached()` or `assert_not_bfcached()` on the main test HTML, to + check the BFCache status. This is important to do to ensure the test would + not fail normally and instead result in `PRECONDITION_FAILED` if the page is + unexpectedly bfcached/not bfcached. +- Check other test expectations on the main test HTML, + +as in [events.html](./events.html) and `runEventTest()` in +[resources/helper.sub.js](resources/helper.sub.js). + +# Asserting PRECONDITION_FAILED for unexpected BFCache eligibility + +To distinguish failures due to unexpected BFCache ineligibility (which might be +acceptable due to different BFCache eligibility criteria across browsers), +`assert_bfcached()` and `assert_not_bfcached()` results in +`PRECONDITION_FAILED` rather than ordinal failures. diff --git a/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/events.html b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/events.html new file mode 100644 index 0000000000000..4b1d3e408ebc2 --- /dev/null +++ b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/events.html @@ -0,0 +1,44 @@ + + + + + + + + diff --git a/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/event-recorder.js b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/event-recorder.js new file mode 100644 index 0000000000000..469286a399212 --- /dev/null +++ b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/event-recorder.js @@ -0,0 +1,54 @@ +// Recording events + +const params = new URLSearchParams(window.location.search); +const uuid = params.get('uuid'); + +// The recorded events are stored in localStorage rather than global variables +// to catch events fired just before navigating out. +function getPushedItems(key) { + return JSON.parse(localStorage.getItem(key) || '[]'); +} + +function pushItem(key, value) { + const array = getPushedItems(key); + array.push(value); + localStorage.setItem(key, JSON.stringify(array)); +} + +window.recordEvent = function(eventName) { + pushItem(uuid + '.observedEvents', eventName); +} + +window.getRecordedEvents = function() { + return getPushedItems(uuid + '.observedEvents'); +} + +// Records events fired on `window` and `document`, with names listed in +// `eventNames`. +function startRecordingEvents(eventNames) { + for (const eventName of eventNames) { + window.addEventListener(eventName, event => { + let result = eventName; + if (event.persisted) { + result += '.persisted'; + } + if (eventName === 'visibilitychange') { + result += '.' + document.visibilityState; + } + recordEvent('window.' + result); + }); + document.addEventListener(eventName, () => { + let result = eventName; + if (eventName === 'visibilitychange') { + result += '.' + document.visibilityState; + } + recordEvent('document.' + result); + }); + } +} + +// When a comma-separated list of event names are given as the `events` +// parameter in the URL, start record the events of the given names. +if (params.get('events')) { + startRecordingEvents(params.get('events').split(',')); +} diff --git a/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/executor.html b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/executor.html new file mode 100644 index 0000000000000..4741fb7690c1b --- /dev/null +++ b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/executor.html @@ -0,0 +1,52 @@ + + + + diff --git a/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js new file mode 100644 index 0000000000000..d61e68a325f03 --- /dev/null +++ b/testing/web-platform/tests/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js @@ -0,0 +1,130 @@ +// Helpers called on the main test HTMLs. +// Functions in `RemoteContext.execute_script()`'s 1st argument are evaluated +// on the executors (`executor.html`), and helpers available on the executors +// are defined in `executor.html`. + +const originSameOrigin = + location.protocol === 'http:' ? + 'http://{{host}}:{{ports[http][0]}}' : + 'https://{{host}}:{{ports[https][0]}}'; +const originSameSite = + location.protocol === 'http:' ? + 'http://{{host}}:{{ports[http][1]}}' : + 'https://{{host}}:{{ports[https][1]}}'; +const originCrossSite = + location.protocol === 'http:' ? + 'http://{{hosts[alt][www]}}:{{ports[http][0]}}' : + 'https://{{hosts[alt][www]}}:{{ports[https][0]}}'; + +const executorPath = + '/html/browsers/browsing-the-web/back-forward-cache/resources/executor.html?uuid='; + +// Asserts that the executor `target` is (or isn't, respectively) +// restored from BFCache. These should be used in the following fashion: +// 1. Call prepareNavigation() on the executor `target`. +// 2. Navigate the executor to another page. +// 3. Navigate back to the executor `target`. +// 4. Call assert_bfcached() or assert_not_bfcached() on the main test HTML. +async function assert_bfcached(target) { + const status = await getBFCachedStatus(target); + assert_implements_optional(status === 'BFCached', + "Should be BFCached but actually wasn't"); +} + +async function assert_not_bfcached(target) { + const status = await getBFCachedStatus(target); + assert_implements_optional(status !== 'BFCached', + 'Should not be BFCached but actually was'); +} + +async function getBFCachedStatus(target) { + const [loadCount, isPageshowFired, isPageshowPersisted] = + await target.execute_script(() => [ + window.loadCount, window.isPageshowFired, window.isPageshowPersisted]); + + if (loadCount === 1 && isPageshowFired === true && + isPageshowPersisted === true) { + return 'BFCached'; + } else if (loadCount === 2 && isPageshowFired === false) { + return 'Not BFCached'; + } else { + // This can occur for example when this is called before first navigating + // away (loadCount = 1, isPageshowFired = false), e.g. when + // 1. sending a script for navigation and then + // 2. calling getBFCachedStatus() without waiting for the completion of + // the script on the `target` page. + assert_unreached( + `Got unexpected BFCache status: loadCount = ${loadCount}, ` + + `isPageshowFired = ${isPageshowFired}, ` + + `isPageshowPersisted = ${isPageshowPersisted}`); + } +} + +// Always call `await remoteContext.execute_script(waitForPageShow);` after +// triggering to navigation to the page, to wait for pageshow event on the +// remote context. +const waitForPageShow = () => window.pageShowPromise; + +// Run a test that navigates A->B->A: +// 1. Page A is opened by `params.openFunc(url)`. +// 2. `params.funcBeforeNavigation` is executed on page A. +// 3. The window is navigated to page B on `params.targetOrigin`. +// 4. The window is back navigated to page A (expecting BFCached). +// +// Events `params.events` (an array of strings) are observed on page A and +// `params.expectedEvents` (an array of strings) is expected to be recorded. +// See `event-recorder.js` for event recording. +// +// Parameters can be omitted. See `defaultParams` below for default. +function runEventTest(params, description) { + const defaultParams = { + openFunc: url => window.open(url, '_blank', 'noopener'), + funcBeforeNavigation: () => {}, + targetOrigin: originCrossSite, + events: ['pagehide', 'pageshow', 'load'], + expectedEvents: [ + 'window.load', + 'window.pageshow', + 'window.pagehide.persisted', + 'window.pageshow.persisted' + ], + } + // Apply defaults. + params = {...defaultParams, ...params}; + + promise_test(async t => { + const pageA = new RemoteContext(token()); + const pageB = new RemoteContext(token()); + + const urlA = executorPath + pageA.context_id + + '&events=' + params.events.join(','); + const urlB = params.targetOrigin + executorPath + pageB.context_id; + + params.openFunc(urlA); + + await pageA.execute_script(waitForPageShow); + await pageA.execute_script(params.funcBeforeNavigation); + await pageA.execute_script( + (url) => { + prepareNavigation(() => { + location.href = url; + }); + }, + [urlB] + ); + + await pageB.execute_script(waitForPageShow); + await pageB.execute_script( + () => { + prepareNavigation(() => { history.back(); }); + } + ); + + await pageA.execute_script(waitForPageShow); + await assert_bfcached(pageA); + + assert_array_equals( + await pageA.execute_script(() => getRecordedEvents()), + params.expectedEvents); + }, description); +}