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

John Wilander
Reported 2017-08-09 18:55:27 PDT
We should increase grandfathering time to 3 days to allow ample time to capture user interaction.
Attachments
Patch (1.37 KB, patch)
2017-08-09 18:57 PDT, John Wilander 		no flags
DetailsFormatted DiffDiff
Patch (1.37 KB, patch)
2017-08-10 16:39 PDT, John Wilander 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-08-09 18:56:02 PDT
<rdar://problem/33818046>
John Wilander
Comment 2 2017-08-09 18:57:02 PDT
Created attachment 317773 [details] Patch
John Wilander
Comment 3 2017-08-09 19:51:39 PDT
Comment on attachment 317773 [details] Patch Thanks, Brent!
WebKit Commit Bot
Comment 4 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>
WebKit Commit Bot
Comment 5 2017-08-09 20:21:09 PDT
All reviewed patches have been landed. Closing bug.
Brent Fulgham
Comment 6 2017-08-10 10:36:42 PDT
Reopening because we think this should be three days of uptime, not three calendar days.
Brent Fulgham
Comment 7 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;
Brent Fulgham
Comment 8 2017-08-10 12:20:24 PDT
Never mind. Let's track other changes as a refinement. Re-closing.
John Wilander
Comment 9 2017-08-10 16:34:33 PDT
We're bumping this to seven days. Reopening.
John Wilander
Comment 10 2017-08-10 16:39:43 PDT
Created attachment 317880 [details] Patch
Brent Fulgham
Comment 11 2017-08-10 17:08:27 PDT
Comment on attachment 317880 [details] Patch r=me
Sam Weinig
Comment 12 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?
Brent Fulgham
Comment 13 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.
John Wilander
Comment 14 2017-08-10 18:16:35 PDT
Comment on attachment 317880 [details] Patch Thanks, Brent!
WebKit Commit Bot
Comment 15 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>
WebKit Commit Bot
Comment 16 2017-08-10 18:47:07 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.