Skip to content

Commit

Permalink
[shared storage] Use separate mojom channels for worklet creation (ad…
Browse files Browse the repository at this point in the history
…dModule) 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}
  • Loading branch information
yaoxiachromium authored and Chromium LUCI CQ committed Oct 19, 2023
1 parent 42ff08d commit 140fde7
Show file tree
Hide file tree
Showing 17 changed files with 723 additions and 941 deletions.
28 changes: 15 additions & 13 deletions chrome/browser/storage/shared_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2017,10 +2017,9 @@ IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest,
GetActiveWebContents(),
content::JsReplace("sharedStorage.worklet.addModule($1)", script_url));

EXPECT_EQ(base::StrCat({"a JavaScript error: \"Error: ",
"sharedStorage.worklet.addModule() can only ",
"be invoked once per browsing context.\"\n"}),
result.error);
EXPECT_THAT(result.error,
testing::HasSubstr("sharedStorage.worklet.addModule() can only "
"be invoked once per browsing context"));

// Navigate away to record `kWorkletNumPerPageHistogram` histogram.
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(),
Expand All @@ -2037,16 +2036,20 @@ IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest,
IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest, Run_NotLoadedError) {
Set3rdPartyCookieAndMainHostAttestationSettingsThenNavigateToMainHostPage();

EXPECT_TRUE(content::ExecJs(GetActiveWebContents(),
R"(
content::EvalJsResult result = content::EvalJs(GetActiveWebContents(), R"(
sharedStorage.run(
'test-operation', {data: {}});
)"));
)");

EXPECT_THAT(
result.error,
testing::HasSubstr(
"sharedStorage.worklet.addModule() has to be called before run()"));

WaitForHistograms({kErrorTypeHistogram});
histogram_tester_.ExpectUniqueSample(
kErrorTypeHistogram,
blink::SharedStorageWorkletErrorType::kRunNonWebVisible, 1);
kErrorTypeHistogram, blink::SharedStorageWorkletErrorType::kRunWebVisible,
1);
}

IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest, Run_NotRegisteredError) {
Expand Down Expand Up @@ -2187,10 +2190,9 @@ IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest,
})()
)");

EXPECT_EQ(base::StrCat({"a JavaScript error: \"Error: ",
"sharedStorage.worklet.addModule() has to be ",
"called before sharedStorage.selectURL().\"\n"}),
result.error);
EXPECT_THAT(result.error,
testing::HasSubstr("sharedStorage.worklet.addModule() has to be "
"called before selectURL()"));

WaitForHistograms({kErrorTypeHistogram});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,16 @@ class SharedStorageWorkletDevToolsAgentHostTest
contents()->NavigateAndCommit(GURL("http://www.google.com"));
RenderFrameHost* main_rfh = web_contents()->GetPrimaryMainFrame();

mojo::PendingAssociatedReceiver<blink::mojom::SharedStorageWorkletHost>
worklet_host;

SharedStorageDocumentServiceImpl* document_service =
SharedStorageDocumentServiceImpl::GetOrCreateForCurrentDocument(
main_rfh);
document_service->AddModuleOnWorklet(
document_service->CreateWorklet(
GURL("http://www.google.com/script.js"),
{blink::mojom::OriginTrialFeature::kSharedStorageAPI},
base::DoNothing());
std::move(worklet_host), base::DoNothing());

SharedStorageWorkletHostManager* manager =
GetSharedStorageWorkletHostManagerForStoragePartition(
Expand Down

0 comments on commit 140fde7

Please sign in to comment.