Skip to content

Commit

Permalink
[shared storage] rename runXXXOperation() and switch to a single regi…
Browse files Browse the repository at this point in the history
…ster() function

- rename runURLSelectionOperation() to selectURL()
- rename runOperation() to run()
- combine registerXXXOperation into a single register() function:
migrate the `operation_definition_map_` map and registration function
from `UnnamedOperationHandler` and `UrlSelectionOperationHandler` to
`SharedStorageWorkletGlobalScope`, and only keep a reference of the maps
in the two handlers.

This CL updates the web & user facing functions, and will be merged
to M103.

A future CL will update other references (i.e. C++ functions, comments)
in rest of the code base, and that will be in M104 after most M103
targets are done.

Updated explainer:
https://github.com/pythagoraskitty/shared-storage/blob/main/README.md

Bug: 1325112
Change-Id: Id857d35e99b74b40e3f219ac00a36622de002dd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3645133
Reviewed-by: Cammie Smith Barnes <cammie@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003398}
  • Loading branch information
yaoxiachromium authored and Chromium LUCI CQ committed May 13, 2022
1 parent 8dde8bc commit a10fe19
Show file tree
Hide file tree
Showing 30 changed files with 237 additions and 311 deletions.
17 changes: 8 additions & 9 deletions chrome/browser/storage/shared_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,11 @@ class SharedStorageChromeBrowserTest
"/shared_storage/customizable_module.js",
run_function_body_replacement));

// We allow Shared Storage for `addModule()` and `runOperation()`, but any
// operations nested within the script run by `runOperation()` will have
// preferences applied according to test parameters. When the latter
// disallow Shared Storage, it siumlates the situation where preferences
// are updated to block Shared Storage during the course of a previously
// allowed `runOperation()` call.
// We allow Shared Storage for `addModule()` and `run()`, but any operations
// nested within the script run by `run()` will have preferences applied
// according to test parameters. When the latter disallow Shared Storage, it
// siumlates the situation where preferences are updated to block Shared
// Storage during the course of a previously allowed `run()` call.
content::SetBypassIsSharedStorageAllowed(/*allow=*/true);

EXPECT_TRUE(content::ExecJs(
Expand All @@ -214,7 +213,7 @@ class SharedStorageChromeBrowserTest
{last_script_message, content::GetSharedStorageDisabledMessage()}));

content::EvalJsResult result = content::EvalJs(execution_target, R"(
sharedStorage.runOperation('test-operation');
sharedStorage.run('test-operation');
)");

script_console_observer.Wait();
Expand Down Expand Up @@ -291,7 +290,7 @@ IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest, RunOperation) {

content::EvalJsResult run_op_result =
content::EvalJs(GetActiveWebContents(), R"(
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': 'customValue'}});
)");

Expand Down Expand Up @@ -326,7 +325,7 @@ IN_PROC_BROWSER_TEST_P(SharedStorageChromeBrowserTest,

content::EvalJsResult run_url_op_result =
content::EvalJs(GetActiveWebContents(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/shared_storage/customizable_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ class TestOperation {
}
}

registerOperation("test-operation", TestOperation);
register("test-operation", TestOperation);

console.log('Finish executing customizable_module.js')
5 changes: 2 additions & 3 deletions chrome/test/data/shared_storage/simple_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ class TestURLSelectionOperation {
}
}

registerOperation("test-operation", TestOperation);
registerURLSelectionOperation(
"test-url-selection-operation", TestURLSelectionOperation);
register("test-operation", TestOperation);
register("test-url-selection-operation", TestURLSelectionOperation);

console.log('Finish executing simple_module.js')
83 changes: 41 additions & 42 deletions content/browser/shared_storage/shared_storage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,16 @@ class TestSharedStorageWorkletHost : public SharedStorageWorkletHost {
return base::Seconds(30);
}

// How many worklet operations have finished. This only include addModule and
// runOperation.
// How many worklet operations have finished. This only include `addModule()`,
// `selectURL()` and `run()`.
size_t worklet_responses_count_ = 0;
size_t expected_worklet_responses_count_ = 0;
base::RunLoop worklet_responses_count_waiter_;

// Whether we should defer messages received from the worklet environment to
// handle them later. This includes request callbacks (e.g. for addModule()
// and runOperation()), as well as commands initiated from the worklet
// (e.g. console.log()).
// handle them later. This includes request callbacks (e.g. for `addModule()`,
// `selectURL()` and `run()`), as well as commands initiated from the worklet
// (e.g. `console.log()`).
bool should_defer_worklet_messages_;
std::vector<base::OnceClosure> pending_worklet_messages_;

Expand Down Expand Up @@ -329,10 +329,10 @@ class SharedStorageBrowserTest : public ContentBrowserTest {
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

EXPECT_TRUE(ExecJs(execution_target, R"(
sharedStorage.runOperation('test-operation');
sharedStorage.run('test-operation');
)"));

// There are 2 "worklet operations": addModule and runOperation.
// There are 2 "worklet operations": `addModule()` and `run()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -501,11 +501,11 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest, RunOperation_Success) {
base::UTF16ToUTF8(console_observer.messages()[1].message));

EXPECT_TRUE(ExecJs(shell(), R"(
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': 'customValue'}});
)"));

// There are 2 "worklet operations": addModule and runOperation.
// There are 2 "worklet operations": `addModule()` and `run()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand All @@ -527,7 +527,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
WebContentsConsoleObserver console_observer(shell()->web_contents());

EXPECT_TRUE(ExecJs(shell(), R"(
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': 'customValue'}});
)"));

Expand All @@ -538,15 +538,15 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EXPECT_EQ(1u, test_worklet_host_manager().GetAttachedWorkletHostsCount());
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

// There are 2 "worklet operations": runOperation and addModule.
// There are 2 "worklet operations": `run()` and `addModule()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);

EXPECT_EQ(3u, console_observer.messages().size());
EXPECT_EQ(
"sharedStorage.worklet.addModule() has to be called before "
"sharedStorage.runOperation().",
"sharedStorage.run().",
base::UTF16ToUTF8(console_observer.messages()[0].message));
EXPECT_EQ(blink::mojom::ConsoleMessageLevel::kError,
console_observer.messages()[0].log_level);
Expand All @@ -568,7 +568,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EvalJsResult result = EvalJs(shell(), R"(
function testFunction() {}
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': testFunction}});
)");

Expand Down Expand Up @@ -607,11 +607,11 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
console_observer.messages()[0].log_level);

EXPECT_TRUE(ExecJs(shell(), R"(
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': 'customValue'}});
)"));

// There are 2 "worklet operations": addModule and runOperation.
// There are 2 "worklet operations": `addModule()` and `run()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -730,7 +730,7 @@ IN_PROC_BROWSER_TEST_F(
->WaitForWorkletResponsesCount(1);

// Three pending messages are expected: two for console.log and one for
// addModule response.
// `addModule()` response.
EXPECT_EQ(3u, test_worklet_host_manager()
.GetKeepAliveWorkletHost()
->pending_worklet_messages()
Expand Down Expand Up @@ -780,7 +780,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
->WaitForWorkletResponsesCount(1);

// Three pending messages are expected: two for console.log and one for
// addModule response.
// `addModule()` response.
EXPECT_EQ(3u, test_worklet_host_manager()
.GetKeepAliveWorkletHost()
->pending_worklet_messages()
Expand Down Expand Up @@ -814,14 +814,14 @@ IN_PROC_BROWSER_TEST_F(

EXPECT_EQ(2u, console_observer.messages().size());

// Configure the worklet host to defer processing the subsequent runOperation
// Configure the worklet host to defer processing the subsequent `run()`
// response.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->set_should_defer_worklet_messages(true);

EXPECT_TRUE(ExecJs(shell(), R"(
sharedStorage.runOperation(
sharedStorage.run(
'test-operation', {data: {'customKey': 'customValue'}})
)"));

Expand All @@ -836,7 +836,7 @@ IN_PROC_BROWSER_TEST_F(
->WaitForWorkletResponsesCount(2);

// Four pending messages are expected: three for console.log and one for
// runOperation response.
// `run()` response.
EXPECT_EQ(4u, test_worklet_host_manager()
.GetKeepAliveWorkletHost()
->pending_worklet_messages()
Expand Down Expand Up @@ -896,7 +896,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest, KeepAlive_SubframeWorklet) {
->WaitForWorkletResponsesCount(1);

// Three pending messages are expected: two for console.log and one for
// addModule response.
// `addModule()` response.
EXPECT_EQ(3u, test_worklet_host_manager()
.GetKeepAliveWorkletHost()
->pending_worklet_messages()
Expand Down Expand Up @@ -981,7 +981,7 @@ IN_PROC_BROWSER_TEST_F(
base::UTF16ToUTF8(console_observer.messages()[1].message));

std::string urn_uuid = EvalJs(shell(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand All @@ -990,7 +990,7 @@ IN_PROC_BROWSER_TEST_F(

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1065,14 +1065,13 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

EvalJsResult result = EvalJs(fenced_frame_rfh_wrapper.get(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["title0.html"], {data: {'mockResult': 0}});
)");

EXPECT_TRUE(result.error.find("sharedStorage.runURLSelectionOperation() is "
"not allowed in fenced frame") !=
std::string::npos);
EXPECT_TRUE(result.error.find("sharedStorage.selectURL() is not allowed in "
"fenced frame") != std::string::npos);
}

IN_PROC_BROWSER_TEST_F(
Expand All @@ -1089,13 +1088,13 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

// Configure the worklet host to defer processing the subsequent
// runURLSelectionOperation response.
// `selectURL()` response.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->set_should_defer_worklet_messages(true);

std::string urn_uuid = EvalJs(shell(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand All @@ -1104,7 +1103,7 @@ IN_PROC_BROWSER_TEST_F(

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1189,7 +1188,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

std::string urn_uuid = EvalJs(iframe, R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand Down Expand Up @@ -1285,13 +1284,13 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

// Configure the worklet host to defer processing the subsequent
// runURLSelectionOperation response.
// `selectURL()` response.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->set_should_defer_worklet_messages(true);

std::string urn_uuid = EvalJs(iframe, R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand All @@ -1305,7 +1304,7 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(0u, test_worklet_host_manager().GetAttachedWorkletHostsCount());
EXPECT_EQ(1u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetKeepAliveWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1382,7 +1381,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

std::string urn_uuid = EvalJs(shell(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 3}});
Expand All @@ -1391,7 +1390,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1449,15 +1448,15 @@ IN_PROC_BROWSER_TEST_F(
)"));

std::string urn_uuid = EvalJs(shell(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html"], {data: {'mockResult':0}});
)")
.ExtractString();

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1487,15 +1486,15 @@ IN_PROC_BROWSER_TEST_F(
)"));

std::string urn_uuid = EvalJs(shell(), R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html"], {data: {'mockResult':-1}});
)")
.ExtractString();

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down Expand Up @@ -1533,7 +1532,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,
EXPECT_EQ(0u, test_worklet_host_manager().GetKeepAliveWorkletHostsCount());

std::string urn_uuid = EvalJs(iframe, R"(
sharedStorage.runURLSelectionOperation(
sharedStorage.selectURL(
'test-url-selection-operation',
["fenced_frames/title0.html", "fenced_frames/title1.html",
"fenced_frames/title2.html"], {data: {'mockResult': 1}});
Expand All @@ -1542,7 +1541,7 @@ IN_PROC_BROWSER_TEST_F(SharedStorageBrowserTest,

EXPECT_TRUE(blink::IsValidUrnUuidURL(GURL(urn_uuid)));

// There are 2 "worklet operations": addModule and runURLSelectionOperation.
// There are 2 "worklet operations": `addModule()` and `selectURL()`.
test_worklet_host_manager()
.GetAttachedWorkletHost()
->WaitForWorkletResponsesCount(2);
Expand Down
Loading

0 comments on commit a10fe19

Please sign in to comment.