diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 7b70bf221..de18073e5 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -9,6 +9,7 @@ import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkRequest import android.net.Uri +import android.os.Build import android.os.Bundle import android.os.Handler import android.os.Looper @@ -19,6 +20,7 @@ import android.view.inputmethod.InputMethodManager import android.webkit.ConsoleMessage import android.webkit.CookieManager import android.webkit.JavascriptInterface +import android.webkit.RenderProcessGoneDetail import android.webkit.ValueCallback import android.webkit.WebChromeClient import android.webkit.WebResourceError @@ -30,6 +32,7 @@ import android.webkit.WebViewClient import android.widget.FrameLayout import android.widget.ProgressBar import android.widget.Toast +import androidx.annotation.RequiresApi import androidx.annotation.VisibleForTesting import androidx.webkit.WebViewAssetLoader import androidx.webkit.WebViewAssetLoader.AssetsPathHandler @@ -94,9 +97,28 @@ const val ASSET_PATH_INDEX = "/assets/index.html" * * - If `dependencies` is provided, the editor loads immediately (fast path) * - If `dependencies` is null, dependencies are fetched asynchronously before loading + * + * ## Renderer Crashes + * + * The view recovers when its WebView's renderer process dies, rather than + * letting Android kill the app. An app's WebViews usually share one renderer, + * though, and Android still kills the app if any WebView alive at the time + * leaves `WebViewClient.onRenderProcessGone` unhandled. Hosts must handle it + * in their own WebViews for the app to survive. */ class GutenbergView : FrameLayout { - private val webView: WebView + private var webView: WebView + + /** Whether [webView] replaced one whose renderer died and has yet to load the editor. */ + private var isWebViewBlank = false + + /** + * Whether the editor reloaded on its own after a renderer death, since + * the user last chose to reload it. The editor reports itself loaded + * before its content finishes rendering, so content that kills the + * renderer would otherwise reload forever. + */ + private var hasRecoveredAutomatically = false @Volatile private var isEditorLoaded = false private var didFireEditorLoaded = false @@ -235,6 +257,9 @@ class GutenbergView : FrameLayout { private var logJsExceptionListener: LogJsExceptionListener? = null private var autocompleterTriggeredListener: AutocompleterTriggeredListener? = null private var modalDialogStateListener: ModalDialogStateListener? = null + + /** Modal dialogs the editor reports open, so a renderer death can report them closed. */ + private val openModalDialogs = mutableSetOf() private var networkRequestListener: NetworkRequestListener? = null private var latestContentProvider: LatestContentProvider? = null private var blockInserterDialog: BlockPickerDialog? = null @@ -254,6 +279,9 @@ class GutenbergView : FrameLayout { /** * Provides access to the internal WebView for tests and advanced use cases. + * + * The instance is replaced when its renderer process dies, so read this + * each time rather than holding on to it. */ val editorWebView: WebView get() = webView @@ -349,10 +377,7 @@ class GutenbergView : FrameLayout { .build() // Create the internal WebView as first child (behind overlays) - webView = WebView(context).apply { - layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) - alpha = 0f - } + webView = createWebView() addView(webView) // Create loading overlay views @@ -389,6 +414,11 @@ class GutenbergView : FrameLayout { } } + private fun createWebView(): WebView = WebView(context).apply { + layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) + alpha = 0f + } + /** * Transitions to the progress bar phase (dependency fetching). */ @@ -514,6 +544,13 @@ class GutenbergView : FrameLayout { onEditorPageStarted(url) } + @RequiresApi(Build.VERSION_CODES.O) + override fun onRenderProcessGone(view: WebView?, detail: RenderProcessGoneDetail?): Boolean { + if (view === webView) onEditorRenderProcessGone(detail?.didCrash() == true) + // Unhandled, Android kills the app along with the renderer. + return true + } + override fun shouldInterceptRequest( view: WebView, request: WebResourceRequest @@ -695,6 +732,7 @@ class GutenbergView : FrameLayout { */ private fun loadEditor(dependencies: EditorDependencies) { this.dependencies = dependencies + isWebViewBlank = false // Derive the asset loader authority from the site URL so that the editor // document shares the site's origin, making REST API and AJAX requests @@ -980,16 +1018,17 @@ class GutenbergView : FrameLayout { * [EditorAvailableListener.onEditorAvailable] when the editor crashes before * it finishes loading. * - * The editor's error boundary caught an error and replaced the editor with an - * error message. React unmounted the editor, which deleted every JavaScript - * `editor` API, so calls to them are refused from this point until the editor - * reloads. + * Either the editor's error boundary caught an error and React unmounted the + * editor, or the web view's renderer process died. Both remove every + * JavaScript `editor` API, so calls to them are refused from this point until + * the editor reloads. * - * GutenbergKit covers the editor with a notice offering to reload it. Until it - * reloads, hosts should disable the controls that depend on the editor — - * history, editor mode — while leaving those that read from their own - * persisted copy, such as saving and closing, available. Re-enable them the - * next time [EditorAvailableListener.onEditorAvailable] is called. + * GutenbergKit either reloads the editor automatically after a renderer death + * or covers it with a notice offering to reload it. Until it reloads, hosts + * should disable the controls that depend on the editor — history, editor + * mode — while leaving those that read from their own persisted copy, such as + * saving and closing, available. Re-enable them the next time + * [EditorAvailableListener.onEditorAvailable] is called. */ fun interface EditorUnavailableListener { fun onEditorUnavailable(view: GutenbergView?) @@ -1179,15 +1218,63 @@ class GutenbergView : FrameLayout { */ internal fun reloadEditor() { isEditorLoaded = false + hasRecoveredAutomatically = false // The reload replaces the page these reads were sent to, so their results // may never arrive. failPendingTitleAndContentReads() handler.post { - showSpinnerPhase() - webView.reload() + if (isWebViewBlank) { + loadEditor(dependencies) + } else { + showSpinnerPhase() + webView.reload() + } + } + } + + /** + * The web view's renderer process died, taking the editor with it. + * + * Android can't use a web view again once its renderer dies, so it is + * replaced, and the editor loads in the new one from the content + * [LatestContentProvider] returns. That happens once: a renderer that dies + * before the editor loads, or again after an automatic recovery, may die + * on every load, so the crash notice offers a reload instead of looping. + */ + private fun onEditorRenderProcessGone(didCrash: Boolean) { + Log.e(TAG, if (didCrash) "The editor's renderer crashed" else "The system killed the editor's renderer") + val wasEditorLoaded = isEditorLoaded + isEditorLoaded = false + // The dead renderer can never answer these reads. + failPendingTitleAndContentReads() + replaceWebView() + blockInserterDialog?.dismiss() + // The dead renderer can't unmount the editor's open dialogs to report + // them closed, so report them here rather than leave navigation blocked. + openModalDialogs.forEach { modalDialogStateListener?.onModalDialogClosed(it) } + openModalDialogs.clear() + editorDidBecomeUnavailableListener?.onEditorUnavailable(this) + if (wasEditorLoaded && !hasRecoveredAutomatically) { + hasRecoveredAutomatically = true + loadEditor(dependencies) + } else { + showEditorCrashPhase() } } + private fun replaceWebView() { + val deadWebView = webView + deadWebView.animate().cancel() + removeView(deadWebView) + deadWebView.destroy() + // A file pick still open would answer the dead web view. + filePathCallback = null + + webView = createWebView() + addView(webView, 0) + isWebViewBlank = true + } + @JavascriptInterface fun onEditorContentChanged() { contentChangeListener?.onContentChanged() @@ -1337,6 +1424,7 @@ class GutenbergView : FrameLayout { @JavascriptInterface fun onModalDialogOpened(dialogType: String) { handler.post { + openModalDialogs.add(dialogType) modalDialogStateListener?.onModalDialogOpened(dialogType) } } @@ -1344,6 +1432,7 @@ class GutenbergView : FrameLayout { @JavascriptInterface fun onModalDialogClosed(dialogType: String) { handler.post { + openModalDialogs.remove(dialogType) modalDialogStateListener?.onModalDialogClosed(dialogType) } } diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt new file mode 100644 index 000000000..4dde7561d --- /dev/null +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewRenderProcessGoneTest.kt @@ -0,0 +1,184 @@ +package org.wordpress.gutenberg + +import android.os.Looper +import android.view.View +import android.webkit.RenderProcessGoneDetail +import kotlinx.coroutines.test.TestScope +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotSame +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.Shadows.shadowOf +import org.wordpress.gutenberg.model.EditorConfiguration +import org.wordpress.gutenberg.model.EditorDependencies + +/** How the editor survives its web view's renderer process dying. */ +@RunWith(RobolectricTestRunner::class) +class GutenbergViewRenderProcessGoneTest { + + private val testScope = TestScope() + + private fun siteView() = GutenbergView( + EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/").build(), + EditorDependencies.empty, + testScope, + RuntimeEnvironment.getApplication() + ).also { shadowOf(Looper.getMainLooper()).idle() } + + /** The editor document, mirroring the fallback in `loadEditor`. */ + private val editorUrl = BuildConfig.GUTENBERG_EDITOR_URL + .ifEmpty { "https://example.com/assets/index.html" } + + private fun loadedSiteView() = siteView().also { view -> + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + } + + private fun killRenderer(view: GutenbergView): Boolean { + val detail = mock(RenderProcessGoneDetail::class.java) + `when`(detail.didCrash()).thenReturn(true) + val webView = view.editorWebView + val handled = webView.webViewClient.onRenderProcessGone(webView, detail) + shadowOf(Looper.getMainLooper()).idle() + return handled + } + + @Test + fun `a renderer death is handled so the app survives it`() { + val view = loadedSiteView() + + assertTrue("an unhandled renderer death kills the app", killRenderer(view)) + } + + @Test + fun `a renderer death replaces the web view`() { + val view = loadedSiteView() + val deadWebView = view.editorWebView + + killRenderer(view) + + assertNotSame("a web view can't be used once its renderer dies", deadWebView, view.editorWebView) + assertNull("the dead web view leaves the hierarchy", deadWebView.parent) + assertSame(view, view.editorWebView.parent) + assertEquals("the new web view stays behind the overlays", 0, view.indexOfChild(view.editorWebView)) + } + + @Test + fun `a renderer death fails reads waiting on the dead renderer`() { + val view = loadedSiteView() + val callback = RecordingTitleAndContentCallback() + // Robolectric's shadow WebView never invokes the ValueCallback, like a + // renderer that dies before it answers. + view.getTitleAndContent("original content", callback) + shadowOf(Looper.getMainLooper()).idle() + + killRenderer(view) + + assertTrue("the host is told the editor is not ready", callback.errors.single() is EditorNotReadyException) + } + + @Test + fun `a renderer death tells the host the editor is unavailable`() { + val view = loadedSiteView() + var unavailable = false + view.setEditorDidBecomeUnavailable { unavailable = true } + + killRenderer(view) + + assertTrue(unavailable) + } + + @Test + fun `a renderer death reports the editor's open dialogs closed`() { + val view = loadedSiteView() + val closed = mutableListOf() + view.setModalDialogStateListener(object : GutenbergView.ModalDialogStateListener { + override fun onModalDialogOpened(dialogType: String) = Unit + + override fun onModalDialogClosed(dialogType: String) { + closed += dialogType + } + }) + view.onModalDialogOpened("block-inspector") + view.onModalDialogOpened("block-inserter") + view.onModalDialogClosed("block-inserter") + shadowOf(Looper.getMainLooper()).idle() + closed.clear() + + killRenderer(view) + + assertEquals("the host would keep its navigation disabled", listOf("block-inspector"), closed) + } + + @Test + fun `a renderer death after the editor loaded loads it again`() { + val view = loadedSiteView() + + killRenderer(view) + + assertEquals(editorUrl, shadowOf(view.editorWebView).lastLoadedUrl) + assertEquals(View.VISIBLE, view.editorWebView.visibility) + } + + @Test + fun `a renderer death before the editor loaded offers a reload instead of looping`() { + val view = siteView() + + killRenderer(view) + + assertNull("loading again may kill the renderer again", shadowOf(view.editorWebView).lastLoadedUrl) + assertEquals("the crash notice covers the editor", View.INVISIBLE, view.editorWebView.visibility) + + view.reloadEditor() + shadowOf(Looper.getMainLooper()).idle() + + assertEquals("the blank web view has no page to reload", editorUrl, shadowOf(view.editorWebView).lastLoadedUrl) + } + + @Test + fun `a renderer death after an automatic recovery offers a reload instead of looping`() { + val view = loadedSiteView() + killRenderer(view) + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + + killRenderer(view) + + assertNull("content that kills the renderer once loaded would reload forever", shadowOf(view.editorWebView).lastLoadedUrl) + assertEquals("the crash notice covers the editor", View.INVISIBLE, view.editorWebView.visibility) + } + + @Test + fun `reloading from the crash notice allows another automatic recovery`() { + val view = loadedSiteView() + killRenderer(view) + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + killRenderer(view) + + view.reloadEditor() + shadowOf(Looper.getMainLooper()).idle() + view.onEditorLoaded() + shadowOf(Looper.getMainLooper()).idle() + killRenderer(view) + + assertEquals(editorUrl, shadowOf(view.editorWebView).lastLoadedUrl) + } + + private class RecordingTitleAndContentCallback : GutenbergView.TitleAndContentCallback { + val errors = mutableListOf() + + override fun onResult(title: CharSequence, content: CharSequence) = Unit + + override fun onError(error: Throwable) { + errors += error + } + } +}