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

Header-Based DirectFromSellerSignals Feature Detection #803

Open
JacobGo opened this issue Sep 13, 2023 · 6 comments
Open

Header-Based DirectFromSellerSignals Feature Detection #803

JacobGo opened this issue Sep 13, 2023 · 6 comments

Comments

@JacobGo
Copy link
Contributor

JacobGo commented Sep 13, 2023

To run an experiment using the header-based directFromSellerSignals spec proposed in #695, we must detect whether this API is supported to avoid moving auction signals into the header server-side with no corresponding client-side mechanism to retrieve them in the auction worklet.

Ideally the client should have a JS visible mechanism that it may invoke to indicate to the server that it can support parsing the Ad-Auction-Signals header and making it available within the auction worklet by providing directFromSellerSignalsHeaderAdSlot.

@JensenPaul
Copy link
Collaborator

It'd be nice if we had a dedicated mechanism to detect APIs like this, but in the mean time I think this should work:

let dfss = false;
navigator.runAdAuction({get directFromSellerSignalsHeaderAdSlot(){dfss = true;}}).catch((e)=>{});

Seems to run pretty quickly, close to 10μs in a loop. I played with a few different ways, but this seemed fastest for a few reasons:

  1. By having an invalid config (missing seller and decisionLogicURL) it avoids doing an IPC which makes it >10x faster
  2. Avoids await by using Promise.catch
  3. Avoids try {} catch {} by using Promise.catch

@dmdabbs
Copy link
Contributor

dmdabbs commented Sep 14, 2023

Thanks @JensenPaul.

If I grok this, it is relying on an attribute fetch within the auction mechanics (to determine whether the config expects DFSSAS inputs?) that (currently) happens prior to basic auction config attribute validation.

I tried to locate this via the DFSS issue (https://bugs.chromium.org/p/chromium/issues/detail?id=1462720) CRs but could not find them. Would you or @caraitto kindly provide a pointer? Nice source browser & tools, BTW!

If I follow the comments in that issue, DFSSAS impl is landing with M118, which is now in Beta channel. Should your snippet work on that version, perhaps with some extra flag enabled?

@caraitto
Copy link
Collaborator

caraitto commented Sep 14, 2023

@dmdabbs

I tried to locate this via the DFSS issue (https://bugs.chromium.org/p/chromium/issues/detail?id=1462720) CRs but could not find them. Would you or @caraitto kindly provide a pointer? Nice source browser & tools, BTW!

This happens in the generated code produced from WebIDL [0]. The WebIDL definition is here [1], and the code generated from the WebIDL that actually reads the property (and therefore executes the get JavaScript code) is here [2] (it's the Chrome OS version, but it should be basically the same on other platforms -- the generated code isn't checked into git but instead it's produced by the build, but Chromium Code Search still indexes it).

[0] https://developer.mozilla.org/en-US/docs/Glossary/WebIDL

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/ad_auction/auction_ad_config.idl;l=42;drc=05bbf9bc1b9dff6e19ae15092fe095e33208ea13

[2] https://source.chromium.org/chromium/chromium/src/+/main:out/chromeos-Debug/gen/third_party/blink/renderer/bindings/modules/v8/v8_auction_ad_config.cc;l=1042;drc=31fb07c05718d671d96c227855bfe97af9e3fb20

If I follow the comments in that issue, DFSSAS impl is landing with M118, which is now in Beta channel. Should your snippet work on that version, perhaps with some extra flag enabled?

We cherry-picked [0] into Chrome 117.0.5938.35; however, currently the feature FledgeDirectFromSellerSignalsHeaderAdSlot is disabled by default. We are working to get approval on our intent to ship (I2S) [1] so that we can turn the feature flag on.

After we turn the flag on, Chrome browser instances will turn on the feature after a browser restart. Typically there is no user prompting UX to restart like there is when an update is ready to be installed. Chrome channels like Canary, Dev, Beta, and Stable can also be targeted separately at different percentages to provide a smooth, stable rollout.

The feature can be enabled locally using this command line switch [2]:

--enable-features=FledgeDirectFromSellerSignalsHeaderAdSlot (it's a comma-separated list of features)

[0] https://crrev.com/c/4803170

[1] https://groups.google.com/a/chromium.org/g/blink-dev/c/JpWOdoPi5Wo

[2] https://www.chromium.org/developers/how-tos/run-chromium-with-flags/ (also applies to official Chrome builds)

@caraitto
Copy link
Collaborator

By the way, there's an open WebIDL feature request to better support feature detection for IDL dictionary members: whatwg/webidl#107.

@dmdabbs
Copy link
Contributor

dmdabbs commented Sep 14, 2023

Thank you for the quick and detailed responses, Caleb (@caraitto)!

Good that Chrome's FillMembersFromV8Object processes attributes in alpha order.

@caraitto
Copy link
Collaborator

Yeah, we're lucky directFromSellerSignalsHeaderAdSlot comes before seller alphabetically -- otherwise, the seller field would have to be passed to the detector, and we'd need some other way to get the C++ to throw to avoid IPC -- we'll definitely want a better long-term solution :)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2023
I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 16, 2023
I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
aarongable pushed a commit to chromium/chromium that referenced this issue Nov 16, 2023
I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 16, 2023
I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 16, 2023
I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 22, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 22, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this issue Nov 24, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this issue Nov 24, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Nov 30, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraittochromium.org>
Reviewed-by: Matt Menke <mmenkechromium.org>
Cr-Commit-Position: refs/heads/main{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188

UltraBlame original commit: b7a446823cf90ff4f71ea6e1ffb4fa7244d87c0e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Nov 30, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraittochromium.org>
Reviewed-by: Matt Menke <mmenkechromium.org>
Cr-Commit-Position: refs/heads/main{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188

UltraBlame original commit: b7a446823cf90ff4f71ea6e1ffb4fa7244d87c0e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Nov 30, 2023
…=testonly

Automatic update from web-platform-tests
Add tests for feature detection logic

I'll be removing the feature flag in a subsequent CL now that we've
shipped, so I don't test the flag off case.

More context on the feature detection method at
WICG/turtledove#803

Bug: 1462720
Change-Id: Ica63b282e3c380aa09f2ee8a6fc93e89ae9ce21f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5034265
Commit-Queue: Caleb Raitto <caraittochromium.org>
Reviewed-by: Matt Menke <mmenkechromium.org>
Cr-Commit-Position: refs/heads/main{#1225565}

--

wpt-commits: e95e83675b27957f4d0e60dffcb4cc6cedcccb3f
wpt-pr: 43188

UltraBlame original commit: b7a446823cf90ff4f71ea6e1ffb4fa7244d87c0e
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

No branches or pull requests

4 participants