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

Specify trigger-data matching #1064

Merged
merged 5 commits into from
Oct 18, 2023
Merged

Conversation

apasel422
Copy link
Collaborator

@apasel422 apasel422 commented Oct 9, 2023

Followup to #1044


Preview | Diff

Note that no verbose debug report is sent when exact matching fails, but
this could be added later.
index.bs Outdated Show resolved Hide resolved
@apasel422 apasel422 marked this pull request as ready for review October 9, 2023 17:19
Copy link
Collaborator

@johnivdel johnivdel left a comment

Choose a reason for hiding this comment

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

I don't think this change includes the requirement that matching=modulous requires a set of contiguous integers starting at 0. Do you plan to spec that separately?

index.bs Outdated Show resolved Hide resolved
@apasel422
Copy link
Collaborator Author

apasel422 commented Oct 10, 2023

I don't think this change includes the requirement that matching=modulous requires a set of contiguous integers starting at 0. Do you plan to spec that separately?

Because the rest of the Full Flex proposal hasn't been specified yet, there is no way for a source to use non-default trigger data. As written, the spec inherently ensures that the source's allowed trigger data is in the range [0, defaultTriggerDataCardinality).

@apasel422 apasel422 merged commit c69aa36 into WICG:main Oct 18, 2023
2 checks passed
@apasel422 apasel422 deleted the spec-td-matching branch October 18, 2023 18:58
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 23, 2023
WICG/attribution-reporting-api#1064

This is gated at parsing time by a new feature that is disabled by
default.

Note that we do not need a DB migration because the default behavior,
modulus, is retained for existing sources that lack the new field.

We expose the field in the Attribution Internals UI and the
experimental DevTools protocol event for source registrations.

We use a wrapper type in both C++ and Mojo because that encapsulation
will be necessary when the fields from
WICG/attribution-reporting-api#1074 are added
in a followup change.

OBSOLETE_HISTOGRAM[Conversions.CreateReportStatus8]=Replaced by Conversions.CreateReportStatus9
OBSOLETE_HISTOGRAM[Conversions.SentVerboseDebugReportType3]=Replaced by Conversions.SentVerboseDebugReportType4
OBSOLETE_HISTOGRAM[Conversions.SourceRegistrationError5]=Replaced by Conversions.SourceRegistrationError6

Bug: 1493766
Change-Id: Icf5624f42f29f06f6e7bf56162e51fafc71b66ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936470
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1213766}
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.

3 participants