Bug 221303 – PCM: earliestTimeToSend should be treated as an independent time value, not relative to timeOfAdClick

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kate Cheney]

Currently the persistent PCM implementation updates earliestTimeToSend after the session is closed. This is not necessary, because earliestTimeToSend is an independent time value and should not need to be updated after a session end/start.

[Radar WebKit Bug Importer]

<rdar://problem/73902668>

[Kate Cheney]

Created attachment 419080 [details]
Patch

[John Wilander]

Comment on attachment 419080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419080&action=review

> Source/WebKit/ChangeLog:14
> +        need any adjustment after a new session.

Just to make sure, we are now going to send the report 24-48 hours after the triggering event, not after the click, right?

> Source/WebKit/ChangeLog:16
> +        No new tests, this will be covered by 

Will be or is?

[Kate Cheney]

Comment on attachment 419080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419080&action=review

>> Source/WebKit/ChangeLog:14
>> +        need any adjustment after a new session.
> 
> Just to make sure, we are now going to send the report 24-48 hours after the triggering event, not after the click, right?

Yes, the time calculation happens when we insert an attributed report into the DB. We call attributeAndGetEarliestTimeToSend() which calculates now + 24-48 hours.

>> Source/WebKit/ChangeLog:16
>> +        No new tests, this will be covered by 
> 
> Will be or is?

Is. This test now marks an attribution as having an earliest time to send of (now - 1hr) and tests that it sends immediately after a session start. I can update the wording.

[John Wilander]

(In reply to katherine_cheney from comment #4)
> Comment on attachment 419080 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419080&action=review
> 
> >> Source/WebKit/ChangeLog:14
> >> +        need any adjustment after a new session.
> > 
> > Just to make sure, we are now going to send the report 24-48 hours after the triggering event, not after the click, right?
> 
> Yes, the time calculation happens when we insert an attributed report into
> the DB. We call attributeAndGetEarliestTimeToSend() which calculates now +
> 24-48 hours.

Good.

> >> Source/WebKit/ChangeLog:16
> >> +        No new tests, this will be covered by 
> > 
> > Will be or is?
> 
> Is. This test now marks an attribution as having an earliest time to send of
> (now - 1hr) and tests that it sends immediately after a session start. I can
> update the wording.

OK, please point out that this patch makes the test check this. Otherwise it sounds like you will have a follow-up patch with a test.

[John Wilander]

Comment on attachment 419080 [details]
Patch

r=me with minor comment on change log.

[Kate Cheney]

Created attachment 419188 [details]
Patch for landing

[EWS]

Committed r272346: <https://trac.webkit.org/changeset/272346>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419188 [details].