Bug 229512

Summary: Remove unnecessary ITP memory store code
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: New BugsAssignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Kate Cheney
Reported 2021-08-25 13:47:44 PDT
Remove unnecessary ITP memory store code
Attachments
Patch (532.89 KB, patch)
2021-08-25 13:51 PDT, Kate Cheney 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (532.84 KB, patch)
2021-08-25 15:14 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (532.92 KB, patch)
2021-08-25 15:58 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (547.35 KB, patch)
2021-08-26 16:56 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (80.63 KB, patch)
2021-09-17 11:30 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Kate Cheney
Comment 1 2021-08-25 13:51:13 PDT
Created attachment 436420 [details] Patch
Kate Cheney
Comment 2 2021-08-25 15:14:09 PDT
Created attachment 436431 [details] Patch
Kate Cheney
Comment 3 2021-08-25 15:58:28 PDT
Created attachment 436433 [details] Patch
John Wilander
Comment 4 2021-08-26 16:18:22 PDT
Comment on attachment 436433 [details] Patch I didn't review this line-by-line. You're change log description makes sense so I'll just ask a few questions: 1) Did you maintain any relevant commenting in the code you're deleting? We may have insights in there we don't want to lose. 2) Did you delete the obsolete test cases? (Sorry to be so lazy) 3) Did you delete the obsolete TestRunner and test API functionality? 4) Did you search for and change text references to ResourceLoadStatisticsMemoryStore and ResourceLoadStatisticsDatabaseStore? 5) Did you remove "Memory" and "Database" from any remaining TestRunner and test API changes.
Kate Cheney
Comment 5 2021-08-26 16:38:21 PDT
(In reply to John Wilander from comment #4) > Comment on attachment 436433 [details] > Patch > > I didn't review this line-by-line. You're change log description makes sense > so I'll just ask a few questions: > 1) Did you maintain any relevant commenting in the code you're deleting? We > may have insights in there we don't want to lose. Yes, I copy-pasted all existing code. > 2) Did you delete the obsolete test cases? (Sorry to be so lazy) Alex removed most test cases when he deleted the plist in https://bugs.webkit.org/show_bug.cgi?id=217063. I see a couple API tests that were missed, I'll remove those as well. > 3) Did you delete the obsolete TestRunner and test API functionality? Not needed, this was done in https://bugs.webkit.org/show_bug.cgi?id=217063. > 4) Did you search for and change text references to > ResourceLoadStatisticsMemoryStore and ResourceLoadStatisticsDatabaseStore? Yes, big find + replace. > 5) Did you remove "Memory" and "Database" from any remaining TestRunner and > test API changes. No! I need to do this. Thanks for the comments!
Kate Cheney
Comment 6 2021-08-26 16:56:27 PDT
Created attachment 436589 [details] Patch
Radar WebKit Bug Importer
Comment 7 2021-09-01 13:48:25 PDT
<rdar://problem/82644309>
Kate Cheney
Comment 8 2021-09-17 11:30:29 PDT
Created attachment 438493 [details] Patch
John Wilander
Comment 9 2021-09-17 15:46:08 PDT
Comment on attachment 438493 [details] Patch r=me
John Wilander
Comment 10 2021-09-17 15:46:56 PDT
I assume you'll take care of the things we discussed above.
Kate Cheney
Comment 11 2021-09-17 16:05:41 PDT
(In reply to John Wilander from comment #10) > I assume you'll take care of the things we discussed above. Yes, I handled test changes in this patch, and will address everything else (keeping commenting, and changing text references) in the part 2 of this patch.
EWS
Comment 12 2021-09-17 16:46:20 PDT
Committed r282711 (241848@main): <https://commits.webkit.org/241848@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438493 [details].
Note You need to log in before you can comment on or make changes to this bug.