Skip to content

Commit

Permalink
Reland "[shared storage] Implement sharedStorage.batchUpdate() for PA…
Browse files Browse the repository at this point in the history
… worklet"

This is a reland of commit f0cab8e2346b2a62aa3be1d5a41ec7737bf335af

The original CL incorrectly attempted to access
args[0].As<v8::Object>() even when the initial IDL validation
(args_converter.is_success() && !args[0]->IsObject()) failed. This
could lead to unpredictable behavior as args[0] might not be a valid
object. This reland fixes this by ensuring that
args[0].As<v8::Object>() is only accessed when
args_converter.is_success() is true.

Code changes: see the diff between Patchset 1 and Patchset 2

Original change's description:
> [shared storage] Implement sharedStorage.batchUpdate() for PA worklet
>
> Add sharedStorage.batchUpdate() function. Parse arguments into
> the 'methods' sequence and a 'with_lock' optional flag, and
> propagate the result to the browser process to invoke the
> `SharedStorageLockManager::SharedStorageBatchUpdate()` API.
>
> This allows developers to perform multiple Shared Storage operations atomically within a single lock, as part of the Web
> Lock integration proposal:
> - WICG/shared-storage#199
> - WICG/shared-storage#205
>
> Fuchsia-Binary-Size: Size increase is unavoidable.
> Bug: 373899210
> Change-Id: Ic6e9f794d78523ec9f6b87f37fb5e91f17635c58
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6072850
> Commit-Queue: Yao Xiao <yaoxia@chromium.org>
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Reviewed-by: Cammie Smith Barnes <cammie@chromium.org>
> Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1401673}

Bug: 373899210
Change-Id: I1782e6af1de95cc643053aa80c2a15f601da56a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6132126
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1403952}
  • Loading branch information
Yao Xiao authored and sadym-chromium committed Jan 14, 2025
1 parent aef684a commit c2f99e2
Showing 1 changed file with 102 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// META: script=/resources/testdriver.js
// META: script=/resources/testdriver-vendor.js
// META: script=/common/utils.js
// META: script=/fledge/tentative/resources/fledge-util.sub.js
// META: script=/common/subset-tests.js
// META: script=/shared-storage/resources/util.js
// META: script=/fenced-frame/resources/utils.js
// META: timeout=long

"use strict;"

subsetTest(promise_test, async test => {
let worklet = await sharedStorage.createWorklet('resources/simple-module.js');

const ancestor_key = token();
let url0 = generateURL("/shared-storage/resources/frame0.html",
[ancestor_key]);
let url1 = generateURL("/shared-storage/resources/frame1.html",
[ancestor_key]);

// Override the default resource path, as we are not running within the Fledge
// repository.
RESOURCE_PATH = '/fledge/tentative/resources/';

const pa_uuid = generateUuid(test);

let biddingLogicURL = createBiddingScriptURL(
{
generateBid:
`
sharedStorage.batchUpdate([
new SharedStorageAppendMethod('key', 'a'),
new SharedStorageAppendMethod('key', 'a')
], {withLock: 'lock1'});
return {};
`
});

let decisionLogicURL = createDecisionScriptURL(pa_uuid);

// Invoke `selectURL()` to perform the following steps:
// 1. Acquires the lock.
// 2. Reads the current value at the given key.
// 3. Waits for 500ms.
// 4. Sets the shared storage value to the read value appended with the given letter.
// 5. Releases the lock.
//
// After 100ms, run a Protected Audience auction that starts a worklet that:
// - Acquires the same named lock.
// - Executes two `append` methods, each appending the same letter.
//
// Expected behavior: After both of them finish, the value at the given key
// should contain the letter repeated three times.
//
// This demonstrates that:
// 1. The `withLock` option is effective, preventing the `batchUpdate()`
// method interfering with the "get and set" operation. If the lock were
// not used, the final value would likely be a single letter.
// 2. `batchUpdate()` correctly executes all `append` methods within the
// batch.
//
// Note: This test remains valid even if the `batchUpdate()` call happens
// outside the critical section protected by the lock within the worklet. The
// test effectively demonstrates mutual exclusion as long as there's a
// reasonable chance for `batchUpdate()` to occur while the worklet is still
// running.
let select_url_result = await worklet.selectURL(
"get-wait-set-within-lock",
[{url: url0}, {url: url1}],
{data: {'key': 'key',
'lock_name': 'lock1',
'append_letter': 'a'},
resolveToConfig: true});

// Busy wait for 100ms.
const startWaitTime = Date.now();
while (Date.now() - startWaitTime < 100) {}

// Run a Protected Audience auction which triggers `append()` with the same
// lock and the same letter.
await joinGroupAndRunBasicFledgeTestExpectingNoWinner(
test,
{
uuid: pa_uuid,
interestGroupOverrides: {
name: pa_uuid,
biddingLogicURL: biddingLogicURL,
},
auctionConfigOverrides: {
decisionLogicURL: decisionLogicURL
}
});

attachFencedFrame(select_url_result, 'opaque-ads');
const result = await nextValueFromServer(ancestor_key);
assert_equals(result, "frame1_loaded");

await verifyKeyValueForOrigin('key', 'aaa', location.origin);

await deleteKeyForOrigin('key', location.origin);
}, 'Test for batchUpdate() with a batch lock in a Protected Audience Worklet context');

0 comments on commit c2f99e2

Please sign in to comment.