Bug 204779

Summary: Resource Load Statistics (experimental): Delete non-cookie website data after 7 days of no user interaction
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit Misc.Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, cdumez, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=204858
Attachments:
Description Flags
Patch none

Description John Wilander 2019-12-02 18:53:15 PST
Trackers are continuing to move cross-site tracking IDs into first-party storage. We should age out script-writable non-cookie website data in alignment with the 7-day cap on client-side cookies.
Comment 1 Radar WebKit Bug Importer 2019-12-02 18:53:43 PST
<rdar://problem/57578989>
Comment 2 John Wilander 2019-12-02 19:23:50 PST
Created attachment 384681 [details]
Patch
Comment 3 John Wilander 2019-12-03 05:59:55 PST
The Win test failures are all over the place and seem unrelated.
Comment 4 Alex Christensen 2019-12-03 15:49:26 PST
Comment on attachment 384681 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:488
> +    postTask([this, mode, completionHandler = WTFMove(completionHandler)]() mutable {

Not for this patch, but I don't like it how in this one file it's ok to use this without protecting it or checking a weak pointer, but everywhere else we need to be careful.  Let's make it so postTask doesn't implicitly protect this and require all callers to protect this.
Comment 5 Chris Dumez 2019-12-03 15:51:07 PST
(In reply to Alex Christensen from comment #4)
> Comment on attachment 384681 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384681&action=review
> 
> > Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:488
> > +    postTask([this, mode, completionHandler = WTFMove(completionHandler)]() mutable {
> 
> Not for this patch, but I don't like it how in this one file it's ok to use
> this without protecting it or checking a weak pointer, but everywhere else
> we need to be careful.  Let's make it so postTask doesn't implicitly protect
> this and require all callers to protect this.

Previously it was like that and people kept forgetting to protect this and we had tons of similar crashes. As a result, I made postTask() protect this. Worse case scenario, the caller protects again, which is a bit inefficient but at least does not cause crashes.
Comment 6 John Wilander 2019-12-03 17:57:50 PST
Thanks for the review!
Comment 7 WebKit Commit Bot 2019-12-03 18:41:58 PST
Comment on attachment 384681 [details]
Patch

Clearing flags on attachment: 384681

Committed r253082: <https://trac.webkit.org/changeset/253082>
Comment 8 WebKit Commit Bot 2019-12-03 18:41:59 PST
All reviewed patches have been landed.  Closing bug.