Skip to content

Recover when the WebView render process is gone - #222

Open
jonashan wants to merge 1 commit into
hotwired:mainfrom
jonashan:fix/render-process-gone-session
Open

jonashan wants to merge 1 commit into
hotwired:mainfrom
jonashan:fix/render-process-gone-session

Conversation

@jonashan

Copy link
Copy Markdown

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 NavigatorHost loses its WebView at the same time.

Root cause

Session sets isRenderProcessGone and calls onRenderProcessGone(), which routes a REPLACE visit to the current location. The new destination's HotwireWebFragmentDelegate.onViewCreated() then calls navigator.createNewSession(), but discards the result. Since the session moved to Navigator (01e2e42), createNewSession() returns a new Session instead of assigning it, so Navigator.session keeps the gone WebView and every later visit in that navigator goes to it. Each discarded Session also creates a WebView that's never destroyed.

Assigning the session isn't enough, because the REPLACE visit doesn't always create a new destination:

  • It goes through the path configuration. In the demo app the root page has presentation: clear_all, which resolves to no navigation at the start destination.
  • While the app is in the background, FragmentNavigator drops it (Ignoring navigate() call: FragmentManager has already saved its state), and the existing destination later restarts with the gone WebView.
  • Only the destination that holds the visit callback is notified. A web page under a web or native bottom sheet keeps its view and attaches the shared WebView again when the sheet is dismissed.

Fix

Recover in place instead of routing a visit:

  • onRenderProcessGone() removes the gone WebView from the destination's view right away and calls onWebViewDetached() (which clears the bridge delegate), then runs the same path as onStart(): attach the session's WebView, set its WebChromeClient, 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.
  • Before a destination attaches the WebView, Navigator.replaceSessionIfRenderProcessGone() removes the gone WebView from its parent, destroys it, and creates a new Session (with Bridge.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.
  • In the background, the session is replaced right away, and the visit waits until the destination is started again (as visits already do).

SwipeRefreshLayout only 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. HotwireSwipeRefreshLayout now 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, Navigator reloads the session on the same WKWebView if 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 .replace visit proposal. On Android the WebView can't be reused, so it's always replaced, and the destination reloads its location without routing.

Known limitations

  • Every 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.
  • There's no guard against a page that kills the render process on every load; it reloads each time.
  • A destroyed WebView stays reachable from 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 changing hotwire_view.xml's hierarchy. I'm happy to switch to that if you prefer.
  • Customizations applied to navigator.session.webView outside Hotwire.config.makeCustomWebView (for example in onNavigatorReady()) aren't applied to the new WebView. turbo-android had an onSessionCreated() hook for this.

Testing

  • Added NavigatorRenderProcessGoneTest (Robolectric, two navigator hosts, the start location configured with clear_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 testRelease passes 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_all root, 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
// crash.mjs (Node 22+)
const [page] = (await (await fetch("http://localhost:9333/json")).json()).filter(p => p.type === "page")
const ws = new WebSocket(page.webSocketDebuggerUrl)
ws.onopen = () => {
  ws.send(JSON.stringify({ id: 1, method: "Page.crash" }))
  setTimeout(() => process.exit(), 1000)
}

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant