Repository navigation
Conversation
After onRenderProcessGone(), the new destination's onViewCreated() calls navigator.createNewSession() but discards the result, so Navigator.session keeps the gone WebView and every later visit in that navigator renders a blank screen. Recover in place instead of routing a REPLACE visit, which path configuration can turn into no navigation (e.g. clear_all on a root page) and which is dropped while the app is in the background: the destination removes the gone WebView from its view right away, the navigator destroys it and creates a new session, and the destination attaches the new WebView and visits its location again. A destination that didn't get the callback (under a native screen or sheet, or not loaded yet) replaces the session before it attaches the WebView. SwipeRefreshLayout only lays out the first child it finds and never resets it, so HotwireSwipeRefreshLayout now also measures and lays out its current WebView. Otherwise a new WebView attached to an existing destination's view stays 0x0.
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.
Symptoms
After the WebView render process dies (a renderer crash, or the system killing it to reclaim memory, often while the app is in the background), the app doesn't recover. The current screen stays blank, tapping links does nothing, and every tab that had loaded a page shows a blank page or a progress indicator until the activity is recreated (for example, by force stopping the app). All WebViews in an app share one render process, so every
NavigatorHostloses its WebView at the same time.Root cause
SessionsetsisRenderProcessGoneand callsonRenderProcessGone(), which routes aREPLACEvisit to the current location. The new destination'sHotwireWebFragmentDelegate.onViewCreated()then callsnavigator.createNewSession(), but discards the result. Since the session moved toNavigator(01e2e42),createNewSession()returns a newSessioninstead of assigning it, soNavigator.sessionkeeps the gone WebView and every later visit in that navigator goes to it. Each discardedSessionalso creates a WebView that's never destroyed.Assigning the session isn't enough, because the
REPLACEvisit doesn't always create a new destination:presentation: clear_all, which resolves to no navigation at the start destination.FragmentNavigatordrops it (Ignoring navigate() call: FragmentManager has already saved its state), and the existing destination later restarts with the gone WebView.Fix
Recover in place instead of routing a visit:
onRenderProcessGone()removes the gone WebView from the destination's view right away and callsonWebViewDetached()(which clears the bridge delegate), then runs the same path asonStart(): attach the session's WebView, set itsWebChromeClient, and visit the destination's location. It doesn't post this work, so a navigation or the destination's destruction right after the callback can't interleave with it.Navigator.replaceSessionIfRenderProcessGone()removes the gone WebView from its parent, destroys it, and creates a newSession(withBridge.initialize()for the new WebView, as before). This also covers destinations that weren't notified: a page under a native screen or sheet, a page on the backstack, and tabs that weren't loaded yet.SwipeRefreshLayoutonly measures and lays out the first child it ever found and never resets it when that child is removed, so a new WebView attached to an existing destination's view stayed 0x0.HotwireSwipeRefreshLayoutnow also measures and lays out its current WebView. Its internal target still references the destroyed WebView until the view is destroyed.On iOS, when the web content process terminates,
Navigatorreloads the session on the sameWKWebViewif the visitable's view controller has a parent and the app is in the foreground. A session whose process terminated in the background is reloaded the same way when the app returns to the foreground; any other session whose process is found terminated then is recreated with a new web view and a.replacevisit proposal. On Android the WebView can't be reused, so it's always replaced, and the destination reloads its location without routing.Known limitations
NavigatorHost, including tabs that aren't visible, reloads its current page right away (or when the app returns to the foreground). Deferring hidden tabs until they're selected would need a hook when a tab becomes visible.SwipeRefreshLayout's private target (and briefly from the framework's cached child order) until that destination's view is destroyed.destroy()releases its render-side resources, so only the Java object is retained, and it doesn't add up per crash for the same view. In an app with five tabs, a few crashes left up to 13 such objects until those pages were navigated away from or the user signed out. Making a stable wrapper view the layout's target would avoid this, and would also make the measure/layout override unnecessary, at the cost of changinghotwire_view.xml's hierarchy. I'm happy to switch to that if you prefer.navigator.session.webViewoutsideHotwire.config.makeCustomWebView(for example inonNavigatorReady()) aren't applied to the new WebView. turbo-android had anonSessionCreated()hook for this.Testing
Added
NavigatorRenderProcessGoneTest(Robolectric, two navigator hosts, the start location configured withclear_all): the current destination, a host that isn't visible, a pushed page and its backstack, a web page under a web bottom sheet and under a native bottom sheet, the app in the background with and without a sheet open, navigating or going back right after the callback, the activity destroyed right after the callback, and a gone WebView that's still attached when the session is replaced. Reverting any part of the fix fails at least one test../gradlew testReleasepasses on JDK 17.Verified on an emulator (Pixel 7, API 35, WebView 124) with the demo app. On this commit: a pushed page and a web bottom sheet over a web page, each in the foreground and in the background. On the same design before the detach was made synchronous, also these screens: a pushed page (in the foreground and in the background), a bridge component page (the overflow menu works afterwards) with the Navigation tab hidden at its
clear_allroot, a modal page, a web bottom sheet over the native numbers screen, and a web bottom sheet and a native bottom sheet over a web page (in the foreground and in the background; the native sheet is a local-only destination, since the demo has none). Each time the current page reloads, the page under a sheet renders after the sheet is dismissed, every tab works when selected, back navigation works, and logcat shows no crashes or destroyed WebView warnings. DevTools lists the same number of WebViews before and after each crash.Also verified in a production app with five tabs, built against this branch and Don't leak the previous Activity and its WebViews when it is destroyed #221 with its own workarounds removed: crashes on a tab root, on a pushed page, a third crash after recovering, in the background, and with a modal and a bottom sheet open (those two added through a local path configuration). Each time the page reloaded in about 1–2 seconds, every tab rendered, and push, back, pull-to-refresh, the file picker and signing out and in again worked. That run was on this design just before the detach was made synchronous.
To reproduce, run the demo app (debug builds have WebView debugging enabled), then crash the render process over the DevTools protocol:
adb forward tcp:9333 localabstract:webview_devtools_remote_$(adb shell pidof -s dev.hotwire.demo) node crash.mjs