[Kotlin] Prevent repeated checkout presentation crashes

platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutBottomSheet.kt

@@ -35,6 +35,12 @@ internal class CheckoutBottomSheet(
     private var dismissing = false
     private var dismissFinalized = false
 
+    /**
+     * Invoked once when this sheet reaches its terminal dismissal state, before the dialog window
+     * is torn down. Lets the presenter release per-presentation resources on every dismissal path.
+     */
+    internal var onDismissFinalized: (() -> Unit)? = null
+
     /**
      * Inflates, configures, and shows the bottom sheet around shared checkout content.
      *
@@ -202,6 +208,8 @@ internal class CheckoutBottomSheet(
         if (dismissFinalized) return
 
         dismissFinalized = true
+        onDismissFinalized?.invoke()
+        onDismissFinalized = null
         destroyPresentedCheckoutView()
         findViewById<CheckoutBottomSheetLayout>(R.id.checkoutKitSheet)?.onDismissRequested = null
         if (!isShowing) return
@@ -230,7 +238,6 @@ internal class CheckoutBottomSheet(
     private fun destroyPresentedCheckoutView() {
         presentedCheckoutView?.let { checkoutView ->
             log.d(LOG_TAG, "Releasing presented checkout view.")
-            checkoutView.retainPreloadOnDestroy = true
             checkoutView.destroy()
             presentedCheckoutView = null
         }

platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt

@@ -494,9 +494,6 @@ internal class CheckoutWebView private constructor(
             invalidate()
         }
 
-        internal fun retainAfterPresentation(view: CheckoutWebView): Boolean =
-            preloadCache.retainAfterPresentation(view)
-
         internal fun discardAfterPresentation(view: CheckoutWebView) {
             preloadCache.discard(view)
         }

platforms/android/lib/src/main/java/com/shopify/checkoutkit/PreloadCache.kt

@@ -128,28 +128,6 @@ internal class PreloadCache(
         }
     }
 
-    /**
-     * Retains a dismissed view when it is still the cached preload and within its time-to-live.
-     *
-     * @return `true` when the view remains cached; otherwise `false` so the caller can destroy it.
-     */
-    fun retainAfterPresentation(view: CheckoutWebView): Boolean {
-        view.markDismissed()
-        val cached = entry
-        return when {
-            cached?.view !== view -> false
-            !cached.isFresh(clock.elapsedRealtime()) -> {
-                ShopifyCheckoutKit.log.d(LOG_TAG, "Discarding expired preloaded WebView after dismissal.")
-                clearEntry()
-                false
-            }
-            else -> {
-                scheduleExpiry(cached)
-                true
-            }
-        }
-    }
-
     fun discard(view: CheckoutWebView) {
         view.markDismissed()
         val cached = entry

platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckout.kt

@@ -85,7 +85,6 @@ public class ShopifyCheckout @MainThread internal constructor(
     private var destroyed = false
     private var webViewResumed = false
     private var headerBorderIsVisible = false
-    internal var retainPreloadOnDestroy = false
     internal var initializationError: CheckoutException? = null
         private set
 
@@ -165,13 +164,9 @@ public class ShopifyCheckout @MainThread internal constructor(
             pauseWebView()
             webView.clearBottomSheetScrollHandoff()
             webView.removeFromParent()
-            if (retainPreloadOnDestroy && CheckoutWebView.retainAfterPresentation(webView)) {
-                log.d(LOG_TAG, "Retaining preloaded checkout WebView after dismissal.")
-            } else {
-                CheckoutWebView.discardAfterPresentation(webView)
-                log.d(LOG_TAG, "Destroying checkout WebView.")
-                webView.destroy()
-            }
+            CheckoutWebView.discardAfterPresentation(webView)
+            log.d(LOG_TAG, "Destroying checkout WebView.")
+            webView.destroy()
         }
         checkoutWebView = null
     }

platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckoutKit.kt

@@ -13,6 +13,20 @@ public object ShopifyCheckoutKit {
 
     internal val log = LogWrapper()
 
+    /**
+     * A presentation currently tracked as on screen, so repeat [present] calls can be refused.
+     */
+    private class LivePresentation(
+        private val sheet: CheckoutBottomSheet,
+        val handle: CheckoutHandle,
+    ) {
+        fun isShowing(): Boolean = sheet.isShowing
+
+        fun tracks(other: CheckoutBottomSheet): Boolean = sheet === other
+    }
+
+    private val livePresentations = mutableMapOf<ComponentActivity, LivePresentation>()
+
     /**
      * Returns the current version of ShopifyCheckoutKit.
 
@@ -203,6 +217,31 @@ public object ShopifyCheckoutKit {
             return null
         }
 
+        val alreadyPresented = livePresentations[context]?.takeIf { it.isShowing() }
+        if (alreadyPresented != null) {
+            log.w("ShopifyCheckoutKit", "A checkout is already presented, ignoring this presentation.")
+        }
+        return alreadyPresented?.handle ?: startPresentation(
+            checkoutUrl = checkoutUrl,
+            context = context,
+            checkoutListener = checkoutListener,
+            protocolClient = protocolClient,
+            webMessageTransport = webMessageTransport,
+        )
+    }
+
+    /**
+     * Builds, starts, and tracks a new bottom-sheet presentation.
+     *
+     * Called only once the activity is usable and no checkout is already on screen for it.
+     */
+    private fun <T : DefaultCheckoutListener> startPresentation(
+        checkoutUrl: String,
+        context: ComponentActivity,
+        checkoutListener: T,
+        protocolClient: CheckoutProtocol.Client?,
+        webMessageTransport: WebMessageTransport,
+    ): CheckoutHandle? {
         log.d("ShopifyCheckoutKit", "Constructing bottom sheet")
         val checkout = CheckoutBottomSheet(
             checkoutUrl = checkoutUrl,
@@ -224,8 +263,18 @@ public object ShopifyCheckoutKit {
         val checkoutStarted = checkout.start()
         if (!checkoutStarted) {
             context.lifecycle.removeObserver(lifecycleObserver)
+            return null
+        }
+
+        val handle = CheckoutHandle { checkout.dismiss() }
+        livePresentations[context] = LivePresentation(sheet = checkout, handle = handle)
+        checkout.onDismissFinalized = {
+            context.lifecycle.removeObserver(lifecycleObserver)
+            if (livePresentations[context]?.tracks(checkout) == true) {
+                livePresentations.remove(context)
+            }
         }
-        return if (checkoutStarted) CheckoutHandle { checkout.dismiss() } else null
+        return handle
     }
 }
 

platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutBottomSheetTest.kt

@@ -538,7 +538,7 @@ class CheckoutBottomSheetTest {
     }
 
     @Test
-    fun `lifecycle failure retains preloaded checkoutView after sheet dismissal`() {
+    fun `lifecycle failure destroys preloaded checkoutView after sheet dismissal`() {
         val listener = mock<DefaultCheckoutListener>()
         CheckoutWebView.preload("https://shopify.com/cart/123", activity, webMessageTransport)
         ShadowLooper.shadowMainLooper().idle()
@@ -549,8 +549,8 @@ class CheckoutBottomSheetTest {
         webView.listener.onCheckoutViewFailedWithError(checkoutException())
         runDismissAnimation()
 
-        assertThat(shadowOf(cachedWebView).wasDestroyCalled()).isFalse()
-        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isSameAs(cachedWebView)
+        assertThat(shadowOf(cachedWebView).wasDestroyCalled()).isTrue()
+        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isNull()
     }
 
     @Test
@@ -592,43 +592,7 @@ class CheckoutBottomSheetTest {
     }
 
     @Test
-    fun `dismiss() retains preloaded checkoutView for another presentation`() {
-        CheckoutWebView.preload("https://shopify.com/cart/123", activity, webMessageTransport)
-        ShadowLooper.shadowMainLooper().idle()
-        val cachedWebView = CheckoutWebView.cachedPreloadViewForTesting()!!
-
-        val sheet = presentBottomSheet("https://shopify.com/cart/123")
-        ShadowLooper.shadowMainLooper().idle()
-
-        val webView = sheet.currentCheckoutWebView()
-        assertThat(webView).isSameAs(cachedWebView)
-        webView.scrollTo(0, 10)
-
-        sheet.dismiss()
-        runDismissAnimation()
-
-        assertThat(sheet.containsChildOfType(CheckoutWebView::class.java)).isFalse()
-        assertThat(shadowOf(cachedWebView).wasDestroyCalled()).isFalse()
-        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isSameAs(cachedWebView)
-        assertThat(shadowOf(cachedWebView).getOnTouchListener()).isNull()
-
-        val nextSheet = presentBottomSheet("https://shopify.com/cart/123")
-        ShadowLooper.shadowMainLooper().idle()
-
-        assertThat(nextSheet.currentCheckoutWebView()).isSameAs(cachedWebView)
-        assertThat(cachedWebView.isPresented).isTrue()
-        assertThat(shadowOf(cachedWebView).getOnTouchListener()).isNotNull
-        assertThat(nextSheet.findViewById<View>(R.id.checkoutKitHeaderBorder)!!.alpha).isEqualTo(1f)
-
-        nextSheet.dismiss()
-        runDismissAnimation()
-
-        assertThat(cachedWebView.isPresented).isFalse()
-        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isSameAs(cachedWebView)
-    }
-
-    @Test
-    fun `dismissed preload reused as embedded checkout has no sheet touch listener`() {
+    fun `dismiss() destroys preloaded checkoutView before another presentation`() {
         val checkoutUrl = "https://shopify.com/cart/123"
         CheckoutWebView.preload(checkoutUrl, activity, webMessageTransport)
         ShadowLooper.shadowMainLooper().idle()
@@ -638,14 +602,15 @@ class CheckoutBottomSheetTest {
 
         sheet.dismiss()
         runDismissAnimation()
-        val embeddedCheckout = ShopifyCheckout.create(activity, checkoutUrl, webMessageTransport) {}
-        val embeddedWebView = embeddedCheckout.findViewById<RelativeLayout>(R.id.checkoutKitContainer)!!
-            .children.first { it is CheckoutWebView } as CheckoutWebView
 
-        assertThat(embeddedWebView).isSameAs(cachedWebView)
-        assertThat(shadowOf(embeddedWebView).getOnTouchListener()).isNull()
+        assertThat(sheet.containsChildOfType(CheckoutWebView::class.java)).isFalse()
+        assertThat(shadowOf(cachedWebView).wasDestroyCalled()).isTrue()
+        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isNull()
+
+        val nextSheet = presentBottomSheet(checkoutUrl)
+        ShadowLooper.shadowMainLooper().idle()
 
-        embeddedCheckout.destroy()
+        assertThat(nextSheet.currentCheckoutWebView()).isNotSameAs(cachedWebView)
     }
 
     @Test

platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolBridgeTest.kt

@@ -634,22 +634,6 @@ class EmbeddedCheckoutProtocolBridgeTest {
         }
     }
 
-    @Test
-    fun `terminal ec error of retained post-presentation checkout does not update consumed preload handle`() {
-        val preload = CheckoutWebView.preload("https://shopify.dev/cart/123", activity, webMessageTransport)!!
-        shadowOf(Looper.getMainLooper()).idle()
-        val view = CheckoutWebView.checkoutViewFor("https://shopify.dev/cart/123", activity, webMessageTransport)
-        view.markPresented()
-        assertThat(CheckoutWebView.retainAfterPresentation(view)).isTrue()
-        val bridge = EmbeddedCheckoutProtocolBridge(view, webMessageTransport, protocolMessageExecutor = directExecutor)
-
-        bridge.receiveMessage(ecErrorMessage(severity = "unrecoverable"))
-        shadowOf(Looper.getMainLooper()).idle()
-
-        assertThat(CheckoutWebView.cachedPreloadViewForTesting()).isNull()
-        assertThat(preload.state).isEqualTo(PreloadState.Loading)
-    }
-
     @Test
     fun `terminal error from foreign view does not evict active cached preload`() {
         CheckoutWebView.preload("https://shopify.dev/cart/123", activity, webMessageTransport)

platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadCacheTest.kt

@@ -17,31 +17,6 @@ class PreloadCacheTest {
         assertThat(PRELOAD_CACHE_HIT_LOG_MESSAGE).isEqualTo("Returning cached preloaded WebView.")
     }
 
-    @Test
-    fun `retaining after presentation schedules expiry for the remaining ttl`() {
-        var now = 1_000L
-        val scheduler = FakePreloadExpiryScheduler()
-        val cache = PreloadCache(scheduler).also {
-            it.clock = object : PreloadCache.Clock() {
-                override fun elapsedRealtime(): Long = now
-            }
-        }
-        val view = mock<CheckoutWebView>()
-        val key = PreloadKey("https://checkout.shopify.com/cart/123")
-
-        cache.store(key, view, activity())
-        now += TimeUnit.MINUTES.toMillis(1)
-        assertThat(cache.take(key)).isSameAs(view)
-
-        assertThat(cache.retainAfterPresentation(view)).isTrue()
-        assertThat(scheduler.scheduledDelayMillis).isEqualTo(TimeUnit.MINUTES.toMillis(4))
-
-        now += TimeUnit.MINUTES.toMillis(4)
-        scheduler.fire()
-
-        assertThat(cache.hasEntry).isFalse()
-    }
-
     @Test
     fun `expiry timer rearms while entry is fresh`() {
         val now = 1_000L
@@ -100,16 +75,15 @@ class PreloadCacheTest {
     }
 
     @Test
-    fun `discard cancels scheduled expiry`() {
+    fun `discard clears consumed preload`() {
         val scheduler = FakePreloadExpiryScheduler()
         val cache = PreloadCache(scheduler)
         val view = mock<CheckoutWebView>()
         val key = PreloadKey("https://checkout.shopify.com/cart/123")
 
         cache.store(key, view, activity())
         assertThat(cache.take(key)).isSameAs(view)
-        assertThat(cache.retainAfterPresentation(view)).isTrue()
-        assertThat(scheduler.isScheduled).isTrue()
+        assertThat(cache.hasEntry).isTrue()
 
         cache.discard(view)
 

platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadObservabilityTest.kt

@@ -316,26 +316,6 @@ class PreloadObservabilityTest {
         )
     }
 
-    @Config(sdk = [26])
-    @Test
-    fun `renderer termination of retained post-presentation checkout does not update consumed preload handle`() {
-        val preload = ShopifyCheckoutKit.preload(url, activity, webMessageTransport)!!
-        ShadowLooper.shadowMainLooper().idle()
-        val view = CheckoutWebView.checkoutViewFor(url, activity, webMessageTransport)
-        view.markPresented()
-        assertThat(CheckoutWebView.retainAfterPresentation(view)).isTrue()
-        assertThat(view.isPreloadRequest).isFalse()
-        val detail = mock<RenderProcessGoneDetail> {
-            whenever(it.didCrash()).thenReturn(false)
-        }
-
-        shadowOf(view).webViewClient.onRenderProcessGone(view, detail)
-        ShadowLooper.shadowMainLooper().idle()
-
-        assertThat(CheckoutWebView.hasCacheEntryForTesting()).isFalse()
-        assertThat(preload.state).isEqualTo(PreloadState.Loading)
-    }
-
     @Test
     fun `http error transitions cached preload to failed`() {
         val preload = ShopifyCheckoutKit.preload(url, activity, webMessageTransport)!!