Fix message ordering in paywall webview communication

[ianrumac]

Changes in this pull request

• Fix multi-page paywalls only reporting entry page view: The paywall runtime was treating template_variables messages that arrived after paywall_open as a fresh load, causing it to discard subsequent page_view events. This made multi-page flows appear as if users dropped off on the first page.

• Implement strict message ordering for webview lifecycle: Refactored PaywallMessageHandler to enforce that template_variables always reaches the webview before paywall_open, even when template construction is slow. This is critical because late-arriving templates reset the runtime state.

• Add recovery mechanism for webview crashes: When a webview is recreated (e.g., after process crash), the handler now properly restores the deferred paywall_open message if the presentation is still active, ensuring paywalls reopen correctly.

Key Implementation Details

• Replaced simple Queue<PaywallMessage> with a structured PendingMessage system that tracks conditional sends
• Introduced enqueueOutbound() to serialize all outbound messages through a job chain, ensuring template builds complete before subsequent messages are evaluated
• Added resetForWebViewReload() to cancel obsolete work and conditionally restore lifecycle messages when webviews are recreated
• Modified passTemplatesToWebView() to use withContext(Dispatchers.Main) with ensureActive() instead of launching, ensuring template sends complete before queued messages proceed
• Updated flushPendingMessagesInternal() to respect the webview loaded state and only send lifecycle messages when appropriate
• Added synchronization around webview state transitions to prevent race conditions between message sends and state updates

Checklist

• All unit tests pass (added comprehensive PaywallMessageOrderingTest with 11 test cases covering ordering, recovery, and edge cases)
• I added/updated tests - Added PaywallMessageOrderingTest.kt with extensive coverage of message ordering scenarios
• I added an entry to CHANGELOG.md for bug fixes

https://claude.ai/code/session_01KD1BdtpZL3MYo5F6R1CcLu

[pullfrog]

ℹ️ No blocking issues — two scope questions and a nit.

Reviewed changes — full read of the 4-file diff at 7f0b756, plus the surrounding PaywallView crash-recovery path, PaywallViewState.Updates, IOScope/MainScope, and the template-build call chain (TemplateLogic → DependencyContainer.makeJsonVariables → DeviceHelper/CoreDataManager).

• Serialized outbound message chain — enqueueOutbound appends each outbound webview send to a job chain (previous?.join(); block()) guarded by outboundLock, replacing the independent ioScope.launch per message.
• template_variables now provably precedes paywall_open — passMessageToWebView and didLoadWebView switched from mainScope.launch to withContext(Dispatchers.Main), so a send completes before the next chained block runs. This is the actual multi-page page_view fix.
• Conditional deferred messages — Queue<PaywallMessage> became ArrayDeque<PendingMessage>, where shouldSend is re-evaluated on the main thread at delivery time.
• Webview-crash recovery — PaywallView.recreateWebview now calls resetForWebViewReload() on the old handler instead of firing handle(PaywallOpen) at the new webview; the reset cancels obsolete jobs, nulls paywalljsVersion, and re-queues a PaywallOpen guarded on isPresented && !closedForBackground && lastOpen === lastOpen.
• New test suite — PaywallMessageOrderingTest with 10 tests, using a SlowVariablesFactory whose template build is deliberately slower than the open it must precede.

I verified the concurrency machinery is sound: the tail-pointer update and the invokeOnCompletion clear (if (lastOutbound === job)) both happen under outboundLock, so FIFO cannot be interleaved; Job.join() resumes normally for a failed or cancelled predecessor, so a broken link never wedges the chain; and no synchronized(outboundLock) block can hold the lock across a suspension (kotlin.synchronized takes a non-suspend lambda), so there is no deadlock cycle between the JS-bridge thread, main, and IO. didLoadWebView calling flushPendingMessagesInternal() from inside its own main-thread block while itself a chained job is also fine — the flush's enqueueOutbound simply joins the still-running job.

Static review only: per repo convention a cold ./gradlew :superwall:testDebugUnitTest exceeds the tooling timeout here, so the new suite was reasoned about rather than executed. Reading it against the removed lines, 4 of the 5 tests covering pre-existing message types would genuinely fail pre-PR.

ℹ️ Transaction and trial messages were folded into the template chain, which they don't need and which drops them on reload

Only template_variables ↔ paywall_open/paywall_close ordering is load-bearing per the CHANGELOG, but TransactionStart, TransactionAbandon, TransactionComplete, TrialStarted, RestoreFailed and BackButtonPressed all moved onto the same chain. That couples their delivery latency to template-build latency with no timeout, and it means resetForWebViewReload's cancellation sweep silently discards an in-flight transaction_complete — only PaywallOpen gets a conditional restore. I traced the chained work and it is local compute plus Room reads today, not network or billing IPC, so this is a scope question rather than a live bug.

Technical details

# Outbound chain scope: transaction messages

## Affected sites
- `superwall/src/main/java/com/superwall/sdk/paywall/view/webview/messaging/PaywallMessageHandler.kt:116-133` — `enqueueOutbound` has no `withTimeout`; the only unblock paths are the block completing, throwing, or `resetForWebViewReload` cancelling.
- `PaywallMessageHandler.kt:278`, `:284`, `:294`, `:304`, `:264`, `:271` — `TransactionStart`, `TransactionAbandon`, `TransactionComplete`, `TrialStarted`, `BackButtonPressed`, `RestoreFailed` moved from independent `ioScope.launch` onto the shared chain.
- `PaywallMessageHandler.kt:137-155` — the reset cancels all of `outboundJobs`, but only re-queues `PaywallOpen`.

## Required outcome
- A decision, recorded in the code, on whether these six message types genuinely need to be ordered behind template construction. If they do not, they should not inherit its latency or its cancellation semantics.
- If they stay on the chain, an explicit answer for what happens to a `transaction_complete` that was in flight when the renderer died.

## Suggested approach (optional)
- Either keep one chain but bound it (e.g. `withTimeoutOrNull` around the template build, matching the existing `withTimeoutOrNull(1000)` in `getState()`), or split into a lifecycle chain (templates + open/close) and leave the event `pass` calls as independent launches as before.

## Open questions for the human
- Is dropping an in-flight `transaction_complete` on webview recreation intended? The new document has no transaction state, so re-sending may be as wrong as dropping — but the choice is currently implicit.

ℹ️ The crash-recovery path that motivates this change is the one path with no test

PaywallView.recreateWebview is the only production caller of resetForWebViewReload, and nothing in src/test or src/androidTest references recreateWebview or onRenderProcessGone. The new tests call resetForWebViewReload() directly, so they pin the handler's behavior but not that recreateWebview calls it before detach/destroyView, nor that deleting the old handle(PaywallMessage.PaywallOpen) line doesn't regress the non-presented (preloaded) case.

Technical details

# Crash-recovery coverage gap

## Affected sites
- `superwall/src/main/java/com/superwall/sdk/paywall/view/PaywallView.kt:1008-1020` — `recreateWebview()`; `resetForWebViewReload()` added at :1010, `handle(PaywallMessage.PaywallOpen)` removed after :1018. Zero test references.
- `PaywallView.kt:955-1001` — the `onRenderProcessGone` → `crashRetries < 3` → `recreateWebview()` branch is likewise uncovered.

## Required outcome
- One test that drives a simulated render-process-gone through `PaywallView` (not through `PaywallMessageHandler` directly) and asserts the paywall reopens for a presented paywall and does not reopen for a preloaded one.

## Suggested approach (optional)
- `PaywallView.webView` is `internal var` and `PaywallViewTest` already runs under Robolectric with `@Config(sdk = [33])`, so asserting on the replacement `SWWebView`'s evaluated scripts after triggering the crash callback should be reachable from `src/test`.

ℹ️ Nitpicks

• New tests never send PaywallMessage.PaywallClose, so the queued-close path through sendLifecycleMessage is uncovered here — the older PaywallMessageHandlerTest only exercises close via the already-loaded path, because it uses Paywall.stub() with paywalljsVersion = "".
• templateVariablesReachWebviewBeforePaywallOpenWhenPaywallIsNotPreloaded, slowInitializationFinishesBeforeOpeningANewPaywall and recoveryDoesNotOpenCachedOrBackgroundedPaywalls never send TemplateParamsAndUserAttributes; their template_variables match comes from didLoadWebView's own resend, since scriptSrc concatenates the templates and htmlSubstitutions into a single evaluate call. They pin page-load ordering rather than the explicit-message ordering the file's doc comment describes.
• recoveryDoesNotOpenCachedOrBackgroundedPaywalls ties isPresented and closedForBackground to the same boolean, so it covers each guard clause's reject direction but never the accept combination — that only holds because deferredPaywallOpenIsSentAfterTheTemplatesOnceTheWebviewLoads covers it separately.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Nit: the event name is inferred by exclusion, so anything that is not PaywallOpen ships as paywall_close. Safe today (both callers are gated by the is PaywallOpen, is PaywallClose -> branch in handle), but adding a third lifecycle message to that branch later would silently mislabel it rather than fail the build.

Suggested change

	val eventName =
	if (pending.message is PaywallMessage.PaywallOpen) {
	SuperwallEvents.PaywallOpen.rawName
	} else {
	SuperwallEvents.PaywallClose.rawName
	}
	val eventName =
	when (pending.message) {
	is PaywallMessage.PaywallOpen -> SuperwallEvents.PaywallOpen.rawName
	is PaywallMessage.PaywallClose -> SuperwallEvents.PaywallClose.rawName
	else -> return@withContext
	}

[pullfrog]

✅ No new issues found in the delta.

Reviewed changes — incremental pass over the one commit pushed since my review of 7f0b756.

• c66baf7 is the automated Update coverage badge [skip ci] commit — only .github/badges/jacoco.svg (45.7% → 46%) and .github/badges/branches.svg (36.5% → 36.8%) changed. No behavioral surface, nothing to review.

No source files changed, so my earlier findings still stand as written against 7f0b756. Worth noting the badge regeneration is itself evidence that CI compiled and ran PaywallMessageOrderingTest, which I could only reason about statically.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}