Skip to content

Use consistent set semantics for impression/conversion sites #139

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 1, 2025
Merged

Use consistent set semantics for impression/conversion sites #139

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70964eb
Make impression-side conversion site a set and use it in matching logic
apasel422 Apr 9, 2025
fb89d1e
Treat empty conversion-side impression sites as match any
apasel422 Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions api.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ that the [=user agent=] record an [=impression=] in the [=impression store=].
navigator.privateAttribution.saveImpression({
histogramIndex: 3,
filterData: 2,
conversionSite: "advertiser.example",
conversionSites: ["advertiser.example"],
lifetimeDays: 7,
});
</xmp>
Expand All @@ -574,7 +574,7 @@ that the [=user agent=] record an [=impression=] in the [=impression store=].
dictionary PrivateAttributionImpressionOptions {
required unsigned long histogramIndex;
unsigned long filterData = 0;
required USVString conversionSite;
sequence<USVString> conversionSites = [];
unsigned long lifetimeDays = 30;
};

Expand Down Expand Up @@ -608,12 +608,12 @@ The arguments to <a method for=PrivateAttribution>saveImpression()</a> are as fo
there is a way to match any filter data, which would not be possible if
there were a default integer value.
</dd>
<dt><dfn>conversionSite</dfn></dt>
<dt><dfn>conversionSites</dfn></dt>
<dd>
The site where [=conversions=] for this impression may occur, identified by
its domain name. The <a method for=PrivateAttribution>measureConversion()</a>
method will only attribute to this impression when called by the indicated
site.
The sites where [=conversions=] for this impression may occur, identified by
their domain names. The <a method for=PrivateAttribution>measureConversion()</a>
method will only attribute to this impression when called by one of the indicated
sites. If empty, any site will match.
Comment on lines +613 to +616
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want this to be empty? Is there any situation where it makes sense to avoid specifying this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you'd prefer, we could instead make the empty list an error.

CC @csharrison

Copy link
Member

Choose a reason for hiding this comment

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

How about we take this (which is unambiguously OK), then we can put the question about empty/not to the broader group.

<dt><dfn>lifetimeDays</dfn></dt>
<dd>
A positive "time to live" (in days) after which the [=impression=] can no
Expand Down Expand Up @@ -777,7 +777,7 @@ The arguments to <a method for=PrivateAttribution>measureConversion()</a> are as
<dt><dfn>filterData</dfn></dt>
<dd>Only [=impressions=] having a [=impression/filter data=] value matching this value will be eligible to match this [=conversion=]. If omitted, all [=impression/filter data=] values will match.</dd>
<dt><dfn>impressionSites</dfn></dt>
<dd>A [=set=] of impression sites. Only [=impressions=] recorded where the [=impression site=] is in this set are eligible to match this [=conversion=].</dd>
<dd>A [=set=] of impression sites. Only [=impressions=] recorded where the [=impression site=] is in this set are eligible to match this [=conversion=]. If empty, any site will match.</dd>
<dt><dfn>intermediarySites</dfn></dt>
<dd>
A [=set=] of sites which called the <a method for=PrivateAttribution>saveImpression()</a> API.
Expand Down Expand Up @@ -1163,7 +1163,7 @@ To <dfn>do attribution and fill a histogram</dfn>, given
1. For each |epoch| from |startEpoch| to |currentEpoch|, inclusive:

1. Let |impressions| be the result of invoking [=common matching logic=]
with |options|, |epoch|, and |now|.
with |options|, |topLevelSite|, |epoch|, and |now|.

1. If |impressions| [=set/is empty|is not empty=]:

Expand Down Expand Up @@ -1197,8 +1197,8 @@ To <dfn>create an all-zero histogram</dfn>, given an integer |size|:
### Common Impression Matching Logic ### {#logic-matching}

To perform <dfn>common matching logic</dfn>, given
<a dictionary lt=PrivateAttributionConversionOptions>|options|</a>, [=epoch=] |epoch|, and
[=moment=] |now|:
<a dictionary lt=PrivateAttributionConversionOptions>|options|</a>,
[=site=] |topLevelSite|, [=epoch=] |epoch|, and [=moment=] |now|:

1. Let |matching| be an [=set/is empty|empty=] [=set=].

Expand All @@ -1214,12 +1214,17 @@ To perform <dfn>common matching logic</dfn>, given
1. If |now| - |lookbackDays| is after |impression|'s [=impression/timestamp=],
[=iteration/continue=].

1. If |impression|'s [=impression/conversion sites=] [=set/is empty|is not empty=]
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it impossible to have an empty set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, this PR currently allows there to be an empty set of conversion sites, unless I made a mistake somewhere.

and [=set/contains|does not contain=] |topLevelSite|,
[=iteration/continue=].

1. If |options|.{{PrivateAttributionConversionOptions/filterData}} [=map/exists=],
and it is not equal to |impression|'s [=impression/filter data=],
[=iteration/continue=].

1. If |options|.{{PrivateAttributionConversionOptions/impressionSites}}
does not [=list/contain=] |impression|'s [=impression/impression site=],
[=list/is empty|is not empty=] and
[=list/contains|does not contain=] |impression|'s [=impression/impression site=],
[=iteration/continue=].

1. [=set/Append=] |impression| to |matching|.
Expand Down
Loading