Skip to content

Pause/play media playback on backgrounded paywalls - #461

Open
ianrumac wants to merge 5 commits into
developfrom
ir/fix/media-playback-pause
Open

Pause/play media playback on backgrounded paywalls#461
ianrumac wants to merge 5 commits into
developfrom
ir/fix/media-playback-pause

Conversation

@ianrumac

Copy link
Copy Markdown
Collaborator

Changes in this pull request

  • Fixes issue with videos looping on non-visible paywalls

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run ktlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

This PR flips setWebContentsDebuggingEnabled to true in SWWebView.prepareWebview(), which turns on chrome://inspect remote debugging for every WebView in every host app's release build. That change is unrelated to media playback and needs reverting before merge.

Reviewed changes — full read of the 11-file diff at 5e19b71, plus the surrounding WebView lifecycle, view-cache reuse and paywall-hosting call sites.

  • New MediaPlaybackScript — an idempotent injected IIFE that installs window.__swMediaPlayback once (a suspended Set plus a capturing document play listener), pauses video/audio when playback isn't allowed, and on resume replays only what it paused, skipping disconnected or ended elements.
  • SWWebView lifecycle wiring — new hostPaused / viewDestroyed state and updateMediaPlayback(), driven from onPause, onResume, onAttachedToWindow, onDetachedFromWindow, onWindowVisibilityChanged, onVisibilityChanged, the onPageStarted preload hook and the OnPageFinished branch.
  • PaywallWebUI gains onPause() / onResume(), forwarded from SuperwallPaywallActivity's own lifecycle callbacks; both unit-test fakes get no-op overrides.
  • Tests — a Robolectric test pinning the allowed computation across pause/visibility/reattach transitions, and an instrumented test running the real script against Chromium playback with a new media-playback.mp4 asset.
  • UnrelatedsetWebContentsDebuggingEnabled(true) and the removal of two setUserAttributes("is_user_eligible_for_dd_offer") calls in UITestHandler.kt.

I confirmed the parts that are easy to get wrong and are in fact right: every new evaluateJavascript call site runs on the main thread (the OnPageFinished one is inside mainScope.launch at SWWebView.kt:424); WebView.onPause() does not pause JavaScript, so issuing the pause script immediately before super.onPause() is safe; and repeated build(false) calls cannot double-register the listener or lose the suspended set.

ℹ️ Only the activity host forwards pause/resume, so embedded paywalls have a narrower guarantee

SuperwallPaywallActivity is the only caller of the new PaywallWebUI.onPause() / onResume(). PaywallComposable renders PaywallView through AndroidView with no lifecycle observer, so an embedded paywall's media is governed purely by the attach and window-visibility overrides. That covers app-backgrounding (window visibility drops), but not a paused-but-visible host — a dialog-themed activity on top, or split-screen where the unfocused app stays visible. The onAttachedToWindow comment explicitly anticipates embedded hosts, so it's worth deciding whether that gap is acceptable or whether PaywallComposable should observe LocalLifecycleOwner.

Technical details
# Embedded (Compose) paywalls never receive host pause/resume

## Affected sites
- `superwall-compose/src/main/java/com/superwall/sdk/compose/PaywallComposable.kt``AndroidView(factory = { viewToRender })` wires `onRelease` only; no `DisposableEffect` on `LocalLifecycleOwner`.
- `superwall/src/main/java/com/superwall/sdk/paywall/view/SuperwallPaywallActivity.kt:791,805` — the only production callers of the new `onResume()`/`onPause()`.
- `superwall/src/main/java/com/superwall/sdk/paywall/view/webview/SWWebView.kt:209` — the comment "A cached view can be attached to a different host, including an embedded one" signals embedded hosts are in scope.

## Required outcome
- Either embedded paywalls get the same "host paused ⇒ media paused" guarantee as activity-hosted ones, or the narrower guarantee is a documented, deliberate choice.

## Open questions for the human
- Is `PaywallComposable` considered in scope for this fix, or is activity-hosted presentation the only surface that needs it right now?

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread superwall/src/main/java/com/superwall/sdk/paywall/view/webview/SWWebView.kt Outdated
Comment thread superwall/src/main/java/com/superwall/sdk/paywall/view/webview/SWWebView.kt Outdated
github-actions Bot and others added 2 commits September 10, 2026 12:06
…/SWWebView.kt

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in this delta — the remote-debugging blocker is reverted. Withholding approval only because the two other threads from the prior review are still open.

Reviewed changes — the one commit since the prior pullfrog review at 5e19b71 (the 358cddf badge commit in between is CI bot output with no behavioral surface).

  • Reverted process-wide WebView remote debuggingSWWebView.prepareWebview() is back to setWebContentsDebuggingEnabled(false) at SWWebView.kt:253, matching develop and closing the chrome://inspect exposure in host-app release builds. It is the only call site in the repo, so nothing else re-enables it.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FY2V8fiWgvPpBhfp1Wjmc9
@ianrumac

Copy link
Copy Markdown
Collaborator Author

Re: embedded Compose paywalls not receiving host pause/resume — out of scope for this PR; activity-hosted presentation is the target here. Embedded paywalls still pause on detach and window-visibility changes (incl. app backgrounding).

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the single commit since the prior pullfrog review at 2bb598c, which answers both remaining inline threads.

  • Documented the mediaLifecycleReady guardSWWebView.kt:186-188 now states outright that View callbacks can run from the superclass constructor before Kotlin initializers, that the JVM field default (false) is what suppresses JS evaluation until then, and that the field must therefore keep its initializer rather than be inlined or converted to a val.
  • Made the detach assertion load-bearingSWWebViewMediaLifecycleTest.kt:46-51 adds a detach-while-resumed step ahead of the existing detach-while-paused one. Every assertion in the test now alternates its expected value, so Robolectric's stale-sticky lastEvaluatedJavascript can no longer mask a missing call.

I traced the mechanism the new test relies on rather than taking it on faith, and it holds. PhoneWindow.setContentViewViewGroup.removeAllViewsInLayout() calls child.dispatchDetachedFromWindow() before clearing child.mParent, and View.dispatchDetachedFromWindow() fires onWindowVisibilityChanged(GONE) first, then onDetachedFromWindow(), and only nulls mAttachInfo last — it never writes mAttachInfo.mWindowVisibility. So at the moment updateMediaPlayback() runs during detach, isAttachedToWindow, isShown and windowVisibility == VISIBLE are all still true and it emits build(true); the onDetachedFromWindow override's direct build(false) is what corrects it. Robolectric shadows none of that dispatch chain, so the real framework code executes and deleting the override does make the new assertion fail on a string mismatch.

Pullfrog  | View workflow run | Using Claude Opus𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant