feat(replay): make screenshot optimizations opt-in - #761
Conversation
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| bitmap = bitmapLease.bitmap | ||
| } else { | ||
| bitmapLease = null | ||
| bitmap = Bitmap.createBitmap(view.width, view.height, Bitmap.Config.ARGB_8888) |
There was a problem hiding this comment.
Low: Unbounded bitmap retention after PixelCopy timeouts
When optimizations are disabled, every capture allocates a new full-resolution bitmap. A timed-out request retains that bitmap until its callback arrives, but unlike the lease-backed path, it does not prevent subsequent captures from allocating more; repeated UI redraws during stalled callbacks can therefore exhaust the host app's memory. Keep full-resolution ARGB_8888 capture while applying equivalent single-request backpressure or another explicit bound to outstanding bitmaps.
PR overviewThis PR makes screenshot optimizations opt-in for Android session replay, using full-resolution bitmap capture when optimizations are disabled. One issue remains: stalled PixelCopy callbacks can allow full-resolution bitmaps to accumulate during repeated UI redraws, potentially exhausting the host app’s memory. The impact depends on timeout conditions and sustained redraw activity, and no reported issues have yet been addressed. Open issues (1)
Fixed/addressed: 0 · PR risk: 4/10 |
Prompt To Fix All With AI### Issue 1
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1715
**Pending bitmaps can leak**
If a default-mode PixelCopy times out, its fresh full-resolution bitmap is left for the callback to recycle. If `uninstall()` quits the callback thread before that callback is delivered, the bitmap is not recycled and is not tracked by `pixelCopyBitmapBuffer.close()`. Multiple delayed captures can therefore retain large ARGB_8888 bitmaps without deterministic cleanup. Please retain ownership of pending default-mode bitmaps so uninstall can release them explicitly.
### Issue 2
posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt:1785
**Cases are not parameterised**
These new tests loop over multiple cases inside ordinary tests, which violates the repository directive to prefer parameterised tests. A failure in one iteration prevents later cases from running and reports all inputs under one result. Please parameterise the optimization modes and dimension pairs before merging. The same pattern appears in the late-callback, transparency, mask-scaling, and invalid-dimension tests.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(replay): make screenshot optimizati..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| bitmap = bitmapLease.bitmap | ||
| } else { | ||
| bitmapLease = null | ||
| bitmap = Bitmap.createBitmap(view.width, view.height, Bitmap.Config.ARGB_8888) |
There was a problem hiding this comment.
If a default-mode PixelCopy times out, its fresh full-resolution bitmap is left for the callback to recycle. If uninstall() quits the callback thread before that callback is delivered, the bitmap is not recycled and is not tracked by pixelCopyBitmapBuffer.close(). Multiple delayed captures can therefore retain large ARGB_8888 bitmaps without deterministic cleanup. Please retain ownership of pending default-mode bitmaps so uninstall can release them explicitly.
Knowledge Base Used: Android session replay
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt
Line: 1715
Comment:
**Pending bitmaps can leak**
If a default-mode PixelCopy times out, its fresh full-resolution bitmap is left for the callback to recycle. If `uninstall()` quits the callback thread before that callback is delivered, the bitmap is not recycled and is not tracked by `pixelCopyBitmapBuffer.close()`. Multiple delayed captures can therefore retain large ARGB_8888 bitmaps without deterministic cleanup. Please retain ownership of pending default-mode bitmaps so uninstall can release them explicitly.
**Knowledge Base Used:** [Android session replay](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-android/-/docs/android-session-replay.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| @Test | ||
| @Config(sdk = [26], shadows = [RecordingShadowPixelCopy::class]) | ||
| fun `screenshot optimization can change while a previous capture is pending`() { | ||
| for (initiallyOptimized in listOf(false, true)) { |
There was a problem hiding this comment.
These new tests loop over multiple cases inside ordinary tests, which violates the repository directive to prefer parameterised tests. A failure in one iteration prevents later cases from running and reports all inputs under one result. Please parameterise the optimization modes and dimension pairs before merging. The same pattern appears in the late-callback, transparency, mask-scaling, and invalid-dimension tests.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt
Line: 1785
Comment:
**Cases are not parameterised**
These new tests loop over multiple cases inside ordinary tests, which violates the repository directive to prefer parameterised tests. A failure in one iteration prevents later cases from running and reports all inputs under one result. Please parameterise the optimization modes and dimension pairs before merging. The same pattern appears in the late-callback, transparency, mask-scaling, and invalid-dimension tests.
**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
💡 Motivation and Context
Stacked on #756. This follow-up must land before the screenshot optimization is released so existing users retain the current capture defaults.
Adds experimental
sessionReplayConfig.optimizeScreenshots, defaulting tofalse:The option is sampled once per capture, including masking and bitmap release. Both modes safely reclaim late bitmaps and discard captures with non-positive source dimensions. The existing
screenshotdefault and all constructor signatures are unchanged.The follow-up also removes redundant casts and updates the changeset to describe the opt-in behavior and minor API addition.
💚 How did you test it?
make test: 355 Android tests and 9 Compose tests passed; 3 existing skips.make testJava: 942 core tests passed.make checkFormat,make api,./gradlew apiCheck, andgit diff --checkpassed.📝 Checklist
🤖 Agent context
Autonomy: Human-driven (agent-assisted).
Implemented and self-reviewed with Pi, Git/GitHub CLI, Gradle, Robolectric, and Java consumer checks. Kept one shared capture/completion pipeline with capture-local mode selection rather than duplicating the screenshot implementation. Human review is required; native/OEM PixelCopy behavior remains outside the local test coverage.