Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional slot-size parameters to trustedBiddingSignalsURL requests #928

Merged
merged 16 commits into from Jan 5, 2024

Conversation

MattMenke2
Copy link
Contributor

@MattMenke2 MattMenke2 commented Nov 22, 2023

This is a slightly modified version of the proposal in issue #869.

This is a slightly modified version of the proposal in issue 869
@MattMenke2
Copy link
Contributor Author

Note that this PR does not yet specify order in the multiple ad-slots case.

@MattMenke2
Copy link
Contributor Author

We could also consider taking a list of options, instead of a mode, which is a bit more extensible. e.g.:

trustedBiddingSignalsExtraParameters: ["slot-size", "all-slots-requested-sizes"]

There's likely no benefit in combining those two, but it allows more easily adding other parameters, if we expect to do so in the future, rather than adding a new field for each option we may want to add.

FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Outdated

The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` (which means they also share an owner).
The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. `trustedBiddingSignalsSlotSizeMode` is one of `none` (which is the default), `slot-size`, and `all-slots-requested-sizes`. In the second case, `&slot-size=<width>,<height>` is appended to the URL, where width and height are the normalized width and height from the `requestedSize` of the top-level auctions AuctionConfig. In the `all-slots-requested-sizes` case, `slotSizes=<width1>,<height1>,<width2>,<height2>,...` is appended, where all sizes are taken from the top-level auction's `allSlotsRequsetedSizes` value. If the corresponding value is not present in the top-level auction, no value is appended. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` and `trustedBiddingSignalsSlotSizeMode` (which means they also share an owner).
Copy link
Contributor

@dmdabbs dmdabbs Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalized width and height from the requestedSize of the top-level auctions AuctionConfig.

To what does normailzed refer?

Nits:

s/auction/auction's
s/allSlotsRequsetedSizes/allSlotsRequestedSizes

Params have been camel cased, so perhaps

&slotSize=<width>,<height>
&slotSizes=<width1>,<height1>,<width2>,<height2>,...

@JensenPaul provided this caveat in the #869 thread:

NOTE: specifying a mode of “thisSlot” or “allSlots” will cause the browser to include the slot size(s) in the trusted bidding signals request for this interest group, which may in turn be coalesced with requests for other interest groups that may not have specified a trustedBiddingSignalsSlotSizeMode.

Verifying that this proposes to alter the coalescing scope (emphasis mine)...

share a trustedBiddingSignalsURL and trustedBiddingSignalsSlotSizeMode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalized width and height from the requestedSize of the top-level auctions AuctionConfig.

To what does normailzed refer?

Sizes are strings with case suffixes, and parsing rules I haven't fully grokked yet. If this allows multiple variants of the same value (e.g., "100 PX", "100px", I don't think we want to preserve the exact details of how the string was formatted, to minimize side channels, and URL variation based on who created the AuctionConfig.

Nits:

s/auction/auction's s/allSlotsRequsetedSizes/allSlotsRequestedSizes

Both fixed.

Params have been camel cased, so perhaps

&slotSize=<width>,<height>
&slotSizes=<width1>,<height1>,<width2>,<height2>,...

Thanks for catching that - also a typo.

@JensenPaul provided this caveat in the #869 thread:

NOTE: specifying a mode of “thisSlot” or “allSlots” will cause the browser to include the slot size(s) in the trusted bidding signals request for this interest group, which may in turn be coalesced with requests for other interest groups that may not have specified a trustedBiddingSignalsSlotSizeMode.

Verifying that this proposes to alter the coalescing scope (emphasis mine)...

share a trustedBiddingSignalsURL and trustedBiddingSignalsSlotSizeMode

I'm concerned about ambiguities here - in the unlikely case one buyer uses single slots for one IG and all slots for others, it would potentially be a coin flip which request we merged with (and would also depend on which other IGs the user has been added to). I'm not really comfortable with that. Particularly if we add more things like this, it just seems like things can get worse and worse. I'd really rather not merge. The only argument I've seen for merging is for the 1-30 day ramp up period when a site starts adding these (if it uses update URLs, can update old IGs pretty quickly), and I think that case is niche enough to not be worth adding more ambiguity to auction behavior.

Copy link
Contributor

@dmdabbs dmdabbs Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about ambiguities here
Particularly if we add more things like this, it just seems like things can get worse and worse.

Yes. Not contesting, just verifying.

and I think that case is niche enough

Not following the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I think that case is niche enough

Not following the case.

The use case is if a buyer starts adding one of these params to its IGs (because it decided it wants to use the feature, either because it was just added, or if a buyer just decides it wants information it didn't need before), there's a ramp up period where some IGs have the new option and some don't, when we aren't merging the trusted signals requests, but doing so would probably be harmless to the buyer, and would improve performance. That seems the main case where merging them would be useful.

FLEDGE.md Outdated Show resolved Hide resolved
FLEDGE.md Show resolved Hide resolved
FLEDGE.md Outdated
'size2': {width: width2, height: height2},
'size3': {width: width3, height: height3},
'size4': {width: width4, height: height4}},
'adSizes': {'size1': {width: 'width1', height: 'height1'},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other size as string thing typos...

Each size has the format {width: widthVal, height: heightVal},
pixel units (e.g. 100 or '100px')
{width: '100sw', height: 50}

Thanks for the reminder re. normalized:

Sizes are strings with case suffixes, and parsing rules I haven't fully grokked yet. If this allows multiple variants of the same value (e.g., "100 PX", "100px", I don't think we want to preserve the exact details of how the string was formatted, to minimize side channels, and URL variation based on who created the AuctionConfig.

Quesitons, perhaps OT from the TBSU parameter, but adjacent...

  • is leading/trailing whitespace removed from sizes (or sizegroup names)
  • what happens when encountering a negative width/height
  • what happens when encountering an unknown width/height suffix ('px,sw,sh')
  • what happens when encountering an inappropriate width or height suffix ('sw,sh')

And specific to the TBSU parameter,

  • pixels are the default (values without suffix are pixels). So will any values specifying px/PX be normalized (either internally [shaves storage bytes]) or in the TBSU param to remove the 'px'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm looking at the regexp our size parser uses, and it looks pretty rigid: R"(^\s*((?:0|(?:[1-9][0-9]*))(?:.[0-9]+)?)(px|sw|sh)?\s*$)".

Looks like the only thing it allows are leading/trailing whitespace, and trailing zeroes. It's case sensitive, does not allow negatives (though does allow 0), allows no whitespace between units, no unrecognized units (so just px, sw, sh, and empty, which is the same as pixels), no leading zeroes / leading "0x", etc. So I think we'll basically just remove leading/trailing whitespace, and potentially trailing 0's and the decimal if there are only zeroes after it. We basically parse the information we can from the value, store that.

When we want a string, we have to serialize that back to a string. We could remember the original string and what we parse it to, but that seems not great in terms of IG size and extra information leakage.

I think if there's demand for it, we could at least remember the "px" vs empty, though that would require a database format update, and we'd have to force pre-existing IGs into one bucket or the other.

Copy link
Contributor

@dmdabbs dmdabbs Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

does not allow negatives (though does allow 0)

What's the use case for a 0 anything banner dimension?

So I think we'll basically just remove ... and potentially trailing 0's and the decimal if there are only zeroes after it.

No legit/practical use for fractional/decimal pixel-specified banner sizes.

We basically parse the information we can from the value, store that.

So you store the extracted string, then.

No, no demand for anything more. Just wanting to understand how it is parsed / normailzed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not allow negatives (though does allow 0)

What's the use case for a 0 anything banner dimension?

None that I'm aware of - I'm just describing what the existing parser does, at least according to my reading of it. Presumably at some point we'll require it to match a valid ad sizes, or at least silently throw away invalid sizes (for better forward compatibility, if new ad sizes are expected to occasionally be added).

Copy link
Contributor

@dmdabbs dmdabbs Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None that I'm aware of - I'm just describing what the existing parser does, at least according to my reading of it.

Appreciate your readout.

Presumably at some point we'll require it to match a valid ad sizes, or at least silently throw away invalid sizes (for better forward compatibility, if new ad sizes are expected to occasionally be added).

Right. In PA there's no notion of 'globally valid' ad sizes. Sure, IAB has standard banner sizes, but in the spec the IG owner determines their creative assets' sizes. And that's fine, since they could have agreements for non-standard sizes with pubs, or use them in O&O cases, &c.

Want to better understand why the browser should permit a top-level seller to embed a 0-anything or fractional dimension banner. That's OT to this new TBS mode param.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now added a description of the two transformations normalized does.

Looking at the code, we have an IsValidAdSize() method that rejects size 0, but we don't actually seem to call it when parsing ad sizes, so I think 0 is actually not allowed, we're just not disallowing it the way we should be (I think we might currently kill a renderer process if we see it? That's not good - anyhow, I'll work on that).

FLEDGE.md Outdated Show resolved Hide resolved
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 29, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 29, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 30, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Switch to component auction.
@MattMenke2
Copy link
Contributor Author

I've switched to using the component auction's sizes, to be a bit more consistent with the requestedSize that's passed to generateBid(). If we want to always use the top-level auction's size instead, we can change both together.

aarongable pushed a commit to chromium/chromium that referenced this pull request Dec 1, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 1, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 1, 2023
This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}
FLEDGE.md Outdated
@@ -597,7 +597,7 @@ Buyers may want to make on-device decisions that take into account real-time dat

https://www.kv-server.example/getvalues?hostname=publisher.com&keys=key1,key2&interestGroupNames=name1,name2&experimentGroupId=12345&slot-size=100,200

The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. `trustedBiddingSignalsSlotSizeMode` is one of `none` (which is the default), `slot-size`, and `all-slots-requested-sizes`. In the second case, `&slotSize=<width>,<height>` is appended to the URL, where width and height are the normalized width and height from the `requestedSize` of the (component) auction's AuctionConfig. In the `all-slots-requested-sizes` case, `&slotSizes=<width1>,<height1>,<width2>,<height2>,...` is appended, where all sizes are taken from the (component) auction's `allSlotsRequestedSizes` value. If the corresponding value is not present in the auction, no value is appended. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` and `trustedBiddingSignalsSlotSizeMode` (which means they also share an owner).
The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. `trustedBiddingSignalsSlotSizeMode` is one of `none` (which is the default), `slot-size`, and `all-slots-requested-sizes`. In the second case, `&slotSize=<width>,<height>` is appended to the URL, where width and height are the normalized width and height from the `requestedSize` of the (component) auction's AuctionConfig. "Normalized" means units are always appended, and trailing zeros are removed, so {width: "0.50sw", height: "10.0" becomes "0.5sw,10px". In the `all-slots-requested-sizes` case, `&slotSizes=<width1>,<height1>,<width2>,<height2>,...` is appended, where all sizes are taken from the (component) auction's `allSlotsRequestedSizes` value. If the corresponding value is not present in the auction, no value is appended. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` and `trustedBiddingSignalsSlotSizeMode` (which means they also share an owner).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adSize section indicates sw/sh are integer not decimal-valued:

For example, the size {width: '100sw', height: 50} describes an ad that is the width of the screen and 50 pixels tall. The size {width: '100sw', height: '200sh'} describes an ad that is the width of the screen and has a 1:2 aspect ratio.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly have no clue what they are. Are they percents as decimals? Percents as numbers? Fenced frame spec doesn't mention them at all. I don't know where to find this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So they're percents out of 100, and the 0.5 was wrong. Apparently that's how vw and vh work in CSS. Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JensenPaul can you shed any light on it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are they percents as decimals? Percents as numbers?

Brought this to mind: They're both!.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 4, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425
@MattMenke2
Copy link
Contributor Author

MattMenke2 commented Dec 5, 2023 via email

jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Dec 5, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 7, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: 6169934ea44403b59b470bbd95bd8077165f3ad9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 7, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: 6169934ea44403b59b470bbd95bd8077165f3ad9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 7, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: 6169934ea44403b59b470bbd95bd8077165f3ad9
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 13, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this pull request Dec 13, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425
FLEDGE.md Outdated

The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` (which means they also share an owner).
The base URL `https://www.kv-server.example/getvalues` comes from the interest group's `trustedBiddingSignalsURL`, the hostname of the top-level webpage where the ad will appear `publisher.com` is provided by the browser, `experimentGroupId` comes from `perBuyerExperimentGroupIds` if provided, `keys` is a list of `trustedBiddingSignalsKeys` strings, and `interestGroupNames` is a list of the names of the interest groups that data is being fetched for. `trustedBiddingSignalsSlotSizeMode` is one of `none` (which is the default), `slot-size`, and `all-slots-requested-sizes`. In the second case, `&slotSize=<width>,<height>` is appended to the URL, where width and height are the normalized width and height from the `requestedSize` of the (component) auction's AuctionConfig. "Normalized" means units are always appended, and trailing zeros are removed, so {width: 62.50sw", height: "10.0" becomes "62.5sw,10px". In the `all-slots-requested-sizes` case, `&slotSizes=<width1>,<height1>,<width2>,<height2>,...` is appended, where all sizes are taken from the (component) auction's `allSlotsRequestedSizes` value. If the corresponding value is not present in the auction, no value is appended. The requests may be coalesced (for efficiency) across any number of interest groups that share a `trustedBiddingSignalsURL` and `trustedBiddingSignalsSlotSizeMode` (which means they also share an owner).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disconnect between the illustrative URL

&slot-size=100,200

and the paragraph

In the second case, &slotSize=<width>,<height> is appended to the URL,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first is a sample URL, rather than a template. The second is telling you the what the values mean. This is really the only place we explicitly mention that width is first (admittedly, the next line demonstrates it is), so I think this is probably the way to go?

Copy link
Contributor

@dmdabbs dmdabbs Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the parameter name disconnect, which in either case is supposed to have fidelity to what Chrome will fetch, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely missed that. You're correct - and this is actually a bug in the current implementation. It should be using camelCase like everything else, but it is not. Thanks so much for catching this before we shipped anything! (Well, mostly - it's enabled on Canary with the wrong value). The all-slots-requested-sizes string also doesn't match the implementation. Anyhow, both issues should be fixed (Though still need to fix Chrome itself)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome. I'll follow the bug you filed.

FLEDGE.md Outdated Show resolved Hide resolved
MattMenke2 added a commit to MattMenke2/turtledove that referenced this pull request Dec 15, 2023
And related auctionConfig field.

This corresponds to the explainer changes in WICG#928.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Dec 16, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: a43d3ba4ddc101fcdfb88e88e50d9332b9021421
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Dec 16, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: a43d3ba4ddc101fcdfb88e88e50d9332b9021421
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Dec 16, 2023
…izeMode to InterestGroups., a=testonly

Automatic update from web-platform-tests
[FLEDGE]:  Add trustedBiddingSignalsSlotSizeMode to InterestGroups.

This CL adds the trustedBiddingSignalsSlotSizeMode to the Protected
Audience API's InterestGroups, and saves them to the database. The
new field currently doesn't have any actual effect.

See WICG/turtledove#928 for API description.

Bug: 1506238
Change-Id: I7133436c813076274822db522ece1a668dba51e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5063314
Commit-Queue: Matt Menke <mmenkechromium.org>
Reviewed-by: Russ Hamilton <behamiltongoogle.com>
Reviewed-by: Ken Buchanan <kenrbchromium.org>
Cr-Commit-Position: refs/heads/main{#1231648}

--

wpt-commits: ba3a0007fbf8711ffeb81df6054327bdc2cc1075
wpt-pr: 43425

UltraBlame original commit: a43d3ba4ddc101fcdfb88e88e50d9332b9021421
FLEDGE.md Outdated
'adSizes': {'size1': {width: '100', height: '100'},
'size2': {width: '100px', height: '200px'},
'size3': {width: '75sw', height: '25sh'},
'size4': {width: '100', height: '25sh'}},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all these different units are a little more distracting than helpful. Maybe just list two ad sizes here, one matching this requestedSize and one not? The available units can go down in the text where you talk about normalization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to trim or rework the sizeGroups section in this CL, as I have no idea what a size group is, and that's really beyond the scope of this PR, so I am not going to trim down the adSizes block.

This PR is also not introducing the ad sizes units, and I'm not really very familiar with them, anyways, so I'm reluctant to dig into them here. I've just removed all the units. I'll leave mucking with them for another PR.

@JensenPaul JensenPaul merged commit cf1c166 into WICG:main Jan 5, 2024
2 checks passed
@dmdabbs
Copy link
Contributor

dmdabbs commented Jan 5, 2024

auctionConfig.requestedSize --> &slotSize
auctionConfig.allSlotsRequestedSizes --> &allSlotsRequestedSizes

You opted not to use &slotSizes or perhaps &allSlotsSizes? Anyway, its shipping so the horse has left the proverbial barn.

github-actions bot added a commit that referenced this pull request Jan 5, 2024
#928)

SHA: cf1c166
Reason: push, by JensenPaul

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
JensenPaul added a commit that referenced this pull request Jan 31, 2024
* Add optional slot-size parameters to trustedBiddingSignalsURL requests

This is a slightly modified version of the proposal in issue 869

* Update FLEDGE.md

Typo

* Update FLEDGE.md

* Update FLEDGE.md

* Update FLEDGE.md

* Update FLEDGE.md

* Update FLEDGE.md

* Update FLEDGE.md

Switch to component auction.

* Update FLEDGE.md

* Update FLEDGE.md

* Update FLEDGE.md

* [spec] Add trustedBiddingSignalsSlotSizeMode

And related auctionConfig field.

This corresponds to the explainer changes in #928.

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Fix compile issues.

* Update spec.bs

* Update spec.bs

* Update spec.bs

* add an extra line to separate </div> from steps

* Partial response to comments.

* Partial changes

* More partial responses

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Fix build?

* Fix merge

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Update spec.bs

* Update spec.bs

Response to Dominic

* Update spec.bs

double->string

* Clean-up ad size conversion

* Update spec.bs

* Update spec.bs

---------

Co-authored-by: Paul Jensen <JensenPaul@users.noreply.github.com>
Co-authored-by: Dominic Farolino <domfarolino@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants