Spec: Allow updating userBiddingSignals. #907
Conversation
spec.bs
Outdated
| <dt>"`userBiddingSignals`" | ||
| <dd> | ||
| 1. If |value| is a [=string=], | ||
| set |ig|'s [=interest group/user bidding signals=] to the result of [=parse a JSON |
There was a problem hiding this comment.
The only problem here is that parse a JSON string to an Infra value expects to be called within a valid ES realm (since it can throw errors, and it operates on JS objects). Unfortunately, this algorithm mostly runs in parallel (while at least one time, it does not). That doesn't seem great, although I realize this algorithm already has this bug from several lines above where we already call this algorithm.
This is in contrast with the Chromium implementation, which uses JSONStringValueSerializer which is vacuumed-off logic isolated from v8 and everything. I believe @jyasskin has raised this issue other times in this specification (maybe looking into a realm-agnostic method of doing the same thing), but I can't find any bugs on this spec, Web IDL, or Infra that represents this. Jeffrey, do you think we could benefit from JSON algorithms that aren't tied to a realm? In the meantime, I'm wondering if we should file a bug on this spec, and tag each instance of this with that bug for tracking.
There was a problem hiding this comment.
Creating a bug for this and noting it everywhere we parse JSON [=in parallel=] SGTM.
There was a problem hiding this comment.
Infra should really let us do this: whatwg/infra#625.
There was a problem hiding this comment.
@jyasskin had a comment about we cannot create JS objects [=in parallel=]. For parsing JSON to infra value, "[=parse a JSON string to an Infra value=] involve creating JS objects also, but because those objects don't escape, it's clear how to implement it without actually creating them".
So we only moved [=parsing JSON bytes to a JavaScript value=] to algorithms that are within single realm, but kept [=parse a JSON string to an Infra value=] in algorithms that run [=in parallel=]. But a "realm-agnostic method" to parse json would be great.
There was a problem hiding this comment.
Thanks for filing that, I couldn't remember if you had already filed something like that or not. OK I think we should make an Issue: block next to this algorithm's usage and point to whatwg/infra#625 from there.
spec.bs
Outdated
| <dt>"`userBiddingSignals`" | ||
| <dd> | ||
| 1. If |value| is a [=string=], | ||
| set |ig|'s [=interest group/user bidding signals=] to the result of [=parse a JSON |
There was a problem hiding this comment.
Creating a bug for this and noting it everywhere we parse JSON [=in parallel=] SGTM.
SHA: ec354d7 Reason: push, by domfarolino Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: ec354d7 Reason: push, by MattMenke2 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Protected Audience now allows an interest group's userBiddingSignals property to be updated
when an interest group update occurs (see change here).
Preview | Diff