Resource Load Statistics: Hold off non-cookie website data deletion until an hour after user interaction

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[John Wilander]

We should wait with non-cookie website data deletion for cases where there's no previous statistics data such as after a reset.

[John Wilander]

<rdar://problem/58550164>

[John Wilander]

Created attachment 390146 [details]
Patch

[Chris Dumez]

Comment on attachment 390146 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:2283
> +    // remove non-cookie website data.

I think this comment could have been on one line.

[John Wilander]

(In reply to Chris Dumez from comment #3)
> Comment on attachment 390146 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=390146&action=review
> 
> > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:2283
> > +    // remove non-cookie website data.
> 
> I think this comment could have been on one line.

Will fix. Thanks!

[John Wilander]

Created attachment 390147 [details]
Patch

[John Wilander]

Comment on attachment 390147 [details]
Patch

Waiting for EWS feedback before landing.

[John Wilander]

The related test failure on the iOS bot is an output ordering text diff. The actual result doesn’t differ: https://ews-build.webkit.org/results/iOS-13-Simulator-WK2-Tests-EWS/r390147-10350/http/tests/resourceLoadStatistics/log-cross-site-load-with-link-decoration-pretty-diff.html

We already have ongoing work to address flakiness in that test.

[WebKit Commit Bot]

Comment on attachment 390147 [details]
Patch

Clearing flags on attachment: 390147

Committed r256090: <https://trac.webkit.org/changeset/256090>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Antoine Bourlon]

Looking at the code and explanations, this doesn't seem to change the immediate deletion of non-cookie data on sites without previous user interaction, after a browser restart or right after the periodic (hourly) data removal. 
Is this intended behavior?

Steps to reproduce here:
https://github.com/cookie-status/cookie-status-dev/issues/9#issuecomment-567076418

[John Wilander]

(In reply to Antoine Bourlon from comment #10)
> Looking at the code and explanations, this doesn't seem to change the
> immediate deletion of non-cookie data on sites without previous user
> interaction, after a browser restart or right after the periodic (hourly)
> data removal. 
> Is this intended behavior?
> 
> Steps to reproduce here:
> https://github.com/cookie-status/cookie-status-dev/issues/9#issuecomment-
> 567076418

As long as the user has interacted with some website at least an hour ago, sites without user interaction the last seven days of use will have their non-cookie website data deleted. That is the intended behavior. Are you seeing something else?

[Antoine Bourlon]

OK that is what I am seeing. 
Noted that "0 days for sites without interaction" is the (commonly misunderstood?) intended behavior. 

By the way "cases where there's no previous statistics" seem pretty rare in real life. ITP will usually start classifying domains after some user interaction, unless the user accesses multiple sites directly from the address bar/bookmarks, without clicking anything. Testing the "data removal after navigation with link decoration from a classified domain" is especially tricky without interacting with a classified domain :)

Anyway, thanks for the clarification!

[John Wilander]

(In reply to Antoine Bourlon from comment #12)
> OK that is what I am seeing. 
> Noted that "0 days for sites without interaction" is the (commonly
> misunderstood?) intended behavior. 
> 
> By the way "cases where there's no previous statistics" seem pretty rare in
> real life. ITP will usually start classifying domains after some user
> interaction, unless the user accesses multiple sites directly from the
> address bar/bookmarks, without clicking anything. Testing the "data removal
> after navigation with link decoration from a classified domain" is
> especially tricky without interacting with a classified domain :)
> 
> Anyway, thanks for the clarification!

Rare, absolutely. This patch was a refinement for the few cases where it happens such as new device, user clears history, new account on macOS, user has run with ITP off and turns it on for the first time, user installs Safari Technology Preview and runs it …