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

Make sendReportTo Doc Apply to All report* functions, Add Beacon Info… #967

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thegreatfatzby
Copy link
Contributor

…, Slight Flow Change

Original intention here was to add what I think is a clarification about how the reporting functions send the signal back to the ad tech's servers. As I was digging through I figured a beacon would be used and I thiiink that is supported by slightly-above-cursory-inspection here.

As I went to write the update I thought it made sense to add it to the paragraph I modified, but wanted it to apply across both reportResult and reportWin, and since that paragraph I think should apply to both as well, I think it makes sense to push it up the "stack" a level. Then, given the move, I changed a few things for flow.

Last thing: as I was writing this PR commit, I realized this might be on the side "not a generic spec piece for any browser implementing Fledge, but still relevant for most readers of this doc"...since I don't think we talked more about that yet, I'm just gonna propose this and we can talk about what makes sense.

…, Slight Flow Change

Original intention here was to add what I think is a clarification about how the reporting functions send the signal back to the ad tech's servers. As I was digging through I figured a beacon would be used and I thiiink that is supported by slightly-above-cursory-inspection [here](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/interest_group/interest_group_auction_reporter.cc;l=264?q=reportResult&ss=chromium%2Fchromium%2Fsrc:content%2Fbrowser%2Finterest_group%2F).

As I went to write the update I thought it made sense to add it to the paragraph I modified, but wanted it to apply across both reportResult and reportWin, and since that paragraph I think should apply to both as well, I think it makes sense to push it up the "stack" a level. Then, given the move, I changed a few things for flow.

Last thing: as I was writing this PR commit, I realized this might be on the side "not a generic spec piece for any browser implementing Fledge, but still relevant for most readers of this doc"...since I don't think we talked more about that yet, I'm just gonna propose this and we can talk about what makes sense.
@MattMenke2
Copy link
Contributor

MattMenke2 commented Jan 19, 2024

I'm not sure it's accurate to say the sendReportTo() methods implement the beacon API. While they will outlive the frame if navigated away, like beacon requests, and currently use no-cors mode, beacons are specified to:

  • Be POSTs, but sendReportTo() sends GETs.
  • Use the frame's settings object (our requests use isolated network partitions, since we don't want the frame to sniff what URLs were fetched via probing the HTTP cache).
  • Go through the page's Service Worker, if there is one. These requests don't do that.
  • Set keepalive to true. I think there are associated rules with this for terminating beacons if too much data is received? We don't match that.
  • The "origin" beacons set is a bit confusing here - I think we currently may set this to the frame's origin, but should probably use that of the script that called sendReportTo() instead, at least for bidders.
  • We don't set initiator type to "beacon", though this, at least, we could easily change. No idea if this affects anything.

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.

2 participants