From a0eae16d4975957cd37be77142c1df11ee06ee9f Mon Sep 17 00:00:00 2001 From: Vailence Date: Mon, 24 Aug 2026 20:20:48 +0500 Subject: [PATCH 01/13] MOBILE-341: Report what the block shows instead of whether it is visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mirror of the same change on iOS. A wrapper that lays the block out itself was told only whether the view is visible, and a boolean cannot tell an error screen from a shimmer: both keep the place occupied, and only the container knows which of them it settled on. A Flutter widget draws the host's screens itself while the native view holds nothing but a stand-in, so it has to know which one to draw. `MindboxEmbeddedBlockAppearance` names what is drawn — placeholder, content, error, collapsed — and `setAppearanceObserver` hands the current value out on subscribing, which the boolean observer never did: a wrapper attaching after the outcome would otherwise hold the place of a block that had already collapsed, with nothing left to say so a second time. The rules behind the choice stay here: `appearanceFor(state)` is the single place that decides, `hasCollapsed` gives way to `hasSettled` plus `shownAppearance`, and the view's own `GONE` is derived from the same answer, so it cannot drift from `COLLAPSED`. Two rules are pinned down along the way. An empty place collapses however the host draws its failures — a host cannot fill the space of a block that was never meant to be there. And the error view is obeyed when it changes: swapped mid-failure it replaces what is on screen, taken away it collapses the block. What neither does is expand a block that has already collapsed; reopening space the layout has reclaimed would make it jump. `setHostVisible(Boolean)` is a second source for the same input as window visibility: a Flutter or React Native screen never leaves the single window the app has, so without it a block spends its waiting budget on a screen nobody is looking at. The window, the host and `release()` now drive one idempotent switch — three sources that can each repeat what another already said — and a release stops the content before letting it go. --- .../embedded/compose/MindboxEmbeddedBlock.kt | 24 +- .../MindboxEmbeddedBlockAppearance.kt | 36 ++ .../embedded/MindboxEmbeddedBlockListener.kt | 7 +- .../embedded/MindboxEmbeddedBlockView.kt | 229 ++++++++++--- ...indboxEmbeddedBlockViewWrapperHooksTest.kt | 319 ++++++++++++++++++ 5 files changed, 553 insertions(+), 62 deletions(-) create mode 100644 sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockAppearance.kt create mode 100644 sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt diff --git a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt index 3e261f53..f4db4996 100644 --- a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt +++ b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import cloud.mindbox.mobile_sdk.annotations.InternalMindboxApi +import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockAppearance import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockListener import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView @@ -29,8 +30,9 @@ import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView * * The behavior mirrors the View one and belongs to the block itself: it is visible while * loading and showing content, and collapses to zero height when the place ends up without - * content — unless the [error] slot is set: a custom error view is a request to keep the - * place, so the block stays and shows it. The callbacks only report the outcome. + * content — unless it failed and the [error] slot is set: that slot is a request to show a + * failure, so the block stays and shows it. An empty place collapses either way. The callbacks + * only report the outcome. * * ```kotlin * MindboxEmbeddedBlock( @@ -49,8 +51,8 @@ import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView * stayed in place showing it. Not necessarily a breakage: an empty place is a normal outcome. * Main thread. * @param placeholder Replaces the SDK's default loading placeholder. Fills the whole block frame. - * @param error The view for a place without content. Setting it also keeps the block visible - * instead of the default collapse. Fills the whole block frame. + * @param error The view for a block that failed. Setting it keeps the block visible instead of + * the default collapse; an empty place collapses regardless. Fills the whole block frame. */ @OptIn(InternalMindboxApi::class) @Composable @@ -70,7 +72,9 @@ public fun MindboxEmbeddedBlock( val context = LocalContext.current key(placeSystemName) { - var isCollapsed by remember { mutableStateOf(false) } + var appearance by remember { + mutableStateOf(MindboxEmbeddedBlockAppearance.PLACEHOLDER) + } val placeholderHost = remember(context) { lazy(LazyThreadSafetyMode.NONE) { @@ -84,10 +88,16 @@ public fun MindboxEmbeddedBlock( } AndroidView( - modifier = (if (isCollapsed) Modifier.height(0.dp).then(modifier) else modifier).fillMaxWidth(), + modifier = ( + if (appearance == MindboxEmbeddedBlockAppearance.COLLAPSED) { + Modifier.height(0.dp).then(modifier) + } else { + modifier + } + ).fillMaxWidth(), factory = { viewContext -> MindboxEmbeddedBlockView(viewContext, placeSystemName).apply { - setVisibilityObserver { isVisible -> isCollapsed = !isVisible } + setAppearanceObserver { shown -> appearance = shown } setListener( object : MindboxEmbeddedBlockListener { override fun onLoad(view: MindboxEmbeddedBlockView) { diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockAppearance.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockAppearance.kt new file mode 100644 index 00000000..651296e8 --- /dev/null +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockAppearance.kt @@ -0,0 +1,36 @@ +package cloud.mindbox.mobile_sdk.embedded + +import cloud.mindbox.mobile_sdk.annotations.InternalMindboxApi + +/** + * How a [MindboxEmbeddedBlockView] occupies its place right now — what is drawn, not what happened. + * + * For wrappers that lay the block out themselves instead of relying on the view's own + * `visibility`. The rules behind the decision stay here: that an empty place shows no error view, + * that a place taken by loading is a place drawn, that a settled block keeps what it shows. A + * wrapper mirrors the answer in its own layout and nothing more, so every wrapper of the SDK shows + * the same thing at the same moment by construction. + * + * Deliberately not part of the public API: available to wrappers through [InternalMindboxApi]. + */ +@InternalMindboxApi +public enum class MindboxEmbeddedBlockAppearance { + + /** The content is loading. The block takes its place and draws a loading screen. */ + PLACEHOLDER, + + /** The block content is shown. */ + CONTENT, + + /** + * The block failed and the host opted into showing it by setting + * [MindboxEmbeddedBlockView.setErrorView]. Never appears for an empty place. + */ + ERROR, + + /** + * The block occupies no space: a failure without a host error view, or an empty place. The + * space goes back to the layout. + */ + COLLAPSED, +} diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockListener.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockListener.kt index 84c4288a..d2f9adc3 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockListener.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockListener.kt @@ -27,9 +27,10 @@ public interface MindboxEmbeddedBlockListener { * The place stays without content — the load failed or timed out, or the config had nothing * to put here. An empty place is a normal outcome, not a breakage. * - * The block already hid itself, unless [MindboxEmbeddedBlockView.setErrorView] is set — then - * it keeps its place and shows that view. Nothing is required here: the block retries by - * itself, and how depends on why the place stayed empty. + * The block already hid itself, unless it failed and [MindboxEmbeddedBlockView.setErrorView] + * is set — then it keeps its place and shows that view. An empty place collapses either way. + * Nothing is required here: the block retries by itself, and how depends on why the place + * stayed empty. * * - The config had no placement for this place — resolved again every time the block comes * back on screen, and on a new session. diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index 97977be3..b25a914a 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -34,7 +34,8 @@ import kotlin.math.abs * **The host owns the size**: give the block an explicit height. The content adapts to that * frame, so the host UI never jumps. While loading, the frame shows a placeholder — the SDK's * default one or the host's own ([setPlaceholderView]). When the place ends up without content the - * block hides itself, unless the host gave it a view to show instead ([setErrorView]). + * block hides itself; a failure can be shown instead of hidden if the host gave it a view for one + * ([setErrorView]). * * ```xml * Unit)? = null + private var appearanceObserver: ((MindboxEmbeddedBlockAppearance) -> Unit)? = null private var placeholderView: View? = null private var errorView: View? = null private val defaultPlaceholder by lazy { EmbeddedBlockDefaultViews.placeholder(context) } @@ -92,7 +94,35 @@ public class MindboxEmbeddedBlockView internal constructor( private var deliveredEvent: BlockEvent? = null private var isDeliveryScheduled = false - private var hasCollapsed = false + + /** + * Whether the block has already settled on a place without content — collapsed, or showing the + * host's error view. + * + * A settled block keeps what it shows. A retry — and the block gets one on every return to the + * screen — is not a reload: the page behind the place is the one that already failed, so it + * answers the same way, while the host watches its layout jerk by the block's height and a + * placeholder flash on every pass across the screen, to show nothing in the end. That holds for + * the error view just as much as for a collapse. + * + * Ended by content that actually appeared. + */ + private var hasSettled = false + + /** What the block shows right now — the one source for both its own visibility and the report. */ + private var shownAppearance = MindboxEmbeddedBlockAppearance.PLACEHOLDER + + /** + * Whether the window shows the block. Tracked rather than read off `windowVisibility`: the + * callback carries the new value while the property still holds the old one. + */ + private var isWindowVisible = false + + /** Whether the wrapper's host still shows the block. Nobody says otherwise until a wrapper does. */ + private var isHostVisible = true + + /** Whether a wrapper has released the block: then it stays stopped whatever the window says. */ + private var isReleased = false private var shownContent: View? = null private var isContentStarted = false private var observedLifecycle: Lifecycle? = null @@ -145,23 +175,70 @@ public class MindboxEmbeddedBlockView internal constructor( */ public fun setPlaceholderView(view: View?) { placeholderView = view - if (state is EmbeddedBlockState.Loading) showContent(currentPlaceholder()) + if (shownAppearance == MindboxEmbeddedBlockAppearance.PLACEHOLDER) { + showContent(currentPlaceholder()) + } } /** - * The view for a place that ended up without content. Setting it also keeps the block - * visible instead of the default collapse. Fills the whole block frame. + * The view for a block that failed. Setting it opts into showing the failure instead of the + * default collapse: the block keeps its place and shows this view. Fills the whole block frame. + * + * Applies to failures only — an empty place always collapses, so a host cannot fill the space + * of a block that was never meant to be there. * - * Applies from the next outcome on: a block that already collapsed stays collapsed until - * its content reloads. + * A view set mid-failure swaps the error screen already shown, and `null` given while one is + * shown takes the failure back down to a collapse — the space returns to the layout. What + * neither does is expand a block that has already collapsed: reopening space the layout has + * reclaimed would make it jump, so such a view takes effect on the next load. */ public fun setErrorView(view: View?) { errorView = view + // Re-decided rather than just swapped: whether a failure is shown at all depends on this + // view existing, so taking it away has to collapse the block and say so, not leave the old + // screen standing. What is on screen is the subject, not the state — a settled block goes on + // showing its error view while the attempt it gets on the way back loads — and a block that + // has already collapsed stays collapsed however this view changes. + if (shownAppearance == MindboxEmbeddedBlockAppearance.ERROR) { + applyState(state) + } } + /** + * Reports how the block occupies its place, for wrappers that lay it out themselves instead of + * relying on this view's own `visibility` — see [MindboxEmbeddedBlockAppearance]. + * + * The current value arrives right away on subscribing: a wrapper that comes after the outcome + * cannot miss what the block already decided. + */ @InternalMindboxApi - public fun setVisibilityObserver(observer: ((isVisible: Boolean) -> Unit)?) { - visibilityObserver = observer + public fun setAppearanceObserver(observer: ((MindboxEmbeddedBlockAppearance) -> Unit)?) { + appearanceObserver = observer + loggingRunCatching { observer?.invoke(shownAppearance) } + } + + /** + * Tells the block whether the host still shows it — a second source for the same input as + * window visibility: the content runs while the window shows it and the host says so. + * + * For wrappers whose whole app lives in one window. In Flutter every screen shares it, so + * leaving a screen never takes the block out of a window: the block would keep waiting — and + * spending its budget — on a screen nobody is looking at, and could collapse before the user + * ever got there. `true` by default, so a wrapper that says nothing behaves as before. + * + * A pause, not a reset: a block hidden mid-load keeps the page it has and the remainder of its + * budget; shown again, it counts that remainder down instead of starting the budget anew. + */ + @InternalMindboxApi + public fun setHostVisible(visible: Boolean) { + if (isHostVisible == visible) return + + isHostVisible = visible + mindboxLogI( + "[EmbeddedBlock] Block '$placeSystemName' was ${if (visible) "shown" else "hidden"} " + + "by the host wrapper", + ) + updateContentActivity() } private val hasCustomErrorView: Boolean @@ -170,17 +247,36 @@ public class MindboxEmbeddedBlockView internal constructor( override fun onAttachedToWindow() { super.onAttachedToWindow() observeHostDestruction() - if (windowVisibility == VISIBLE) startContent() + isWindowVisible = windowVisibility == VISIBLE + updateContentActivity() } override fun onDetachedFromWindow() { - pauseContent() + isWindowVisible = false + updateContentActivity() super.onDetachedFromWindow() } override fun onWindowVisibilityChanged(visibility: Int) { super.onWindowVisibilityChanged(visibility) - if (visibility == VISIBLE) startContent() else pauseContent() + isWindowVisible = visibility == VISIBLE + updateContentActivity() + } + + /** + * Whether anybody is looking at the block. The window answers that for a native host; a wrapper + * whose app has a single window answers the rest through [setHostVisible], and a released block + * is not looked at by definition. + */ + private val isEffectivelyVisible: Boolean + get() = isWindowVisible && isHostVisible && !isReleased + + /** + * Three sources drive one switch — the window, the wrapper's host, a release — and each of them + * can repeat what another has already said, so the switch is idempotent. + */ + private fun updateContentActivity() { + if (isEffectivelyVisible) startContent() else pauseContent() } private fun startContent() { @@ -207,7 +303,14 @@ public class MindboxEmbeddedBlockView internal constructor( @InternalMindboxApi public fun release() { + if (isReleased) return + mindboxLogI("[EmbeddedBlock] Released by the host wrapper, freeing content") + isReleased = true + appearanceObserver = null + // A released block is not looked at, whatever the window says: the switch hears that here, + // or it would go on believing the content runs and pause a controller already released. + updateContentActivity() detachFromHost() loggingRunCatching { contentController.release() } } @@ -248,58 +351,80 @@ public class MindboxEmbeddedBlockView internal constructor( } private fun applyState(state: EmbeddedBlockState) { - when (state) { - is EmbeddedBlockState.Ready -> { - val readyContent = contentController.contentView - if (readyContent == null) { - mindboxLogW("[EmbeddedBlock] Ready content has no view, treating it as a failure") - this.state = EmbeddedBlockState.Failed - return - } - mindboxLogI("[EmbeddedBlock] Content ready") - showContent(readyContent) - } - is EmbeddedBlockState.Loading -> { + if (state is EmbeddedBlockState.Ready && contentController.contentView == null) { + mindboxLogW("[EmbeddedBlock] Ready content has no view, treating it as a failure") + this.state = EmbeddedBlockState.Failed + return + } + + // Read before it is written: a settled block's answer for `Loading` is the appearance it + // already had. + val appearance = appearanceFor(state) + shownAppearance = appearance + hasSettled = when (appearance) { + // A place without content, however it is drawn, is what the block settles on. + MindboxEmbeddedBlockAppearance.COLLAPSED, + MindboxEmbeddedBlockAppearance.ERROR, + -> true + // And content that actually appeared ends it: from here the block is an ordinary one + // again, and the next load it really starts is entitled to its placeholder. + MindboxEmbeddedBlockAppearance.CONTENT -> false + MindboxEmbeddedBlockAppearance.PLACEHOLDER -> hasSettled + } + + when (appearance) { + MindboxEmbeddedBlockAppearance.PLACEHOLDER -> { mindboxLogI("[EmbeddedBlock] Content loading, showing the placeholder") showContent(currentPlaceholder()) } - - is EmbeddedBlockState.Empty -> { - mindboxLogI("[EmbeddedBlock] Nothing to show for this place") - showErrorView() + // The guard above already turned a `Ready` without a view into a failure, so there is + // one to show here; the null-safe call is what keeps that guarantee from being an `!!`. + MindboxEmbeddedBlockAppearance.CONTENT -> { + mindboxLogI("[EmbeddedBlock] Content ready") + contentController.contentView?.let { showContent(it) } + } + MindboxEmbeddedBlockAppearance.ERROR -> { + mindboxLogI("[EmbeddedBlock] Content failed, showing the host's error view") + errorView?.let { showContent(it) } } - is EmbeddedBlockState.Failed -> { - mindboxLogI("[EmbeddedBlock] Content failed, showing the error state") - showErrorView() + MindboxEmbeddedBlockAppearance.COLLAPSED -> { + mindboxLogI("[EmbeddedBlock] Nothing to show for this place, collapsing") + clearContent() } } - applyDefaultVisibility(state) + visibility = if (appearance == MindboxEmbeddedBlockAppearance.COLLAPSED) GONE else VISIBLE + loggingRunCatching { appearanceObserver?.invoke(appearance) } scheduleDelivery() } - private fun applyDefaultVisibility(state: EmbeddedBlockState) { - val isVisible = when { - state.nothingToShow -> hasCustomErrorView - state is EmbeddedBlockState.Loading -> !hasCollapsed - else -> true - } - hasCollapsed = when { - state is EmbeddedBlockState.Ready -> false - state.nothingToShow -> !isVisible - else -> hasCollapsed + private fun appearanceFor(state: EmbeddedBlockState): MindboxEmbeddedBlockAppearance = + when (state) { + // A block that already settled keeps what it shows even while it loads anew: a retry + // earns neither the space the host has reclaimed nor a placeholder over the error view. + // What it does not keep is an error view the host has taken away in the meantime — + // there is nothing left to draw, so the block collapses. + is EmbeddedBlockState.Loading -> when { + !hasSettled -> MindboxEmbeddedBlockAppearance.PLACEHOLDER + shownAppearance == MindboxEmbeddedBlockAppearance.ERROR && !hasCustomErrorView -> + MindboxEmbeddedBlockAppearance.COLLAPSED + else -> shownAppearance + } + is EmbeddedBlockState.Ready -> MindboxEmbeddedBlockAppearance.CONTENT + // A failure is shown only to those who opted in explicitly; for the rest it collapses. + is EmbeddedBlockState.Failed -> + if (hasCustomErrorView) { + MindboxEmbeddedBlockAppearance.ERROR + } else { + MindboxEmbeddedBlockAppearance.COLLAPSED + } + // An empty place always collapses: a host cannot fill the space of a block that was + // never meant to be there, however it drew its failures. + is EmbeddedBlockState.Empty -> MindboxEmbeddedBlockAppearance.COLLAPSED } - visibility = if (isVisible) VISIBLE else GONE - loggingRunCatching { visibilityObserver?.invoke(isVisible) } - } private fun currentPlaceholder(): View = placeholderView ?: defaultPlaceholder - private fun showErrorView() { - val view = errorView - if (view == null) clearContent() else showContent(view) - } - private fun showContent(content: View): Unit = loggingRunCatching { if (content === shownContent) return@loggingRunCatching shownContent?.let { removeView(it) } diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt new file mode 100644 index 00000000..aa347365 --- /dev/null +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt @@ -0,0 +1,319 @@ +package cloud.mindbox.mobile_sdk.embedded + +import android.app.Activity +import android.os.Looper +import android.view.View +import android.widget.LinearLayout +import cloud.mindbox.mobile_sdk.inapp.domain.models.InAppType +import cloud.mindbox.mobile_sdk.models.InAppStub +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import java.io.Closeable + +/** + * What a wrapper sees and says: the appearance the block reports, and the host visibility it is + * told about. A wrapper lays the block out itself, so it needs the decision the view would have + * applied to its own `visibility` — and it has to be able to say that its screen went away when the + * window cannot say it. + */ +@RunWith(RobolectricTestRunner::class) +class MindboxEmbeddedBlockViewWrapperHooksTest { + + private class FakeBlocksRegistry : EmbeddedBlocksRegistry { + var lastHandle: EmbeddedBlockHandle? = null + + override fun register(placeSystemName: String, handle: EmbeddedBlockHandle): Closeable { + lastHandle = handle + return Closeable { lastHandle = null } + } + + override fun onBlockAppeared(placeSystemName: String) = Unit + + override fun startListening() = Unit + } + + private class ReadyProvider(context: Activity) : EmbeddedContentProvider { + override var onStateChange: ((EmbeddedBlockState) -> Unit)? = null + override val contentView: View = View(context) + + var startCount = 0 + var pauseCount = 0 + + override fun start() { + startCount++ + onStateChange?.invoke(EmbeddedBlockState.Ready) + } + + override fun pause() { + pauseCount++ + } + + override fun release() = Unit + } + + private val activity: Activity = Robolectric.buildActivity(Activity::class.java).setup().get() + private val blocksRegistry = FakeBlocksRegistry() + private lateinit var provider: ReadyProvider + + private fun buildView(): MindboxEmbeddedBlockView = + MindboxEmbeddedBlockView( + activity, + null, + "main-screen-top", + EmbeddedBlockContentController( + placeSystemName = "main-screen-top", + providerFactory = { _, _ -> ReadyProvider(activity).also { provider = it } }, + blocksRegistry = { blocksRegistry }, + ), + ) + + private fun attach(view: MindboxEmbeddedBlockView) { + activity.setContentView(LinearLayout(activity).apply { addView(view, 500, 300) }) + idle() + dispatchWindowVisibility(view, View.VISIBLE) + idle() + } + + private fun idle() { + shadowOf(Looper.getMainLooper()).idle() + } + + /** Resolves the place to content, which is what builds the provider. */ + private fun showContent() { + blocksRegistry.lastHandle?.onContentResolved(InAppStub.getEmbedded() as InAppType.Embedded) + idle() + } + + /** Fails the page behind a place that already resolved to content. */ + private fun failContent() { + provider.onStateChange?.invoke(EmbeddedBlockState.Failed) + idle() + } + + @Test + fun `subscribing hands out the appearance the block already has`() { + val view = buildView() + attach(view) + blocksRegistry.lastHandle?.onContentResolved(null) + idle() + + // The place settled before anybody subscribed, and its only report would otherwise be lost: + // a wrapper built after the outcome would sit on a loading screen forever. + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + + assertEquals(listOf(MindboxEmbeddedBlockAppearance.COLLAPSED), seen) + } + + @Test + fun `the appearance follows the block from loading to content`() { + val view = buildView() + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + + attach(view) + assertEquals(MindboxEmbeddedBlockAppearance.PLACEHOLDER, seen.last()) + + showContent() + + assertEquals(MindboxEmbeddedBlockAppearance.CONTENT, seen.last()) + } + + @Test + fun `an empty place collapses even when the host set an error view`() { + val view = buildView() + view.setErrorView(View(activity)) + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + + blocksRegistry.lastHandle?.onContentResolved(null) + idle() + + // An error view opts into showing a failure, not into filling a place that was never meant + // to hold anything. + assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) + assertEquals(View.GONE, view.visibility) + } + + @Test + fun `a host that hides the block pauses its content and shows it again on return`() { + val view = buildView() + attach(view) + showContent() + val startsWhenShown = provider.startCount + + view.setHostVisible(false) + idle() + assertEquals(1, provider.pauseCount) + + view.setHostVisible(true) + idle() + + // The window never changed — only the wrapper's word did, and it drives the same switch. + assertTrue(provider.startCount > startsWhenShown) + } + + @Test + fun `the same host visibility twice changes nothing`() { + val view = buildView() + attach(view) + showContent() + + view.setHostVisible(false) + view.setHostVisible(false) + idle() + + assertEquals(1, provider.pauseCount) + } + + @Test + fun `taking the error view away collapses the failure it was showing`() { + val view = buildView() + view.setErrorView(View(activity)) + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + showContent() + failContent() + assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) + + view.setErrorView(null) + + // Whether a failure is shown at all is this view's doing, so taking it away is not a swap of + // screens — it is the block going back to the collapse it would have had, and saying so. + assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) + assertEquals(View.GONE, view.visibility) + } + + @Test + fun `one error view swapped for another keeps the failure shown`() { + val view = buildView() + view.setErrorView(View(activity)) + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + showContent() + failContent() + + val next = View(activity) + view.setErrorView(next) + + assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) + assertEquals(View.VISIBLE, view.visibility) + } + + @Test + fun `an error view given to an already collapsed block does not expand it`() { + val view = buildView() + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + showContent() + failContent() + assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) + + view.setErrorView(View(activity)) + + // Reopening space the layout has already reclaimed would make it jump: the view waits for + // the next load. + assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) + assertEquals(View.GONE, view.visibility) + } + + @Test + fun `taking the error view away collapses a failure still shown after a return`() { + val view = buildView() + view.setErrorView(View(activity)) + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + showContent() + failContent() + assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) + + // Off the screen and back: the failed block drops its page and waits for a new answer, so + // the state is `Loading` again while the error view is still the thing on screen. + dispatchWindowVisibility(view, View.GONE) + idle() + dispatchWindowVisibility(view, View.VISIBLE) + idle() + assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) + + view.setErrorView(null) + + assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) + assertEquals(View.GONE, view.visibility) + } + + @Test + fun `an error view swapped after a return replaces the one still on screen`() { + val view = buildView() + view.setErrorView(View(activity)) + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + attach(view) + showContent() + failContent() + + dispatchWindowVisibility(view, View.GONE) + idle() + dispatchWindowVisibility(view, View.VISIBLE) + idle() + + val next = View(activity) + view.setErrorView(next) + + assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) + assertEquals(View.VISIBLE, view.visibility) + assertEquals(view, next.parent) + } + + @Test + fun `a released block stops its content before letting it go`() { + val view = buildView() + attach(view) + showContent() + + view.release() + idle() + + // Released while the window still says it is shown: the switch has to hear it here, or it + // goes on believing the content runs and pauses a controller that is already released. + assertEquals(1, provider.pauseCount) + } + + @Test + fun `a wrapper callback that throws on subscribing does not reach the host`() { + val view = buildView() + attach(view) + + view.setAppearanceObserver { throw IllegalStateException("the channel is not ready yet") } + + // The same callback is guarded everywhere else; the immediate call on subscribing cannot be + // the one place where a wrapper's exception takes the host down with it. + val seen = mutableListOf() + view.setAppearanceObserver { seen.add(it) } + assertEquals(1, seen.size) + } + + @Test + fun `a released block stays stopped even when the window says it is shown`() { + val view = buildView() + attach(view) + showContent() + view.release() + idle() + val startsBefore = provider.startCount + + dispatchWindowVisibility(view, View.VISIBLE) + idle() + + assertEquals(startsBefore, provider.startCount) + } +} From bba4d3da469207e0a9b037d7cc04ee885cc05887 Mon Sep 17 00:00:00 2001 From: Vailence Date: Mon, 24 Aug 2026 20:21:15 +0500 Subject: [PATCH 02/13] MOBILE-341: Count a show where somebody is looking and hold what failed off screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend hears about blocks the user was actually shown. A page rendering behind another screen was counted anyway: the block reported the show the moment the page said `contentRendered`, whether or not anybody could see it. The show is now counted where the block is looked at — at the render if it is on screen, otherwise by the `start()` that brings it back. `timeToDisplay` follows the same split. Measured at the show, it would have carried the user's absence from the screen on top of the wait for the page, so the render time is frozen where the render was reported. Failures are held the same way, and for the same reason: one that happened off screen is kept and sent when the block comes back. Only the first is kept — the block reports the outcome it came back to, and a silent page repeating itself adds nothing to that. --- .../webview/EmbeddedBlockWebViewHolder.kt | 86 ++++++++++++++++--- .../webview/EmbeddedBlockWebViewHolderTest.kt | 67 +++++++++++++++ 2 files changed, 143 insertions(+), 10 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt index 43d32d42..49927a26 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt @@ -44,6 +44,7 @@ import cloud.mindbox.mobile_sdk.logger.mindboxLogW import cloud.mindbox.mobile_sdk.managers.DbManager import cloud.mindbox.mobile_sdk.managers.GatewayManager import cloud.mindbox.mobile_sdk.models.Configuration +import cloud.mindbox.mobile_sdk.models.Milliseconds import cloud.mindbox.mobile_sdk.models.Timestamp import cloud.mindbox.mobile_sdk.models.getShortUserAgent import cloud.mindbox.mobile_sdk.models.operation.request.FailureReason @@ -139,15 +140,43 @@ internal class EmbeddedBlockWebViewHolder( private val isUserPresent: Boolean get() = isActive && !isReleased + /** + * A failure that happened behind another screen, held until somebody looks at the block. + * + * The backend hears about blocks the user was shown, and a page that failed off screen was not + * one — the same rule the show follows. Only the first is kept: the block reports the outcome it + * came back to, and a silent page repeating itself adds nothing to that. + */ + @Volatile private var heldFailure: HeldFailure? = null + + private data class HeldFailure( + val failureReason: FailureReason, + val errorDescription: String, + val throwable: Throwable?, + ) + + /** + * How long the page took to render, frozen where the render was reported. + * + * The show can be counted much later — a page that finished behind another screen is counted by + * the [start] that brings it back — and `timeToDisplay` measures the wait for the page, not the + * user's absence from the screen. + */ + @Volatile private var renderedTimeToDisplay: Milliseconds? = null + override fun start() { if (isReleased) return isActive = true + flushHeldFailure() if (!isLoadRequested) { isLoadRequested = true load() return } report(lastState) + // A page that rendered behind another screen is only shown now, so this is where its show + // is counted. + if (lastState == EmbeddedBlockState.Ready) accountForShow() } override fun pause() { @@ -161,6 +190,7 @@ internal class EmbeddedBlockWebViewHolder( if (commonBridgeActionsLazy.isInitialized()) { commonBridgeActions.tearDown() } + heldFailure = null cancelPendingResponses("Embedded block content is released") webViewController?.let { controller -> controller.setEventListener(null) @@ -265,12 +295,10 @@ internal class EmbeddedBlockWebViewHolder( "description=${error.description}, url=${error.url}" ) if (error.isForMainFrame == true) { - inAppFailureTracker.sendFailureWithContext( - inAppId = inAppId, + sendFailure( failureReason = FailureReason.WEBVIEW_PRESENTATION_FAILED, errorDescription = "Embedded block WebView error: code=${error.code}, " + "description=${error.description}, url=${error.url}", - tags = null ) report(EmbeddedBlockState.Failed) } @@ -388,8 +416,11 @@ internal class EmbeddedBlockWebViewHolder( report(EmbeddedBlockState.Empty) return BridgeMessage.SUCCESS_PAYLOAD } + renderedTimeToDisplay = timeProvider.elapsedSince(attemptStartedAt) report(EmbeddedBlockState.Ready) - accountForShow() + // Only what somebody is looking at counts as shown; a page that rendered off screen is + // counted by the `start()` that brings it back. + if (isActive) accountForShow() return BridgeMessage.SUCCESS_PAYLOAD } @@ -407,11 +438,9 @@ internal class EmbeddedBlockWebViewHolder( */ private fun refusedContentReport(reason: String): Throwable { mindboxLogE("[EmbeddedBlock] contentRendered refused: $reason") - inAppFailureTracker.sendFailureWithContext( - inAppId = inAppId, + sendFailure( failureReason = FailureReason.PRESENTATION_FAILED, errorDescription = "The embedded block page reported contentRendered with an unusable payload: $reason", - tags = null ) report(EmbeddedBlockState.Failed) return IllegalArgumentException(reason) @@ -424,7 +453,7 @@ internal class EmbeddedBlockWebViewHolder( private fun accountForShow() { if (didAccountForShow) return didAccountForShow = true - val timeToDisplay = timeProvider.elapsedSince(attemptStartedAt) + val timeToDisplay = renderedTimeToDisplay ?: timeProvider.elapsedSince(attemptStartedAt) Mindbox.mindboxScope.launch { loggingRunCatchingSuspending { inAppInteractor.recordBlockShow(inAppId, timeToDisplay, gatedTags()) @@ -478,13 +507,50 @@ internal class EmbeddedBlockWebViewHolder( onContentPageLoaded(content) } - private fun reportLoadFailure(description: String, throwable: Throwable?) { + /** + * Sent when the block is on screen, held for the return when it is not — see [heldFailure]. + */ + private fun sendFailure( + failureReason: FailureReason, + errorDescription: String, + throwable: Throwable? = null, + ) { + if (!isActive) { + mindboxLogI( + "[EmbeddedBlock] $failureReason for $inAppId happened off screen, holding the " + + "report until the block is looked at" + ) + if (heldFailure == null) { + heldFailure = HeldFailure(failureReason, errorDescription, throwable) + } + return + } + inAppFailureTracker.sendFailureWithContext( + inAppId = inAppId, + failureReason = failureReason, + errorDescription = errorDescription, + throwable = throwable, + tags = null + ) + } + + private fun flushHeldFailure() { + val held = heldFailure ?: return + heldFailure = null inAppFailureTracker.sendFailureWithContext( inAppId = inAppId, + failureReason = held.failureReason, + errorDescription = held.errorDescription, + throwable = held.throwable, + tags = null + ) + } + + private fun reportLoadFailure(description: String, throwable: Throwable?) { + sendFailure( failureReason = FailureReason.WEBVIEW_LOAD_FAILED, errorDescription = description, throwable = throwable, - tags = null ) webViewController?.executeOnViewThread { report(EmbeddedBlockState.Failed) } ?: mainHandler.post { if (!isReleased) report(EmbeddedBlockState.Failed) } diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt index f1a75bb3..dfb885b3 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt @@ -12,6 +12,7 @@ import cloud.mindbox.mobile_sdk.inapp.domain.interfaces.PermissionManager import cloud.mindbox.mobile_sdk.inapp.domain.interfaces.PermissionStatus import cloud.mindbox.mobile_sdk.inapp.domain.interfaces.interactors.InAppInteractor import cloud.mindbox.mobile_sdk.inapp.presentation.InAppMessageManager +import cloud.mindbox.mobile_sdk.inapp.domain.interfaces.managers.InAppFailureTracker import cloud.mindbox.mobile_sdk.inapp.presentation.InAppWebViewCachePolicy import cloud.mindbox.mobile_sdk.inapp.presentation.view.MindboxWebPageRegistry import cloud.mindbox.mobile_sdk.inapp.presentation.view.WebViewAction @@ -20,7 +21,10 @@ import cloud.mindbox.mobile_sdk.managers.GatewayManager import cloud.mindbox.mobile_sdk.managers.MindboxEventManager import cloud.mindbox.mobile_sdk.models.Configuration import cloud.mindbox.mobile_sdk.models.InAppStub +import cloud.mindbox.mobile_sdk.models.Milliseconds import cloud.mindbox.mobile_sdk.models.Timestamp +import cloud.mindbox.mobile_sdk.models.operation.request.FailureReason +import cloud.mindbox.mobile_sdk.utils.SystemTimeProvider import com.google.gson.JsonObject import com.google.gson.JsonParser import com.google.gson.JsonPrimitive @@ -60,6 +64,13 @@ class EmbeddedBlockWebViewHolderTest { private val inAppInteractor: InAppInteractor = mockk() private val inAppMessageManager: InAppMessageManager = mockk(relaxUnitFun = true) private val webPageRegistry: MindboxWebPageRegistry = mockk(relaxUnitFun = true) + private val inAppFailureTracker: InAppFailureTracker = mockk(relaxed = true) + + /** What [SystemTimeProvider] would return: the tests move it by hand. */ + private var elapsed = 0L + private val timeProvider: SystemTimeProvider = mockk { + every { elapsedSince(any()) } answers { Milliseconds(elapsed) } + } private val permissionManager: PermissionManager = mockk { every { getCameraPermissionStatus() } returns PermissionStatus.DENIED every { getLocationPermissionStatus() } returns PermissionStatus.DENIED @@ -80,6 +91,8 @@ class EmbeddedBlockWebViewHolderTest { every { inAppInteractor } returns this@EmbeddedBlockWebViewHolderTest.inAppInteractor every { permissionManager } returns this@EmbeddedBlockWebViewHolderTest.permissionManager every { appContext } returns application + every { inAppFailureTracker } returns this@EmbeddedBlockWebViewHolderTest.inAppFailureTracker + every { timeProvider } returns this@EmbeddedBlockWebViewHolderTest.timeProvider every { webViewCachePolicy } returns mockk { every { isCacheEnabled } returns false } @@ -731,6 +744,60 @@ class EmbeddedBlockWebViewHolderTest { assertEquals("response", lastOutgoingMessage()?.get("type")?.asString) } + @Test + fun `a refusal off screen is held until the block comes back`() { + coEvery { inAppInteractor.recordBlockShow(any(), any(), any()) } just runs + startAndAwaitPageLoad() + + holder.pause() + postFromPage(request(action = "contentRendered", payload = """{"count":"many"}""")) + + // The backend hears about blocks the user was shown, and this page failed behind another + // screen. + verify(exactly = 0) { inAppFailureTracker.sendFailure(any(), any(), any(), any()) } + + holder.start() + + verify(exactly = 1) { + inAppFailureTracker.sendFailure("embedded-id", FailureReason.PRESENTATION_FAILED, any(), any()) + } + } + + @Test + fun `a page that rendered off screen counts its show when the block comes back`() { + coEvery { inAppInteractor.recordBlockShow(any(), any(), any()) } just runs + startAndAwaitPageLoad() + + holder.pause() + postFromPage(request(action = "contentRendered", payload = """{"count":3}""")) + + coVerify(exactly = 0) { inAppInteractor.recordBlockShow(any(), any(), any()) } + + holder.start() + + coVerify(exactly = 1, timeout = 5_000L) { + inAppInteractor.recordBlockShow("embedded-id", any(), any()) + } + } + + @Test + fun `the counted show carries the time the render took, not the time off screen`() { + coEvery { inAppInteractor.recordBlockShow(any(), any(), any()) } just runs + startAndAwaitPageLoad() + + holder.pause() + elapsed = 1_000L + postFromPage(request(action = "contentRendered", payload = """{"count":3}""")) + // Two minutes on another screen, which is not time the user spent waiting for this page. + elapsed = 121_000L + + holder.start() + + coVerify(exactly = 1, timeout = 5_000L) { + inAppInteractor.recordBlockShow("embedded-id", Milliseconds(1_000L), any()) + } + } + @Test fun `release destroys the webview and start after release does nothing`() { startAndAwaitPageLoad() From f3fae6e6e995a72cd4532f6bc9e721c8c62b82ba Mon Sep 17 00:00:00 2001 From: Vailence Date: Tue, 25 Aug 2026 17:13:45 +0500 Subject: [PATCH 03/13] MOBILE-341: Let a wrapper set the block's timeout in code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The budget only ever came from `app:mindboxTimeoutMs`, and a wrapper has no XML to put it in: the Flutter and Compose hosts both build the block programmatically and were stuck with the default 30 s, while the iOS container has taken a `timeout:` since it was written. The programmatic constructor takes one now, and the internal one prefers it over the attribute — a block built in code has no attributes to read, and one inflated from XML has no caller to ask, so neither path loses anything. Nothing new decides what a bad value means: a non-positive budget still falls back to the default and says so in the log, in the one place that already did that. --- .../embedded/compose/MindboxEmbeddedBlock.kt | 6 +- .../embedded/MindboxEmbeddedBlockView.kt | 17 +++++- .../MindboxEmbeddedBlockViewCollapseTest.kt | 2 +- .../MindboxEmbeddedBlockViewLookupTest.kt | 57 +++++++++++++++++++ ...indboxEmbeddedBlockViewWrapperHooksTest.kt | 2 +- 5 files changed, 79 insertions(+), 5 deletions(-) diff --git a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt index f4db4996..654b319e 100644 --- a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt +++ b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt @@ -45,6 +45,9 @@ import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView * @param placeSystemName The place identifier matched against the config's `inlineBlocks` * section. Changing it recreates the block for the new place. Blocks with the same name work * independently, each with its own content. + * @param timeoutMs How long the block waits to learn what it shows before collapsing as empty, in + * milliseconds. `null` means the SDK default of 30 s. Fixed when the block is created, as the + * place is: a new value given to a block already on screen is ignored. * @param onLoad The block is shown and visible. Main thread. * @param onFail The place ends up without content — the load failed or timed out, or the * config had nothing to put here. The block collapsed, or — if the [error] slot is set — @@ -59,6 +62,7 @@ import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView public fun MindboxEmbeddedBlock( placeSystemName: String, modifier: Modifier = Modifier, + timeoutMs: Long? = null, onLoad: () -> Unit = {}, onFail: () -> Unit = {}, placeholder: (@Composable () -> Unit)? = null, @@ -96,7 +100,7 @@ public fun MindboxEmbeddedBlock( } ).fillMaxWidth(), factory = { viewContext -> - MindboxEmbeddedBlockView(viewContext, placeSystemName).apply { + MindboxEmbeddedBlockView(viewContext, placeSystemName, timeoutMs).apply { setAppearanceObserver { shown -> appearance = shown } setListener( object : MindboxEmbeddedBlockListener { diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index b25a914a..7c9d8184 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -56,9 +56,12 @@ public class MindboxEmbeddedBlockView internal constructor( context: Context, attrs: AttributeSet?, placeSystemName: String?, + configTimeout: Milliseconds? = null, private val contentController: EmbeddedBlockContentController = EmbeddedBlockContentController( placeSystemName = placeSystemName.orNullIfBlank(), - configTimeout = readConfigTimeout(context, attrs), + // What the caller asked for, and only failing that what XML says: a block built in code has + // no attributes to read, and one built from XML has no caller to ask. + configTimeout = configTimeout ?: readConfigTimeout(context, attrs), providerFactory = { content, attemptStartedAt -> EmbeddedBlockContentFactory.createProvider(context, content, attemptStartedAt) }, @@ -71,10 +74,20 @@ public class MindboxEmbeddedBlockView internal constructor( attrs: AttributeSet? = null, ) : this(context, attrs, readPlaceSystemName(context, attrs)) + /** + * Creates a block for [placeSystemName] in code, where there is no XML to carry the attributes. + * + * @param timeoutMs How long the block waits to learn what it shows before collapsing as empty, + * in milliseconds — the same budget `app:mindboxTimeoutMs` sets from XML. `null` means the SDK + * default of 30 s. An answer that arrives after that no longer expands the block; the next + * attempt starts when the block enters the window again. + */ + @JvmOverloads public constructor( context: Context, placeSystemName: String, - ) : this(context, null, placeSystemName) + timeoutMs: Long? = null, + ) : this(context, null, placeSystemName, timeoutMs?.let(::Milliseconds)) public val placeSystemName: String? = placeSystemName.orNullIfBlank() private var listener: MindboxEmbeddedBlockListener = DefaultListener diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt index de58a803..58d2ea86 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt @@ -55,7 +55,7 @@ class MindboxEmbeddedBlockViewCollapseTest { activity, null, "main-screen-top", - EmbeddedBlockContentController( + contentController = EmbeddedBlockContentController( placeSystemName = "main-screen-top", providerFactory = { _, _ -> ReadyProvider(activity) }, blocksRegistry = { blocksRegistry }, diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt index 240ccfaf..ec288dcd 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt @@ -78,6 +78,63 @@ class MindboxEmbeddedBlockViewLookupTest { assertEquals(listOf("fail"), listener.events) } + @Test + fun `a timeout given in code is the one the block waits out`() { + // The wrapper path: no XML to carry `mindboxTimeoutMs`, so the budget comes as an argument. + val view = MindboxEmbeddedBlockView(activity, "main-screen-top", timeoutMs = 5_000L) + val listener = RecordingListener() + view.setListener(listener) + + attach(view) + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(4L)) + + assertEquals(View.VISIBLE, view.visibility) + assertTrue(listener.events.isEmpty()) + + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(2L)) + + // Six seconds in — long past the five it was given, and long before the default thirty. + assertEquals(View.GONE, view.visibility) + assertEquals(listOf("fail"), listener.events) + } + + @Test + fun `a block given no timeout in code still reads the one from xml`() { + val attrs = Robolectric.buildAttributeSet() + .addAttribute(R.attr.mindboxPlaceSystemName, "main-screen-top") + .addAttribute(R.attr.mindboxTimeoutMs, "5000") + .build() + + val view = MindboxEmbeddedBlockView(activity, attrs) + val listener = RecordingListener() + view.setListener(listener) + + attach(view) + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(6L)) + + assertEquals(View.GONE, view.visibility) + assertEquals(listOf("fail"), listener.events) + } + + @Test + fun `a non-positive timeout is not a budget, and the default stands`() { + // Honoured literally it would collapse every block before the config had a chance. + val view = MindboxEmbeddedBlockView(activity, "main-screen-top", timeoutMs = 0L) + val listener = RecordingListener() + view.setListener(listener) + + attach(view) + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(29L)) + + assertEquals(View.VISIBLE, view.visibility) + assertTrue(listener.events.isEmpty()) + + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(2L)) + + assertEquals(View.GONE, view.visibility) + assertEquals(listOf("fail"), listener.events) + } + @Test fun `a block without a place name hides itself`() { // XML without the attribute (or a programmatic view with no name): nothing to match diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt index aa347365..76c05407 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt @@ -65,7 +65,7 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { activity, null, "main-screen-top", - EmbeddedBlockContentController( + contentController = EmbeddedBlockContentController( placeSystemName = "main-screen-top", providerFactory = { _, _ -> ReadyProvider(activity).also { provider = it } }, blocksRegistry = { blocksRegistry }, From d61efd8d2d9b6474e2947c7467e92a4fe4aa37ab Mon Sep 17 00:00:00 2001 From: Vailence Date: Tue, 25 Aug 2026 22:15:10 +0500 Subject: [PATCH 04/13] MOBILE-341: Keep a settled block settled when its retry fails too --- .../embedded/MindboxEmbeddedBlockView.kt | 22 +++++-- .../MindboxEmbeddedBlockViewCollapseTest.kt | 60 ++++++++++++++++++- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index 7c9d8184..ce17db5d 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -203,7 +203,8 @@ public class MindboxEmbeddedBlockView internal constructor( * A view set mid-failure swaps the error screen already shown, and `null` given while one is * shown takes the failure back down to a collapse — the space returns to the layout. What * neither does is expand a block that has already collapsed: reopening space the layout has - * reclaimed would make it jump, so such a view takes effect on the next load. + * reclaimed would make it jump, so such a view takes effect on a load that starts the cycle + * anew, never on the silent retry a return to the screen brings. */ public fun setErrorView(view: View?) { errorView = view @@ -425,12 +426,21 @@ public class MindboxEmbeddedBlockView internal constructor( } is EmbeddedBlockState.Ready -> MindboxEmbeddedBlockAppearance.CONTENT // A failure is shown only to those who opted in explicitly; for the rest it collapses. - is EmbeddedBlockState.Failed -> - if (hasCustomErrorView) { - MindboxEmbeddedBlockAppearance.ERROR - } else { + // A settled block keeps what it shows when the retry fails too: an error view given + // after the collapse must not reopen space the layout has reclaimed. The exception is + // the loading branch's one — an error view the host has taken away leaves nothing to + // draw, so the block collapses. + is EmbeddedBlockState.Failed -> when { + !hasSettled -> + if (hasCustomErrorView) { + MindboxEmbeddedBlockAppearance.ERROR + } else { + MindboxEmbeddedBlockAppearance.COLLAPSED + } + shownAppearance == MindboxEmbeddedBlockAppearance.ERROR && !hasCustomErrorView -> MindboxEmbeddedBlockAppearance.COLLAPSED - } + else -> shownAppearance + } // An empty place always collapses: a host cannot fill the space of a block that was // never meant to be there, however it drew its failures. is EmbeddedBlockState.Empty -> MindboxEmbeddedBlockAppearance.COLLAPSED diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt index 58d2ea86..c3b5b7b9 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt @@ -47,17 +47,33 @@ class MindboxEmbeddedBlockViewCollapseTest { override fun release() = Unit } + private class FailingProvider(context: Activity) : EmbeddedContentProvider { + override var onStateChange: ((EmbeddedBlockState) -> Unit)? = null + override val contentView: View = View(context) + + override fun start() { + onStateChange?.invoke(EmbeddedBlockState.Failed) + } + + override fun pause() = Unit + + override fun release() = Unit + } + private val activity: Activity = Robolectric.buildActivity(Activity::class.java).setup().get() private val blocksRegistry = FakeBlocksRegistry() + private var lastProvider: EmbeddedContentProvider? = null - private fun buildView(): MindboxEmbeddedBlockView = + private fun buildView( + provider: () -> EmbeddedContentProvider = { ReadyProvider(activity) }, + ): MindboxEmbeddedBlockView = MindboxEmbeddedBlockView( activity, null, "main-screen-top", contentController = EmbeddedBlockContentController( placeSystemName = "main-screen-top", - providerFactory = { _, _ -> ReadyProvider(activity) }, + providerFactory = { _, _ -> provider().also { lastProvider = it } }, blocksRegistry = { blocksRegistry }, ), ) @@ -137,5 +153,45 @@ class MindboxEmbeddedBlockViewCollapseTest { assertEquals(View.VISIBLE, view.visibility) } + @Test + fun `an error view given after the collapse does not reopen the space when the retry fails too`() { + val view = buildView(provider = { FailingProvider(activity) }) + attach(view) + blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) + idle() + assertEquals(View.GONE, view.visibility) + + view.setErrorView(View(activity)) + leaveAndReturn(view) + blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) + idle() + + // The retry failed the same way the block already settled on: the error view is remembered + // for a load that starts the cycle anew, not for a pass across the screen. + assertEquals(View.GONE, view.visibility) + } + + @Test + fun `an error view given after the collapse shows once content has started the cycle anew`() { + val providers = ArrayDeque(listOf(FailingProvider(activity), ReadyProvider(activity))) + val view = buildView(provider = { providers.removeFirst() }) + attach(view) + blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) + idle() + assertEquals(View.GONE, view.visibility) + view.setErrorView(View(activity)) + + // Content that actually appeared ends the settlement — the block is an ordinary one again... + leaveAndReturn(view) + blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) + idle() + assertEquals(View.VISIBLE, view.visibility) + + // ...so the next failure is entitled to the error view the host gave meanwhile. + lastProvider?.onStateChange?.invoke(EmbeddedBlockState.Failed) + idle() + assertEquals(View.VISIBLE, view.visibility) + } + private fun embeddedContent(): InAppType.Embedded = InAppStub.getEmbedded() } From 614f0c1a86d0a1d516e9c4f756ab1bae000d8e5f Mon Sep 17 00:00:00 2001 From: Vailence Date: Tue, 25 Aug 2026 22:15:12 +0500 Subject: [PATCH 05/13] MOBILE-341: Hand a failure held off screen to start atomically --- .../webview/EmbeddedBlockWebViewHolder.kt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt index 49927a26..11ddbff2 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt @@ -64,6 +64,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull import org.json.JSONObject import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicReference @OptIn(InternalMindboxApi::class) internal class EmbeddedBlockWebViewHolder( @@ -147,7 +148,7 @@ internal class EmbeddedBlockWebViewHolder( * one — the same rule the show follows. Only the first is kept: the block reports the outcome it * came back to, and a silent page repeating itself adds nothing to that. */ - @Volatile private var heldFailure: HeldFailure? = null + private val heldFailure = AtomicReference(null) private data class HeldFailure( val failureReason: FailureReason, @@ -190,7 +191,7 @@ internal class EmbeddedBlockWebViewHolder( if (commonBridgeActionsLazy.isInitialized()) { commonBridgeActions.tearDown() } - heldFailure = null + heldFailure.set(null) cancelPendingResponses("Embedded block content is released") webViewController?.let { controller -> controller.setEventListener(null) @@ -520,9 +521,11 @@ internal class EmbeddedBlockWebViewHolder( "[EmbeddedBlock] $failureReason for $inAppId happened off screen, holding the " + "report until the block is looked at" ) - if (heldFailure == null) { - heldFailure = HeldFailure(failureReason, errorDescription, throwable) - } + heldFailure.compareAndSet(null, HeldFailure(failureReason, errorDescription, throwable)) + // Failures come off mindboxScope while start() flips the block visible on the main + // thread: a start() between the check above and the hold has flushed before the failure + // was there. Re-check and flush; getAndSet keeps the report single whichever side wins. + if (isActive) flushHeldFailure() return } inAppFailureTracker.sendFailureWithContext( @@ -535,8 +538,7 @@ internal class EmbeddedBlockWebViewHolder( } private fun flushHeldFailure() { - val held = heldFailure ?: return - heldFailure = null + val held = heldFailure.getAndSet(null) ?: return inAppFailureTracker.sendFailureWithContext( inAppId = inAppId, failureReason = held.failureReason, From 582229a7365f4b27b0bc8d02c4be96c9bb3727ec Mon Sep 17 00:00:00 2001 From: Vailence Date: Wed, 26 Aug 2026 18:12:41 +0500 Subject: [PATCH 06/13] MOBILE-341: Answer the targeting question under both of its names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pages already shipped ask `checkInappsTargeting`; the name became `filterShowableInapps` in the rename, and iOS answers both. Android answered only the new one, so on a shipped page the block stayed silent: the page waited out its three seconds, drew an empty feed, reported contentRendered: 0 and collapsed for no reason of its own. A second enum constant rather than a Gson `alternate`: `alternate` is read-only, so the answer would go back named `filterShowableInapps` while the page waits for one to the name it asked under — and an answer it does not recognise is the same silence as none. The test that asserted the old name was cut now proves both spellings get the identical answer, mirroring the iOS suite. --- .../webview/EmbeddedBlockWebViewHolder.kt | 2 ++ .../inapp/presentation/view/WebViewAction.kt | 14 ++++++++++++++ .../webview/EmbeddedBlockWebViewHolderTest.kt | 17 ++++++++++++----- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt index 11ddbff2..940cbeff 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt @@ -326,6 +326,8 @@ internal class EmbeddedBlockWebViewHolder( register(WebViewAction.CONTENT_RENDERED, ::handleContentRenderedAction) register(WebViewAction.SHOW_IN_APP, ::handleShowInAppAction) registerSuspend(WebViewAction.FILTER_SHOWABLE_INAPPS, ::handleFilterShowableInappsAction) + // The same question under the name pages already shipped send — see CHECK_INAPPS_TARGETING. + registerSuspend(WebViewAction.CHECK_INAPPS_TARGETING, ::handleFilterShowableInappsAction) } } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt index db3ddfc5..56d534a9 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt @@ -82,6 +82,20 @@ public enum class WebViewAction { @SerializedName("filterShowableInapps") FILTER_SHOWABLE_INAPPS, + /** + * The same question as [FILTER_SHOWABLE_INAPPS] under the name pages already shipped send. + * + * A second constant and not a Gson `alternate`: `alternate` is read-only, so the answer would go + * back named `filterShowableInapps` while the page waits for one to the name it asked under — + * and an answer the page does not recognise is the same silence as none. An unanswered question + * is a page that renders an empty feed, then a `contentRendered: 0`, then a block that collapses + * and reports an empty place for no reason of its own. + * + * Both spellings are answered until the web side settles on one, in sync with iOS. + */ + @SerializedName("checkInappsTargeting") + CHECK_INAPPS_TARGETING, + @SerializedName("localState.changed") LOCAL_STATE_CHANGED, diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt index dfb885b3..8d19c861 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt @@ -417,15 +417,22 @@ class EmbeddedBlockWebViewHolderTest { verify(exactly = 0) { inAppMessageManager.showInAppById(any(), any()) } } + // Every other test asks with `filterShowableInapps`, so this is where the shipped pages' + // spelling proves it is answered identically — under the name it asked with, or the page does + // not recognise the answer as its own. @Test - fun `the old checkInappsTargeting name is not spoken anymore`() { - // Cut hard, in sync with iOS: the action never shipped, so no installed SDK speaks it. + fun `the shipped checkInappsTargeting name is answered the same way`() { + coEvery { inAppInteractor.filterShowableInAppIds(listOf("story-1", "story-2")) } returns + listOf("story-1") startAndAwaitPageLoad() - postFromPage(request(action = "checkInappsTargeting", payload = """{"inappIds":["story-1"]}""")) + postFromPage(request(action = "checkInappsTargeting", payload = """{"inappIds":["story-1","story-2"]}""")) + await { lastOutgoingMessage()?.get("action")?.asString == "checkInappsTargeting" } - coVerify(exactly = 0) { inAppInteractor.filterShowableInAppIds(any()) } - assertTrue(lastOutgoingMessage()?.get("action")?.asString != "checkInappsTargeting") + assertEquals("response", lastOutgoingMessage()!!.get("type").asString) + val payload = lastOutgoingPayload()!! + assertEquals(1, payload.getAsJsonArray("inappIds").size()) + assertEquals("story-1", payload.getAsJsonArray("inappIds").get(0).asString) } @Test From c8fcc68c85f066d0ddbb5a4cbe192cd437b99822 Mon Sep 17 00:00:00 2001 From: Vailence Date: Wed, 26 Aug 2026 18:13:02 +0500 Subject: [PATCH 07/13] MOBILE-341: Say out loud that Compose keeps the timeout it was built with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The budget is settled in the AndroidView factory, so a new timeoutMs handed to a live block is dropped. The View has no such trap — its timeout is a constructor argument — but the composable takes the parameter on every recomposition, and the Flutter widget already warns about the same thing. Compose was the only wrapper that stayed silent, which leaves a host debugging a budget it thinks it set against the clock. Once per value, not per frame: the effect is keyed on timeoutMs. --- .../embedded/compose/MindboxEmbeddedBlock.kt | 24 ++++++++- .../compose/MindboxEmbeddedBlockTest.kt | 53 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt index 654b319e..d2eb62d3 100644 --- a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt +++ b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt @@ -3,6 +3,7 @@ package cloud.mindbox.mobile_sdk.embedded.compose import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.key import androidx.compose.runtime.mutableStateOf @@ -14,10 +15,12 @@ import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView +import cloud.mindbox.mobile_sdk.Mindbox import cloud.mindbox.mobile_sdk.annotations.InternalMindboxApi import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockAppearance import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockListener import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView +import cloud.mindbox.mobile_sdk.logger.Level /** * An embedded Mindbox block as a composable. @@ -47,7 +50,8 @@ import cloud.mindbox.mobile_sdk.embedded.MindboxEmbeddedBlockView * independently, each with its own content. * @param timeoutMs How long the block waits to learn what it shows before collapsing as empty, in * milliseconds. `null` means the SDK default of 30 s. Fixed when the block is created, as the - * place is: a new value given to a block already on screen is ignored. + * place is: a new value given to a block already on screen is ignored, and the block says so in + * the log. Wrap the block in a `key()` of your own to build one on a different budget. * @param onLoad The block is shown and visible. Main thread. * @param onFail The place ends up without content — the load failed or timed out, or the * config had nothing to put here. The block collapsed, or — if the [error] slot is set — @@ -80,6 +84,24 @@ public fun MindboxEmbeddedBlock( mutableStateOf(MindboxEmbeddedBlockAppearance.PLACEHOLDER) } + // The budget is settled when the block is built and cannot be talked out of it afterwards, + // and a value quietly dropped is the kind of thing a host debugs against the clock. The View + // has no such trap — its timeout is a constructor argument — but this composable takes the + // parameter on every recomposition, so it says what it does with a new one, as the Flutter + // widget does. Once per value, not per frame: the effect is keyed on it. + val creationTimeoutMs = remember { timeoutMs } + if (timeoutMs != creationTimeoutMs) { + LaunchedEffect(timeoutMs) { + Mindbox.writeLog( + "[EmbeddedBlock] Block '$placeSystemName' was given timeoutMs=$timeoutMs after " + + "creation and keeps $creationTimeoutMs: the timeout is fixed when the block " + + "is created. Wrap the block in a key() of your own to build one on a " + + "different budget.", + Level.WARN, + ) + } + } + val placeholderHost = remember(context) { lazy(LazyThreadSafetyMode.NONE) { ComposeView(context).apply { setContent { currentPlaceholder?.invoke() } } diff --git a/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt b/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt index 1b9ea9e0..d8d1d148 100644 --- a/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt +++ b/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt @@ -13,12 +13,14 @@ import androidx.compose.ui.test.assertHeightIsEqualTo import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.unit.dp +import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf +import org.robolectric.shadows.ShadowLog @RunWith(RobolectricTestRunner::class) class MindboxEmbeddedBlockTest { @@ -159,4 +161,55 @@ class MindboxEmbeddedBlockTest { compose.onNodeWithTag("block").assertDoesNotExist() } + + @Test + fun `a budget changed after creation is ignored, and said out loud`() { + // The factory runs once, so the budget is settled there. A value quietly dropped is the kind + // of thing a host debugs against the clock — the View has no such trap, its timeout being a + // constructor argument, but this composable takes the parameter on every recomposition. + val timeout = mutableStateOf(5_000) + + compose.setContent { + MindboxEmbeddedBlock( + placeSystemName = "main-screen-top", + modifier = Modifier.height(120.dp), + timeoutMs = timeout.value, + ) + } + settle() + assertTrue(timeoutWarnings().isEmpty()) + + compose.runOnUiThread { timeout.value = 60_000 } + settle() + + val said = timeoutWarnings() + assertEquals(1, said.size) + assertTrue(said.single().contains("timeoutMs=60000")) + assertTrue(said.single().contains("keeps 5000")) + + // Said once per value, not once per frame: a recomposition on the same value adds nothing. + compose.runOnUiThread { timeout.value = 60_000 } + settle() + assertEquals(1, timeoutWarnings().size) + } + + @Test + fun `a budget left alone says nothing`() { + compose.setContent { + MindboxEmbeddedBlock( + placeSystemName = "main-screen-top", + modifier = Modifier.height(120.dp), + timeoutMs = 5_000, + ) + } + settle() + compose.runOnUiThread { compose.activity.setTitle("recompose") } + settle() + + assertTrue(timeoutWarnings().isEmpty()) + } + + private fun timeoutWarnings(): List = ShadowLog.getLogs() + .filter { log -> log.msg?.contains("was given timeoutMs=") == true } + .map { log -> log.msg } } From df51655f4b0c8b0e3d2d1c08a049c9174194b2ba Mon Sep 17 00:00:00 2001 From: Vailence Date: Wed, 26 Aug 2026 19:36:54 +0500 Subject: [PATCH 08/13] MOBILE-341: Keep comments only where they document the public API --- .../embedded/compose/MindboxEmbeddedBlock.kt | 5 -- .../compose/MindboxEmbeddedBlockTest.kt | 4 -- .../embedded/MindboxEmbeddedBlockView.kt | 60 ------------------- .../webview/EmbeddedBlockWebViewHolder.kt | 25 -------- .../MindboxEmbeddedBlockViewCollapseTest.kt | 4 -- .../MindboxEmbeddedBlockViewLookupTest.kt | 3 - ...indboxEmbeddedBlockViewWrapperHooksTest.kt | 23 ------- .../webview/EmbeddedBlockWebViewHolderTest.kt | 7 --- 8 files changed, 131 deletions(-) diff --git a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt index d2eb62d3..0899999d 100644 --- a/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt +++ b/mindbox-embedded-compose/src/main/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlock.kt @@ -84,11 +84,6 @@ public fun MindboxEmbeddedBlock( mutableStateOf(MindboxEmbeddedBlockAppearance.PLACEHOLDER) } - // The budget is settled when the block is built and cannot be talked out of it afterwards, - // and a value quietly dropped is the kind of thing a host debugs against the clock. The View - // has no such trap — its timeout is a constructor argument — but this composable takes the - // parameter on every recomposition, so it says what it does with a new one, as the Flutter - // widget does. Once per value, not per frame: the effect is keyed on it. val creationTimeoutMs = remember { timeoutMs } if (timeoutMs != creationTimeoutMs) { LaunchedEffect(timeoutMs) { diff --git a/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt b/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt index d8d1d148..1491eb9e 100644 --- a/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt +++ b/mindbox-embedded-compose/src/test/java/cloud/mindbox/mobile_sdk/embedded/compose/MindboxEmbeddedBlockTest.kt @@ -164,9 +164,6 @@ class MindboxEmbeddedBlockTest { @Test fun `a budget changed after creation is ignored, and said out loud`() { - // The factory runs once, so the budget is settled there. A value quietly dropped is the kind - // of thing a host debugs against the clock — the View has no such trap, its timeout being a - // constructor argument, but this composable takes the parameter on every recomposition. val timeout = mutableStateOf(5_000) compose.setContent { @@ -187,7 +184,6 @@ class MindboxEmbeddedBlockTest { assertTrue(said.single().contains("timeoutMs=60000")) assertTrue(said.single().contains("keeps 5000")) - // Said once per value, not once per frame: a recomposition on the same value adds nothing. compose.runOnUiThread { timeout.value = 60_000 } settle() assertEquals(1, timeoutWarnings().size) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index ce17db5d..b88462b4 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -59,8 +59,6 @@ public class MindboxEmbeddedBlockView internal constructor( configTimeout: Milliseconds? = null, private val contentController: EmbeddedBlockContentController = EmbeddedBlockContentController( placeSystemName = placeSystemName.orNullIfBlank(), - // What the caller asked for, and only failing that what XML says: a block built in code has - // no attributes to read, and one built from XML has no caller to ask. configTimeout = configTimeout ?: readConfigTimeout(context, attrs), providerFactory = { content, attemptStartedAt -> EmbeddedBlockContentFactory.createProvider(context, content, attemptStartedAt) @@ -107,34 +105,10 @@ public class MindboxEmbeddedBlockView internal constructor( private var deliveredEvent: BlockEvent? = null private var isDeliveryScheduled = false - - /** - * Whether the block has already settled on a place without content — collapsed, or showing the - * host's error view. - * - * A settled block keeps what it shows. A retry — and the block gets one on every return to the - * screen — is not a reload: the page behind the place is the one that already failed, so it - * answers the same way, while the host watches its layout jerk by the block's height and a - * placeholder flash on every pass across the screen, to show nothing in the end. That holds for - * the error view just as much as for a collapse. - * - * Ended by content that actually appeared. - */ private var hasSettled = false - - /** What the block shows right now — the one source for both its own visibility and the report. */ private var shownAppearance = MindboxEmbeddedBlockAppearance.PLACEHOLDER - - /** - * Whether the window shows the block. Tracked rather than read off `windowVisibility`: the - * callback carries the new value while the property still holds the old one. - */ private var isWindowVisible = false - - /** Whether the wrapper's host still shows the block. Nobody says otherwise until a wrapper does. */ private var isHostVisible = true - - /** Whether a wrapper has released the block: then it stays stopped whatever the window says. */ private var isReleased = false private var shownContent: View? = null private var isContentStarted = false @@ -208,11 +182,6 @@ public class MindboxEmbeddedBlockView internal constructor( */ public fun setErrorView(view: View?) { errorView = view - // Re-decided rather than just swapped: whether a failure is shown at all depends on this - // view existing, so taking it away has to collapse the block and say so, not leave the old - // screen standing. What is on screen is the subject, not the state — a settled block goes on - // showing its error view while the attempt it gets on the way back loads — and a block that - // has already collapsed stays collapsed however this view changes. if (shownAppearance == MindboxEmbeddedBlockAppearance.ERROR) { applyState(state) } @@ -277,18 +246,9 @@ public class MindboxEmbeddedBlockView internal constructor( updateContentActivity() } - /** - * Whether anybody is looking at the block. The window answers that for a native host; a wrapper - * whose app has a single window answers the rest through [setHostVisible], and a released block - * is not looked at by definition. - */ private val isEffectivelyVisible: Boolean get() = isWindowVisible && isHostVisible && !isReleased - /** - * Three sources drive one switch — the window, the wrapper's host, a release — and each of them - * can repeat what another has already said, so the switch is idempotent. - */ private fun updateContentActivity() { if (isEffectivelyVisible) startContent() else pauseContent() } @@ -322,8 +282,6 @@ public class MindboxEmbeddedBlockView internal constructor( mindboxLogI("[EmbeddedBlock] Released by the host wrapper, freeing content") isReleased = true appearanceObserver = null - // A released block is not looked at, whatever the window says: the switch hears that here, - // or it would go on believing the content runs and pause a controller already released. updateContentActivity() detachFromHost() loggingRunCatching { contentController.release() } @@ -371,17 +329,12 @@ public class MindboxEmbeddedBlockView internal constructor( return } - // Read before it is written: a settled block's answer for `Loading` is the appearance it - // already had. val appearance = appearanceFor(state) shownAppearance = appearance hasSettled = when (appearance) { - // A place without content, however it is drawn, is what the block settles on. MindboxEmbeddedBlockAppearance.COLLAPSED, MindboxEmbeddedBlockAppearance.ERROR, -> true - // And content that actually appeared ends it: from here the block is an ordinary one - // again, and the next load it really starts is entitled to its placeholder. MindboxEmbeddedBlockAppearance.CONTENT -> false MindboxEmbeddedBlockAppearance.PLACEHOLDER -> hasSettled } @@ -391,8 +344,6 @@ public class MindboxEmbeddedBlockView internal constructor( mindboxLogI("[EmbeddedBlock] Content loading, showing the placeholder") showContent(currentPlaceholder()) } - // The guard above already turned a `Ready` without a view into a failure, so there is - // one to show here; the null-safe call is what keeps that guarantee from being an `!!`. MindboxEmbeddedBlockAppearance.CONTENT -> { mindboxLogI("[EmbeddedBlock] Content ready") contentController.contentView?.let { showContent(it) } @@ -414,10 +365,6 @@ public class MindboxEmbeddedBlockView internal constructor( private fun appearanceFor(state: EmbeddedBlockState): MindboxEmbeddedBlockAppearance = when (state) { - // A block that already settled keeps what it shows even while it loads anew: a retry - // earns neither the space the host has reclaimed nor a placeholder over the error view. - // What it does not keep is an error view the host has taken away in the meantime — - // there is nothing left to draw, so the block collapses. is EmbeddedBlockState.Loading -> when { !hasSettled -> MindboxEmbeddedBlockAppearance.PLACEHOLDER shownAppearance == MindboxEmbeddedBlockAppearance.ERROR && !hasCustomErrorView -> @@ -425,11 +372,6 @@ public class MindboxEmbeddedBlockView internal constructor( else -> shownAppearance } is EmbeddedBlockState.Ready -> MindboxEmbeddedBlockAppearance.CONTENT - // A failure is shown only to those who opted in explicitly; for the rest it collapses. - // A settled block keeps what it shows when the retry fails too: an error view given - // after the collapse must not reopen space the layout has reclaimed. The exception is - // the loading branch's one — an error view the host has taken away leaves nothing to - // draw, so the block collapses. is EmbeddedBlockState.Failed -> when { !hasSettled -> if (hasCustomErrorView) { @@ -441,8 +383,6 @@ public class MindboxEmbeddedBlockView internal constructor( MindboxEmbeddedBlockAppearance.COLLAPSED else -> shownAppearance } - // An empty place always collapses: a host cannot fill the space of a block that was - // never meant to be there, however it drew its failures. is EmbeddedBlockState.Empty -> MindboxEmbeddedBlockAppearance.COLLAPSED } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt index 940cbeff..c3326ddd 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt @@ -141,13 +141,6 @@ internal class EmbeddedBlockWebViewHolder( private val isUserPresent: Boolean get() = isActive && !isReleased - /** - * A failure that happened behind another screen, held until somebody looks at the block. - * - * The backend hears about blocks the user was shown, and a page that failed off screen was not - * one — the same rule the show follows. Only the first is kept: the block reports the outcome it - * came back to, and a silent page repeating itself adds nothing to that. - */ private val heldFailure = AtomicReference(null) private data class HeldFailure( @@ -156,13 +149,6 @@ internal class EmbeddedBlockWebViewHolder( val throwable: Throwable?, ) - /** - * How long the page took to render, frozen where the render was reported. - * - * The show can be counted much later — a page that finished behind another screen is counted by - * the [start] that brings it back — and `timeToDisplay` measures the wait for the page, not the - * user's absence from the screen. - */ @Volatile private var renderedTimeToDisplay: Milliseconds? = null override fun start() { @@ -175,8 +161,6 @@ internal class EmbeddedBlockWebViewHolder( return } report(lastState) - // A page that rendered behind another screen is only shown now, so this is where its show - // is counted. if (lastState == EmbeddedBlockState.Ready) accountForShow() } @@ -326,7 +310,6 @@ internal class EmbeddedBlockWebViewHolder( register(WebViewAction.CONTENT_RENDERED, ::handleContentRenderedAction) register(WebViewAction.SHOW_IN_APP, ::handleShowInAppAction) registerSuspend(WebViewAction.FILTER_SHOWABLE_INAPPS, ::handleFilterShowableInappsAction) - // The same question under the name pages already shipped send — see CHECK_INAPPS_TARGETING. registerSuspend(WebViewAction.CHECK_INAPPS_TARGETING, ::handleFilterShowableInappsAction) } } @@ -421,8 +404,6 @@ internal class EmbeddedBlockWebViewHolder( } renderedTimeToDisplay = timeProvider.elapsedSince(attemptStartedAt) report(EmbeddedBlockState.Ready) - // Only what somebody is looking at counts as shown; a page that rendered off screen is - // counted by the `start()` that brings it back. if (isActive) accountForShow() return BridgeMessage.SUCCESS_PAYLOAD } @@ -510,9 +491,6 @@ internal class EmbeddedBlockWebViewHolder( onContentPageLoaded(content) } - /** - * Sent when the block is on screen, held for the return when it is not — see [heldFailure]. - */ private fun sendFailure( failureReason: FailureReason, errorDescription: String, @@ -524,9 +502,6 @@ internal class EmbeddedBlockWebViewHolder( "report until the block is looked at" ) heldFailure.compareAndSet(null, HeldFailure(failureReason, errorDescription, throwable)) - // Failures come off mindboxScope while start() flips the block visible on the main - // thread: a start() between the check above and the hold has flushed before the failure - // was there. Re-check and flush; getAndSet keeps the report single whichever side wins. if (isActive) flushHeldFailure() return } diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt index c3b5b7b9..f8a4367f 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewCollapseTest.kt @@ -166,8 +166,6 @@ class MindboxEmbeddedBlockViewCollapseTest { blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) idle() - // The retry failed the same way the block already settled on: the error view is remembered - // for a load that starts the cycle anew, not for a pass across the screen. assertEquals(View.GONE, view.visibility) } @@ -181,13 +179,11 @@ class MindboxEmbeddedBlockViewCollapseTest { assertEquals(View.GONE, view.visibility) view.setErrorView(View(activity)) - // Content that actually appeared ends the settlement — the block is an ordinary one again... leaveAndReturn(view) blocksRegistry.lastHandle?.onContentResolved(embeddedContent()) idle() assertEquals(View.VISIBLE, view.visibility) - // ...so the next failure is entitled to the error view the host gave meanwhile. lastProvider?.onStateChange?.invoke(EmbeddedBlockState.Failed) idle() assertEquals(View.VISIBLE, view.visibility) diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt index ec288dcd..8faf056a 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt @@ -80,7 +80,6 @@ class MindboxEmbeddedBlockViewLookupTest { @Test fun `a timeout given in code is the one the block waits out`() { - // The wrapper path: no XML to carry `mindboxTimeoutMs`, so the budget comes as an argument. val view = MindboxEmbeddedBlockView(activity, "main-screen-top", timeoutMs = 5_000L) val listener = RecordingListener() view.setListener(listener) @@ -93,7 +92,6 @@ class MindboxEmbeddedBlockViewLookupTest { shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(2L)) - // Six seconds in — long past the five it was given, and long before the default thirty. assertEquals(View.GONE, view.visibility) assertEquals(listOf("fail"), listener.events) } @@ -118,7 +116,6 @@ class MindboxEmbeddedBlockViewLookupTest { @Test fun `a non-positive timeout is not a budget, and the default stands`() { - // Honoured literally it would collapse every block before the config had a chance. val view = MindboxEmbeddedBlockView(activity, "main-screen-top", timeoutMs = 0L) val listener = RecordingListener() view.setListener(listener) diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt index 76c05407..5e71d968 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt @@ -15,12 +15,6 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf import java.io.Closeable -/** - * What a wrapper sees and says: the appearance the block reports, and the host visibility it is - * told about. A wrapper lays the block out itself, so it needs the decision the view would have - * applied to its own `visibility` — and it has to be able to say that its screen went away when the - * window cannot say it. - */ @RunWith(RobolectricTestRunner::class) class MindboxEmbeddedBlockViewWrapperHooksTest { @@ -83,13 +77,11 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { shadowOf(Looper.getMainLooper()).idle() } - /** Resolves the place to content, which is what builds the provider. */ private fun showContent() { blocksRegistry.lastHandle?.onContentResolved(InAppStub.getEmbedded() as InAppType.Embedded) idle() } - /** Fails the page behind a place that already resolved to content. */ private fun failContent() { provider.onStateChange?.invoke(EmbeddedBlockState.Failed) idle() @@ -102,8 +94,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { blocksRegistry.lastHandle?.onContentResolved(null) idle() - // The place settled before anybody subscribed, and its only report would otherwise be lost: - // a wrapper built after the outcome would sit on a loading screen forever. val seen = mutableListOf() view.setAppearanceObserver { seen.add(it) } @@ -135,8 +125,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { blocksRegistry.lastHandle?.onContentResolved(null) idle() - // An error view opts into showing a failure, not into filling a place that was never meant - // to hold anything. assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) assertEquals(View.GONE, view.visibility) } @@ -155,7 +143,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { view.setHostVisible(true) idle() - // The window never changed — only the wrapper's word did, and it drives the same switch. assertTrue(provider.startCount > startsWhenShown) } @@ -185,8 +172,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { view.setErrorView(null) - // Whether a failure is shown at all is this view's doing, so taking it away is not a swap of - // screens — it is the block going back to the collapse it would have had, and saying so. assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) assertEquals(View.GONE, view.visibility) } @@ -220,8 +205,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { view.setErrorView(View(activity)) - // Reopening space the layout has already reclaimed would make it jump: the view waits for - // the next load. assertEquals(MindboxEmbeddedBlockAppearance.COLLAPSED, seen.last()) assertEquals(View.GONE, view.visibility) } @@ -237,8 +220,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { failContent() assertEquals(MindboxEmbeddedBlockAppearance.ERROR, seen.last()) - // Off the screen and back: the failed block drops its page and waits for a new answer, so - // the state is `Loading` again while the error view is still the thing on screen. dispatchWindowVisibility(view, View.GONE) idle() dispatchWindowVisibility(view, View.VISIBLE) @@ -283,8 +264,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { view.release() idle() - // Released while the window still says it is shown: the switch has to hear it here, or it - // goes on believing the content runs and pauses a controller that is already released. assertEquals(1, provider.pauseCount) } @@ -295,8 +274,6 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { view.setAppearanceObserver { throw IllegalStateException("the channel is not ready yet") } - // The same callback is guarded everywhere else; the immediate call on subscribing cannot be - // the one place where a wrapper's exception takes the host down with it. val seen = mutableListOf() view.setAppearanceObserver { seen.add(it) } assertEquals(1, seen.size) diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt index 8d19c861..a8b46317 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt @@ -66,7 +66,6 @@ class EmbeddedBlockWebViewHolderTest { private val webPageRegistry: MindboxWebPageRegistry = mockk(relaxUnitFun = true) private val inAppFailureTracker: InAppFailureTracker = mockk(relaxed = true) - /** What [SystemTimeProvider] would return: the tests move it by hand. */ private var elapsed = 0L private val timeProvider: SystemTimeProvider = mockk { every { elapsedSince(any()) } answers { Milliseconds(elapsed) } @@ -417,9 +416,6 @@ class EmbeddedBlockWebViewHolderTest { verify(exactly = 0) { inAppMessageManager.showInAppById(any(), any()) } } - // Every other test asks with `filterShowableInapps`, so this is where the shipped pages' - // spelling proves it is answered identically — under the name it asked with, or the page does - // not recognise the answer as its own. @Test fun `the shipped checkInappsTargeting name is answered the same way`() { coEvery { inAppInteractor.filterShowableInAppIds(listOf("story-1", "story-2")) } returns @@ -759,8 +755,6 @@ class EmbeddedBlockWebViewHolderTest { holder.pause() postFromPage(request(action = "contentRendered", payload = """{"count":"many"}""")) - // The backend hears about blocks the user was shown, and this page failed behind another - // screen. verify(exactly = 0) { inAppFailureTracker.sendFailure(any(), any(), any(), any()) } holder.start() @@ -795,7 +789,6 @@ class EmbeddedBlockWebViewHolderTest { holder.pause() elapsed = 1_000L postFromPage(request(action = "contentRendered", payload = """{"count":3}""")) - // Two minutes on another screen, which is not time the user spent waiting for this page. elapsed = 121_000L holder.start() From 2a278135e9beb60cb31b62ca28cb80ae406ad282 Mon Sep 17 00:00:00 2001 From: Vailence Date: Thu, 27 Aug 2026 13:20:53 +0500 Subject: [PATCH 09/13] MOBILE-341: Stop answering the targeting question under its old name iOS dropped its half of this, so the pair no longer holds: `checkInappsTargeting` was the pre-rename spelling, and answering both names on Android alone only keeps a name alive that no platform is committed to. `filterShowableInapps` is the one name the bridge speaks. The constant, its registration, and the reverted test go back to what they were before the second spelling was added: a page that still asks the old name gets no answer and the interactor is never consulted. --- .../embedded/webview/EmbeddedBlockWebViewHolder.kt | 1 - .../inapp/presentation/view/WebViewAction.kt | 14 -------------- .../webview/EmbeddedBlockWebViewHolderTest.kt | 13 ++++--------- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt index c3326ddd..c24dabfc 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolder.kt @@ -310,7 +310,6 @@ internal class EmbeddedBlockWebViewHolder( register(WebViewAction.CONTENT_RENDERED, ::handleContentRenderedAction) register(WebViewAction.SHOW_IN_APP, ::handleShowInAppAction) registerSuspend(WebViewAction.FILTER_SHOWABLE_INAPPS, ::handleFilterShowableInappsAction) - registerSuspend(WebViewAction.CHECK_INAPPS_TARGETING, ::handleFilterShowableInappsAction) } } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt index 56d534a9..db3ddfc5 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/presentation/view/WebViewAction.kt @@ -82,20 +82,6 @@ public enum class WebViewAction { @SerializedName("filterShowableInapps") FILTER_SHOWABLE_INAPPS, - /** - * The same question as [FILTER_SHOWABLE_INAPPS] under the name pages already shipped send. - * - * A second constant and not a Gson `alternate`: `alternate` is read-only, so the answer would go - * back named `filterShowableInapps` while the page waits for one to the name it asked under — - * and an answer the page does not recognise is the same silence as none. An unanswered question - * is a page that renders an empty feed, then a `contentRendered: 0`, then a block that collapses - * and reports an empty place for no reason of its own. - * - * Both spellings are answered until the web side settles on one, in sync with iOS. - */ - @SerializedName("checkInappsTargeting") - CHECK_INAPPS_TARGETING, - @SerializedName("localState.changed") LOCAL_STATE_CHANGED, diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt index a8b46317..e90344f1 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/webview/EmbeddedBlockWebViewHolderTest.kt @@ -417,18 +417,13 @@ class EmbeddedBlockWebViewHolderTest { } @Test - fun `the shipped checkInappsTargeting name is answered the same way`() { - coEvery { inAppInteractor.filterShowableInAppIds(listOf("story-1", "story-2")) } returns - listOf("story-1") + fun `the old checkInappsTargeting name is not spoken anymore`() { startAndAwaitPageLoad() - postFromPage(request(action = "checkInappsTargeting", payload = """{"inappIds":["story-1","story-2"]}""")) - await { lastOutgoingMessage()?.get("action")?.asString == "checkInappsTargeting" } + postFromPage(request(action = "checkInappsTargeting", payload = """{"inappIds":["story-1"]}""")) - assertEquals("response", lastOutgoingMessage()!!.get("type").asString) - val payload = lastOutgoingPayload()!! - assertEquals(1, payload.getAsJsonArray("inappIds").size()) - assertEquals("story-1", payload.getAsJsonArray("inappIds").get(0).asString) + coVerify(exactly = 0) { inAppInteractor.filterShowableInAppIds(any()) } + assertTrue(lastOutgoingMessage()?.get("action")?.asString != "checkInappsTargeting") } @Test From 194cee24e43c76473d5feec896effafe7a335879 Mon Sep 17 00:00:00 2001 From: Vailence Date: Thu, 27 Aug 2026 22:12:43 +0500 Subject: [PATCH 10/13] MOBILE-341: Report an outcome once, however many passes the screen takes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deliverPendingEvent wrote LOADING into deliveredEvent, so the outcome that followed always looked new — even when it was the one the host had already heard. The controller reports Loading again on every start(), and that delivery lands before the resolve answers, so a place that is not in the config called onFail on every pass: every foreground, every re-attach of a recycled row, every setHostVisible(true) from Flutter. The listener KDoc promises each outcome once, and iOS already behaves that way — it returns nil for .loading and leaves the record alone. Loading is no longer an event at all: the delivery returns early and the record keeps the last real outcome. The LOADING case of the enum and its branch are gone with it. --- .../embedded/MindboxEmbeddedBlockView.kt | 7 ++++--- .../MindboxEmbeddedBlockViewLookupTest.kt | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index b88462b4..9365d39e 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -95,7 +95,7 @@ public class MindboxEmbeddedBlockView internal constructor( private val defaultPlaceholder by lazy { EmbeddedBlockDefaultViews.placeholder(context) } private val mainHandler = Handler(Looper.getMainLooper()) - private enum class BlockEvent { LOADING, LOADED, FAILED } + private enum class BlockEvent { LOADED, FAILED } private var state: EmbeddedBlockState = EmbeddedBlockState.Loading set(value) { @@ -409,10 +409,12 @@ public class MindboxEmbeddedBlockView internal constructor( private fun deliverPendingEvent(): Unit = loggingRunCatching { isDeliveryScheduled = false + // Loading is not an outcome, so the record of what was delivered is left alone: writing it + // down would make the outcome that follows look new even when it is the one already heard. val event = when { state is EmbeddedBlockState.Ready -> BlockEvent.LOADED state.nothingToShow -> BlockEvent.FAILED - else -> BlockEvent.LOADING + else -> return@loggingRunCatching } if (event == deliveredEvent) return@loggingRunCatching @@ -420,7 +422,6 @@ public class MindboxEmbeddedBlockView internal constructor( when (event) { BlockEvent.LOADED -> listener.onLoad(this) BlockEvent.FAILED -> listener.onFail(this) - BlockEvent.LOADING -> Unit } } diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt index 8faf056a..c865f8b4 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt @@ -176,6 +176,26 @@ class MindboxEmbeddedBlockViewLookupTest { assertNull(view.placeSystemName) } + @Test + fun `a place that stays empty reports its outcome once across passes`() { + val view = MindboxEmbeddedBlockView(activity, "main-screen-top") + val listener = RecordingListener() + view.setListener(listener) + + attach(view) + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(31L)) + + assertEquals(listOf("fail"), listener.events) + + // A second pass across the screen: the same empty place is the same outcome, not news. + (view.parent as LinearLayout).removeView(view) + shadowOf(Looper.getMainLooper()).idle() + attach(view) + shadowOf(Looper.getMainLooper()).idleFor(Duration.ofSeconds(31L)) + + assertEquals(listOf("fail"), listener.events) + } + @Test fun `the block behaves with no listener at all`() { // The show/hide behavior belongs to the block, not to the host's callbacks. From fedf59ee43fe7a435be4f5130fe0a2bc67840f84 Mon Sep 17 00:00:00 2001 From: Vailence Date: Thu, 27 Aug 2026 22:14:49 +0500 Subject: [PATCH 11/13] MOBILE-341: Stop the registry from outliving the blocks it registered handlesByPlace is a process-wide map, and it held its handles strongly. The only paths that ever closed a registration were release() from a wrapper and the onDestroy of a ViewTreeLifecycleOwner, so a block in a plain Dialog, in a PopupWindow, or in an app that swaps its own views had neither: findViewTreeLifecycleOwner() is empty there, nothing was installed, and closing the host leaked the block, its view, its WebView and the Activity behind them. iOS has a third exit in deinit; Android had two. The map now keeps weak references and prunes the cleared ones whenever a place is looked up, so letting the view go is enough on Android as well. Nothing else changes: the handle is the content controller itself, which the view holds for as long as it lives. --- .../embedded/EmbeddedBlocksRegistry.kt | 57 +++++++++++++------ .../embedded/EmbeddedBlocksRegistryTest.kt | 32 +++++++++++ 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt index 3f9060f3..deb599a6 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.launch import java.io.Closeable +import java.lang.ref.WeakReference /** * One registered block: how the registry talks back to a view. @@ -40,7 +41,10 @@ internal class EmbeddedBlocksRegistryImpl( private val scopeProvider: () -> CoroutineScope = { Mindbox.mindboxScope }, ) : EmbeddedBlocksRegistry { - private val handlesByPlace = mutableMapOf>() + // Weak on purpose: a host with no lifecycle owner to say goodbye — a plain Dialog, a + // PopupWindow, an app that swaps views itself — only lets its block go, and a strong entry in + // this process-wide map would keep the block, its view and the Activity behind it alive. + private val handlesByPlace = mutableMapOf>>() private val resolvingPlaces = mutableSetOf() private val reResolveQueuedPlaces = mutableMapOf() @@ -85,16 +89,15 @@ internal class EmbeddedBlocksRegistryImpl( val place = placeSystemName.trim() runOnMain { restartChannelsIfDead() - handlesByPlace.getOrPut(place) { mutableListOf() }.add(handle) + handlesByPlace.getOrPut(place) { mutableListOf() }.add(WeakReference(handle)) mindboxLogI("[EmbeddedBlock] Block registered for place '$place'") } return Closeable { runOnMain { - handlesByPlace[place]?.remove(handle) - if (handlesByPlace[place]?.isEmpty() == true) { - handlesByPlace.remove(place) - reResolveQueuedPlaces.remove(place) + handlesByPlace[place]?.removeAll { reference -> + reference.get().let { registered -> registered === handle || registered == null } } + forgetPlaceIfEmpty(place) mindboxLogI("[EmbeddedBlock] Block unregistered from place '$place'") } } @@ -108,9 +111,25 @@ internal class EmbeddedBlocksRegistryImpl( } } + private fun liveHandles(place: String): List { + val references = handlesByPlace[place] ?: return emptyList() + val handles = references.mapNotNull { reference -> reference.get() } + if (handles.size != references.size) { + references.removeAll { reference -> reference.get() == null } + forgetPlaceIfEmpty(place) + } + return handles + } + + private fun forgetPlaceIfEmpty(place: String) { + if (handlesByPlace[place]?.isEmpty() != true) return + handlesByPlace.remove(place) + reResolveQueuedPlaces.remove(place) + } + private fun onPlaceEvent(place: String, triggerEvent: InAppEventType) { - val handles = handlesByPlace[place] - if (handles.isNullOrEmpty()) { + val handles = liveHandles(place) + if (handles.isEmpty()) { mindboxLogI("[EmbeddedBlock] Operation matched place '$place' but no block is registered, dropping") return } @@ -156,23 +175,27 @@ internal class EmbeddedBlocksRegistryImpl( } private fun invalidateAll(reason: String) { - handlesByPlace.forEach { (place, handles) -> - if (handles.any { handle -> handle.isActive }) { - mindboxLogI("[EmbeddedBlock] Re-resolving place '$place' ($reason)") - resolvePlace(place) - } else { - mindboxLogI("[EmbeddedBlock] Place '$place' is paused, nowhere to display — skipping ($reason)") + handlesByPlace.keys.toList().forEach { place -> + val handles = liveHandles(place) + when { + handles.isEmpty() -> Unit + handles.any { handle -> handle.isActive } -> { + mindboxLogI("[EmbeddedBlock] Re-resolving place '$place' ($reason)") + resolvePlace(place) + } + else -> + mindboxLogI("[EmbeddedBlock] Place '$place' is paused, nowhere to display — skipping ($reason)") } } } private fun deliver(place: String, content: InAppType.Embedded?) { - val handles = handlesByPlace[place] - if (handles.isNullOrEmpty()) { + val handles = liveHandles(place) + if (handles.isEmpty()) { mindboxLogW("[EmbeddedBlock] No block is registered for place '$place', dropping the content") return } - handles.toList().forEach { handle -> + handles.forEach { handle -> loggingRunCatching { handle.onContentResolved(content) } } } diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistryTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistryTest.kt index 35f6ba70..a25870f3 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistryTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistryTest.kt @@ -18,11 +18,13 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.Shadows.shadowOf +import java.lang.ref.WeakReference /** * The central controller is deliberately dumb: a registry and a router. Both directions call @@ -129,6 +131,36 @@ class EmbeddedBlocksRegistryTest { coVerify(exactly = 0) { interactor.selectInAppForPlace(any(), any()) } } + @Test + fun `a block whose host is gone stops holding its place`() { + val operation = InAppEventType.OrdinalEvent(EventType.AsyncOperation("story-operation")) + val controller = controller() + var handle: RecordingHandle? = RecordingHandle() + val handleReference = WeakReference(handle) + controller.register(place, handle!!) + idleMain() + + // A host without a lifecycle owner never says goodbye — it only lets the block go. The + // registry must not be what keeps it, and the Activity behind it, in this process. + handle = null + awaitCollection(handleReference) + + scope.launch { placeEvents.emit(EmbeddedPlaceEvent(place, operation)) } + idleMain() + + coVerify(exactly = 0) { interactor.selectInAppForPlace(any(), any()) } + } + + private fun awaitCollection(reference: WeakReference<*>) { + repeat(50) { + if (reference.get() == null) return + System.gc() + System.runFinalization() + Thread.sleep(10L) + } + assertNull("the registry is still holding the handle of a host that is gone", reference.get()) + } + @Test fun `operation for a paused place is skipped and the next appearance resolves fresh`() { val operation = InAppEventType.OrdinalEvent(EventType.AsyncOperation("story-operation")) From 3029b17cd41d5c7741d12db4b345571d972a7e41 Mon Sep 17 00:00:00 2001 From: Vailence Date: Thu, 27 Aug 2026 22:17:36 +0500 Subject: [PATCH 12/13] MOBILE-341: Make release one way, and say what it is for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A released block still took a listener: setListener passed the identity check, cleared the record of what was delivered and scheduled a delivery, so a dead block called the host back once. setAppearanceObserver rearmed the observer release had just dropped, and a re-attached released view registered hostDestroyObserver on the host lifecycle again — a stale registration, since the content stays stopped. iOS closes both doors with `guard !isReleased` on the delegate. Both setters and the lifecycle observation now check isReleased, and release() has the KDoc its iOS twin has: what it stops, that it is for wrappers rather than host applications, and that it is one way. --- .../embedded/MindboxEmbeddedBlockView.kt | 17 ++++++++ ...indboxEmbeddedBlockViewWrapperHooksTest.kt | 40 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index 9365d39e..a22a9608 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -141,6 +141,8 @@ public class MindboxEmbeddedBlockView internal constructor( } public fun setListener(listener: MindboxEmbeddedBlockListener?) { + if (isReleased) return + val next = listener ?: DefaultListener // The same listener is not a new subscriber. A host rebinds it on every recycled row, and // replaying an outcome it already heard would have it rebuild its layout again — which @@ -196,6 +198,8 @@ public class MindboxEmbeddedBlockView internal constructor( */ @InternalMindboxApi public fun setAppearanceObserver(observer: ((MindboxEmbeddedBlockAppearance) -> Unit)?) { + if (isReleased) return + appearanceObserver = observer loggingRunCatching { observer?.invoke(shownAppearance) } } @@ -268,6 +272,7 @@ public class MindboxEmbeddedBlockView internal constructor( } private fun observeHostDestruction(): Unit = loggingRunCatching { + if (isReleased) return@loggingRunCatching val lifecycle = findViewTreeLifecycleOwner()?.lifecycle ?: return@loggingRunCatching if (lifecycle === observedLifecycle) return@loggingRunCatching observedLifecycle?.removeObserver(hostDestroyObserver) @@ -275,6 +280,18 @@ public class MindboxEmbeddedBlockView internal constructor( lifecycle.addObserver(hostDestroyObserver) } + /** + * Stops the block for good: the content stops, the host screen is no longer observed, the + * listener is dropped, and the block does not start again even while it stays in a window. + * + * For wrappers whose own object graph dies before the view leaves the window — a platform-view + * factory keeps the view for as long as the platform sees fit, and the block should stop when + * the screen is gone rather than when the last reference is. A host application needs nothing: + * a block that leaves the window pauses itself, and the destruction of the host screen frees it. + * + * One way only: a released block stays released, and [setListener] and [setAppearanceObserver] + * on it do nothing. + */ @InternalMindboxApi public fun release() { if (isReleased) return diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt index 5e71d968..ed643273 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewWrapperHooksTest.kt @@ -279,6 +279,46 @@ class MindboxEmbeddedBlockViewWrapperHooksTest { assertEquals(1, seen.size) } + @Test + fun `a released block takes no new listener`() { + val view = buildView() + attach(view) + showContent() + view.release() + idle() + + val heard = mutableListOf() + view.setListener( + object : MindboxEmbeddedBlockListener { + override fun onLoad(view: MindboxEmbeddedBlockView) { + heard.add("load") + } + + override fun onFail(view: MindboxEmbeddedBlockView) { + heard.add("fail") + } + }, + ) + idle() + + assertTrue(heard.isEmpty()) + } + + @Test + fun `a released block takes no new appearance observer`() { + val view = buildView() + attach(view) + showContent() + view.release() + idle() + + val seen = mutableListOf() + view.setAppearanceObserver { appearance -> seen.add(appearance) } + idle() + + assertTrue(seen.isEmpty()) + } + @Test fun `a released block stays stopped even when the window says it is shown`() { val view = buildView() From aeb8143d6332101f34c62632761e0dcd98816dea Mon Sep 17 00:00:00 2001 From: Vailence Date: Fri, 28 Aug 2026 01:50:04 +0500 Subject: [PATCH 13/13] MOBILE-341: Take a place name as it is, and only ask whether it is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android trimmed the name in five places — the config mapping, the selection, the place filter, the registry's host input and the operation events — while iOS trimmed only the config. A name copied from the admin panel with a trailing space therefore worked on Android and silently collapsed on iOS: the same widget, the same config. Rather than teach the other side to trim, both now compare what they were given. The only question left about a name is whether it is empty, so the view asks isNotEmpty instead of isNotBlank and the variant validator asks isNullOrEmpty instead of isNullOrBlank: a name of spaces is a name like any other, and a place nobody spelled that way simply never resolves. --- .../embedded/EmbeddedBlocksRegistry.kt | 16 +++++++--------- .../embedded/MindboxEmbeddedBlockView.kt | 6 +++--- .../mobile_sdk/inapp/data/mapper/InAppMapper.kt | 2 +- .../data/validators/EmbeddedVariantValidator.kt | 2 +- .../inapp/domain/InAppFilteringManagerImpl.kt | 7 +++---- .../inapp/domain/InAppInteractorImpl.kt | 15 +++++++-------- .../MindboxEmbeddedBlockViewLookupTest.kt | 16 ++++++++++++++-- .../inapp/data/mapper/EmbeddedMapperTest.kt | 4 ++-- .../validators/EmbeddedVariantValidatorTest.kt | 7 ++++++- .../inapp/domain/EmbeddedFilteringManagerTest.kt | 4 ++-- 10 files changed, 46 insertions(+), 33 deletions(-) diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt index deb599a6..9816af39 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/EmbeddedBlocksRegistry.kt @@ -72,7 +72,7 @@ internal class EmbeddedBlocksRegistryImpl( scope.launch { inAppInteractor.listenEmbeddedPlaceEvents().collect { placeEvent -> runOnMain { - onPlaceEvent(placeEvent.placeSystemName.trim(), placeEvent.triggerEvent) + onPlaceEvent(placeEvent.placeSystemName, placeEvent.triggerEvent) } } }, @@ -86,28 +86,26 @@ internal class EmbeddedBlocksRegistryImpl( } override fun register(placeSystemName: String, handle: EmbeddedBlockHandle): Closeable { - val place = placeSystemName.trim() runOnMain { restartChannelsIfDead() - handlesByPlace.getOrPut(place) { mutableListOf() }.add(WeakReference(handle)) - mindboxLogI("[EmbeddedBlock] Block registered for place '$place'") + handlesByPlace.getOrPut(placeSystemName) { mutableListOf() }.add(WeakReference(handle)) + mindboxLogI("[EmbeddedBlock] Block registered for place '$placeSystemName'") } return Closeable { runOnMain { - handlesByPlace[place]?.removeAll { reference -> + handlesByPlace[placeSystemName]?.removeAll { reference -> reference.get().let { registered -> registered === handle || registered == null } } - forgetPlaceIfEmpty(place) - mindboxLogI("[EmbeddedBlock] Block unregistered from place '$place'") + forgetPlaceIfEmpty(placeSystemName) + mindboxLogI("[EmbeddedBlock] Block unregistered from place '$placeSystemName'") } } } override fun onBlockAppeared(placeSystemName: String) { - val place = placeSystemName.trim() runOnMain { restartChannelsIfDead() - resolvePlace(place) + resolvePlace(placeSystemName) } } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt index a22a9608..1f3f0000 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockView.kt @@ -58,7 +58,7 @@ public class MindboxEmbeddedBlockView internal constructor( placeSystemName: String?, configTimeout: Milliseconds? = null, private val contentController: EmbeddedBlockContentController = EmbeddedBlockContentController( - placeSystemName = placeSystemName.orNullIfBlank(), + placeSystemName = placeSystemName.orNullIfEmpty(), configTimeout = configTimeout ?: readConfigTimeout(context, attrs), providerFactory = { content, attemptStartedAt -> EmbeddedBlockContentFactory.createProvider(context, content, attemptStartedAt) @@ -87,7 +87,7 @@ public class MindboxEmbeddedBlockView internal constructor( timeoutMs: Long? = null, ) : this(context, null, placeSystemName, timeoutMs?.let(::Milliseconds)) - public val placeSystemName: String? = placeSystemName.orNullIfBlank() + public val placeSystemName: String? = placeSystemName.orNullIfEmpty() private var listener: MindboxEmbeddedBlockListener = DefaultListener private var appearanceObserver: ((MindboxEmbeddedBlockAppearance) -> Unit)? = null private var placeholderView: View? = null @@ -447,7 +447,7 @@ public class MindboxEmbeddedBlockView internal constructor( } } -private fun String?.orNullIfBlank(): String? = this?.takeIf { it.isNotBlank() } +private fun String?.orNullIfEmpty(): String? = this?.takeIf { it.isNotEmpty() } private fun readPlaceSystemName(context: Context, attrs: AttributeSet?): String? { if (attrs == null) return null diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/InAppMapper.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/InAppMapper.kt index fcd40d4e..c3377977 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/InAppMapper.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/InAppMapper.kt @@ -275,7 +275,7 @@ internal class InAppMapper { is PayloadDto.EmbeddedDto -> { InAppType.Embedded( inAppId = inAppDto.id, - placeSystemName = payloadDto.placeSystemName!!.trim(), + placeSystemName = payloadDto.placeSystemName!!, layers = mapBackgroundLayers( payloadDto.content?.background?.layers ?.filterIsInstance() diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidator.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidator.kt index 77e1a35d..ebc9764d 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidator.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidator.kt @@ -20,7 +20,7 @@ internal class EmbeddedVariantValidator( ) return false } - if (item.placeSystemName.isNullOrBlank()) { + if (item.placeSystemName.isNullOrEmpty()) { mindboxLogW("InApp is invalid. Embedded variant has no placeSystemName") return false } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppFilteringManagerImpl.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppFilteringManagerImpl.kt index 0cdb7b79..ca03f82a 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppFilteringManagerImpl.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppFilteringManagerImpl.kt @@ -53,14 +53,13 @@ internal class InAppFilteringManagerImpl( inApps: List, placeSystemName: String ): List { - val requestedPlace = placeSystemName.trim() return inApps.filter { inApp -> inApp.embeddedVariants().any { variant -> - val matches = variant.placeSystemName == requestedPlace - if (!matches && variant.placeSystemName.equals(requestedPlace, ignoreCase = true)) { + val matches = variant.placeSystemName == placeSystemName + if (!matches && variant.placeSystemName.equals(placeSystemName, ignoreCase = true)) { mindboxLogW( "Place names differ only in letter case: config has " + - "'${variant.placeSystemName}', the block asked for '$requestedPlace'. " + + "'${variant.placeSystemName}', the block asked for '$placeSystemName'. " + "The comparison is case-sensitive, the candidate is skipped" ) } diff --git a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppInteractorImpl.kt b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppInteractorImpl.kt index da75b928..2766e964 100644 --- a/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppInteractorImpl.kt +++ b/sdk/src/main/java/cloud/mindbox/mobile_sdk/inapp/domain/InAppInteractorImpl.kt @@ -116,29 +116,28 @@ internal class InAppInteractorImpl( placeSystemName: String, triggerEvent: InAppEventType, ): InAppType.Embedded? { - val requestedPlace = placeSystemName.trim() val inApps = mobileConfigRepository.getInAppsSection() inAppRepository.saveCurrentSessionInApps(inApps) // The same chain as the event path: the display style does not change "when to show". val candidates = abTestFilteredInApps(inApps) - .let { inAppFilteringManager.filterEmbeddedInAppsByPlace(it, requestedPlace) } + .let { inAppFilteringManager.filterEmbeddedInAppsByPlace(it, placeSystemName) } val winner = chooseAmongCandidates( - logLabel = "Place '$requestedPlace'", + logLabel = "Place '$placeSystemName'", candidates = candidates, triggerEvent = triggerEvent, selectVariant = { candidate -> candidate.form.variants .filterIsInstance() - .firstOrNull { variant -> variant.placeSystemName == requestedPlace } + .firstOrNull { variant -> variant.placeSystemName == placeSystemName } } ) ?: run { - logI("Place '$requestedPlace': nothing to show") + logI("Place '$placeSystemName': nothing to show") return null } if (!areShowLimitsAllowed(winner)) { - logI("Place '$requestedPlace': in-app ${winner.id} is blocked by the show limits") + logI("Place '$placeSystemName': in-app ${winner.id} is blocked by the show limits") return null } // Place resolves repeat for reasons that offer nothing new — the block reappears, a config @@ -147,11 +146,11 @@ internal class InAppInteractorImpl( if (sessionStorageManager.placeTargetingReportedInSession.add(winner.id)) { inAppProcessingManager.sendTargetedInApp(winner) } else { - logI("Place '$requestedPlace': in-app ${winner.id} already sent its targeting this session") + logI("Place '$placeSystemName': in-app ${winner.id} already sent its targeting this session") } return winner.form.variants .filterIsInstance() - .firstOrNull { variant -> variant.placeSystemName == requestedPlace } + .firstOrNull { variant -> variant.placeSystemName == placeSystemName } } private suspend fun abTestFilteredInApps(inApps: List): List = diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt index c865f8b4..053b779b 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/embedded/MindboxEmbeddedBlockViewLookupTest.kt @@ -167,8 +167,8 @@ class MindboxEmbeddedBlockViewLookupTest { } @Test - fun `a blank place name is the same as none`() { - val view = MindboxEmbeddedBlockView(activity, " ") + fun `an empty place name is the same as none`() { + val view = MindboxEmbeddedBlockView(activity, "") attach(view) @@ -176,6 +176,18 @@ class MindboxEmbeddedBlockViewLookupTest { assertNull(view.placeSystemName) } + @Test + fun `a place name of spaces is a name like any other`() { + // Only emptiness is checked: the name goes to the config as it was given, padding and all, + // and a place nobody named that way simply never resolves. + val view = MindboxEmbeddedBlockView(activity, " ") + + attach(view) + + assertEquals(" ", view.placeSystemName) + assertEquals(View.VISIBLE, view.visibility) + } + @Test fun `a place that stays empty reports its outcome once across passes`() { val view = MindboxEmbeddedBlockView(activity, "main-screen-top") diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/EmbeddedMapperTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/EmbeddedMapperTest.kt index cfcc08e7..42bebb0c 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/EmbeddedMapperTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/mapper/EmbeddedMapperTest.kt @@ -78,7 +78,7 @@ class EmbeddedMapperTest { } @Test - fun `place system name is trimmed on mapping`() { + fun `place system name is mapped as it is`() { val dto = baseDto.copy( form = FormDto(variants = listOf(InAppStub.getEmbeddedDto().copy(placeSystemName = " main-screen-top "))) ) @@ -86,7 +86,7 @@ class EmbeddedMapperTest { val variant = mapper.mapToInAppConfig(config(dto)).inApps.single() .form.variants.single() as InAppType.Embedded - assertEquals("main-screen-top", variant.placeSystemName) + assertEquals(" main-screen-top ", variant.placeSystemName) } @Test diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidatorTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidatorTest.kt index 0c1db908..ca9b1498 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidatorTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/data/validators/EmbeddedVariantValidatorTest.kt @@ -41,7 +41,12 @@ class EmbeddedVariantValidatorTest { @Test fun `missing place system name is invalid`() { assertFalse(validator.isValid(valid.copy(placeSystemName = null))) - assertFalse(validator.isValid(valid.copy(placeSystemName = " "))) + assertFalse(validator.isValid(valid.copy(placeSystemName = ""))) + } + + @Test + fun `a place system name of spaces is a name`() { + assertTrue(validator.isValid(valid.copy(placeSystemName = " "))) } @Test diff --git a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/domain/EmbeddedFilteringManagerTest.kt b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/domain/EmbeddedFilteringManagerTest.kt index 4ee57875..4eb90263 100644 --- a/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/domain/EmbeddedFilteringManagerTest.kt +++ b/sdk/src/test/java/cloud/mindbox/mobile_sdk/inapp/domain/EmbeddedFilteringManagerTest.kt @@ -36,12 +36,12 @@ class EmbeddedFilteringManagerTest { } @Test - fun `place comparison trims whitespace on both sides`() { + fun `place comparison takes the padding as part of the name`() { val embedded = embeddedInApp() val result = manager.filterEmbeddedInAppsByPlace(listOf(embedded), " main-screen-top ") - assertEquals(listOf(embedded), result) + assertTrue(result.isEmpty()) } @Test