Swizzling (#364)

* Swizzling

• Ensures that UNNotificationCenter setDelegate: will never get called twice, which is useful if other SDK's are swizzling the same method incorrectly in a way that invites race conditions

* Add Test

• Adds a test to make sure that calling UNUserNotificationCenter +setDelegate: does not swizzle the same object twice.

* Add Comment

• Adds a comment to explain what is happening in the registerAsUNNotificationCenterDelegate() method which looks a bit confusing

Author: Nightsd01

iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj

@@ -153,6 +153,7 @@
 		CAABF34B205B15780042F8E5 /* OneSignalExtensionBadgeHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = CAABF34A205B15780042F8E5 /* OneSignalExtensionBadgeHandler.m */; };
 		CAABF34C205B157B0042F8E5 /* OneSignalExtensionBadgeHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = CAABF34A205B15780042F8E5 /* OneSignalExtensionBadgeHandler.m */; };
 		CAABF34D205B157B0042F8E5 /* OneSignalExtensionBadgeHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = CAABF34A205B15780042F8E5 /* OneSignalExtensionBadgeHandler.m */; };
+		CAB411AE208931EE005A70D1 /* DummyNotificationCenterDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = CAB411AD208931EE005A70D1 /* DummyNotificationCenterDelegate.m */; };
 		CAEA1C66202BB3C600FBFE9E /* OSEmailSubscription.h in Headers */ = {isa = PBXBuildFile; fileRef = CA810FCF202BA97300A60FED /* OSEmailSubscription.h */; };
 /* End PBXBuildFile section */
 
@@ -292,6 +293,8 @@
 		CAA4ED0020646762005BD59B /* BadgeTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BadgeTests.m; sourceTree = "<group>"; };
 		CAABF349205B15780042F8E5 /* OneSignalExtensionBadgeHandler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = OneSignalExtensionBadgeHandler.h; sourceTree = "<group>"; };
 		CAABF34A205B15780042F8E5 /* OneSignalExtensionBadgeHandler.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = OneSignalExtensionBadgeHandler.m; sourceTree = "<group>"; };
+		CAB411AC208931EE005A70D1 /* DummyNotificationCenterDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DummyNotificationCenterDelegate.h; sourceTree = "<group>"; };
+		CAB411AD208931EE005A70D1 /* DummyNotificationCenterDelegate.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = DummyNotificationCenterDelegate.m; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -427,6 +430,8 @@
 				4529DED41FA823B900CEAB1D /* TestHelperFunctions.m */,
 				4529DEF41FA8460C00CEAB1D /* UnitTestAppDelegate.h */,
 				4529DEF51FA8460C00CEAB1D /* UnitTestAppDelegate.m */,
+				CAB411AC208931EE005A70D1 /* DummyNotificationCenterDelegate.h */,
+				CAB411AD208931EE005A70D1 /* DummyNotificationCenterDelegate.m */,
 			);
 			path = UnitTests;
 			sourceTree = "<group>";
@@ -836,6 +841,7 @@
 				4529DEE11FA82AB300CEAB1D /* NSBundleOverrider.m in Sources */,
 				912412441E73342200E41FD7 /* UNUserNotificationCenter+OneSignal.m in Sources */,
 				9124123C1E73342200E41FD7 /* OneSignalWebView.m in Sources */,
+				CAB411AE208931EE005A70D1 /* DummyNotificationCenterDelegate.m in Sources */,
 				4529DEF01FA8433500CEAB1D /* NSLocaleOverrider.m in Sources */,
 				CA08FC751FE99B00004C445F /* OneSignalClient.m in Sources */,
 				9129C6BA1E89E59B009CB6A0 /* OSPermission.m in Sources */,

iOS_SDK/OneSignalSDK/Source/OneSignalHelper.m

@@ -540,6 +540,14 @@ +(NSString*)randomStringWithLength:(int)length {
 
 + (void)registerAsUNNotificationCenterDelegate {
     let curNotifCenter = [UNUserNotificationCenter currentNotificationCenter];
+    
+    /*
+        Sets the OneSignal shared instance as a delegate of UNUserNotificationCenter
+        OneSignal does not implement the delegate methods, we simply set it as a delegate
+        in order to swizzle the UNUserNotificationCenter methods in case the developer
+        does not set a UNUserNotificationCenter delegate themselves
+    */
+    
     if (!curNotifCenter.delegate)
         curNotifCenter.delegate = (id)[self sharedInstance];
 }

iOS_SDK/OneSignalSDK/Source/UNUserNotificationCenter+OneSignal.m

@@ -68,6 +68,10 @@ @implementation OneSignalUNUserNotificationCenter
 // But rather in one of the subclasses
 static NSArray* delegateUNSubclasses = nil;
 
+//ensures setDelegate: swizzles will never get executed twice for the same delegate object
+//captures a weak reference to avoid retain cycles
+__weak static id previousDelegate;
+
 + (void)swizzleSelectors {
     injectToProperClass(@selector(setOneSignalUNDelegate:), @selector(setDelegate:), @[], [OneSignalUNUserNotificationCenter class], [UNUserNotificationCenter class]);
 
@@ -120,6 +124,13 @@ - (void)onesignalGetNotificationSettingsWithCompletionHandler:(void(^)(UNNotific
 //  - Selector will be called once if developer does not set a UNUserNotificationCenter delegate.
 //  - Selector will be called a 2nd time if the developer does set one.
 - (void) setOneSignalUNDelegate:(id)delegate {
+    if (previousDelegate == delegate) {
+        [self setOneSignalUNDelegate:delegate];
+        return;
+    }
+    
+    previousDelegate = delegate;
+    
     [OneSignal onesignal_Log:ONE_S_LL_VERBOSE message:@"OneSignalUNUserNotificationCenter setOneSignalUNDelegate Fired!"];
 
     delegateUNClass = getClassWithProtocolInHierarchy([delegate class], @protocol(UNUserNotificationCenterDelegate));

iOS_SDK/OneSignalSDK/UnitTests/DummyNotificationCenterDelegate.h

@@ -0,0 +1,14 @@
+//
+//  DummyNotificationCenterDelegate.h
+//  UnitTests
+//
+//  Created by Brad Hesse on 4/19/18.
+//  Copyright © 2018 Hiptic. All rights reserved.
+//
+
+#import <Foundation/Foundation.h>
+#import <UserNotifications/UserNotifications.h>
+
+@interface DummyNotificationCenterDelegate : NSObject <UNUserNotificationCenterDelegate>
+
+@end

iOS_SDK/OneSignalSDK/UnitTests/DummyNotificationCenterDelegate.m

@@ -0,0 +1,21 @@
+//
+//  DummyNotificationCenterDelegate.m
+//  UnitTests
+//
+//  Created by Brad Hesse on 4/19/18.
+//  Copyright © 2018 Hiptic. All rights reserved.
+//
+
+#import "DummyNotificationCenterDelegate.h"
+
+@implementation DummyNotificationCenterDelegate
+
+-(void)userNotificationCenter:(UNUserNotificationCenter *)center willPresentNotification:(UNNotification *)notification withCompletionHandler:(void (^)(UNNotificationPresentationOptions))completionHandler {
+    
+}
+
+-(void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler {
+    
+}
+
+@end

iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m

@@ -79,6 +79,8 @@
 #import "OneSignalClientOverrider.h"
 #import "OneSignalCommonDefines.h"
 
+#import "DummyNotificationCenterDelegate.h"
+
 @interface OneSignalHelper (TestHelper)
 + (NSString*)downloadMediaAndSaveInBundle:(NSString*)urlString;
 @end
@@ -1850,4 +1852,28 @@ - (void)testHandlingMediaUrlExtensions {
     XCTAssertNotNil(cacheName);
 }
 
+//tests to make sure that UNNotificationCenter setDelegate: duplicate calls don't double-swizzle for the same object
+- (void)testSwizzling {
+    UNUserNotificationCenter *center = [UNUserNotificationCenter currentNotificationCenter];
+    
+    DummyNotificationCenterDelegate *delegate = [[DummyNotificationCenterDelegate alloc] init];
+    
+    IMP original = class_getMethodImplementation([delegate class], @selector(userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler:));
+    
+    center.delegate = delegate;
+    
+    IMP swizzled = class_getMethodImplementation([delegate class], @selector(userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler:));
+    
+    XCTAssertNotEqual(original, swizzled);
+    
+    //calling setDelegate: a second time on the same object should not re-exchange method implementations
+    //thus the new method implementation should still be the same, swizzled == newSwizzled should be true
+    center.delegate = delegate;
+    
+    IMP newSwizzled = class_getMethodImplementation([delegate class], @selector(userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler:));
+    
+    XCTAssertNotEqual(original, newSwizzled);
+    XCTAssertEqual(swizzled, newSwizzled);
+}
+
 @end