Destroy LilicoWebView when tabs and WebViewActivity tear down#2995
Open
jim-daf wants to merge 1 commit into
Open
Destroy LilicoWebView when tabs and WebViewActivity tear down#2995jim-daf wants to merge 1 commit into
jim-daf wants to merge 1 commit into
Conversation
The browser keeps a list of LilicoWebView instances in BrowserTabs.kt and never calls destroy on them. popBrowserTab only removes the view from its parent and clearBrowserTabs only empties the list. WebViewActivity has the same gap, the WebView is held by the layout for the lifetime of the activity and is dropped on the floor when the user leaves. The chromium-side resources behind a WebView (renderer process, GPU buffers, JS heap, parcel buffers) stay live until destroy is called, so each navigation that creates or rotates a tab leaks memory. The OOM cluster of crash reports against ExploreFragment and the dialogs that hover above the browser are consistent with that leak. The same root cause shows up in the RxJava OutOfMemory wrappers and in the various 16 to 64 byte allocation failures reported from random Parcel and SystemSensor sites. Changes: - Add a private LilicoWebView.destroyWebView extension that runs the standard cleanup sequence (stopLoading, clear callback, about:blank, clearHistory, detach, destroy) inside runCatching. - Call destroyWebView from popBrowserTab after the view is removed from the container. - Call destroyWebView for every tab inside clearBrowserTabs before the list is emptied. - Override onDestroy in WebViewActivity to apply the same cleanup.
There was a problem hiding this comment.
Pull request overview
This PR addresses WebView-backed memory leaks in the in-app dApp browser by ensuring LilicoWebView instances are explicitly torn down when tabs are closed/cleared and when WebViewActivity is destroyed.
Changes:
- Added a
LilicoWebView.destroyWebView()teardown helper inBrowserTabs.ktand invoked it when popping tabs and clearing all tabs. - Updated
clearBrowserTabs()to destroy each tab’s WebView before clearing the global tab list. - Added
onDestroy()cleanup inWebViewActivityto explicitly destroy itsLilicoWebView.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| app/src/main/java/com/flowfoundation/wallet/page/common/WebViewActivity.kt | Adds onDestroy() cleanup steps to stop and destroy the activity-owned LilicoWebView. |
| app/src/main/java/com/flowfoundation/wallet/page/browser/tools/BrowserTabs.kt | Introduces a WebView teardown helper and applies it to tab pop/clear paths to reduce persistent WebView memory usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+119
to
+123
| loadUrl("about:blank") | ||
| clearHistory() | ||
| removeFromParent() | ||
| removeAllViews() | ||
| destroy() |
Comment on lines
+117
to
+120
| stopLoading() | ||
| setWebViewCallback(null) | ||
| loadUrl("about:blank") | ||
| clearHistory() |
| // chromium-side resources (renderer process, GPU buffers, JS heap) stay | ||
| // alive until the GC happens to collect the wrapper, which is a major | ||
| // contributor to the OOM reports filed against ExploreFragment. | ||
| tabs.forEach { runCatching { it.webView.destroyWebView() }.onFailure { loge(it) } } |
Comment on lines
+32
to
+35
| web.stopLoading() | ||
| web.setWebViewCallback(null) | ||
| web.loadUrl("about:blank") | ||
| web.clearHistory() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
BrowserTabs.ktkeeps a process-widemutableListOf<BrowserTab>()where each tab owns aLilicoWebView. Two things happen to that list as the user uses the in-app dApp browser:popBrowserTabremoves the tab entry and detaches the WebView from its parent container, but it never callsdestroy()on the WebView.clearBrowserTabs(invoked fromreleaseBrowserwhen the user leaves the browser, and from a few other places) just runstabs.clear()and does not destroy the WebViews either.WebViewActivityhas the same gap. The activity holds aLilicoWebViewthrough the inflated layout for its full lifetime and never callsdestroyononDestroy.A
WebViewis mostly a thin Java handle on top of a chromium renderer process, a JS heap, GPU buffers, and a pile of native parcel buffers. None of that is freed untildestroy()is called. Until then the GC cannot reclaim the wrapper either, because the platform keeps internal references back into it. So every dApp the user opens leaks a renderer worth of memory, and the leak persists across the user backing out of the browser, switching wallets, or closing tabs.Why this matches the open OOM reports
Several open issues report
OutOfMemoryErrorfrom very different call sites: ExploreFragment, FclSignMessageDialog, EVMSignMessageDialog, generic 16 to 64 byte allocation failures insideParcel.nativeCreate, RxJavaOnErrorNotImplementedExceptionwrappers around OOM, and so on. The common thread is that none of those allocation sites are large by themselves, they only fail when the process heap is already saturated. A leaking WebView per browser session is a very plausible cause of that saturation, and the fact that the failures cluster on devices with a long browser usage session is consistent with that pattern.This PR is not a claim that every one of those reports is fully explained by this single leak, but stopping the leak removes the most obvious systemic source of pressure and should reduce the rate noticeably.
Related reports that should benefit:
Changes
LilicoWebView.destroyWebView()extension inBrowserTabs.kt. It runs the standard cleanup recommended by the Android team insiderunCatching: stop loading, drop the webview callback, navigate toabout:blank, clear history, detach from any parent, drop child views, thendestroy.destroyWebView()frompopBrowserTabafter the existingremoveWebViewcall.destroyWebView()for every tab insideclearBrowserTabsbeforetabs.clear()runs.onDestroyinWebViewActivityto apply the same cleanup, so the standalone activity stops leaking too.