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

Description Kate Cheney 2021-08-25 13:47:44 PDT
Remove unnecessary ITP memory store code
Comment 1 Kate Cheney 2021-08-25 13:51:13 PDT
Created attachment 436420 [details]
Patch
Comment 2 Kate Cheney 2021-08-25 15:14:09 PDT
Created attachment 436431 [details]
Patch
Comment 3 Kate Cheney 2021-08-25 15:58:28 PDT
Created attachment 436433 [details]
Patch
Comment 4 John Wilander 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.
Comment 5 Kate Cheney 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!
Comment 6 Kate Cheney 2021-08-26 16:56:27 PDT
Created attachment 436589 [details]
Patch
Comment 7 Radar WebKit Bug Importer 2021-09-01 13:48:25 PDT
<rdar://problem/82644309>
Comment 8 Kate Cheney 2021-09-17 11:30:29 PDT
Created attachment 438493 [details]
Patch
Comment 9 John Wilander 2021-09-17 15:46:08 PDT
Comment on attachment 438493 [details]
Patch

r=me
Comment 10 John Wilander 2021-09-17 15:46:56 PDT
I assume you'll take care of the things we discussed above.
Comment 11 Kate Cheney 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.
Comment 12 EWS 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].