We added ephemeral measurement for direct response advertising in https://bugs.webkit.org/show_bug.cgi?id=228984. We added support for same-site triggering events in https://bugs.webkit.org/show_bug.cgi?id=233173. These two features should work together.
<rdar://87423294>
Created attachment 449006 [details] [fast-cq] Patch
Comment on attachment 449006 [details] [fast-cq] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449006&action=review > Source/WebKit/NetworkProcess/NetworkSession.cpp:375 > + if (ephemeralMeasurement.isNeitherSameSiteNorCrossSiteTriggeringEvent(redirectDomain, firstPartyForCookies, attributionTriggerData)) I think the code would read nicer if this were phrased positively, like this: if (!ephemeralMeasurement.isSameSiteOrCrossSiteTriggeringEvent(...)) return;
(In reply to Alex Christensen from comment #3) > Comment on attachment 449006 [details] > [fast-cq] Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449006&action=review > > > Source/WebKit/NetworkProcess/NetworkSession.cpp:375 > > + if (ephemeralMeasurement.isNeitherSameSiteNorCrossSiteTriggeringEvent(redirectDomain, firstPartyForCookies, attributionTriggerData)) > > I think the code would read nicer if this were phrased positively, like this: > if (!ephemeralMeasurement.isSameSiteOrCrossSiteTriggeringEvent(...)) > return; I did consider that, but opted against it because we can't say for sure with the rather shallow analysis we do in that function that it *is* a triggering event. But we can say for sure that it *isn't* a triggering event. Unfortunate for readability but I don't want to take on the risk of someone calling that function and then assuming it's a fully validated triggering event based on the boolean return value. Makes sense?
sounds reasonable
(In reply to John Wilander from comment #4) > (In reply to Alex Christensen from comment #3) > > Comment on attachment 449006 [details] > > [fast-cq] Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=449006&action=review > > > > > Source/WebKit/NetworkProcess/NetworkSession.cpp:375 > > > + if (ephemeralMeasurement.isNeitherSameSiteNorCrossSiteTriggeringEvent(redirectDomain, firstPartyForCookies, attributionTriggerData)) > > > > I think the code would read nicer if this were phrased positively, like this: > > if (!ephemeralMeasurement.isSameSiteOrCrossSiteTriggeringEvent(...)) > > return; > > I did consider that, but opted against it because we can't say for sure with > the rather shallow analysis we do in that function that it *is* a triggering > event. But we can say for sure that it *isn't* a triggering event. > Unfortunate for readability but I don't want to take on the risk of someone > calling that function and then assuming it's a fully validated triggering > event based on the boolean return value. > > Makes sense? I at one point had the function named isLikely… which felt vague and not really what the caller wanted.
win bot test flakiness unrelated.
The mac-AS-debug-wk2 bot is hitting an XPC or process bootstrap crash.
Comment on attachment 449006 [details] [fast-cq] Patch The remaining bots are taking forever to run. Thanks for the review, Alex!
Committed r287970 (245999@main): <https://commits.webkit.org/245999@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449006 [details].