Bug 229512 - Remove unnecessary ITP memory store code
Summary: Remove unnecessary ITP memory store code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-08-25 13:47 PDT by Kate Cheney
Modified: 2021-09-17 16:46 PDT (History)
2 users (show)

See Also:


Attachments
Patch (532.89 KB, patch)
2021-08-25 13:51 PDT, Kate Cheney
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (532.84 KB, patch)
2021-08-25 15:14 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (532.92 KB, patch)
2021-08-25 15:58 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (547.35 KB, patch)
2021-08-26 16:56 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch (80.63 KB, patch)
2021-09-17 11:30 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].