[Fix] Prevent re-swizzling when there is another swizzler #1100
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The test added in this commit was based on what FlutterFire does which was observed to get stuck in an infinite loop. We need to ensure we swizzle a class only once, we will fix this in the next commit and explain why.
This was added to prevent an infinite loops when another library that swizzles the same selectors is present in the app. This was only true if the other library correctly forwards the event to the original selector, however this is almost always the case as otherwise this other SDK would create side effects if it missed the forwarding step. Basically something can swizzle after OneSignal and swizzle it out of first inline. However this is ok since that other swizzling library will just think OneSignal's implementation is the original so therefore OneSignal will still get the event. We just have trust the other SDK swizzling is forwarding.
Ensure we only swizzle an UNUserNotificationCenterDelegate class once. This was added to prevent an infinite loops when another library that swizzles the same selectors is present in the app. See the previous commit where this same logic was added for AppDelegate for more details.
jkasten2
force-pushed
the
fix/prevent_reswizzling_when_there_is_another_swizzler
branch
from
704451a
to
c568ace
Compare
emawby
approved these changes
Jun 2, 2022
Base automatically changed from
omit_objc_getClassList_calls_for_swizzling
to
main
June 2, 2022 17:41
jkasten2
deleted the
fix/prevent_reswizzling_when_there_is_another_swizzler
branch
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
Prevent infinite loops between OneSignal and other SDKs that also swizzle.
Details
Motivation
After a recent swizzling change (commit hasn't gotten into a release yet) it was discovered that it introduced a bug with FlutterFire which causes an infinite loop between the two with the
AppDelegate. This is due to the factFlutterFireclears theAppDelegateand re-assigns it and OneSignal re-swizzles it self back. Then when anAppDelegateselector fires from iOS OneSignal and FlutterFire call each other in a loop due to this.Scope
Effects swizzling for both
AppDelegateandUNUserNotificationCenterDelegate.Testing
Unit testing
Added a unit test that reproduced the exact FlutterFire issue.
Manual testing
Ensured issue no longer happens on the Flutter project with FlutterFire and OneSignal. Tested on an iPhone 6s with iOS 14.4.1 and sent a notification while the app was in the foreground, ensured "onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler: Fired!" was printed to the log.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is