Repository navigation
fix(replay): never copy an app-defined layer's presentation - #1064
turnipdabeets merged 4 commits into
Conversation
Copying a layer for presentation() runs its class's init(layer:), which traps for a Swift CALayer subclass that doesn't implement it. Replay now measures masks from the model under an app-defined layer while nothing in its chain animates, masks the whole window while something does, and skips motion-band frames for windows that hold one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
PostHog Review · Standard · reviewed e15704f: 2 Must fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
posthog-ios Compliance ReportDate: 2026-11-13T17:13:10.580247+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 2986ms |
| Format Validation.Event Has Uuid | ✅ | 2902ms |
| Format Validation.Event Has Lib Properties | ✅ | 2833ms |
| Format Validation.Distinct Id Is String | ✅ | 2962ms |
| Format Validation.Token Is Present | ✅ | 556ms |
| Format Validation.Custom Properties Preserved | ✅ | 2880ms |
| Format Validation.Event Has Timestamp | ✅ | 2764ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 2803ms |
| Retry Behavior.Retries On 503 | ✅ | 11758ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 4839ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 4874ms |
| Retry Behavior.Respects Retry After Header | ✅ | 7873ms |
| Retry Behavior.Implements Backoff | ✅ | 15584ms |
| Retry Behavior.Retries On 500 | ✅ | 9240ms |
| Retry Behavior.Retries On 502 | ✅ | 9350ms |
| Retry Behavior.Retries On 504 | ✅ | 9329ms |
| Retry Behavior.Max Retries Respected | ❌ | 21848ms |
| Deduplication.Generates Unique Uuids | ✅ | 1035ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 9370ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 16482ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 7941ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 3034ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 3014ms |
| Compression.Sends Gzip When Enabled | ✅ | 2968ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 469ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 304ms |
| Batch Format.Multiple Events Batched Together | ✅ | 2992ms |
| Error Handling.Does Not Retry On 403 | ✅ | 4824ms |
| Error Handling.Does Not Retry On 413 | ✅ | 4834ms |
| Error Handling.Retries On 408 | ✅ | 9446ms |
Failures
retry_behavior.max_retries_respected
Expected 4 requests, got 6
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 2895ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 2837ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 2885ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 2748ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 2880ms |
| Request Payload.Groups Round Trip | ✅ | 2881ms |
| Request Payload.Groups Default To Empty Object | ✅ | 2866ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 2875ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 2879ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 2834ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 97ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 471ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 5611ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 2999ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 2914ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 2989ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 3014ms |
…z-order can't be read
…aced image could draw
ioannisj
left a comment
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
No new findings from me. The open thread on failingClosedWhileAnimating is real though: a non-animated move under an app-defined layer gets masked at its new model position while the old one is still on screen, and a test that passes on the base branch fails here. I added the repro details in that thread.
…r an app-defined layer Under an app-defined layer, mask rects come from the model, which runs ahead of what's drawn until Core Animation commits. The bridge and background captures can measure mid-turn, so a view moved without animation was masked at its new spot while the screen still showed it at the old one. Flush first when the window holds an app-defined layer; at the run-loop opportunity there's nothing to commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
579b4fa
into
fix/replay-cleared-content-masking
… screen updates (#1054) * fix(replay): keep cleared labels and replaced images masked until the screen updates * fix(replay): bound replaced-image masks under a fill gravity and keep wireframes on the model image * fix(replay): mask private image view children and bound replaced images without private layer state * fix(replay): never copy an app-defined layer's presentation (#1064) * fix(replay): never copy an app-defined layer's presentation Copying a layer for presentation() runs its class's init(layer:), which traps for a Swift CALayer subclass that doesn't implement it. Replay now measures masks from the model under an app-defined layer while nothing in its chain animates, masks the whole window while something does, and skips motion-band frames for windows that hold one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(replay): keep masks behind a cover when an app-defined sibling's z-order can't be read * fix(replay): mask an unclipped app-defined image view wherever a replaced image could draw * fix(replay): commit pending layer changes before measuring masks under an app-defined layer Under an app-defined layer, mask rects come from the model, which runs ahead of what's drawn until Core Animation commits. The bridge and background captures can measure mid-turn, so a view moved without animation was masked at its new spot while the screen still showed it at the old one. Flush first when the window holds an app-defined layer; at the run-loop opportunity there's nothing to commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anna Garcia <11654201+turnipdabeets@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Anna Garcia <anna.g@posthog.com>
…cated Brings in #1054 (cleared labels and replaced images stay masked, and the app-defined CALayer presentation guard from #1064). - isLabelSensitive keeps 4.0's image-attachment check alongside #1054's cleared-text check. - toWireframe stays removed (4.0 has no wireframe mode); main's hasSensitiveImageAttachment is dropped for 4.0's existing one. - #1054's wireframe-only test is dropped, its fixture uses RRScreenshotFrame, and the new replay test files are gated on the SessionReplay trait like the other replay suites.
💡 Motivation and Context
Follow-up to #1054, and the narrower version of the guard from #1008 discussed in #1054 (comment).
Screenshot replay calls
CALayer.presentation(), which copies a layer through its class'sinit(layer:). A Swift layer subclass that declares its own initializer but notinit(layer:)traps there (Use of unimplemented initializer 'init(layer:)'), crashing the host app. That's onmaintoday withscreenshotMode, and 4.0 makes screenshots the only mode.The runtime can't tell a trapping subclass from one that implements
init(layer:). Both get their owninitWithLayer:from the app binary, and they look identical once stripped. So, like #1008, every layer class whoseinitWithLayer:comes from neitherCALayernor a system image counts as app-defined, and replay never copies one. Measured on an iOS 27.2 simulator:layer.presentation()drawHierarchyWhat changes where an app-defined layer is involved:
LinearGradientscreens.zPosition,isHidden, replaced image contents) go throughreadablePresentation. When an app-defined layer is animating and can't be read, the code falls back to whatever keeps content masked.drawHierarchy.Behavior changes (only for apps that use a custom
CALayersubclass)session-replay-privacyitem 11 allows skipping on screen changes. If frozen scrolls get reported, the follow-up is to draw matched-sample frames withdrawHierarchyand the drift-inflated rects.ph-no-captureview in such a chain are also dropped for that time.PostHogMaskFailClosedTesttests faked in-flight animations withCALayersubclasses that overridepresentation(). Those now count as app-defined, so the tests hold real animations on plain views instead.💚 How did you test it?
The simulator runs below used an iOS 27.2 simulator, Xcode 27.2 beta, and
IPHONEOS_DEPLOYMENT_TARGET=15.0overridden on the command line only. Xcode 26.4.1's simulators wouldn't connect locally.PostHogAppLayerPresentationTest(7 tests):init(layer:)calls.TrapLayer(noinit(layer:), above a masked field), as a temporary test:Fatal error: Use of unimplemented initializer 'init(layer:)' for class 'PostHogTests.TrapLayer'.PostHogClearedContentMaskingTest,PostHogMaskFailClosedTest,PostHogMaskingCharacterizationTest,PostHogMaskScenarioTest,PostHogReactNativeMaskingTestandPostHogMaskFlattenedContentTestpass. The one known-limitation test is still recorded as a known issue.PostHogMaskPresentationTestandPostHogMaskPresentationPrivacyTestpass (31 tests).make lintandmake checkTestRegistrationpass. ThebuildSdkbuilds pass for iOS, macOS, Mac Catalyst, tvOS, watchOS, visionOS andswift build; I ran them withoutxcpretty, which isn't installed locally.make testfails only inPostHogStorageTest, the app-group container tests, with the same 13 issues on fix(replay): keep cleared labels and replaced images masked until the screen updates #1054's branch, so that's the local environment rather than this change.Not tested on a physical device: there, system frameworks load from the dyld shared cache, and I haven't confirmed that
dladdrstill reports/System/Library/paths for them. If it doesn't, every layer counts as app-defined. That fails closed, but every app would lose motion frames. Also not checked: whether Lottie's and expo-linear-gradient's layer classes count as app-defined in a real app.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, at @turnipdabeets's direction, as a handoff for @ioannisj. A probe test settled which calls copy a layer. The classifier is #1008's, since nothing at runtime separates a trapping class from a correct one. Masking from the model while nothing animates is the narrowing over #1008. Two review passes followed: they found a fallback that failed open (
isVisibleForMaskingon an animating, collapsed, clipping app layer), which is fixed and tested, and the motion-frame cost, which is disclosed above.🤖 Generated with Claude Code