Bug 206601

Summary: Delete the ITP storage file that is not being used (plist or database file) when switching to a new storage type
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, cdumez, commit-queue, conrad_shultz, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Kate Cheney 2020-01-22 11:23:55 PST 
Whichever storage mechanism is enabled, the other persistent file should be deleted (either the plist or the db file).
Comment 1 Kate Cheney 2020-01-22 11:24:31 PST 
rdar://problem/58696521
Comment 2 Kate Cheney 2020-01-22 11:32:33 PST 
Created attachment 388450 [details]
Patch
Comment 3 Alex Christensen 2020-01-24 13:28:59 PST 
Comment on attachment 388450 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:167
> +            auto persistentStorageFilePath = FileSystem::pathByAppendingComponent(resourceLoadStatisticsDirectory, "full_browsing_session_resourceLog.plist");

Let's name this variable "legacyPlistFilePath" to indicate that it is something we are migrating from and intent to remove support for in the future.
Comment 4 Kate Cheney 2020-01-24 17:41:27 PST 
Created attachment 388750 [details]
Patch
Comment 5 Kate Cheney 2020-01-24 17:41:56 PST 
This was causing API test failures, I had to make some changes to the tests in this patch. Alex, I thought you might want to take another look before confirming r+.
Comment 6 WebKit Commit Bot 2020-01-27 11:16:28 PST 
Comment on attachment 388750 [details]
Patch

Clearing flags on attachment: 388750

Committed r255156: <https://trac.webkit.org/changeset/255156>
Comment 7 WebKit Commit Bot 2020-01-27 11:16:30 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2020-01-27 11:17:17 PST 
<rdar://problem/58926444>
Comment 9 Conrad Shultz 2020-02-04 13:09:55 PST 
Comment on attachment 388750 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:168
> +            if (FileSystem::fileExists(legacyPlistFilePath))

It's generally an anti-pattern to check for file-existence. Can we just attempt deletion and handle failure as needed?

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:178
> +            if (FileSystem::fileExists(databaseStorageFilePath)) {

Ditto.