Bug 182812

Summary: Resource Load Statistics: Make sure WebResourceLoadStatisticsStore::mergeWithDataFromDecoder() can ingest older plist versions and not reset the database
Product: WebKit Reporter: John Wilander <wilander>
Component: WebKit Misc.Assignee: John Wilander <wilander>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, dbates, ews-watchlist, japhet, jlewis3, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

John Wilander
Reported 2018-02-14 14:53:00 PST
We should not drop existing statistics if we come across a plist with an older statistics model version. Instead we should ingest what we have and continue from there. This will re-introduce logic that we had earlier but got lost in the refactoring. And to be fair, it wasn't really needed since all the older versions where internal and never shipped.
Attachments
Patch (7.49 KB, patch)
2018-02-14 15:02 PST, John Wilander 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.29 KB, patch)
2018-02-15 13:15 PST, John Wilander 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
John Wilander
Comment 1 2018-02-14 14:53:20 PST
rdar://problem/37511406
John Wilander
Comment 2 2018-02-14 15:02:37 PST
Created attachment 333849 [details] Patch
Brent Fulgham
Comment 3 2018-02-14 16:25:47 PST
Comment on attachment 333849 [details] Patch Looks good. r=me.
John Wilander
Comment 4 2018-02-14 16:29:39 PST
Comment on attachment 333849 [details] Patch Thanks, Brent!
WebKit Commit Bot
Comment 5 2018-02-14 16:52:34 PST
Comment on attachment 333849 [details] Patch Clearing flags on attachment: 333849 Committed r228495: <https://trac.webkit.org/changeset/228495>
WebKit Commit Bot
Comment 6 2018-02-14 16:52:36 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 7 2018-02-15 09:11:55 PST
This broke performance tests with unexpected logging: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/236/steps/perf-test/logs/stdio
Matt Lewis
Comment 9 2018-02-15 09:19:27 PST
Reverted r228495 for reason: This caused mulitple perf tests to fail consistently. Committed r228514: <https://trac.webkit.org/changeset/228514>
John Wilander
Comment 10 2018-02-15 11:54:45 PST
(In reply to Alexey Proskuryakov from comment #7) > This broke performance tests with unexpected logging: > https://build.webkit.org/builders/ > Apple%20Sierra%20Release%20WK2%20%28Perf%29/builds/236/steps/perf-test/logs/ > stdio I have no idea why perf tests are ingesting statistics of model 1. Either this is some special statistics plist, a super old test, or a workaround for something else. One way to fix this is to avoid model 1.
John Wilander
Comment 11 2018-02-15 13:15:31 PST
Created attachment 333934 [details] Patch for landing
John Wilander
Comment 12 2018-02-15 13:16:52 PST
Removed logging for model version upgrades all together. It's mostly the early return for skipped ingestion that we need to log. The other one was intended to help with bug investigations since it would tell us if an upgrade had taken place.
WebKit Commit Bot
Comment 13 2018-02-15 13:50:19 PST
Comment on attachment 333934 [details] Patch for landing Clearing flags on attachment: 333934 Committed r228532: <https://trac.webkit.org/changeset/228532>
WebKit Commit Bot
Comment 14 2018-02-15 13:50:21 PST
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.