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 lookback window #914

Merged
merged 10 commits into from
Aug 3, 2023
Merged

Specify lookback window #914

merged 10 commits into from
Aug 3, 2023

Conversation

agarant
Copy link
Contributor

@agarant agarant commented Jul 25, 2023

Fix #769

Support lookback window by adding an optional reserved keyword "_lookback_window" to trigger's filters.

When set, the field is parsed as a duration in seconds.

To decide if a filter matches, we first check the lookback window. If available, the duration since the source was registered must be smaller or equal to the lookback window duration.

Backward incompatibility: By introducing a new reserved keyword, we break source registrations which are currently using the "_lookback_window" key.


Preview | Diff

@agarant agarant marked this pull request as ready for review July 26, 2023 14:49
Copy link
Collaborator

@apasel422 apasel422 left a comment

Choose a reason for hiding this comment

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

Please reference #769 in the PR description.

Also, could you provide a high-level explanation of what types of operations this supports and how it's used syntactically?

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
Copy link
Collaborator

@apasel422 apasel422 left a comment

Choose a reason for hiding this comment

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

Spec changes LGTM, but I'm withholding any judgment on the compatibility issue.

index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
@agarant agarant merged commit 0ae86dc into WICG:main Aug 3, 2023
1 check passed
@agarant agarant deleted the spec-lookback-window branch August 3, 2023 17:27
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 4, 2023
Support lookback window by adding an optional reserved keyword
`lookback_window` to trigger's filters.

spec: WICG/attribution-reporting-api#914

When set, the field is parsed as a duration in seconds.

To decide if a filter matches, we first check the lookback window. If available, the duration since the source was registered must be smaller or equal to the lookback window duration.

Backward compatibility: by introducing a new reserved keyword, the spec
breaks source registrations which are currently using the `lookback_window` key. To ease migration, we don't yet prevent sources
from using the `lookback_window` key and we process a trigger's
`lookback_window` key as other filter keys when its associated value is
a list of strings.

OBSOLETE_HISTOGRAM[Conversions.SourceRegistrationError3]=Replaced with
Conversions.SourceRegistrationError4 due to addition of new enum value.

Change-Id: Ie87e9613493fa3b00e04567d712f7ef64e193e89
Bug: 1469316,1468973
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727801
Reviewed-by: John Delaney <[email protected]>
Commit-Queue: Anthony Garant <[email protected]>
Reviewed-by: Andrew Paseltiner <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1179638}
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.

Consider having browser provided filters for lookback window
6 participants