-
-
Notifications
You must be signed in to change notification settings - Fork 372
fix(session-replay): Add exemption for CameraUI traversal for iOS 26.0 #6045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
❌ 9 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9389467 | 1218.62 ms | 1244.86 ms | 26.24 ms |
| 5d238d3 | 1228.94 ms | 1253.04 ms | 24.10 ms |
| bc0a04c | 1226.83 ms | 1255.04 ms | 28.21 ms |
| 67e8e3e | 1220.08 ms | 1229.23 ms | 9.15 ms |
| 3ec47ae | 1231.02 ms | 1256.67 ms | 25.65 ms |
| f0e2579 | 1224.82 ms | 1245.49 ms | 20.67 ms |
| 8047b99 | 1226.37 ms | 1246.63 ms | 20.26 ms |
| 701b301 | 1226.10 ms | 1245.57 ms | 19.47 ms |
| 07d7e83 | 1211.71 ms | 1240.08 ms | 28.37 ms |
| 570f725 | 1206.00 ms | 1238.96 ms | 32.96 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9389467 | 23.75 KiB | 866.51 KiB | 842.76 KiB |
| 5d238d3 | 23.75 KiB | 913.62 KiB | 889.88 KiB |
| bc0a04c | 23.75 KiB | 933.32 KiB | 909.57 KiB |
| 67e8e3e | 23.75 KiB | 919.91 KiB | 896.16 KiB |
| 3ec47ae | 23.75 KiB | 919.88 KiB | 896.13 KiB |
| f0e2579 | 23.75 KiB | 969.22 KiB | 945.47 KiB |
| 8047b99 | 23.75 KiB | 855.37 KiB | 831.62 KiB |
| 701b301 | 23.75 KiB | 867.16 KiB | 843.41 KiB |
| 07d7e83 | 23.75 KiB | 913.27 KiB | 889.52 KiB |
| 570f725 | 23.74 KiB | 913.38 KiB | 889.63 KiB |
Previous results on branch: issue-5647
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1b6626 | 1242.37 ms | 1262.79 ms | 20.42 ms |
| 384aab3 | 1232.67 ms | 1260.67 ms | 28.00 ms |
| 697c280 | 1206.27 ms | 1231.76 ms | 25.50 ms |
| f8f9ada | 1214.33 ms | 1249.85 ms | 35.52 ms |
| 9793ab3 | 1231.33 ms | 1248.90 ms | 17.57 ms |
| 29dc424 | 1233.29 ms | 1257.29 ms | 24.01 ms |
| afcf7f2 | 1200.98 ms | 1234.06 ms | 33.08 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1b6626 | 23.75 KiB | 969.24 KiB | 945.49 KiB |
| 384aab3 | 23.75 KiB | 963.05 KiB | 939.30 KiB |
| 697c280 | 23.75 KiB | 969.24 KiB | 945.49 KiB |
| f8f9ada | 23.75 KiB | 938.51 KiB | 914.76 KiB |
| 9793ab3 | 23.75 KiB | 963.10 KiB | 939.35 KiB |
| 29dc424 | 23.75 KiB | 969.23 KiB | 945.48 KiB |
| afcf7f2 | 23.75 KiB | 933.31 KiB | 909.56 KiB |
|
Using |
|
@cursor review |
The mapRedactRegion method could create duplicate redaction regions for views that met both general redaction criteria and isViewSubtreeIgnored condition. This happened when clipsToBounds was false, allowing both code paths to add regions for the same view. Fixed by moving the isViewSubtreeIgnored check to the beginning with an early return, ensuring views are processed only once. Added comprehensive tests to verify deduplication works correctly while maintaining proper redaction behavior."
|
@philipphofmann we can wait with this PR until after #6075 so we have verification in CI that the tests work as expected. |
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for tackling this complex crash. I have a few questions.
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
|
@philipphofmann is the new UI test failing on your machine too? It passes on my machine but fails in CI. |
It runs successfully on my machine. I tested it with Xcode 26 RC and iPhone 17 Pro iOS 26 simulator. |
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
📜 Description
This pull request introduces a targeted workaround in the
SentryUIRedactBuilderto handle a crash related toCameraUI.ChromeSwiftUIViewon iOS 26 when built with Xcode 16. The workaround ensures that the redaction process skips subtrees of this specific internal camera view to prevent the crash, while maintaining normal redaction behavior for other views.SentryUIRedactBuilderto detect and skip subtrees ofCameraUI.ChromeSwiftUIViewwhen running on iOS 26+ to prevent crashes from unimplemented initializers. (cameraSwiftUIViewClassObjectId,isViewSubtreeIgnored,redactRegionsFor(view:)) [1] [2] [3]view.debugDescriptioninstead oflayer.namefor more consistent identification. [1] [2]ExtrasViewController.swiftto present the camera UI usingUIImagePickerControllerfor testing camera permission flows when pressing the buttonShow Camera UI.💡 Motivation and Context
Fixes #5647
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.Full Investigation
The issue could be reproduced when using Xcode 16.4 to run on iOS 26.0 Beta 6 or later, where accessing the
layer.subLayersduring the redaction geometry calculations while displaying the standard Camera UI caused a crash:During my investigation I found the class to be part of the private framework
CameraUI, located at/Library/Developer/CoreSimulator/Volumes/iOS_23A5326a/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 26.0.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/CameraUI.frameworkwhen running on iOS 26.0 Beta 6.My approach to resolve the issue is excluding the view hierarchy of the camera UI from traversal and instead fully redact the UI, as it can be considered personal information anyways. I found the
CameraUI.ModeLoupeLayerto be part of the view hierarchy of theCameraUI.ChromeSwiftUIView, so I use it to decide if the subview hierarchy should be ignored from traversal.While working on the unit tests to verify the changes redaction algorithm, I encountered issues in creating an instance of the
CameraUI.ChromeSwiftUIViewas it is a private framework which can not be imported directly into the unit tests.Instead I tried to find the class using
NSClassFromString("CameraUI.ChromeSwiftUIView")which also returnednilbecause the framework was not yet loaded. I was able to resolve this by creating an instance ofUIImagePickerController(the original cause of the crash) which then transitively imports the frameworkCameraUI.To analyse the class, I created the following dump tool to access the available Objective-C runtime information:
This resulted in the following output:
This shows that the
initWithCoderandinitWithFrameexist as expected, but as shown in the crash message might not be implemented.My initial approach to get an instance of the view was using the instance of the
UIImagePickerControllerand its subviews for the redaction testing. Unfortunately the image picker controller seems to be empty at the beginning and loads additional views after being displayed - most likely also due to permissions checking.❌ Instance of UIImagePickerController
My second attempt was using the
NSClassFromStringto create the reference of the class type to then call it's init method with or without a frame:I concluded that it’s a Swift class that likely has a Swift-only designated initializer (internal/private, not @objc) and the ObjC inits (-init, -initWithFrame:, -initWithCoder:) are intentionally trapped to fatalError.
❌ Invoking init methods of
CameraUI.ChromeSwiftUIViewvia Objective-C runtimeMy next approach was circumventing the traps by swizzling the init methods of the
ChromeSwiftUIViewwith the init methods of it's superclassUIView:✅ Swizzling the type initializer
As the solution above is replacing class type methods which can have side effects, I also explored the option of raw object initialization using the Objective-C runtime
✅ Swizzling the instance initializer
This implementation still relies on Objective-C, but I wanted to check if I can directly call the private Swift initializer too. I started to look into the LLVM symbol table dumper of the
CameraUIframework to see if I can find anyTo summarize this output, the available symbols only include the
_TtC8CameraUI17ChromeSwiftUIViewdemangles toCameraUI.ChromeSwiftUIView(Swift class bridged to ObjC) and class and metaclass exist in the ObjC runtime (soNSClassFromStringworks), but symbol lookup by name ((lldb) image lookup -rn ChromeSwiftUIView) finds nothing because the relevant symbols aren’t exported.❌ This means we can not
dlsyma Swift initializer to invoke it directly and instance swizzling is the best approach.