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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Kate Cheney

Reported 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).

Attachments
Patch (3.20 KB, patch) 2020-01-22 11:32 PST, Kate Cheney	no flags	Details Formatted Diff Diff
Patch (9.06 KB, patch) 2020-01-24 17:41 PST, Kate Cheney	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Kate Cheney

Comment 1 2020-01-22 11:24:31 PST

rdar://problem/58696521

Kate Cheney

Comment 2 2020-01-22 11:32:33 PST

Created attachment 388450 [details] Patch

Alex Christensen

Comment 3 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.

Kate Cheney

Comment 4 2020-01-24 17:41:27 PST

Created attachment 388750 [details] Patch

Kate Cheney

Comment 5 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+.

WebKit Commit Bot

Comment 6 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>

WebKit Commit Bot

Comment 7 2020-01-27 11:16:30 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2020-01-27 11:17:17 PST

<rdar://problem/58926444>

Conrad Shultz

Comment 9 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.

Note You need to log in before you can comment on or make changes to this bug.