[shared storage] Use separate mojom channels for worklet creation (addModule) and for other operations (selectURL/run)
What: Currently, The mojom::SharedStorageDocumentService channel
is used to create the worklet (i.e. handle addModule), and is also
used to route future worklet operations to SharedStorageWorkletHost.
This CL decouples the two things. Note that the new channel is
still associated with mojom::SharedStorageDocumentService (which is
associated with the default navigation mojom channel), so that
worklet operations before navigating away can be handled reliably.
We also combine the handling of "creating worklet" and "addModule"
because they always occur together. By doing this, we can remove some
assertions on addModule status.
Also, move more permission checks to the renderer, and
ReportbadMessage at mojom boundary: 1) addModule can only be called
once, 2) addModule must be called before selectURL()/run(),
3) selectURL()/run() cannot be called if a previous call didn't
have {keepAlive: true}
Why: This prepares for a potential future change to allow multiple
worklets per Document. Nevertheless, a decoupled architecture is
generally better.
Bug: 1218540
Change-Id: Idc2b5df326ba75806f293d881fb9fb00fe80601e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4947893
Reviewed-by: Cammie Smith Barnes <cammie@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212380}
diff --git a/shared-storage/run-operation.tentative.https.sub.html b/shared-storage/run-operation.tentative.https.sub.html
index c6c7080..1ef0d25 100644
--- a/shared-storage/run-operation.tentative.https.sub.html
+++ b/shared-storage/run-operation.tentative.https.sub.html
@@ -1,16 +1,19 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
+<script src="/shared-storage/resources/util.js"></script>
<body>
<script>
'use strict';
-promise_test(() => {
+promise_test(async t => {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
return sharedStorage.run("operation1", {keepAlive: true});
}, 'run()');
-promise_test(() => {
+promise_test(async t => {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
return sharedStorage.run("operation1",
{data: {'custom-key': 'custom-value'},
keepAlive: true});
@@ -18,6 +21,7 @@
promise_test(async t => {
try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
await sharedStorage.run("operation1", {data: window});
} catch (e) {
assert_equals(e.name, 'DataCloneError');
diff --git a/shared-storage/run-url-selection-operation-without-add-module.tentative.https.sub.html b/shared-storage/run-url-selection-operation-without-add-module.tentative.https.sub.html
index 0626684..8041a9c 100644
--- a/shared-storage/run-url-selection-operation-without-add-module.tentative.https.sub.html
+++ b/shared-storage/run-url-selection-operation-without-add-module.tentative.https.sub.html
@@ -15,136 +15,6 @@
{resolveToConfig: resolve_to_config, keepAlive: true}));
}, 'selectURL() without addModule, ' +
'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation",
- [{url: "1"}, {url: "2"}, {url: "3"}, {url: "4"}, {url: "5"},
- {url: "6"}, {url: "7"}, {url: "8"}, {url: "9"}],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message, 'Length of the \"urls\" parameter is not valid.');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with urls array length too big, ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation",
- [],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message, 'Length of the \"urls\" parameter is not valid.');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with empty urls array, ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation", [{
- reportingMetadata: {
- 'click': "https://google.com"
- }
- }],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'TypeError');
- assert_equals(true, e.message.startsWith(
- 'Failed to execute \'selectURL\' on \'SharedStorage\': ' +
- 'Failed to read the \'url\' property from ' +
- '\'SharedStorageUrlWithMetadata\':') &&
- e.message.endsWith('Failed to read the \'url\' property from ' +
- '\'SharedStorageUrlWithMetadata\': Required member is undefined.'));
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with missing url, ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation", [{
- url: "https://#"
- }],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message,
- 'The url \"https://#\" is invalid.');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with invalid url, ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation", [{
- url: "/shared-storage/resources/frame0.html",
- reportingMetadata: {
- 'click': "https://#"
- }
- }],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message,
- 'The metadata for the url at index 0 has an invalid ' +
- 'or non-HTTPS report_url parameter \"https://#\".');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with invalid report url ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation", [{
- url: "/shared-storage/resources/frame0.html",
- reportingMetadata: {
- 'click': "http://google.com"
- }
- }],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message, 'The metadata for the url at index 0 has an ' +
- 'invalid or non-HTTPS report_url parameter \"http://google.com\".');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with http report url, ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
-
- promise_test(async t => {
- try {
- await sharedStorage.selectURL(
- "test-url-selection-operation", [{
- url: "/shared-storage/resources/frame0.html",
- reportingMetadata: {}
- }],
- {resolveToConfig: resolve_to_config, keepAlive: true});
- } catch (e) {
- assert_equals(e.name, 'DataError');
- assert_equals(e.message, 'selectURL could not get reportingMetadata ' +
- 'object attributes');
- return;
- }
- assert_unreached("did not reject");
- }, 'selectURL() with invalid reportingMetadata ' +
- 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
}
</script>
diff --git a/shared-storage/run-url-selection-operation.tentative.https.sub.html b/shared-storage/run-url-selection-operation.tentative.https.sub.html
index 68f420a..01cb9fd 100644
--- a/shared-storage/run-url-selection-operation.tentative.https.sub.html
+++ b/shared-storage/run-url-selection-operation.tentative.https.sub.html
@@ -46,6 +46,143 @@
const result2 = await nextValueFromServer(ancestor_key);
assert_equals(result2, "frame0_loaded");
}, 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation",
+ [{url: "1"}, {url: "2"}, {url: "3"}, {url: "4"}, {url: "5"},
+ {url: "6"}, {url: "7"}, {url: "8"}, {url: "9"}],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message, 'Length of the \"urls\" parameter is not valid.');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with urls array length too big, ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation",
+ [],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message, 'Length of the \"urls\" parameter is not valid.');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with empty urls array, ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation", [{
+ reportingMetadata: {
+ 'click': "https://google.com"
+ }
+ }],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'TypeError');
+ assert_equals(true, e.message.startsWith(
+ 'Failed to execute \'selectURL\' on \'SharedStorage\': ' +
+ 'Failed to read the \'url\' property from ' +
+ '\'SharedStorageUrlWithMetadata\':') &&
+ e.message.endsWith('Failed to read the \'url\' property from ' +
+ '\'SharedStorageUrlWithMetadata\': Required member is undefined.'));
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with missing url, ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation", [{
+ url: "https://#"
+ }],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message,
+ 'The url \"https://#\" is invalid.');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with invalid url, ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation", [{
+ url: "/shared-storage/resources/frame0.html",
+ reportingMetadata: {
+ 'click': "https://#"
+ }
+ }],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message,
+ 'The metadata for the url at index 0 has an invalid ' +
+ 'or non-HTTPS report_url parameter \"https://#\".');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with invalid report url ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation", [{
+ url: "/shared-storage/resources/frame0.html",
+ reportingMetadata: {
+ 'click': "http://google.com"
+ }
+ }],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message, 'The metadata for the url at index 0 has an ' +
+ 'invalid or non-HTTPS report_url parameter \"http://google.com\".');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with http report url, ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
+
+ promise_test(async t => {
+ try {
+ await addModuleOnce("/shared-storage/resources/simple-module.js");
+ await sharedStorage.selectURL(
+ "test-url-selection-operation", [{
+ url: "/shared-storage/resources/frame0.html",
+ reportingMetadata: {}
+ }],
+ {resolveToConfig: resolve_to_config, keepAlive: true});
+ } catch (e) {
+ assert_equals(e.name, 'DataError');
+ assert_equals(e.message, 'selectURL could not get reportingMetadata ' +
+ 'object attributes');
+ return;
+ }
+ assert_unreached("did not reject");
+ }, 'selectURL() with invalid reportingMetadata ' +
+ 'selectURL() resolves to ' + (resolve_to_config ? 'config' : 'urn:uuid'));
}
</script>
