diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutBottomSheet.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutBottomSheet.kt index 16650ed62..55a16f432 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutBottomSheet.kt +++ b/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(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 } diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt index 50a894fcf..535cc9c84 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutWebView.kt +++ b/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) } diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/PreloadCache.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/PreloadCache.kt index f568c1e16..61322ab06 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/PreloadCache.kt +++ b/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 diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckout.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckout.kt index 28051be3a..d6bca7d02 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckout.kt +++ b/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 } diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckoutKit.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckoutKit.kt index 03bbc0b1c..9d052b320 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/ShopifyCheckoutKit.kt +++ b/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() + /** * 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 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 } } diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutBottomSheetTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutBottomSheetTest.kt index 9f1c9f1d3..eb2516e43 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutBottomSheetTest.kt +++ b/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() 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(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(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 diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolBridgeTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolBridgeTest.kt index 06fc02198..296c8d351 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolBridgeTest.kt +++ b/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) diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadCacheTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadCacheTest.kt index 18540e4c5..ea4ffaddb 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadCacheTest.kt +++ b/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() - 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,7 +75,7 @@ class PreloadCacheTest { } @Test - fun `discard cancels scheduled expiry`() { + fun `discard clears consumed preload`() { val scheduler = FakePreloadExpiryScheduler() val cache = PreloadCache(scheduler) val view = mock() @@ -108,8 +83,7 @@ class PreloadCacheTest { 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) diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadObservabilityTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadObservabilityTest.kt index c22e5ecbd..bdfc172b4 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/PreloadObservabilityTest.kt +++ b/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 { - 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)!! diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/ShopifyCheckoutKitTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/ShopifyCheckoutKitTest.kt index 26172fc8d..ba8c965d7 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/ShopifyCheckoutKitTest.kt +++ b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/ShopifyCheckoutKitTest.kt @@ -3,6 +3,7 @@ package com.shopify.checkoutkit import android.widget.RelativeLayout import androidx.activity.ComponentActivity import androidx.core.view.children +import androidx.lifecycle.LifecycleRegistry import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Before @@ -16,6 +17,7 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf import org.robolectric.shadows.ShadowDialog import org.robolectric.shadows.ShadowLooper +import java.util.concurrent.TimeUnit @RunWith(RobolectricTestRunner::class) class ShopifyCheckoutKitTest { @@ -84,6 +86,93 @@ class ShopifyCheckoutKitTest { } } + @Test + fun `present ignores a second call while a checkout is showing`() { + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { activityController -> + val activity = activityController.get() + + val first = presentCheckout(activity) + val second = presentCheckout(activity) + + assertThat(first).isNotNull + assertThat(ShadowDialog.getShownDialogs()).hasSize(1) + assertThat(second).isSameAs(first) + } + } + + @Test + fun `present tracks active checkouts independently for each activity`() { + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { firstActivityController -> + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { secondActivityController -> + val firstActivity = firstActivityController.get() + val secondActivity = secondActivityController.get() + + val first = presentCheckout(firstActivity) + presentCheckout(secondActivity) + val repeatedFirst = presentCheckout(firstActivity) + + assertThat(ShadowDialog.getShownDialogs()).hasSize(2) + assertThat(repeatedFirst).isSameAs(first) + } + } + } + + @Test + fun `present shows a new checkout after the previous one is dismissed`() { + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { activityController -> + val activity = activityController.get() + val first = presentCheckout(activity) + layoutLatestSheet() + + first?.dismiss() + ShadowLooper.idleMainLooper(1, TimeUnit.SECONDS) + val second = presentCheckout(activity) + + assertThat(second).isNotNull + assertThat(second).isNotSameAs(first) + assertThat(ShadowDialog.getShownDialogs()).hasSize(2) + } + } + + @Test + fun `present releases its lifecycle observer once checkout is dismissed`() { + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { activityController -> + val activity = activityController.get() + val registry = activity.lifecycle as LifecycleRegistry + val observerCountBeforePresent = registry.observerCount + + val checkout = presentCheckout(activity) + layoutLatestSheet() + assertThat(registry.observerCount).isEqualTo(observerCountBeforePresent + 1) + + checkout?.dismiss() + ShadowLooper.idleMainLooper(1, TimeUnit.SECONDS) + + assertThat(registry.observerCount).isEqualTo(observerCountBeforePresent) + } + } + + @Test + fun `present uses a fresh WebView after dismissing a preloaded checkout`() { + Robolectric.buildActivity(ComponentActivity::class.java).setup().use { activityController -> + val activity = activityController.get() + preload(PRELOAD_URL, activity) + ShadowLooper.shadowMainLooper().idle() + val cachedView = CheckoutWebView.cachedPreloadViewForTesting()!! + + val first = presentCheckout(activity, PRELOAD_URL) + layoutLatestSheet() + assertThat(latestSheetCheckoutWebView()).isSameAs(cachedView) + + first?.dismiss() + ShadowLooper.idleMainLooper(1, TimeUnit.SECONDS) + presentCheckout(activity, PRELOAD_URL) + + assertThat(shadowOf(cachedView).wasDestroyCalled()).isTrue() + assertThat(latestSheetCheckoutWebView()).isNotSameAs(cachedView) + } + } + @Test fun `activity destroy dismisses checkout without waiting for sheet animation`() { Robolectric.buildActivity(ComponentActivity::class.java).setup().use { activityController -> @@ -202,7 +291,32 @@ class ShopifyCheckoutKitTest { ShopifyCheckoutKit.preload(url, activity, webMessageTransport) } + private fun presentCheckout( + activity: ComponentActivity, + url: String = "https://shopify.dev", + ): CheckoutHandle? { + return ShopifyCheckoutKit.present( + url, + activity, + noopDefaultCheckoutListener(), + webMessageTransport = webMessageTransport, + ) + } + + private fun layoutLatestSheet() { + val sheet = ShadowDialog.getLatestDialog() as CheckoutBottomSheet + sheet.findViewById(R.id.checkoutKitSheet)!! + .layout(0, 0, TEST_SHEET_SIZE, TEST_SHEET_SIZE) + } + + private fun latestSheetCheckoutWebView(): CheckoutWebView { + val sheet = ShadowDialog.getLatestDialog() as CheckoutBottomSheet + return sheet.findViewById(R.id.checkoutKitContainer)!! + .children.first { it is CheckoutWebView } as CheckoutWebView + } + private companion object { private const val TEST_SHEET_SIZE = 1000 + private const val PRELOAD_URL = "https://shopify.dev/cart/123" } }