-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
fix: Crash for Custom ViewController init on iOS 15 #1361
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1361 +/- ##
==========================================
- Coverage 95.41% 95.40% -0.02%
==========================================
Files 152 153 +1
Lines 5479 5500 +21
==========================================
+ Hits 5228 5247 +19
- Misses 251 253 +2
Continue to review full report at Codecov.
|
Tests/SentryTests/Integrations/SentryUIViewControllerSwizzlingTests.swift
Outdated
Show resolved
Hide resolved
Tests/SentryTests/Integrations/SentryUIViewControllerSwizzlingTests.swift
Outdated
Show resolved
Hide resolved
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.
Good solution!
📜 Description
Swizzle all custom ViewControllers. First, fetch all subclasses of ViewController and then swizzle them. As there is no straightforward way to get all sub-classes in Objective-C, the code first retrieves all classes in the runtime, iterates over all classes, and checks for every class if UIViewController is a parent. Cause loading all classes can take a few milliseconds, do this on a background thread, which should be fine because the SDK swizzles the root view controller and its children.
Previously, the code intercepted the ViewController initializers with swizzling to swizzle the lifecycle methods. This approach led to ViewControllers crashing when using a convenience initializer, see GH-1355. The error occurred because our swizzling logic adds the method to swizzle if the class doesn't implement it. It seems like adding an extra initializer causes problems with the rules for initialization in Swift, see https://docs.swift.org/swift-book/LanguageGuide/Initialization.html#ID216.
💡 Motivation and Context
GH-1355
💚 How did you test it?
Unit tests and on a real device.
I also profiled swizzling the view controllers on https://github.com/home-assistant/iOS, which has a couple of frameworks, on a simulator. It took around 120ms on the background thread. I also wanted to test a release build on a real device, but I had trouble with the signing certificates. The SDK needs around 80 ms for the iOS-Swift sample project to swizzle on a simulator and around 20 ms on an iPhone 12.
Added 1000 ViewControllers and 10k classes generated with
generate-classes.sh
and to iOS-Swift and profiled iOS-Swift.📝 Checklist
🔮 Next steps