Fix memory leaks by cleaning up BrowserViewModel references #1148
Conversation
…maining tab content disappear
| @@ -282,6 +287,8 @@ struct RootView: View { | |||
| // but that's not comming from our window | |||
| return | |||
| } | |||
| windowTracker.current = nil // remove the reference to this window, see guard above | |||
There was a problem hiding this comment.
a very non-obvious problem that was macOS specific
| @@ -290,12 +297,9 @@ struct RootView: View { | |||
| let browser = BrowserViewModel.getCached(tabID: tabID) | |||
| // tab closed by user | |||
| browser.pauseVideoWhenNotInPIP() | |||
| Task { @MainActor [weak browser] in | |||
There was a problem hiding this comment.
this is already called from navigation.deleteTab(tabID: tabID) a few lines below.
| @@ -132,11 +132,24 @@ final class CompactViewController: UIHostingController<AnyView>, UISearchControl | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Fixing by forcing SwiftUI body function switch between a browser based and non browser based views on a higher level, therefore we don't need to reference browserViewModel in the .loading state.
| @@ -168,6 +177,10 @@ final class BrowserViewModel: NSObject, ObservableObject, | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| deinit { | |||
| debugPrint("🧨 BrowserViewModel deinit 🧨") | |||
There was a problem hiding this comment.
leaving this in, to have a clear indicator that the clean up is working, while we work on other features and change the code.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1148 +/- ##
==========================================
- Coverage 25.09% 24.13% -0.96%
==========================================
Files 136 136
Lines 7098 7179 +81
==========================================
- Hits 1781 1733 -48
- Misses 5317 5446 +129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@BPerlakiH Inguess zou have remarked that the CI does not pass? Please ask for review only if the CI pass and keep the PR in draft as long as it does not. |
Fixes: #1016
Many things had to change in order to clean up the BrowserViewModel references, this (rather large object) is holding a reference to the entire WKWebView for each tab, so it's important to clean it up properly once the tab is closed, otherwise we are using more and more memory.
Tested on iPhone, iPad and macOS.