Skip to content
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

Remove +[UIApplication load], Remove -[UIApplication setDelegate:] swizzle #259

Closed
wants to merge 7 commits into from

Conversation

alexhillc
Copy link

@alexhillc alexhillc commented Aug 1, 2017

  • Remove +[UIApplication load] in OneSignal class implementation in order to prevent swizzling before the library is instantiated (IF it is instantiated at all)
  • Remove -[UIApplication setDelegate:] swizzle in favor of manually calling +[OneSignal swizzleSelectors] during init, which kicks off all required method swizzling. This method also wraps the sensitive code in a dispatch_once block in order to prevent multiple swizzles from happening if init is called more than once

The use cases for making these changes:

  • Prevent swizzling if the SDK is not going to be instantiated during the app lifecycle
  • Ties swizzle methods to the init method rather than depending on the swizzled -[UIApplication setDelegate:]. This allows the SDK to lie "dormant" while not in use

Not sure if this will be appreciated, but I figured if I was going to make changes, I may as well PR them!

This change is Reviewable

  order to prevent swizzling before the library is instantiated (IF it
is instantiated at all)
- Remove `-[UIApplication setDelegate:]` swizzle in favor of manually
  calling `+[OneSignal swizzleSelectors]` during init, which kicks off
all required method swizzling. This method also wraps the sensitive code in a
`dispatch_once` block in order to prevent multiple swizzles from
happening if init is called more than once

- Force swizzle before all tests execute to accomodate previous commit
- Add some pretty prints to make debugging easier

- Attempts to swizzle the current UNUserNotificationCenterDelegate
  during +[UNUserNotificationCenter swizzleSelectors] just in case the
delegate has already been set by the time this method is called

- Fixes potential memory leak in Reachability
- Missing -[UIViewController viewDidLoad] super call

- Update binary

- Revert memory leak "fix"
@jkasten2
Copy link
Member

@alexhillc Thanks for sharing the changes. Some of our wrapping SDKs for other frameworks such as Unity, Cordova, Xamarin, etc might be effected by this. Before merging this in it will need to be tested for these cases. We may need a flag to disabling sizzling with a flag instead depending if this effects them.

@michalinas michalinas mentioned this pull request Oct 27, 2017
@emawby emawby closed this Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants