fix(webview): harden SafeWebViewLayout for renderer death after teardown

AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt

@@ -73,7 +73,8 @@ private const val SVG_IMAGE = "image/svg+xml"
 class MultimediaImageFragment :
     MultimediaFragment(R.layout.fragment_multimedia_image),
     OnWebViewRecreatedListener {
-    private val binding by viewBinding(FragmentMultimediaImageBinding::bind)
+    @VisibleForTesting
+    internal val binding by viewBinding(FragmentMultimediaImageBinding::bind)
 
     /** The image on screen, re-rendered if the WebView's render process dies */
     private var previewedImage: Uri? = null

AnkiDroid/src/main/java/com/ichi2/anki/pages/Statistics.kt

@@ -78,7 +78,11 @@ class Statistics : PageFragment(R.layout.page_statistics) {
         val printManager = getSystemService(requireContext(), PrintManager::class.java) ?: return
         val currentDateTime = getTimestamp(TimeManager.time)
         val jobName = "${getString(R.string.app_name)}-stats-$currentDateTime"
-        val printAdapter = webViewLayout.createPrintDocumentAdapter(jobName)
+        val printAdapter =
+            webViewLayout.createPrintDocumentAdapter(jobName) ?: run {
+                Timber.w("Skipping stats PDF export; WebView is destroyed")
+                return
+            }
         pendingPrintJob =
             printManager.print(
                 jobName,

AnkiDroid/src/main/java/com/ichi2/anki/workarounds/SafeWebViewLayout.kt

@@ -3,6 +3,7 @@
 package com.ichi2.anki.workarounds
 
 import android.content.Context
+import android.print.PrintDocumentAdapter
 import android.util.AttributeSet
 import android.view.ViewGroup
 import android.webkit.CookieManager
@@ -11,6 +12,7 @@ import android.webkit.WebSettings
 import android.webkit.WebView
 import android.widget.FrameLayout
 import androidx.annotation.MainThread
+import androidx.core.view.ancestors
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.findFragment
 import com.ichi2.anki.BuildConfig
@@ -25,17 +27,27 @@ open class SafeWebViewLayout :
     constructor(context: Context, attrs: AttributeSet?) : this(context, attrs, 0)
     constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr)
 
+    private enum class WebViewState {
+        ACTIVE,
+        DESTROYED_RECOVERABLE,
+        DESTROYED_TERMINAL,
+    }
+
     private var webView: WebView = createWebView()
 
+    private var webViewState = WebViewState.ACTIVE
+
     var scrollBars: Int = webView.scrollBarStyle
         set(value) {
-            webView.scrollBarStyle = value
             field = value
+            if (warnIfNotActive("scrollBars setter")) return
+            webView.scrollBarStyle = value
         }
 
     @NeedsTest("Verify background color applies to inner WebView")
     override fun setBackgroundColor(color: Int) {
         super.setBackgroundColor(color)
+        if (warnIfNotActive("setBackgroundColor")) return
         webView.setBackgroundColor(color)
     }
 
@@ -45,38 +57,50 @@ open class SafeWebViewLayout :
         addView(webView, webViewLayoutParams)
     }
 
+    // Not guarded when not [WebViewState.ACTIVE]: callers rarely use these after destroy and a no-op is impossible.
     val settings: WebSettings get() = webView.settings
 
     @Suppress("DEPRECATION")
     val scale get() = webView.scale
 
     @MainThread
     fun setWebViewClient(webViewClient: SafeWebViewClient) {
+        if (warnIfNotActive("setWebViewClient")) return
         webViewClient.setOnRenderProcessGoneListener(this)
         webView.webViewClient = webViewClient
     }
 
     @MainThread
     fun setWebChromeClient(webChromeClient: WebChromeClient) {
+        if (warnIfNotActive("setWebChromeClient")) return
         webView.webChromeClient = webChromeClient
     }
 
     @MainThread
     fun evaluateJavascript(
         script: String,
         resultCallback: ((String) -> Unit)? = null,
-    ) = webView.evaluateJavascript(script) { callback ->
-        resultCallback?.invoke(callback)
+    ) {
+        if (warnIfNotActive("evaluateJavascript")) return
+        webView.evaluateJavascript(script) { callback ->
+            resultCallback?.invoke(callback)
+        }
     }
 
     @MainThread
     fun addJavascriptInterface(
         javascriptInterface: Any,
         name: String,
-    ) = webView.addJavascriptInterface(javascriptInterface, name)
+    ) {
+        if (warnIfNotActive("addJavascriptInterface")) return
+        webView.addJavascriptInterface(javascriptInterface, name)
+    }
 
     @MainThread
-    fun loadUrl(url: String) = webView.loadUrl(url)
+    fun loadUrl(url: String) {
+        if (warnIfNotActive("loadUrl")) return
+        webView.loadUrl(url)
+    }
 
     @MainThread
     fun loadDataWithBaseURL(
@@ -85,51 +109,150 @@ open class SafeWebViewLayout :
         mimeType: String?,
         encoding: String?,
         historyUrl: String?,
-    ) = webView.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl)
+    ) {
+        if (warnIfNotActive("loadDataWithBaseURL")) return
+        webView.loadDataWithBaseURL(baseUrl, data, mimeType, encoding, historyUrl)
+    }
 
-    fun setAcceptThirdPartyCookies(accept: Boolean) = CookieManager.getInstance().setAcceptThirdPartyCookies(webView, accept)
+    fun setAcceptThirdPartyCookies(accept: Boolean) {
+        if (warnIfNotActive("setAcceptThirdPartyCookies")) return
+        CookieManager.getInstance().setAcceptThirdPartyCookies(webView, accept)
+    }
 
     @MainThread
-    fun goBack() = webView.goBack()
+    fun goBack() {
+        if (warnIfNotActive("goBack")) return
+        webView.goBack()
+    }
 
     @MainThread
-    fun pageUp() = webView.pageUp(false)
+    fun pageUp(): Boolean {
+        if (warnIfNotActive("pageUp")) return false
+        return webView.pageUp(false)
+    }
 
     @MainThread
-    fun pageDown() = webView.pageDown(false)
+    fun pageDown(): Boolean {
+        if (warnIfNotActive("pageDown")) return false
+        return webView.pageDown(false)
+    }
 
     @MainThread
-    fun reload() = webView.reload()
+    fun reload() {
+        if (warnIfNotActive("reload")) return
+        webView.reload()
+    }
 
     @MainThread
-    fun focusOnWebView() = webView.requestFocus()
+    fun focusOnWebView() {
+        if (warnIfNotActive("focusOnWebView")) return
+        webView.requestFocus()
+    }
 
     @MainThread
-    fun destroy() = webView.destroy()
+    fun destroy() {
+        when (webViewState) {
+            WebViewState.ACTIVE -> webView.destroy()
+            // Crash cleanup already destroyed the native WebView; promote to terminal so reattach
+            // does not recover.
+            WebViewState.DESTROYED_RECOVERABLE -> Unit
+            WebViewState.DESTROYED_TERMINAL -> {
+                Timber.w("destroy called after WebView was destroyed")
+                return
+            }
+        }
+        webViewState = WebViewState.DESTROYED_TERMINAL
+    }
 
     @MainThread
     fun scrollVerticallyBy(y: Int) {
+        if (warnIfNotActive("scrollVerticallyBy")) return
         if (webView.canScrollVertically(y)) {
             webView.scrollBy(0, y)
         }
     }
 
     @MainThread
-    fun createPrintDocumentAdapter(documentName: String) = webView.createPrintDocumentAdapter(documentName)
+    fun createPrintDocumentAdapter(documentName: String): PrintDocumentAdapter? {
+        if (warnIfNotActive("createPrintDocumentAdapter")) return null
+        return webView.createPrintDocumentAdapter(documentName)
+    }
 
-    override fun setOnScrollChangeListener(l: OnScrollChangeListener?) = webView.setOnScrollChangeListener(l)
+    override fun setOnScrollChangeListener(l: OnScrollChangeListener?) {
+        if (warnIfNotActive("setOnScrollChangeListener")) return
+        webView.setOnScrollChangeListener(l)
+    }
 
+    /**
+     * Replaces the terminated inner [WebView] after a render process crash when recreation is possible.
+     *
+     * When recreation is skipped (layout not in a usable fragment/window state), the terminated
+     * [WebView] is destroyed and not replaced; further calls on this layout are guarded until
+     * [onAttachedToWindow] recreates the inner [WebView] from [WebViewState.DESTROYED_RECOVERABLE].
+     */
     override fun onRenderProcessGone(webView: WebView) {
+        if (webView !== this.webView) {
+            destroyWebView(webView)
+            return
+        }
+
+        // Always remove and destroy the terminated WebView first. Android requires this even when
+        // we skip recreation (e.g. fragment view already gone). See:
+        // https://developer.android.com/develop/ui/views/layout/webapps/handle-termination
         removeView(webView)
         webView.destroy()
 
+        val fragment =
+            try {
+                findFragment<Fragment>()
+            } catch (e: IllegalStateException) {
+                Timber.w(e, "skipping WebView recreation; layout is not attached to a Fragment")
+                webViewState = WebViewState.DESTROYED_RECOVERABLE
+                return
+            }
+        if (fragment.view == null) {
+            Timber.w("skipping WebView recreation; fragment view is gone")
+            webViewState = WebViewState.DESTROYED_RECOVERABLE
+            return
+        }
+        if (!isAttachedToWindow) {
+            Timber.w("skipping WebView recreation; layout is not attached to a window")
+            webViewState = WebViewState.DESTROYED_RECOVERABLE
+            return
+        }
+
+        recreateInnerWebView(fragment)
+    }
+
+    private fun recreateInnerWebView(fragment: Fragment) {
+        val previousWebView = this.webView
+        if (previousWebView.parent == this) {
+            removeView(previousWebView)
+        }
         this.webView = createWebView()
+        webViewState = WebViewState.ACTIVE
         addView(this.webView, webViewLayoutParams)
-
-        val fragment = findFragment<Fragment>()
         (fragment as? OnWebViewRecreatedListener)?.onWebViewRecreated(this.webView)
     }
 
+    private fun tryRecoverDestroyedWebViewIfNeeded(fragment: Fragment) {
+        if (webViewState != WebViewState.DESTROYED_RECOVERABLE) return
+        val fragmentView = fragment.view ?: return
+        if (this === fragmentView || ancestors.any { it === fragmentView }) {
+            recreateInnerWebView(fragment)
+        } else {
+            Timber.w("skipping WebView recovery; layout is not in the fragment's current view hierarchy")
+        }
+    }
+
+    private fun warnIfNotActive(methodName: String): Boolean {
+        if (webViewState != WebViewState.ACTIVE) {
+            Timber.w("$methodName called after WebView was destroyed")
+            return true
+        }
+        return false
+    }
+
     override fun onAttachedToWindow() {
         super.onAttachedToWindow()
 
@@ -140,6 +263,10 @@ open class SafeWebViewLayout :
                 // findFragment throws if the View is not attached to a Fragment.
                 // This can happen in scenarios like Android Studio previews
                 // or if the view is added directly to an Activity.
+                if (webViewState == WebViewState.DESTROYED_RECOVERABLE) {
+                    Timber.w(e, "SafeWebViewLayout not attached to a Fragment; skipping WebView recovery")
+                    return
+                }
                 if (BuildConfig.DEBUG && !isInEditMode) {
                     throw IllegalStateException(
                         "SafeWebViewLayout must be used within a Fragment",
@@ -159,7 +286,10 @@ open class SafeWebViewLayout :
             } else {
                 Timber.w("Fragment does not implement OnWebViewRecreatedListener. WebView recreation may not be handled")
             }
+            return
         }
+
+        tryRecoverDestroyedWebViewIfNeeded(fragment)
     }
 
     /**
@@ -169,7 +299,21 @@ open class SafeWebViewLayout :
      */
     @MainThread
     fun safeDestroy() {
-        destroyWebView(webView, this)
+        when (webViewState) {
+            WebViewState.ACTIVE -> {
+                destroyWebView(webView, this)
+                // Mark destroyed even if [destroyWebView] partially failed; using a partially torn-down
+                // WebView is unsafe.
+            }
+            // Crash cleanup already destroyed the native WebView; promote to terminal so reattach
+            // does not recover.
+            WebViewState.DESTROYED_RECOVERABLE -> Unit
+            WebViewState.DESTROYED_TERMINAL -> {
+                Timber.w("safeDestroy called after WebView was destroyed")
+                return
+            }
+        }
+        webViewState = WebViewState.DESTROYED_TERMINAL
     }
 
     companion object {

AnkiDroid/src/test/java/com/ichi2/anki/multimedia/MultimediaImageFragmentTest.kt

@@ -7,11 +7,14 @@ package com.ichi2.anki.multimedia
 import android.annotation.SuppressLint
 import android.content.Intent
 import android.net.Uri
+import android.webkit.WebView
 import androidx.core.net.toUri
 import androidx.core.os.bundleOf
+import androidx.fragment.app.commitNow
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.ichi2.anki.RobolectricTest
 import com.ichi2.anki.multimedia.MultimediaActivity.Companion.EXTRA_MEDIA_OPTIONS
+import com.ichi2.anki.workarounds.SafeWebViewLayout
 import com.ichi2.testutils.launchFragmentInContainer
 import com.ichi2.testutils.withFragment
 import org.hamcrest.MatcherAssert.assertThat
@@ -20,9 +23,13 @@ import org.hamcrest.Matchers.notNullValue
 import org.hamcrest.Matchers.nullValue
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.robolectric.Shadows.shadowOf
 import java.io.File
 
-/** A picked or shared `file://` must only resolve to a file inside our own cache. */
+/**
+ * URI resolution for picked/shared images, and [SafeWebViewLayout] lifecycle regression (issue 21952).
+ * [SafeWebViewLayout.onRenderProcessGone] requires API 26+; Robolectric's default SDK (targetSdk) satisfies that.
+ */
 @RunWith(AndroidJUnit4::class)
 class MultimediaImageFragmentTest : RobolectricTest() {
     @Test
@@ -66,6 +73,23 @@ class MultimediaImageFragmentTest : RobolectricTest() {
         assertThat(PickedImage(Intent().setData(content)).trustedUri, equalTo(content))
     }
 
+    @Test
+    fun `onRenderProcessGone after view is destroyed cleans up the dead WebView - issue 21952`() =
+        withImageFragment {
+            val layout = binding.multimediaWebView
+            val webView = layout.getChildAt(0) as WebView
+            parentFragmentManager.commitNow { detach(this@withImageFragment) }
+            assertThat(view, nullValue())
+
+            layout.onRenderProcessGone(webView)
+
+            assertThat("the dead WebView is destroyed", shadowOf(webView).wasDestroyCalled(), equalTo(true))
+            assertThat("the dead WebView is removed from its parent", webView.parent, nullValue())
+            assertThat("no replacement WebView is created", layout.childCount, equalTo(0))
+
+            layout.safeDestroy()
+        }
+
     private fun withImageFragment(block: MultimediaImageFragment.() -> Unit) =
         launchFragmentInContainer<MultimediaImageFragment>(
             bundleOf(EXTRA_MEDIA_OPTIONS to MultimediaImageFragment.ImageOptions.GALLERY),