Bug 182812 - Resource Load Statistics: Make sure WebResourceLoadStatisticsStore::mergeWithDataFromDecoder() can ingest older plist versions and not reset the database
Summary: Resource Load Statistics: Make sure WebResourceLoadStatisticsStore::mergeWith...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-14 14:53 PST by John Wilander
Modified: 2018-02-15 13:50 PST (History)
8 users (show)

See Also:

Attachments
Patch (7.49 KB, patch)
2018-02-14 15:02 PST, John Wilander 		no flags Details | Formatted Diff | Diff
Patch for landing (7.29 KB, patch)
2018-02-15 13:15 PST, John Wilander 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 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.
Comment 1 John Wilander 2018-02-14 14:53:20 PST 
rdar://problem/37511406
Comment 2 John Wilander 2018-02-14 15:02:37 PST 
Created attachment 333849 [details]
Patch
Comment 3 Brent Fulgham 2018-02-14 16:25:47 PST 
Comment on attachment 333849 [details]
Patch

Looks good. r=me.
Comment 4 John Wilander 2018-02-14 16:29:39 PST 
Comment on attachment 333849 [details]
Patch

Thanks, Brent!
Comment 5 WebKit Commit Bot 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>
Comment 6 WebKit Commit Bot 2018-02-14 16:52:36 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Alexey Proskuryakov 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
Comment 9 Matt Lewis 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>
Comment 10 John Wilander 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.
Comment 11 John Wilander 2018-02-15 13:15:31 PST 
Created attachment 333934 [details]
Patch for landing
Comment 12 John Wilander 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.
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2018-02-15 13:50:21 PST 
All reviewed patches have been landed.  Closing bug.