Bug 175414

Summary: Resource Load Statistics: Increase grandfathering time to 7 days
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit2Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, sam, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description John Wilander 2017-08-09 18:55:27 PDT 
We should increase grandfathering time to 3 days to allow ample time to capture user interaction.
Comment 1 Radar WebKit Bug Importer 2017-08-09 18:56:02 PDT 
<rdar://problem/33818046>
Comment 2 John Wilander 2017-08-09 18:57:02 PDT 
Created attachment 317773 [details]
Patch
Comment 3 John Wilander 2017-08-09 19:51:39 PDT 
Comment on attachment 317773 [details]
Patch

Thanks, Brent!
Comment 4 WebKit Commit Bot 2017-08-09 20:21:08 PDT 
Comment on attachment 317773 [details]
Patch

Clearing flags on attachment: 317773

Committed r220505: <http://trac.webkit.org/changeset/220505>
Comment 5 WebKit Commit Bot 2017-08-09 20:21:09 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Brent Fulgham 2017-08-10 10:36:42 PDT 
Reopening because we think this should be three days of uptime, not three calendar days.
Comment 7 Brent Fulgham 2017-08-10 10:38:15 PDT 
We should change the grandfathering code to operate based on uptime.

I think this just means modifying WebResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRemoveWebsiteDataFor to use this as the test:

    bool shouldCheckForGrandfathering = m_operatingDates.size() > 3;
Comment 8 Brent Fulgham 2017-08-10 12:20:24 PDT 
Never mind. Let's track other changes as a refinement. Re-closing.
Comment 9 John Wilander 2017-08-10 16:34:33 PDT 
We're bumping this to seven days. Reopening.
Comment 10 John Wilander 2017-08-10 16:39:43 PDT 
Created attachment 317880 [details]
Patch
Comment 11 Brent Fulgham 2017-08-10 17:08:27 PDT 
Comment on attachment 317880 [details]
Patch

r=me
Comment 12 Sam Weinig 2017-08-10 17:16:47 PDT 
Please be aware that changes to the WebKit directory need approval from a WebKit owner.

To that end, how is this change being tested? Can an automated test be written?
Comment 13 Brent Fulgham 2017-08-10 17:25:37 PDT 
(In reply to Sam Weinig from comment #12)
> To that end, how is this change being tested? Can an automated test be
> written?

Existing tests cover this logic. Because of the lengths of time involved, they run with override timeouts that aren't affected by this change to a constant.

If you want to track detailed testing in Safari, please follow the associated Radar.
Comment 14 John Wilander 2017-08-10 18:16:35 PDT 
Comment on attachment 317880 [details]
Patch

Thanks, Brent!
Comment 15 WebKit Commit Bot 2017-08-10 18:47:06 PDT 
Comment on attachment 317880 [details]
Patch

Clearing flags on attachment: 317880

Committed r220571: <http://trac.webkit.org/changeset/220571>
Comment 16 WebKit Commit Bot 2017-08-10 18:47:07 PDT 
All reviewed patches have been landed.  Closing bug.