Move over our integration with Private Aggregation into our spec. #1312
Conversation
cache is remaining.
morlovich
changed the title
morlovich
changed the title
|
FYI @alexmturner |
There was a problem hiding this comment.
do we send PAgg contributions to PAgg host somewhere?
(I'll not pay much attention to those moved algorithms at this moment, since this PR may not be a good place to fix them anyways. Will take another look sometime in the future. Won't let that block this PR).
| [=environment settings object=] |settings|, perform the following steps. They return a failure if | ||
| failing to fetch the script or wasm, otherwise a [=tuple=] of ([=list=] of [=generated bids=], | ||
| [=bid debug reporting info=], [=list=] of [=real time reporting contributions=]). | ||
| |multiBidLimit|, an [=interest group=] |ig|, a [=reporting context=] |reportingContext| and a |
There was a problem hiding this comment.
nit
| |multiBidLimit|, an [=interest group=] |ig|, a [=reporting context=] |reportingContext| and a | |
| |multiBidLimit|, an [=interest group=] |ig|, a [=reporting context=] |reportingContext|, a |
The "process the Private Aggregation contributions for an auction" algorithm eventually calls "Process contributions for a batching scope" from P.Agg. which does stuff like that. The wiring of that got changed quite a bit in this version, though. |
Private aggregation contributions are also only sent to PAgg host after navigated to winning ad if there's one, right? |
Good point (and at end if there is no winner). I guess I need to save leaderInfo for that somehow. Probably just stick it into the reporting context, and then have a helper walk the reportingContextMap and call Process contributions for auction on each one... Need to be careful not to get the reserved.win/non-reserved handling wrong for losing component auctions, though. |
|
https://wicg.github.io/turtledove/#asynchronously-finish-reporting handles other reports after navigation when there's a winner. |
Done, I think. |
There was a problem hiding this comment.
generally LGTM.
I'll take a look at the moved algorithms in the near future when I get more time (I've reviewed most of them when they were added to PAgg spec, so should be fine for now, and this PR is not a good place to fix anything if there is any problem anyways).
| @@ -859,10 +859,13 @@ The <dfn for=Navigator method>runAdAuction(|config|)</dfn> method steps are: | |||
| 1. [=Send report=] with |reportUrl| and |settings|. | |||
| 1. [=Send real time reports=] with |auctionReportInfo|'s | |||
| [=auction report info/real time reporting contributions map=] and |settings|. | |||
| 1. [=Process the Private Aggregation contributions for an auction tree=] given |auctionConfig|, | |||
There was a problem hiding this comment.
nit: is there a better name for this, or simply call it "process private aggregation contributions"? not a fan of introducing another new term for auctions.
| @@ -1050,7 +1054,8 @@ To <dfn>fill in a pending fenced frame config</dfn> given a [=fenced frame confi | |||
| <div algorithm> | |||
|
|
|||
| To <dfn>asynchronously finish reporting</dfn> given a | |||
| [=fencedframetype/fenced frame reporting map=] |reportingMap|, [=leading bid info=] |leadingBidInfo|, | |||
| [=fencedframetype/fenced frame reporting map=] |reportingMap|, [=auction config=] |auctionConfig|, | |||
| [=reporting context map=] |reportingContextMap|, [=leading bid info=] |leadingBidInfo|, | |||
There was a problem hiding this comment.
maybe possible to combine |reportingContextMap| and |auctionReportInfo| in some way. But won't be a request for this PR. We can think about that in the future.
| @@ -3610,6 +3618,13 @@ A <dfn>reporting context</dfn> is a [=struct=] with the following [=struct/items | |||
| contribution entries=]. | |||
| : <dfn>private aggregation allowed</dfn> | |||
| :: A [=boolean=], initially false. | |||
| : <dfn>local leader info</dfn> | |||
| :: A [=leading bid info=], describing information on what, if anything, won this particular | |||
| subauction. | |||
This largely follows how things were done when Private Aggregation was patching us, with a few notable differences:
adding a separate reporting context for top-level so that the batching stuff doesn't get confused. (forDebugOnly
doesn't care, it's just one more context to merge stuff from).
generateBid we don't know that in advance); so we buffer them and only collect them later;
... in a follow up we need to monkey-patch regular contributeToHistogram in a follow so it also doesn't update
instantly.
implement; though still slightly off (the property just... isn't there before the script run).
A lot of stuff about how metrics are evaluated remains hand-wavy. I suspect I'll implement my new metrics first, and then re-build the old ones on top of whatever gets developed.
Preview | Diff