GULAppDelegateSwizzler - remote notifications methods and tvOS support #2698
Conversation
…DelegateSwizzler [WIP]" This reverts commit b2eabf2.
…public interface with no assumptions on the implementation details
…lementation details assumptions
Conflicts: Example/Messaging/Tests/FIRMessagingRemoteNotificationsProxyTest.m
…s applied to all targets. Move the hook to the top level to avoid confusion.
…etProxyOriginalDelegateOnceToken] at the beginning of each test.
…sts added. Cleanup
…essage:] before calling delegate.
|
Consider targeting the PR to |
|
@paulb777 This PR is just a part of |
…Notification:` for tvOS just in case
| static char const *const kGULHandleBackgroundSessionIMPKey = "GUL_handleBackgroundSessionIMP"; | ||
| static char const *const kGULOpenURLOptionsIMPKey = "GUL_openURLOptionsIMP"; | ||
|
|
||
| #if TARGET_OS_IOS |
There was a problem hiding this comment.
Nit: Move this to the end of this list and add a comment mentioning this method is only available on iOS and not on tvOS.
| fromClass:realClass]; | ||
| NSValue *continueUserActivityIMPPointer = [NSValue valueWithPointer:continueUserActivityIMP]; | ||
|
|
||
| #if TARGET_OS_IOS |
There was a problem hiding this comment.
Nit: Move this to the end of the method.
| SEL didReceiveRemoteNotificationWithCompletionSEL = | ||
| @selector(application:didReceiveRemoteNotification:fetchCompletionHandler:); | ||
| if ([anObject respondsToSelector:didReceiveRemoteNotificationWithCompletionSEL]) { | ||
| // Only add the application:didReceiveRemoteNotification:fetchCompletionHandler: method if |
| OBJC_ASSOCIATION_RETAIN_NONATOMIC); | ||
| } | ||
|
|
||
| #if TARGET_OS_IOS |
There was a problem hiding this comment.
Nit: move to end of the list.
| * | ||
| * @return the current UIApplication if in an app, or nil if in extension or if it doesn't exist. | ||
| */ | ||
| + (nullable UIApplication *)sharedApplication; |
There was a problem hiding this comment.
Why have you moved this to the public header? Do consumers of the AppDelegateSwizzler need this?
There was a problem hiding this comment.
@tejasd I think, I needed it in one of the versions of Messaging, but forgot to remove.
4 participants
Changes required for #2591.
This is a part of PR #2683. I have to merge the changes first to make sure macOS support is available for FirebaseMessaging