Minor improvements to RemoteContext helper
Some of these help with testing prerendering using the helper.
* Allow the executorCreator option to createContext() to return a promise, which gets awaited. This allows it to consist of running script in another RemoteContextWrapper, e.g. to insert <script type=speculationrules>. (If it is sync, there is no change in behavior; it just waits an extra microtask.) Although previously you could still do async things in it and that would generally work (e.g. window creation via window.open() is essentially async), this change ensures any promise rejections are propagated.
* Add a navigateTo() method to RemoteContextWrapper, to make some of the simpler cases where you would use navigate() easier. (Notably, this is what you would use for prerendering.)
* Renamed addHtml() to addHTML(), following https://w3ctag.github.io/design-principles/#casing-rules.
* Fixed cases where an executeScript() was not being awaited, notably in calls to navigate() or the navigation helpers, and in execution context creation.
* Clean up a variety of confusing options passing and defaulting code and documentation. There was an overuse of null (instead of the JavaScript-idiomatic default of undefined) which caused optional argument forwarding to need to be manual. As part of this, also fixed up the documentation to denote optional arguments using the appropriate syntax, and to document options as object properties instead of documenting them as if they were arguments.
* Other minor code and documentation cleanups
Change-Id: I3d001f5921aa643e3c1d66d8e4e40b264c6636cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3841185
Reviewed-by: Fergal Daly <fergal@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1039622}
diff --git a/html/browsers/browsing-the-web/remote-context-helper-tests/addHtml.window.js b/html/browsers/browsing-the-web/remote-context-helper-tests/addHTML.window.js
similarity index 92%
rename from html/browsers/browsing-the-web/remote-context-helper-tests/addHtml.window.js
rename to html/browsers/browsing-the-web/remote-context-helper-tests/addHTML.window.js
index 216febb..df461b0 100644
--- a/html/browsers/browsing-the-web/remote-context-helper-tests/addHtml.window.js
+++ b/html/browsers/browsing-the-web/remote-context-helper-tests/addHTML.window.js
@@ -16,7 +16,7 @@
const main = await rcHelper.addWindow();
await assertSimplestScriptRuns(main);
- await main.addHtml('<div id=div-id>div-content</div>');
+ await main.addHTML('<div id=div-id>div-content</div>');
await assertFunctionRuns(
main, () => document.getElementById('div-id').textContent, 'div-content');
});
diff --git a/html/browsers/browsing-the-web/remote-context-helper-tests/createContext-bad-executorCreator.window.js b/html/browsers/browsing-the-web/remote-context-helper-tests/createContext-bad-executorCreator.window.js
new file mode 100644
index 0000000..52acc0e
--- /dev/null
+++ b/html/browsers/browsing-the-web/remote-context-helper-tests/createContext-bad-executorCreator.window.js
@@ -0,0 +1,22 @@
+// META: title=RemoteContextHelper createContext with throwing/rejecting executorCreators.
+// META: script=/common/dispatcher/dispatcher.js
+// META: script=/common/get-host-info.sub.js
+// META: script=/common/utils.js
+// META: script=/resources/testharness.js
+// META: script=/resources/testharnessreport.js
+// META: script=/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
+
+'use strict';
+
+promise_test(async t => {
+ const rcHelper = new RemoteContextHelper();
+
+ const err = new Error('something bad!');
+ promise_rejects_exactly(
+ t, err, rcHelper.createContext({ executorCreator() { throw err; } }),
+ 'Sync exception must be rethrown');
+
+ promise_rejects_exactly(
+ t, err, rcHelper.createContext({ executorCreator() { return Promise.reject(err); } }),
+ 'Async rejection must be rethrown');
+});
diff --git a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-bfcache.window.js b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-bfcache.window.js
index bb7cb3b..9c94fc7 100644
--- a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-bfcache.window.js
+++ b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-bfcache.window.js
@@ -28,7 +28,7 @@
await assertSimplestScriptRuns(rc2);
// Navigate back.
- rc2.historyBack();
+ await rc2.historyBack();
// Verify that the document was BFCached.
assert_true(await rc1.executeScript(() => {
diff --git a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-helpers.window.js b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-helpers.window.js
index 50085d1..c4cb3b2 100644
--- a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-helpers.window.js
+++ b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-helpers.window.js
@@ -17,13 +17,13 @@
const rc2 = await rc1.navigateToNew();
await assertSimplestScriptRuns(rc2);
- rc2.historyBack();
+ await rc2.historyBack();
await assertSimplestScriptRuns(rc1);
- rc1.historyForward();
+ await rc1.historyForward();
await assertSimplestScriptRuns(rc2);
const rc3 = await rc2.navigateToNew();
- rc3.historyGo(-2);
+ await rc3.historyGo(-2);
await assertSimplestScriptRuns(rc1);
});
diff --git a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-same-document.window.js b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-same-document.window.js
index 3d6e576..e2b6711 100644
--- a/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-same-document.window.js
+++ b/html/browsers/browsing-the-web/remote-context-helper-tests/navigation-same-document.window.js
@@ -28,9 +28,7 @@
const newLocation = oldLocation + '#fragment';
// Navigate to same document.
- rc.navigate((newLocation) => {
- location = newLocation;
- }, [newLocation]);
+ await rc.navigateTo(newLocation);
// Verify that the window navigated.
await assertLocationIs(rc, newLocation);
diff --git a/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js b/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
index 3880e3c..300eded 100644
--- a/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
+++ b/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
@@ -67,7 +67,7 @@
if (!origin) {
return location.origin;
}
- if (origin.indexOf('/') == -1) {
+ if (!origin.includes('/')) {
const origins = get_host_info();
if (origin in origins) {
return origins[origin];
@@ -93,18 +93,20 @@
*/
class RemoteContextConfig {
/**
- * @param {string} origin A URL or a key in `get_host_info()`. @see finalizeOrigin for how origins are handled.
- * @param {string[]} scripts A list of script URLs. The current document
- * will be used as the base for relative URLs.
- * @param {[string, string][]} headers A list of pairs of name and value.
- * The executor will be served with these headers set.
- * @param {string} startOn If supplied, the executor will start when this
- * event occurs, e.g. "pageshow",
- * (@see window.addEventListener). This only makes sense for window-based executors, not worker-based.
- *
+ * @param {Object} [options]
+ * @param {string} [options.origin] A URL or a key in `get_host_info()`.
+ * @see finalizeOrigin for how origins are handled.
+ * @param {string[]} [options.scripts] A list of script URLs. The current
+ * document will be used as the base for relative URLs.
+ * @param {[string, string][]} [options.headers] A list of pairs of name
+ * and value. The executor will be served with these headers set.
+ * @param {string} [options.startOn] If supplied, the executor will start
+ * when this event occurs, e.g. "pageshow",
+ * (@see window.addEventListener). This only makes sense for
+ * window-based executors, not worker-based.
*/
constructor(
- {origin = null, scripts = [], headers = [], startOn = null} = {}) {
+ {origin, scripts = [], headers = [], startOn} = {}) {
this.origin = origin;
this.scripts = scripts;
this.headers = headers;
@@ -115,7 +117,7 @@
* If `config` is not already a `RemoteContextConfig`, one is constructed
* using `config`.
* @private
- * @param {object} config
+ * @param {object} [config]
* @returns
*/
static ensure(config) {
@@ -138,16 +140,16 @@
origin = extraConfig.origin;
}
let startOn = this.startOn;
- if (extraConfig.startOn !== null) {
+ if (extraConfig.startOn) {
startOn = extraConfig.startOn;
}
const headers = this.headers.concat(extraConfig.headers);
const scripts = this.scripts.concat(extraConfig.scripts);
return new RemoteContextConfig({
- origin: origin,
- headers: headers,
- scripts: scripts,
- startOn: startOn,
+ origin,
+ headers,
+ scripts,
+ startOn,
});
}
}
@@ -169,7 +171,7 @@
* @param {RemoteContextConfig|object} config The configuration
* for this remote context.
*/
- constructor(config = null) {
+ constructor(config) {
this.config = RemoteContextConfig.ensure(config);
}
@@ -177,17 +179,19 @@
* Creates a new remote context and returns a `RemoteContextWrapper` giving
* access to it.
* @private
- * @param {(url: string) => RemoteContext} executorCreator A function that
- * takes a URL and returns a context, e.g. an iframe or window.
- * @param {RemoteContextConfig|object|null} extraConfig If
- * supplied, extra configuration for this remote context to be merged
- * with `this`'s existing config. If it's not a `RemoteContextConfig`, it
- * will be used to construct a new one.
- * @returns {RemoteContextWrapper}
+ * @param {Object} options
+ * @param {(url: string) => Promise<undefined>} options.executorCreator A
+ * function that takes a URL and causes the browser to navigate some
+ * window to that URL, e.g. via an iframe or a new window.
+ * @param {RemoteContextConfig|object} [options.extraConfig] If supplied,
+ * extra configuration for this remote context to be merged with
+ * `this`'s existing config. If it's not a `RemoteContextConfig`, it
+ * will be used to construct a new one.
+ * @returns {Promise<RemoteContextWrapper>}
*/
async createContext({
- executorCreator: executorCreator,
- extraConfig = null,
+ executorCreator,
+ extraConfig,
isWorker = false,
}) {
const config =
@@ -212,27 +216,25 @@
url.searchParams.append('startOn', config.startOn);
}
- executorCreator(url);
- return new RemoteContextWrapper(new RemoteContext(uuid), this);
+ await executorCreator(url.href);
+ return new RemoteContextWrapper(new RemoteContext(uuid), this, url.href);
}
/**
* Creates a window with a remote context. @see createContext for
- * @param {RemoteContextConfig|object} extraConfig Will be
+ * @param {RemoteContextConfig|object} [extraConfig] Will be
* merged with `this`'s config.
- * @param {string} options.target Passed to `window.open` as the 2nd
- * argument
- * @param {string} options.features Passed to `window.open` as the 3rd
- * argument
- * @returns {RemoteContextWrapper}
+ * @param {Object} [options]
+ * @param {string} [options.target] Passed to `window.open` as the
+ * 2nd argument
+ * @param {string} [options.features] Passed to `window.open` as the
+ * 3rd argument
+ * @returns {Promise<RemoteContextWrapper>}
*/
- addWindow(extraConfig = null, options = {
- target: null,
- features: null,
- }) {
+ addWindow(extraConfig, options) {
return this.createContext({
executorCreator: windowExecutorCreator(options),
- extraConfig: extraConfig,
+ extraConfig,
});
}
}
@@ -256,10 +258,7 @@
url.searchParams.append('pipe', formattedHeaders.join('|'));
}
- function windowExecutorCreator({target, features}) {
- if (!target) {
- target = '_blank';
- }
+ function windowExecutorCreator({target = '_blank', features} = {}) {
return url => {
window.open(url, target, features);
};
@@ -268,7 +267,7 @@
function elementExecutorCreator(
remoteContextWrapper, elementName, attributes) {
return url => {
- remoteContextWrapper.executeScript((url, elementName, attributes) => {
+ return remoteContextWrapper.executeScript((url, elementName, attributes) => {
const el = document.createElement(elementName);
for (const attribute in attributes) {
el.setAttribute(attribute, attributes[attribute]);
@@ -287,7 +286,7 @@
function navigateExecutorCreator(remoteContextWrapper) {
return url => {
- remoteContextWrapper.navigate((url) => {
+ return remoteContextWrapper.navigate((url) => {
window.location = url;
}, [url]);
};
@@ -315,8 +314,8 @@
* Executes a script in the remote context.
* @param {function} fn The script to execute.
* @param {any[]} args An array of arguments to pass to the script.
- * @returns {any} The return value of the script (after being serialized and
- * deserialized).
+ * @returns {Promise<any>} The return value of the script (after
+ * being serialized and deserialized).
*/
async executeScript(fn, args) {
return this.context.execute_script(fn, args);
@@ -325,9 +324,9 @@
/**
* Adds a string of HTML to the executor's document.
* @param {string} html
- * @returns
+ * @returns {Promise<undefined>}
*/
- async addHtml(html) {
+ async addHTML(html) {
return this.executeScript((htmlSource) => {
document.body.insertAdjacentHTML('beforebegin', htmlSource);
}, [html]);
@@ -337,7 +336,7 @@
* Adds scripts to the executor's document.
* @param {string[]} urls A list of URLs. URLs are relative to the current
* document.
- * @returns
+ * @returns {Promise<undefined>}
*/
async addScripts(urls) {
if (!urls) {
@@ -350,28 +349,28 @@
/**
* Adds an iframe to the current document.
- * @param {RemoteContextConfig} extraConfig
- * @param {[string, string][]} options.attributes A list of pairs of strings
+ * @param {RemoteContextConfig} [extraConfig]
+ * @param {[string, string][]} [attributes] A list of pairs of strings
* of attribute name and value these will be set on the iframe element
* when added to the document.
- * @returns {RemoteContextWrapper} The remote context.
+ * @returns {Promise<RemoteContextWrapper>} The remote context.
*/
- addIframe(extraConfig = null, attributes = {}) {
+ addIframe(extraConfig, attributes = {}) {
return this.helper.createContext({
executorCreator: elementExecutorCreator(this, 'iframe', attributes),
- extraConfig: extraConfig,
+ extraConfig,
});
}
/**
* Adds a dedicated worker to the current document.
- * @param {RemoteContextConfig} extraConfig
- * @returns {RemoteContextWrapper} The remote context.
+ * @param {RemoteContextConfig} [extraConfig]
+ * @returns {Promise<RemoteContextWrapper>} The remote context.
*/
- addWorker(extraConfig = null) {
+ addWorker(extraConfig) {
return this.helper.createContext({
executorCreator: workerExecutorCreator(),
- extraConfig: extraConfig,
+ extraConfig,
isWorker: true,
});
}
@@ -389,8 +388,9 @@
*
* Foolproof rule:
* - The script must perform exactly one navigation.
- * - If that navigation is a same-document history navigation, you must
- * `await` the result of `waitUntilLocationIs`.
+ * - If that navigation is a same-document history traversal, you must
+ * `await` the result of `waitUntilLocationIs`. (Same-document non-traversal
+ * navigations do not need this extra step.)
*
* More complex rules:
* - The script must perform a navigation. If it performs no navigation,
@@ -403,22 +403,48 @@
*
* @param {function} fn The script to execute.
* @param {any[]} args An array of arguments to pass to the script.
+ * @returns {Promise<undefined>}
*/
navigate(fn, args) {
- this.executeScript((fnText, args) => {
+ return this.executeScript((fnText, args) => {
executeScriptToNavigate(fnText, args);
}, [fn.toString(), args]);
}
/**
- * Navigates the context to a new document running an executor.
- * @param {RemoteContextConfig} extraConfig
- * @returns {RemoteContextWrapper} The remote context.
+ * Navigates to the given URL, by executing a script in the remote
+ * context that will perform navigation with the `location.href`
+ * setter.
+ *
+ * Be aware that performing a cross-document navigation using this
+ * method will cause this `RemoteContextWrapper` to become dormant,
+ * since the remote context it points to is no longer active and
+ * able to receive messages. You also won't be able to reliably
+ * tell when the navigation finishes; the returned promise will
+ * fulfill when the script finishes running, not when the navigation
+ * is done. As such, this is most useful for testing things like
+ * unload behavior (where it doesn't matter) or prerendering (where
+ * there is already a `RemoteContextWrapper` for the destination).
+ * For other cases, using `navigateToNew()` will likely be better.
+ *
+ * @param {string|URL} url The URL to navigate to.
+ * @returns {Promise<undefined>}
*/
- async navigateToNew(extraConfig = null) {
+ navigateTo(url) {
+ return this.navigate(url => {
+ location.href = url;
+ }, [url.toString()]);
+ }
+
+ /**
+ * Navigates the context to a new document running an executor.
+ * @param {RemoteContextConfig} [extraConfig]
+ * @returns {Promise<RemoteContextWrapper>} The remote context.
+ */
+ async navigateToNew(extraConfig) {
return this.helper.createContext({
executorCreator: navigateExecutorCreator(this),
- extraConfig: extraConfig,
+ extraConfig,
});
}
@@ -436,14 +462,14 @@
async waitUntilLocationIs(expectedLocation) {
return this.executeScript(async (expectedLocation) => {
- if (location == expectedLocation) {
+ if (location.href === expectedLocation) {
return;
}
// Wait until the location updates to the expected one.
await new Promise(resolve => {
const listener = addEventListener('hashchange', (event) => {
- if (event.newURL == expectedLocation) {
+ if (event.newURL === expectedLocation) {
removeEventListener(listener);
resolve();
}
@@ -455,43 +481,43 @@
/**
* Performs a history traversal.
* @param {integer} n How many steps to traverse. @see history.go
- * @param {string} expectedLocation If supplied will be passed to @see waitUntilLocationIs.
- * @returns The return value of `waitUntilLocationIs` or nothing.
+ * @param {string} [expectedLocation] If supplied will be passed to @see waitUntilLocationIs.
+ * @returns {Promise<undefined>}
*/
- async historyGo(n, expectedLocation = null) {
- this.navigate((n) => {
+ async historyGo(n, expectedLocation) {
+ await this.navigate((n) => {
history.go(n);
}, [n]);
if (expectedLocation) {
- return this.waitUntilLocationIs(expectedLocation);
+ await this.waitUntilLocationIs(expectedLocation);
}
}
/**
* Performs a history traversal back.
- * @param {string} expectedLocation If supplied will be passed to @see waitUntilLocationIs.
- * @returns The return value of `waitUntilLocationIs` or nothing.
+ * @param {string} [expectedLocation] If supplied will be passed to @see waitUntilLocationIs.
+ * @returns {Promise<undefined>}
*/
- async historyBack(expectedLocation = null) {
- this.navigate(() => {
+ async historyBack(expectedLocation) {
+ await this.navigate(() => {
history.back();
});
if (expectedLocation) {
- return this.waitUntilLocationIs(expectedLocation);
+ await this.waitUntilLocationIs(expectedLocation);
}
}
/**
* Performs a history traversal back.
- * @param {string} expectedLocation If supplied will be passed to @see waitUntilLocationIs.
- * @returns The return value of `waitUntilLocationIs` or nothing.
+ * @param {string} [expectedLocation] If supplied will be passed to @see waitUntilLocationIs.
+ * @returns {Promise<undefined>}
*/
- async historyForward(expectedLocation = null) {
- this.navigate(() => {
+ async historyForward(expectedLocation) {
+ await this.navigate(() => {
history.forward();
});
if (expectedLocation) {
- return this.waitUntilLocationIs(expectedLocation);
+ await this.waitUntilLocationIs(expectedLocation);
}
}
}
