From d2873f8baa05262802e39246e2d1283650e4ba61 Mon Sep 17 00:00:00 2001 From: Daniel Kift Date: Tue, 21 Nov 2023 16:58:59 +0000 Subject: [PATCH 1/4] override on render process gone --- .../com/shopify/checkoutkit/CheckoutDialog.kt | 4 +++- .../com/shopify/checkoutkit/CheckoutWebView.kt | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/com/shopify/checkoutkit/CheckoutDialog.kt b/lib/src/main/java/com/shopify/checkoutkit/CheckoutDialog.kt index e404eb54..4b02a3fd 100644 --- a/lib/src/main/java/com/shopify/checkoutkit/CheckoutDialog.kt +++ b/lib/src/main/java/com/shopify/checkoutkit/CheckoutDialog.kt @@ -107,7 +107,9 @@ internal class CheckoutDialog( setOnCancelListener { checkoutEventProcessor.onCheckoutCanceled() - (checkoutWebView.parent as ViewGroup).removeView(checkoutWebView) + checkoutWebView.parent?.let { + (checkoutWebView.parent as ViewGroup).removeView(checkoutWebView) + } } header.setOnMenuItemClickListener { diff --git a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt index b3184539..74ea234e 100644 --- a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt +++ b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt @@ -26,13 +26,16 @@ import android.annotation.SuppressLint import android.content.Context import android.graphics.Color.TRANSPARENT import android.net.Uri +import android.os.Build import android.os.Handler import android.os.Looper import android.util.AttributeSet import android.view.KeyEvent import android.view.View +import android.view.ViewGroup import android.view.ViewGroup.LayoutParams import android.view.ViewGroup.LayoutParams.MATCH_PARENT +import android.webkit.RenderProcessGoneDetail import android.webkit.WebResourceError import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse @@ -116,6 +119,20 @@ internal class CheckoutWebView(context: Context, attributeSet: AttributeSet? = n checkoutBridge.getEventProcessor().onCheckoutViewLoadComplete() } + override fun onRenderProcessGone(view: WebView, detail: RenderProcessGoneDetail): Boolean { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + if (!detail.didCrash()) { + // Renderer was killed because system ran out of memory. + + // Removing the view from `CheckoutWebViewContainer will trigger a cache clear + // and call webView.destroy() + (view.parent as ViewGroup).removeView(view) + return true + } + } + return super.onRenderProcessGone(view, detail) + } + override fun shouldOverrideUrlLoading( view: WebView?, request: WebResourceRequest? From 67b887f96c4312f76f2b89a63b5caecb8a4ccb67 Mon Sep 17 00:00:00 2001 From: Daniel Kift Date: Wed, 22 Nov 2023 11:17:56 +0000 Subject: [PATCH 2/4] add tests --- .../shopify/checkoutkit/CheckoutWebView.kt | 2 +- .../checkoutkit/CheckoutWebViewClientTest.kt | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt index 74ea234e..f8341727 100644 --- a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt +++ b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt @@ -130,7 +130,7 @@ internal class CheckoutWebView(context: Context, attributeSet: AttributeSet? = n return true } } - return super.onRenderProcessGone(view, detail) + return false } override fun shouldOverrideUrlLoading( diff --git a/lib/src/test/java/com/shopify/checkoutkit/CheckoutWebViewClientTest.kt b/lib/src/test/java/com/shopify/checkoutkit/CheckoutWebViewClientTest.kt index c35a6ba2..1189ebd6 100644 --- a/lib/src/test/java/com/shopify/checkoutkit/CheckoutWebViewClientTest.kt +++ b/lib/src/test/java/com/shopify/checkoutkit/CheckoutWebViewClientTest.kt @@ -23,6 +23,7 @@ package com.shopify.checkoutkit import android.net.Uri +import android.webkit.RenderProcessGoneDetail import android.webkit.WebResourceError import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse @@ -39,6 +40,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.Robolectric import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config @RunWith(RobolectricTestRunner::class) class CheckoutWebViewClientTest { @@ -147,6 +149,47 @@ class CheckoutWebViewClientTest { verify(checkoutWebViewEventProcessor).onCheckoutViewLoadComplete() } + @Test + fun `onRenderProcessGone should return false if sdk version is too low to check detail#didCrash()`() { + val view = viewWithProcessor(activity).withParent() + val webViewClient = view.CheckoutWebViewClient() + val detail = mock() + whenever(detail.didCrash()).thenReturn(false) + + val result = webViewClient.onRenderProcessGone(view, detail) + + assertThat(result).isFalse + assertThat(view.parent).isNotNull + } + + @Config(sdk = [26]) + @Test + fun `onRenderProcessGone should do nothing if the renderer crashed`() { + val view = viewWithProcessor(activity).withParent() + val webViewClient = view.CheckoutWebViewClient() + val detail = mock() + whenever(detail.didCrash()).thenReturn(true) + + val result = webViewClient.onRenderProcessGone(view, detail) + + assertThat(result).isFalse + assertThat(view.parent).isNotNull + } + + @Config(sdk = [26]) + @Test + fun `onRenderProcessGone should remove the view from its parent if the render process crashed due to low memory`() { + val view = viewWithProcessor(activity).withParent() + val webViewClient = view.CheckoutWebViewClient() + val detail = mock() + whenever(detail.didCrash()).thenReturn(false) + + val result = webViewClient.onRenderProcessGone(view, detail) + + assertThat(result).isTrue + assertThat(view.parent).isNull() + } + private fun mockWebRequest(uri: Uri, forMainFrame: Boolean = false): WebResourceRequest { val mockRequest = mock() whenever(mockRequest.url).thenReturn(uri) @@ -181,4 +224,10 @@ class CheckoutWebViewClientTest { whenever(mock.reasonPhrase).thenReturn(description) return mock } + + private fun CheckoutWebView.withParent(): CheckoutWebView { + val container = CheckoutWebViewContainer(activity) + container.addView(this) + return this + } } From 756cbb42fb0851d0bf4348462f4f9705312f6a4f Mon Sep 17 00:00:00 2001 From: Daniel Kift Date: Wed, 22 Nov 2023 13:22:17 +0000 Subject: [PATCH 3/4] single if statement/expression --- .../com/shopify/checkoutkit/CheckoutWebView.kt | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt index f8341727..bccda3b0 100644 --- a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt +++ b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt @@ -120,15 +120,13 @@ internal class CheckoutWebView(context: Context, attributeSet: AttributeSet? = n } override fun onRenderProcessGone(view: WebView, detail: RenderProcessGoneDetail): Boolean { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - if (!detail.didCrash()) { - // Renderer was killed because system ran out of memory. - - // Removing the view from `CheckoutWebViewContainer will trigger a cache clear - // and call webView.destroy() - (view.parent as ViewGroup).removeView(view) - return true - } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !detail.didCrash()) { + // Renderer was killed because system ran out of memory. + + // Removing the view from `CheckoutWebViewContainer will trigger a cache clear + // and call webView.destroy() + (view.parent as ViewGroup).removeView(view) + return true } return false } From a405675ddbf4be8d7546221f53886bf86a0c98c0 Mon Sep 17 00:00:00 2001 From: Daniel Kift Date: Wed, 22 Nov 2023 13:23:37 +0000 Subject: [PATCH 4/4] if expression over statement --- .../main/java/com/shopify/checkoutkit/CheckoutWebView.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt index bccda3b0..ba328e44 100644 --- a/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt +++ b/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt @@ -120,15 +120,16 @@ internal class CheckoutWebView(context: Context, attributeSet: AttributeSet? = n } override fun onRenderProcessGone(view: WebView, detail: RenderProcessGoneDetail): Boolean { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !detail.didCrash()) { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !detail.didCrash()) { // Renderer was killed because system ran out of memory. // Removing the view from `CheckoutWebViewContainer will trigger a cache clear // and call webView.destroy() (view.parent as ViewGroup).removeView(view) - return true + true + } else { + false } - return false } override fun shouldOverrideUrlLoading(