fix(replay): verify mask alignment instead of discarding frames on any redraw

[arnohillen]

💡 Motivation and Context

Closes #596.

In screenshot mode, a frame was discarded whenever its window redrew during the PixelCopy capture, unless an animation-type heuristic matched (hasTransientState from #529, surface/texture views from #649). Most animated content matches neither signal: indeterminate spinners (ProgressDialog), animated GIFs (Glide/Coil), Lottie (which never sets transient state, so the #529 exemption structurally could not fire for it), Material progress indicators, and Compose infinite animations all redraw per frame on the UI thread. On screens showing any of them, essentially every capture logged "Session Replay screenshot discarded due to screen changes" and the replay showed nothing. On top of that, the draw-dirty flags were single fields shared across all tracked windows, so an animating loader dialog also blanked captures of the static activity behind it.

The guard exists for a real reason (#254 / #234): mask rects are computed from live views after the pixels are frozen, so a structural change mid-capture can drift masks off sensitive content. This PR keeps that protection but stops proxying it with "did anything redraw":

• Per-window draw state: isOnDrawnCalled/didLayoutSinceReset move into a WindowDrawState on ViewTreeSnapshotStatus. PixelCopy copies a single window's surface and masks come from that window's own tree, so one window's draws say nothing about another window's mask alignment.
• Verified masks instead of heuristics: mask rects are sampled before PixelCopy.request and again in the callback. A dirty frame is kept only when both walks agree, no layout pass ran, and neither walk was poisoned. Pixel-only animation redraws pass this check no matter which library drives them; structural changes still discard. The hasTransientState/surface-view exemptions are deleted: frames they legitimately kept have stable geometry and pass rect equality anyway, and frames they kept with moving masked geometry were unsafe to keep at all.
• Fail closed on unknowable geometry: a walk is poisoned (frame discarded) when it meets a rendered view it cannot place (legacy view.animation, transient state mid-animation), and when the Compose semantics pass times out. Previously such views were silently pruned from the walk, which would have shipped them unmasked. A timed-out PixelCopy latch also no longer ships the bitmap before masks are painted.

Behavior is monotone for safety: no frame that was previously discarded for a genuine structural change is now kept, and the discard log line is unchanged for support diagnostics. The default wireframe mode is untouched, and the Flutter/RN forced-screenshot bridge inherits the fix through the same path.

💚 How did you test it?

• Unit tests for the keep/discard predicate: clean frame, stable rects, moved rects, appeared/disappeared rects, layout pass, poisoned walks.
• Deterministic end-to-end Robolectric tests through generateSnapshot + ShadowPixelCopy, using a hook view that injects state changes exactly between the pre- and post-copy walks: dirty-but-stable frame kept (the fix path), masked widget moved mid-capture discarded, layout mid-capture discarded, masked view mid legacy animation fails closed (regression test for the walk-pruning hole), and a redraw+layout in another window no longer discards this window's capture (fails under the old shared-flag code).
• Full posthog-android unit test suite, apiCheck, and spotlessCheck pass locally.

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• I updated the docs if needed.
• No breaking change or entry added to the changelog.

If releasing new changes

• Ran pnpm changeset to generate a changeset file

⚡ Performance

The verification machinery sits on two hot paths: the endpoint walks (twice per ~1s capture) and the draw-time walk, which runs on the main thread on every frame while a capture is in flight, on exactly the animated screens this PR unblocks. Follow-up commits make those paths allocation-free and add fail-fast exits, with discard semantics unchanged:

• The draw-time walk streams each mask rect against the capture baseline and stops on the first mismatch, instead of building a List<Rect> per frame and comparing it under the lock.
• Walk-confined scratch Rect/Point objects replace the per-view Rect()/Point() allocations, and a primitive int set replaces the boxing MutableSet<Int> for cycle detection.
• Doomed captures fail fast: a draw or layout during the pre-walk skips the ~10MB ARGB_8888 bitmap and PixelCopy entirely (with a bounded re-arm so a busy screen still captures); once a capture's verdict is sealed as discard, per-frame sampling and the post-copy walk stop.
• The per-frame draw callback is lock-free (a monotone draw counter, main thread is the single writer) and allocation-free: ~7-10 ns when no capture is in flight.
• The redundant rootView attach check (verified against AOSP) and the per-walk toString() copies of TextView text/hint are gone.

What the numbers show. Two committed benchmarks, both runnable with POSTHOG_BENCHMARK=1 ./gradlew :posthog-android:testReleaseUnitTest --tests '<name>' (they never run in CI):

PostHogReplayMaskWalkMicroBenchmark isolates the changed data structures (median of 5 batches x 500k ops, 3 runs; bytes/op are exact per-thread allocation counts):

Primitive	Before	After
Visited-set, 60 views	568 ns, 3,936 B	233 ns, 0 B (2.4x faster)
Visited-set, 550 views	5.8 µs, 34,688 B	2.0 µs, 16 B (2.9x faster)
Rect compare, matching frame (40 rects)	310 ns, 1,456 B	326 ns, 0 B (same speed, zero alloc)
Rect compare, mismatching frame (40 rects)	306 ns, 1,456 B	21 ns, 0 B (15x faster, early exit)

PostHogReplayMaskWalkBenchmark runs the full walk on a real 60/550-view tree. An A/B comparison (4 alternating runs against the pre-optimization commit) shows wall-clock parity within noise there: on Robolectric the walk is dominated by shadow-framework calls both versions make identically. That benchmark's value is the allocation counter and as a regression harness, not as proof of speedup.

So the honest claim is: on device, the SDK's own per-frame cost during capture windows (one Rect+Point per visited view, one boxed Integer+map entry per view, one stored Rect per masked view, a full-list comparison under a lock, ~KB-scale garbage per frame at 60-120 fps) drops to zero allocation and less work per view, mismatching frames stop at the first divergent rect, and unkeepable captures skip the bitmap+PixelCopy+encode entirely. Framework-call count per walk is unchanged.

✅ Correctness evidence

All of this runs in CI on every push:

• Exhaustive protocol check (WindowDrawStateProtocolTest): every placement of up to three mutate/draw/layout events across the capture pipeline's five gaps (1,096 schedules, enumerated, count asserted) runs against the real WindowDrawState. Kept frames provably never coexist with a layout, a draw overlapping the pre-walk, or a draw whose geometry differs from the baseline; stable-geometry schedules (pixel-only animations, the bug this PR fixes) are provably always kept.
• Property tests (PostHogReplayMaskWalkPropertyTest): the streaming comparison returns exactly the same verdict as the old build-a-list-and-compare across 10,000 randomized baseline mutations (element changed / inserted / removed / swapped / truncated); the primitive int set matches a HashSet oracle across 55,000 operations including the zero-sentinel edge case and growth.
• Behavioral tests: 280 unit tests total, including end-to-end Robolectric captures pinning every keep/discard scenario (stable redraw kept; moved/appeared/disappeared masks discarded; layout discarded; poisoned walks fail closed; pre-walk draws re-arm then capture; in-flight draw samples fail closed; per-window isolation).
• Adversarial review: the capture protocol went through three rounds of independent adversarial verification (multiple reviewers per round, prompted to construct PII-leak interleavings against the real code). The one design they refuted (keeping pre-walk-overlapping frames when rect geometry agrees) was discarded; the shipped protocol is the one that survived.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

• Authored with PostHog Code (Claude). Root cause was established with a multi-agent investigation over the SDK source, AOSP internals (ProgressBar/AnimatedVectorDrawable/ViewRootImpl), and the fix history of chore: do not capture screenshot during screen changes #254/fix: stop screenshot frames being dropped during animations #529/fix(replay): keep frames on screens with continuous surface rendering (e.g. Rive) #649, then adversarially verified claim by claim.
• Notable findings that shaped the design: setHasTransientState is only called by ViewPropertyAnimator, Editor, Transition, and view-translation in the framework, so the fix: stop screenshot frames being dropped during animations #529 exemption never fired for Lottie or any drawable-level animation; sibling SDKs (posthog-ios, Sentry Android) avoid this bug class by sampling masks synchronously with the frame, which is what the rect pre/post verification approximates while keeping PixelCopy off the main thread.
• An enumerate-more-animation-types fix (like closed PR fix: capture session replay screenshots during animated loaders #606) was considered and rejected: it cannot cover Compose or future libraries and keeps unverified keep paths. Rect verification subsumes the heuristics.
• The diff itself was adversarially reviewed by independent agents (PII, concurrency, regression, efficacy lenses); the confirmed findings (legacy-animation walk pruning, Compose walk timeout, stale exemption flag) are fixed and regression-tested in this PR.

---

Created with PostHog Code

[greptile-apps]

Security Review

Two fail-open paths remain in screenshot masking: poisoned walks are accepted when no redraw is observed, and a late timed-out PixelCopy callback can erase state belonging to a newer capture.

Prompt To Fix All With AI

### Issue 1
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1174-1176
**Poisoned clean frames bypass masking**

When either mask walk is poisoned without an intervening redraw, this early return accepts the frame and paints only the incomplete post-walk rectangles, causing sensitive content to be emitted without its required mask.

**How this was verified:** The capture resets the draw flag before walking, while timeout and unstable-geometry paths poison a walk independently of that flag.

```suggestion
        if (preWalk.poisoned || postWalk.poisoned) {
            return false
        }
        if (!drawState.isOnDrawnCalled) {
            return true
        }
```

### Issue 2
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1284-1287
**Stale callbacks erase capture state**

If a PixelCopy callback arrives after its one-second timeout and another capture for the same window has started, this old callback resets the shared `WindowDrawState`, erasing draw or layout changes from the newer capture and allowing a screenshot with misaligned masks to be accepted.

**How this was verified:** The timeout releases the capture executor while the pending callback retains the same per-window state and resets it from its `finally` block.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(replay): verify mask alignment inste..." | Re-trigger Greptile}

[marandaneto]

@arnohillen, this requires manual testing; otherwise, we risk leaking PII. Have you tested this, or are you purely relying on the unit tests?

[posthog]

🦔 ReviewHog reviewed this pull request

Found 1 must fix, 1 should fix, 0 consider.

Published 2 findings (view the review).

[posthog]

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

ReviewHog Report

Changes

Issues: 6 issues

Files (3)

• .changeset/replay-animated-screens-capture.md
• posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt
• posthog-android/src/main/java/com/posthog/android/replay/internal/ViewTreeSnapshotStatus.kt

[dustinbyrne]

Verified: I ran the Android sample app in screenshot-based session replay mode with a continuously animated loading spinner visible. Before the patch, every capture attempt during the animation was discarded and no replay frames were sent; after the patch, frames were captured continuously with no discard errors, and the animation was represented by changing screenshot payloads.

[marandaneto]

Verified: I ran the Android sample app in screenshot-based session replay mode with a continuously animated loading spinner visible. Before the patch, every capture attempt during the animation was discarded and no replay frames were sent; after the patch, frames were captured continuously with no discard errors, and the animation was represented by changing screenshot payloads.

awesome, @dustinbyrne @arnohillen worth testing this on react native as well since react native relies on that and sometimes there are some incompatibilities

[dustinbyrne]

working as expected on RN

[posthog]

ReviewHog Report

Changes

Issues: 2 issues

Files (5)

• .changeset/replay-animated-screens-capture.md
• posthog-android/build.gradle.kts
• posthog-android/gradle.lockfile
• posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt
• posthog-android/src/main/java/com/posthog/android/replay/internal/ViewTreeSnapshotStatus.kt

[dustinbyrne]

@arnohillen what do you think about putting this behind a configuration variable? my concern is that this could end up having a performance impact, especially on lower-end hardware or cases with an unusual amount of layout rects (just a guess, happy to hear push back on this).

if so, i'm happy to do the work!

[arnohillen]

@arnohillen what do you think about putting this behind a configuration variable? my concern is that this could end up having a performance impact, especially on lower-end hardware or cases with an unusual amount of layout rects (just a guess, happy to hear push back on this).

if so, i'm happy to do the work!

Yes, I think that's a great idea! I'm not 100% certain of this PR so it would be good to be able to revert quickly in case of any issues.

[dustinbyrne]

Yes, I think that's a great idea! I'm not 100% certain of this PR so it would be good to be able to revert quickly in case of any issues.

sounds good, i'll add that in. i've done a good amount of testing with this branch and the overall strategy seems sound. though, i'm not 100% on what we'd consider to promote this to be default on at the moment. we don't have any measurements we can rely on to definitively state that this is performant in all cases.

regarding rollback, yes, i agree - that is the scary part. there's really no way of rolling back mobile SDKs considering updating the SDK would require a somewhat lengthy resubmission process and the actual roll out to users can be slow / has a long tail.