spec: update bid counts and previous wins #638
Conversation
spec.bs
Outdated
| 1. If |config|["{{AuctionAdConfig/signal}}"] [=map/exists=], then: | ||
| 1. Let |signal| be |config|["{{AuctionAdConfig/signal}}"]. | ||
| 1. If |signal| is [=AbortSignal/aborted=], then [=reject=] |p| with |signal|'s | ||
| [=AbortSignal/abort reason=] and return |p|. | ||
| 1. [=AbortSignal/Add|Add the following abort steps=] to |signal|: | ||
| 1. [=Reject=] |p| with |signal|’s [=AbortSignal/abort reason=]. | ||
| 1. TODO: Update bidCount for interest groups that participated in the auction. | ||
| 1. Evaluate [=update bid counts=] with |bidIgs|. |
There was a problem hiding this comment.
At this point bidIgs might still be being mutated by the steps running in parallel, right? Whenever this sort of thing comes up in spec code review I find myself asking how this works in the implementation? Just from reading the spec I'd have to imagine that (1) the browser process is continually sending IPCs to the render process, telling it to update its local copy of bidIgs, and (2) whenever runAdAuction() gets aborted, the renderer calls this update, bid count algorithm with its copy of bidIgs that it currently has at the time.
Is that actually what happens? I feel like it isn't, just because update bid counts is also at least sometimes called "in parallel" by the parallel queue steps below. Is this meant to be called exclusively in parallel? Do we really need this main-thread invocation here?
Separate question: Do we have some sort of cancellation mechanism such that when runAdAuction gets aborted, the in parallel steps know to stop running? Do we need one? It might be good to document this and the effects of it either way in a Note: in the spec.
There was a problem hiding this comment.
bidIgs shouldn't be mutated in parallel. The auction should have completed by now (i.e. either aborted or completed successfully). I don't think we have spec'd a mechanism to actually stop the script runner execution yet in the case of an abort. In the successful case I think it should have have finished previously on line 506 before we use bidIgs on line 531.
SHA: 5e23bfa Reason: push, by JensenPaul Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Preview | Diff