From 1cd7ebf350021aa8e23fd6cd928668d324ba2098 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 24 Feb 2025 18:02:55 -0500 Subject: [PATCH 01/22] Add unit test for didReceiveResponse --- .ruby-version | 2 +- .../FPRNSURLSessionInstrumentTest.m | 22 +++++++++++++++++++ .../FPRNSURLSessionInstrumentTestDelegates.h | 7 ++++++ .../FPRNSURLSessionInstrumentTestDelegates.m | 7 ++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/.ruby-version b/.ruby-version index fa376edcab7..a4dd9dba4fb 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -ruby-2.7 +2.7.4 diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index ffffbe17c17..8925bc85ded 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -482,6 +482,28 @@ - (void)testDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { [instrument deregisterInstrumentors]; } +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testDownload"]; + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; + [downloadTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled); + }]; + [instrument deregisterInstrumentors]; +} + /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h index 722f508fa84..56c2dce7c00 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h @@ -67,6 +67,13 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic) BOOL URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled; + +/** Set to YES when + * URLSession:dataTask:didReceiveResponse:completionHandler: is called, used + * for testing. + */ +@property(nonatomic) BOOL URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled; + @end /** This class implements the methods necessary to cancel and resume a download. */ diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m index 28525bd4967..f7a851d114f 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m @@ -95,6 +95,13 @@ - (void)URLSession:(NSURLSession *)session self.URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled = YES; } +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask +didReceiveResponse:(NSURLResponse *)response + completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + self.URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled = YES; +} + @end @interface FPRNSURLSessionTestDownloadDelegate () From 3ad8823d6f02d6478cdc6d3c695d0ed7a9bddbef Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 24 Feb 2025 18:07:53 -0500 Subject: [PATCH 02/22] style --- .../Instruments/FPRNSURLSessionInstrumentTestDelegates.h | 1 - .../Instruments/FPRNSURLSessionInstrumentTestDelegates.m | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h index 56c2dce7c00..cab640b93e8 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h @@ -67,7 +67,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic) BOOL URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled; - /** Set to YES when * URLSession:dataTask:didReceiveResponse:completionHandler: is called, used * for testing. diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m index f7a851d114f..8f3afb74fab 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m @@ -96,9 +96,9 @@ - (void)URLSession:(NSURLSession *)session } - (void)URLSession:(NSURLSession *)session - dataTask:(NSURLSessionDataTask *)dataTask -didReceiveResponse:(NSURLResponse *)response - completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + dataTask:(NSURLSessionDataTask *)dataTask + didReceiveResponse:(NSURLResponse *)response + completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { self.URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled = YES; } From 95ebfc3735651b61061c649193e8498c72a98064 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 24 Feb 2025 18:11:22 -0500 Subject: [PATCH 03/22] Fix test to data task --- .../FPRNSURLSessionInstrumentTest.m | 39 +++++++++++-------- .../FPRNSURLSessionInstrumentTestDelegates.h | 12 +++--- .../FPRNSURLSessionInstrumentTestDelegates.m | 2 +- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 8925bc85ded..ff824157b0f 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -484,23 +484,28 @@ - (void)testDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { - FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; - [instrument registerInstrumentors]; - FPRNSURLSessionCompleteTestDelegate *delegate = - [[FPRNSURLSessionCompleteTestDelegate alloc] init]; - NSURLSessionConfiguration *configuration = - [NSURLSessionConfiguration defaultSessionConfiguration]; - NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:delegate - delegateQueue:nil]; - NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testDownload"]; - NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; - [downloadTask resume]; - XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); - [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, - GCDWebServerResponse *_Nonnull response) { - XCTAssertTrue(delegate.URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled); - }]; + FPRNSURLSessionInstrument *instrument; + NSURLSessionDataTask *dataTask; + @autoreleasepool { + instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; + dataTask = [session dataTaskWithURL:URL]; + [dataTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + }]; + } [instrument deregisterInstrumentors]; } diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h index cab640b93e8..1fe8a746943 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h @@ -52,6 +52,12 @@ NS_ASSUME_NONNULL_BEGIN /** Set to YES when URLSession:dataTask:didReceiveData: is called, used for testing. */ @property(nonatomic) BOOL URLSessionDataTaskDidReceiveDataCalled; +/** Set to YES when + * URLSession:dataTask:didReceiveResponse:completionHandler: is called, used + * for testing. + */ +@property(nonatomic) BOOL URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled; + /** Set to YES when URLSession:downloadTask:didFinishDownloadingToURL: is called, used for testing. */ @property(nonatomic) BOOL URLSessionDownloadTaskDidFinishDownloadingToURLCalled; @@ -67,12 +73,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic) BOOL URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled; -/** Set to YES when - * URLSession:dataTask:didReceiveResponse:completionHandler: is called, used - * for testing. - */ -@property(nonatomic) BOOL URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled; - @end /** This class implements the methods necessary to cancel and resume a download. */ diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m index 8f3afb74fab..c9fbe183813 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m @@ -99,7 +99,7 @@ - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveResponse:(NSURLResponse *)response completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { - self.URLSessionDownloadTaskDidReceiveResponseCompletionHandlerCalled = YES; + self.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled = YES; } @end From c630b230c84d24e1b8f22dd54183cbc9da6113c3 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 24 Feb 2025 18:12:03 -0500 Subject: [PATCH 04/22] Fix unit test --- .../Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 1 - 1 file changed, 1 deletion(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index ff824157b0f..142c40a2bba 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -503,7 +503,6 @@ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From 2842cfaea8f354c55d703890189704a3faf602a0 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 25 Feb 2025 17:41:21 -0500 Subject: [PATCH 05/22] Add a unit test for a delegate that's a proxy --- .../FPRNSURLSessionInstrumentTest.m | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 142c40a2bba..aabcf1bba29 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -62,6 +62,35 @@ - (void)forwardInvocation:(NSInvocation *)invocation { @end +@interface FPRNSURLSessionDelegateProxy : NSProxy { + // The wrapped session object. + id _delegate; +} + +/** @return an instance of the session proxy. */ +- (instancetype)initWithDelegate:(id )delegate; + +@end + +@implementation FPRNSURLSessionDelegateProxy + +- (instancetype)initWithDelegate:(id)delegate { + if (self) { + _delegate = delegate; + } + return self; +} + +- (NSMethodSignature *)methodSignatureForSelector:(SEL)selector { + return [_delegate methodSignatureForSelector:selector]; +} + +- (void)forwardInvocation:(NSInvocation *)invocation { + [invocation invokeWithTarget:_delegate]; +} + +@end + @interface FPRNSURLSessionInstrumentTest : FPRTestCase /** Test server to create connections to. */ @@ -432,6 +461,31 @@ - (void)testDelegateURLSessionTaskWillPerformHTTPRedirectionNewRequestCompletion [instrument deregisterInstrumentors]; } +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandler { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testRedirect"]; + NSURLRequest *request = [NSURLRequest requestWithURL:URL]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURLSessionTask *task = [session dataTaskWithRequest:request]; + [task resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:task]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue( + delegate.URLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandlerCalled); + }]; + [instrument deregisterInstrumentors]; +} + /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testDelegateURLSessionDataTaskDidReceiveData { FPRNSURLSessionInstrument *instrument; @@ -503,6 +557,7 @@ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From b8825c6f08defc2b6c1d63eb1dc8a653aacdd761 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 25 Feb 2025 17:43:03 -0500 Subject: [PATCH 06/22] Update comments --- .../Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index aabcf1bba29..b5584d129e9 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -63,11 +63,11 @@ - (void)forwardInvocation:(NSInvocation *)invocation { @end @interface FPRNSURLSessionDelegateProxy : NSProxy { - // The wrapped session object. + // The wrapped delegate object. id _delegate; } -/** @return an instance of the session proxy. */ +/** @return an instance of the delegate proxy. */ - (instancetype)initWithDelegate:(id )delegate; @end From e82d4341a810fdf676754593e9bd9968fbbfc0e3 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 25 Feb 2025 17:46:46 -0500 Subject: [PATCH 07/22] Fix test --- .../FPRNSURLSessionInstrumentTest.m | 49 +++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index b5584d129e9..3ca4711f118 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -461,31 +461,6 @@ - (void)testDelegateURLSessionTaskWillPerformHTTPRedirectionNewRequestCompletion [instrument deregisterInstrumentors]; } -/** Tests that the called delegate selector is wrapped and calls through. */ -- (void)testProxyDelegateURLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandler { - FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; - [instrument registerInstrumentors]; - FPRNSURLSessionCompleteTestDelegate *delegate = - [[FPRNSURLSessionCompleteTestDelegate alloc] init]; - FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; - NSURLSessionConfiguration *configuration = - [NSURLSessionConfiguration defaultSessionConfiguration]; - NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testRedirect"]; - NSURLRequest *request = [NSURLRequest requestWithURL:URL]; - NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:proxyDelegate - delegateQueue:nil]; - NSURLSessionTask *task = [session dataTaskWithRequest:request]; - [task resume]; - XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:task]); - [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, - GCDWebServerResponse *_Nonnull response) { - XCTAssertTrue( - delegate.URLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandlerCalled); - }]; - [instrument deregisterInstrumentors]; -} - /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testDelegateURLSessionDataTaskDidReceiveData { FPRNSURLSessionInstrument *instrument; @@ -536,6 +511,30 @@ - (void)testDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { [instrument deregisterInstrumentors]; } +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testDownload"]; + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; + [downloadTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDownloadTaskDidFinishDownloadingToURLCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + }]; + [instrument deregisterInstrumentors]; +} + /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { FPRNSURLSessionInstrument *instrument; From a2974d226dde4bc334efcaebeb29424b9c74f30c Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 25 Feb 2025 17:49:45 -0500 Subject: [PATCH 08/22] correct test --- .../FPRNSURLSessionInstrumentTest.m | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 3ca4711f118..9178816f58e 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -512,31 +512,34 @@ - (void)testDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { } /** Tests that the called delegate selector is wrapped and calls through. */ -- (void)testProxyDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { - FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; - [instrument registerInstrumentors]; - FPRNSURLSessionCompleteTestDelegate *delegate = - [[FPRNSURLSessionCompleteTestDelegate alloc] init]; - FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; - NSURLSessionConfiguration *configuration = - [NSURLSessionConfiguration defaultSessionConfiguration]; - NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:proxyDelegate - delegateQueue:nil]; - NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testDownload"]; - NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; - [downloadTask resume]; - XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); - [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, - GCDWebServerResponse *_Nonnull response) { - XCTAssertTrue(delegate.URLSessionDownloadTaskDidFinishDownloadingToURLCalled); - XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); - }]; +- (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { + FPRNSURLSessionInstrument *instrument; + NSURLSessionDataTask *dataTask; + @autoreleasepool { + instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; + dataTask = [session dataTaskWithURL:URL]; + [dataTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + }]; + } [instrument deregisterInstrumentors]; } /** Tests that the called delegate selector is wrapped and calls through. */ -- (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { +- (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { FPRNSURLSessionInstrument *instrument; NSURLSessionDataTask *dataTask; @autoreleasepool { @@ -544,10 +547,11 @@ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [instrument registerInstrumentors]; FPRNSURLSessionCompleteTestDelegate *delegate = [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:delegate + delegate:proxyDelegate delegateQueue:nil]; NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; dataTask = [session dataTaskWithURL:URL]; @@ -556,7 +560,6 @@ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From c8393215ca86052b9c67eab6d0b9c86bfd92a4c1 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 27 Feb 2025 14:58:58 -0500 Subject: [PATCH 09/22] Add swizzling for URLSession:dataTask:didReceiveResponse:completionHandler: --- .../FPRNSURLSessionDelegateInstrument.m | 36 +++++++++++++++++++ .../FPRNSURLSessionInstrumentTest.m | 12 +++++-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index c636d3e2ba7..87b2da68b17 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -33,8 +33,42 @@ static dispatch_queue_t GetInstrumentationQueue(void) { return queue; } +typedef void (^FPRDataTaskDelegateCompletionHandler)(NSURLSessionResponseDisposition); + #pragma mark - NSURLSessionTaskDelegate methods +/** Instruments URLSession:dataTask:didReceiveResponse:completionHandler: + * + * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. + */ +FOUNDATION_STATIC_INLINE +NS_EXTENSION_UNAVAILABLE("Firebase Performance is not supported for extensions.") +void InstrumentURLSessionTaskDidReceiveResponseCompletionHandler( + FPRClassInstrumentor *instrumentor) { + SEL selector = @selector(URLSession:dataTask:didReceiveResponse:completionHandler:); + FPRSelectorInstrumentor *selectorInstrumentor = + [instrumentor instrumentorForInstanceSelector:selector]; + if (selectorInstrumentor) { + IMP currentIMP = selectorInstrumentor.currentIMP; + [selectorInstrumentor + setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionTask *task, + NSURLResponse *response, + FPRDataTaskDelegateCompletionHandler completionHandler) { + @try { + FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:task]; + [trace didCompleteRequestWithResponse:task.response error:task.error]; + [FPRNetworkTrace removeNetworkTraceFromObject:task]; + } @catch (NSException *exception) { + FPRLogWarning(kFPRNetworkTraceNotTrackable, @"Unable to track network request."); + } @finally { + typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionTask *, + NSURLResponse *, FPRDataTaskDelegateCompletionHandler); + ((OriginalImp)currentIMP)(object, selector, session, task, response, completionHandler); + } + }]; + } +} + /** Instruments URLSession:task:didCompleteWithError:. * * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. @@ -247,6 +281,8 @@ - (void)registerObject:(id)object { CopySelector(@selector(URLSession: task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:), instrumentor); + CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), + instrumentor); // NSURLSessionDataDelegate methods. CopySelector(@selector(URLSession:dataTask:didReceiveData:), instrumentor); diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 9178816f58e..1574e4642a9 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -62,13 +62,13 @@ - (void)forwardInvocation:(NSInvocation *)invocation { @end -@interface FPRNSURLSessionDelegateProxy : NSProxy { +@interface FPRNSURLSessionDelegateProxy : NSProxy { // The wrapped delegate object. id _delegate; } /** @return an instance of the delegate proxy. */ -- (instancetype)initWithDelegate:(id )delegate; +- (instancetype)initWithDelegate:(id)delegate; @end @@ -85,6 +85,10 @@ - (NSMethodSignature *)methodSignatureForSelector:(SEL)selector { return [_delegate methodSignatureForSelector:selector]; } +- (BOOL)respondsToSelector:(SEL)aSelector { + return [_delegate respondsToSelector:aSelector]; +} + - (void)forwardInvocation:(NSInvocation *)invocation { [invocation invokeWithTarget:_delegate]; } @@ -547,7 +551,8 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [instrument registerInstrumentors]; FPRNSURLSessionCompleteTestDelegate *delegate = [[FPRNSURLSessionCompleteTestDelegate alloc] init]; - FPRNSURLSessionDelegateProxy *proxyDelegate = [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration @@ -560,6 +565,7 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From 0792561989dbd6fc381b3ffc144aabddc38436e2 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 27 Feb 2025 15:05:44 -0500 Subject: [PATCH 10/22] Use the instrumentation function --- .../Network/Delegates/FPRNSURLSessionDelegateInstrument.m | 1 + 1 file changed, 1 insertion(+) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index 87b2da68b17..0af8c1972a5 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -255,6 +255,7 @@ - (void)registerClass:(Class)aClass { // NSURLSessionTaskDelegate methods. InstrumentURLSessionTaskDidCompleteWithError(instrumentor); InstrumentURLSessionTaskDidSendBodyDataTotalBytesSentTotalBytesExpectedToSend(instrumentor); + InstrumentURLSessionTaskDidReceiveResponseCompletionHandler(instrumentor); // NSURLSessionDataDelegate methods. InstrumentURLSessionDataTaskDidReceiveData(instrumentor); From 2205da28705f7de9290aa3a4416065e0eb9720c3 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 27 Feb 2025 16:02:08 -0500 Subject: [PATCH 11/22] Correct the network trace behavior --- .../Delegates/FPRNSURLSessionDelegate.m | 8 +++ .../FPRNSURLSessionDelegateInstrument.m | 68 +++++++++---------- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m index 75a44a52ae0..87b668ffd06 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m @@ -55,6 +55,14 @@ - (void)URLSession:(NSURLSession *)session [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; } +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask + didReceiveResponse:(NSURLResponse *)response + completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; + [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; +} + - (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didFinishDownloadingToURL:(NSURL *)location { diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index 0af8c1972a5..3b152a8a54b 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -37,38 +37,6 @@ static dispatch_queue_t GetInstrumentationQueue(void) { #pragma mark - NSURLSessionTaskDelegate methods -/** Instruments URLSession:dataTask:didReceiveResponse:completionHandler: - * - * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. - */ -FOUNDATION_STATIC_INLINE -NS_EXTENSION_UNAVAILABLE("Firebase Performance is not supported for extensions.") -void InstrumentURLSessionTaskDidReceiveResponseCompletionHandler( - FPRClassInstrumentor *instrumentor) { - SEL selector = @selector(URLSession:dataTask:didReceiveResponse:completionHandler:); - FPRSelectorInstrumentor *selectorInstrumentor = - [instrumentor instrumentorForInstanceSelector:selector]; - if (selectorInstrumentor) { - IMP currentIMP = selectorInstrumentor.currentIMP; - [selectorInstrumentor - setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionTask *task, - NSURLResponse *response, - FPRDataTaskDelegateCompletionHandler completionHandler) { - @try { - FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:task]; - [trace didCompleteRequestWithResponse:task.response error:task.error]; - [FPRNetworkTrace removeNetworkTraceFromObject:task]; - } @catch (NSException *exception) { - FPRLogWarning(kFPRNetworkTraceNotTrackable, @"Unable to track network request."); - } @finally { - typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionTask *, - NSURLResponse *, FPRDataTaskDelegateCompletionHandler); - ((OriginalImp)currentIMP)(object, selector, session, task, response, completionHandler); - } - }]; - } -} - /** Instruments URLSession:task:didCompleteWithError:. * * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. @@ -158,6 +126,38 @@ void InstrumentURLSessionDataTaskDidReceiveData(FPRClassInstrumentor *instrument } } +/** Instruments URLSession:dataTask:didReceiveResponse:completionHandler: + * + * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. + */ +FOUNDATION_STATIC_INLINE +NS_EXTENSION_UNAVAILABLE("Firebase Performance is not supported for extensions.") +void InstrumentURLSessionTaskDidReceiveResponseCompletionHandler( + FPRClassInstrumentor *instrumentor) { + SEL selector = @selector(URLSession:dataTask:didReceiveResponse:completionHandler:); + FPRSelectorInstrumentor *selectorInstrumentor = + [instrumentor instrumentorForInstanceSelector:selector]; + if (selectorInstrumentor) { + IMP currentIMP = selectorInstrumentor.currentIMP; + [selectorInstrumentor + setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionTask *task, + NSURLResponse *response, + FPRDataTaskDelegateCompletionHandler completionHandler) { + @try { + FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; + [trace didReceiveData:task.]; + [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; + } @catch (NSException *exception) { + FPRLogWarning(kFPRNetworkTraceNotTrackable, @"Unable to track network request."); + } @finally { + typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionTask *, + NSURLResponse *, FPRDataTaskDelegateCompletionHandler); + ((OriginalImp)currentIMP)(object, selector, session, task, response, completionHandler); + } + }]; + } +} + #pragma mark - NSURLSessionDownloadDelegate methods. /** Instruments URLSession:downloadTask:didFinishDownloadingToURL:. @@ -282,11 +282,11 @@ - (void)registerObject:(id)object { CopySelector(@selector(URLSession: task:didSendBodyData:totalBytesSent:totalBytesExpectedToSend:), instrumentor); - CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), - instrumentor); // NSURLSessionDataDelegate methods. CopySelector(@selector(URLSession:dataTask:didReceiveData:), instrumentor); + CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), + instrumentor); // NSURLSessionDownloadDelegate methods. CopySelector(@selector(URLSession:downloadTask:didFinishDownloadingToURL:), instrumentor); From 19ca07c947d056c76fbd98e63e1e5a3677a5906c Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 27 Feb 2025 16:07:41 -0500 Subject: [PATCH 12/22] Fix types --- .../Network/Delegates/FPRNSURLSessionDelegateInstrument.m | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index 3b152a8a54b..16e1563e259 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -140,17 +140,16 @@ void InstrumentURLSessionTaskDidReceiveResponseCompletionHandler( if (selectorInstrumentor) { IMP currentIMP = selectorInstrumentor.currentIMP; [selectorInstrumentor - setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionTask *task, + setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionDataTask *task, NSURLResponse *response, FPRDataTaskDelegateCompletionHandler completionHandler) { @try { - FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; - [trace didReceiveData:task.]; + FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:task]; [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; } @catch (NSException *exception) { FPRLogWarning(kFPRNetworkTraceNotTrackable, @"Unable to track network request."); } @finally { - typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionTask *, + typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionDataTask *, NSURLResponse *, FPRDataTaskDelegateCompletionHandler); ((OriginalImp)currentIMP)(object, selector, session, task, response, completionHandler); } From 688236596d1cd873f3d39add4590a4ad52e7ee39 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 27 Feb 2025 16:11:59 -0500 Subject: [PATCH 13/22] Function name --- .../Network/Delegates/FPRNSURLSessionDelegateInstrument.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index 16e1563e259..8b82240295e 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -132,7 +132,7 @@ void InstrumentURLSessionDataTaskDidReceiveData(FPRClassInstrumentor *instrument */ FOUNDATION_STATIC_INLINE NS_EXTENSION_UNAVAILABLE("Firebase Performance is not supported for extensions.") -void InstrumentURLSessionTaskDidReceiveResponseCompletionHandler( +void InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler( FPRClassInstrumentor *instrumentor) { SEL selector = @selector(URLSession:dataTask:didReceiveResponse:completionHandler:); FPRSelectorInstrumentor *selectorInstrumentor = @@ -254,7 +254,7 @@ - (void)registerClass:(Class)aClass { // NSURLSessionTaskDelegate methods. InstrumentURLSessionTaskDidCompleteWithError(instrumentor); InstrumentURLSessionTaskDidSendBodyDataTotalBytesSentTotalBytesExpectedToSend(instrumentor); - InstrumentURLSessionTaskDidReceiveResponseCompletionHandler(instrumentor); + InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler(instrumentor); // NSURLSessionDataDelegate methods. InstrumentURLSessionDataTaskDidReceiveData(instrumentor); From af4bfe257f7ad26281887be7ef257cf88b19b440 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 13:11:17 -0500 Subject: [PATCH 14/22] call completion handler with NSURLSessionResponseAllow --- .../Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m | 1 + 1 file changed, 1 insertion(+) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m index 87b668ffd06..3d221ed5810 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m @@ -61,6 +61,7 @@ - (void)URLSession:(NSURLSession *)session completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; + completionHandler(NSURLSessionResponseAllow); } - (void)URLSession:(NSURLSession *)session From 62e126de24a7463572f409ab392c7febda99dc70 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 13:25:07 -0500 Subject: [PATCH 15/22] Call completionHandler callback in the test delegate --- .../FPRNSURLSessionInstrumentTestDelegates.m | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m index c9fbe183813..9d84a70aa43 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m @@ -72,6 +72,14 @@ - (void)URLSession:(NSURLSession *)session self.URLSessionDataTaskDidReceiveDataCalled = YES; } +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask + didReceiveResponse:(NSURLResponse *)response + completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + self.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled = YES; + completionHandler(NSURLSessionResponseAllow); +} + #pragma mark - NSURLSessionDownloadDelegate methods - (void)URLSession:(NSURLSession *)session @@ -95,13 +103,6 @@ - (void)URLSession:(NSURLSession *)session self.URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled = YES; } -- (void)URLSession:(NSURLSession *)session - dataTask:(NSURLSessionDataTask *)dataTask - didReceiveResponse:(NSURLResponse *)response - completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { - self.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled = YES; -} - @end @interface FPRNSURLSessionTestDownloadDelegate () From 1ad3623e454d52ee30fe3e3eb561416b8ab8787b Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 14:10:57 -0500 Subject: [PATCH 16/22] Update the NSProxy unit test --- .../Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 1574e4642a9..40566ead75c 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -565,7 +565,8 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + // URLSession:task:didCompleteWithError: isn't called in the case when the delegate is an NSProxy, and thus this won't be nil. + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From 15dd716d10fe322fce2cf01a36c048044f30e04c Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 14:17:37 -0500 Subject: [PATCH 17/22] Comment out swizzling the delegate method --- .../Network/Delegates/FPRNSURLSessionDelegate.m | 16 ++++++++-------- .../FPRNSURLSessionDelegateInstrument.m | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m index 3d221ed5810..a58c02ddca7 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m @@ -55,14 +55,14 @@ - (void)URLSession:(NSURLSession *)session [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; } -- (void)URLSession:(NSURLSession *)session - dataTask:(NSURLSessionDataTask *)dataTask - didReceiveResponse:(NSURLResponse *)response - completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { - FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; - [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; - completionHandler(NSURLSessionResponseAllow); -} +//- (void)URLSession:(NSURLSession *)session +// dataTask:(NSURLSessionDataTask *)dataTask +// didReceiveResponse:(NSURLResponse *)response +// completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { +// FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; +// [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; +// completionHandler(NSURLSessionResponseAllow); +//} - (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index 8b82240295e..aa3bb771472 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -254,10 +254,11 @@ - (void)registerClass:(Class)aClass { // NSURLSessionTaskDelegate methods. InstrumentURLSessionTaskDidCompleteWithError(instrumentor); InstrumentURLSessionTaskDidSendBodyDataTotalBytesSentTotalBytesExpectedToSend(instrumentor); - InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler(instrumentor); // NSURLSessionDataDelegate methods. InstrumentURLSessionDataTaskDidReceiveData(instrumentor); + // InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler(instrumentor); + // NSURLSessionDownloadDelegate methods. InstrumentURLSessionDownloadTaskDidFinishDownloadToURL(instrumentor); @@ -284,8 +285,8 @@ - (void)registerObject:(id)object { // NSURLSessionDataDelegate methods. CopySelector(@selector(URLSession:dataTask:didReceiveData:), instrumentor); - CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), - instrumentor); +// CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), +// instrumentor); // NSURLSessionDownloadDelegate methods. CopySelector(@selector(URLSession:downloadTask:didFinishDownloadingToURL:), instrumentor); From 8a3b49cb4aeb83b836ced9ff247c056ffcc459a4 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 14:49:42 -0500 Subject: [PATCH 18/22] Add additional info around NSProxy --- .../Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m | 2 +- .../Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m index 57394071199..b34f0bac100 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m @@ -157,9 +157,9 @@ void InstrumentSessionWithConfigurationDelegateDelegateQueue( ThrowExceptionBecauseInstrumentHasBeenDeallocated(selector, instrumentedClass); } if (delegate) { + // TODO(#14478): Investigate workaround for NSProxy. [delegateInstrument registerClass:[delegate class]]; [delegateInstrument registerObject:delegate]; - } else { delegate = [[FPRNSURLSessionDelegate alloc] init]; } diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 40566ead75c..e613e1f5af1 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -565,7 +565,7 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - // URLSession:task:didCompleteWithError: isn't called in the case when the delegate is an NSProxy, and thus this won't be nil. + // URLSession:task:didCompleteWithError: isn't called in the case when the delegate is an NSProxy, and thus this isn't nil (when it should). XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } From 4e3034ca898407f72fc6dcab34f200e81ace01d2 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 14:59:40 -0500 Subject: [PATCH 19/22] Revert the changes to swizzle the additional delegate method --- .../Delegates/FPRNSURLSessionDelegate.m | 9 ----- .../FPRNSURLSessionDelegateInstrument.m | 37 ------------------- 2 files changed, 46 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m index a58c02ddca7..75a44a52ae0 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.m @@ -55,15 +55,6 @@ - (void)URLSession:(NSURLSession *)session [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; } -//- (void)URLSession:(NSURLSession *)session -// dataTask:(NSURLSessionDataTask *)dataTask -// didReceiveResponse:(NSURLResponse *)response -// completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { -// FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:dataTask]; -// [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; -// completionHandler(NSURLSessionResponseAllow); -//} - - (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didFinishDownloadingToURL:(NSURL *)location { diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index aa3bb771472..c636d3e2ba7 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -33,8 +33,6 @@ static dispatch_queue_t GetInstrumentationQueue(void) { return queue; } -typedef void (^FPRDataTaskDelegateCompletionHandler)(NSURLSessionResponseDisposition); - #pragma mark - NSURLSessionTaskDelegate methods /** Instruments URLSession:task:didCompleteWithError:. @@ -126,37 +124,6 @@ void InstrumentURLSessionDataTaskDidReceiveData(FPRClassInstrumentor *instrument } } -/** Instruments URLSession:dataTask:didReceiveResponse:completionHandler: - * - * @param instrumentor The FPRClassInstrumentor to add the selector instrumentor to. - */ -FOUNDATION_STATIC_INLINE -NS_EXTENSION_UNAVAILABLE("Firebase Performance is not supported for extensions.") -void InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler( - FPRClassInstrumentor *instrumentor) { - SEL selector = @selector(URLSession:dataTask:didReceiveResponse:completionHandler:); - FPRSelectorInstrumentor *selectorInstrumentor = - [instrumentor instrumentorForInstanceSelector:selector]; - if (selectorInstrumentor) { - IMP currentIMP = selectorInstrumentor.currentIMP; - [selectorInstrumentor - setReplacingBlock:^(id object, NSURLSession *session, NSURLSessionDataTask *task, - NSURLResponse *response, - FPRDataTaskDelegateCompletionHandler completionHandler) { - @try { - FPRNetworkTrace *trace = [FPRNetworkTrace networkTraceFromObject:task]; - [trace checkpointState:FPRNetworkTraceCheckpointStateResponseReceived]; - } @catch (NSException *exception) { - FPRLogWarning(kFPRNetworkTraceNotTrackable, @"Unable to track network request."); - } @finally { - typedef void (*OriginalImp)(id, SEL, NSURLSession *, NSURLSessionDataTask *, - NSURLResponse *, FPRDataTaskDelegateCompletionHandler); - ((OriginalImp)currentIMP)(object, selector, session, task, response, completionHandler); - } - }]; - } -} - #pragma mark - NSURLSessionDownloadDelegate methods. /** Instruments URLSession:downloadTask:didFinishDownloadingToURL:. @@ -257,8 +224,6 @@ - (void)registerClass:(Class)aClass { // NSURLSessionDataDelegate methods. InstrumentURLSessionDataTaskDidReceiveData(instrumentor); - // InstrumentURLSessionDataTaskDidReceiveResponseCompletionHandler(instrumentor); - // NSURLSessionDownloadDelegate methods. InstrumentURLSessionDownloadTaskDidFinishDownloadToURL(instrumentor); @@ -285,8 +250,6 @@ - (void)registerObject:(id)object { // NSURLSessionDataDelegate methods. CopySelector(@selector(URLSession:dataTask:didReceiveData:), instrumentor); -// CopySelector(@selector(URLSession:dataTask:didReceiveResponse:completionHandler:), -// instrumentor); // NSURLSessionDownloadDelegate methods. CopySelector(@selector(URLSession:downloadTask:didFinishDownloadingToURL:), instrumentor); From c5cc371ff1733fcf02c856fc7283ca2499d99b52 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 16:17:34 -0500 Subject: [PATCH 20/22] Add proxy swizzling to FPRNSURLSessionDelegateInstrument --- .../Sources/Instrumentation/FPRObjectInstrumentor.h | 6 ++++++ .../Sources/Instrumentation/FPRProxyObjectHelper.h | 11 +++++++++++ .../Sources/Instrumentation/FPRProxyObjectHelper.m | 11 +++++++++++ .../Delegates/FPRNSURLSessionDelegateInstrument.m | 10 ++++++++++ .../Network/FPRNSURLSessionInstrument.m | 9 ++++++--- .../Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 3 +-- 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h b/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h index b016c070ef8..1716a56d4aa 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h +++ b/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h @@ -33,6 +33,12 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)registerObject:(id)object; +/** Registers an instance of the delegate class to be instrumented. + * + * @param proxy The instance to instrument. + */ +- (void)registerProxy:(id)proxy; + @end /** This class allows the instrumentation of specific objects by isa swizzling specific instances diff --git a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h index a746e8e0237..dadf4d50a09 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h +++ b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h @@ -31,4 +31,15 @@ forSuperclass:(Class)superclass varFoundHandler:(void (^)(id ivar))varFoundHandler; +/** Registers a proxy object for a given class and runs the onSuccess block whenever an ivar of the + * given class is discovered on the proxy object. + * + * @param proxy The proxy object whose ivars will be iterated. + * @param protocol The protocol all ivars will be compared against. See varFoundHandler. + * @param varFoundHandler The block to run when an ivar conformsToProtocol:protocol. + */ ++ (void)registerProxyObject:(id)proxy + forProtocol:(Protocol *)protocol + varFoundHandler:(void (^)(id ivar))varFoundHandler; + @end diff --git a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.m b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.m index ae46231ee5a..58a98c2a549 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.m +++ b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.m @@ -29,4 +29,15 @@ + (void)registerProxyObject:(id)proxy } } ++ (void)registerProxyObject:(id)proxy + forProtocol:(Protocol *)protocol + varFoundHandler:(void (^)(id ivar))varFoundHandler { + NSArray *ivars = [GULSwizzler ivarObjectsForObject:proxy]; + for (id ivar in ivars) { + if ([ivar conformsToProtocol:protocol]) { + varFoundHandler(ivar); + } + } +} + @end diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m index c636d3e2ba7..6541caa2e27 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegateInstrument.m @@ -18,6 +18,7 @@ #import "FirebasePerformance/Sources/Instrumentation/FPRClassInstrumentor.h" #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h" #import "FirebasePerformance/Sources/Instrumentation/FPRNetworkTrace.h" +#import "FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h" #import "FirebasePerformance/Sources/Instrumentation/FPRSelectorInstrumentor.h" #import "FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLSessionDelegate.h" #import "FirebasePerformance/Sources/Instrumentation/Network/FPRNetworkInstrumentHelpers.h" @@ -263,4 +264,13 @@ - (void)registerObject:(id)object { }); } +- (void)registerProxy:(id)proxy { + [FPRProxyObjectHelper registerProxyObject:proxy + forProtocol:@protocol(NSURLSessionDelegate) + varFoundHandler:^(id ivar) { + [self registerClass:[ivar class]]; + [self registerObject:ivar]; + }]; +} + @end diff --git a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m index b34f0bac100..610ba0fcc8c 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/FPRNSURLSessionInstrument.m @@ -157,9 +157,12 @@ void InstrumentSessionWithConfigurationDelegateDelegateQueue( ThrowExceptionBecauseInstrumentHasBeenDeallocated(selector, instrumentedClass); } if (delegate) { - // TODO(#14478): Investigate workaround for NSProxy. - [delegateInstrument registerClass:[delegate class]]; - [delegateInstrument registerObject:delegate]; + if ([delegate isProxy]) { + [delegateInstrument registerProxy:delegate]; + } else { + [delegateInstrument registerClass:[delegate class]]; + [delegateInstrument registerObject:delegate]; + } } else { delegate = [[FPRNSURLSessionDelegate alloc] init]; } diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index e613e1f5af1..1574e4642a9 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -565,8 +565,7 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - // URLSession:task:didCompleteWithError: isn't called in the case when the delegate is an NSProxy, and thus this isn't nil (when it should). - XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } [instrument deregisterInstrumentors]; From 004da2ed98e779c43ee165cef24930bb9d97050b Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Fri, 28 Feb 2025 16:25:12 -0500 Subject: [PATCH 21/22] Add proxy swizzling to NSURLCollection delegate --- .../Delegates/FPRNSURLConnectionDelegateInstrument.m | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.m index a01fb2180a6..8123ae56b62 100644 --- a/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.m +++ b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.m @@ -17,6 +17,7 @@ #import "FirebasePerformance/Sources/Instrumentation/FPRClassInstrumentor.h" #import "FirebasePerformance/Sources/Instrumentation/FPRInstrument_Private.h" #import "FirebasePerformance/Sources/Instrumentation/FPRNetworkTrace.h" +#import "FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h" #import "FirebasePerformance/Sources/Instrumentation/FPRSelectorInstrumentor.h" #import "FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegate.h" #import "FirebasePerformance/Sources/Instrumentation/Network/FPRNetworkInstrumentHelpers.h" @@ -304,4 +305,13 @@ - (void)registerObject:(id)object { }); } +- (void)registerProxy:(id)proxy { + [FPRProxyObjectHelper registerProxyObject:proxy + forProtocol:@protocol(NSURLSessionDelegate) + varFoundHandler:^(id ivar) { + [self registerClass:[ivar class]]; + [self registerObject:ivar]; + }]; +} + @end From f216d0706588e56c7e3fceded3e2ff6faa7928dc Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 6 Mar 2025 16:44:22 -0500 Subject: [PATCH 22/22] Add more tests for an NSProxy delegate --- .../FPRNSURLSessionInstrumentTest.m | 254 +++++++++++++++++- 1 file changed, 248 insertions(+), 6 deletions(-) diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index 1574e4642a9..92d754e8385 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -542,6 +542,240 @@ - (void)testDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [instrument deregisterInstrumentors]; } +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionTestDownloadDelegate *delegate = + [[FPRNSURLSessionTestDownloadDelegate alloc] init]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; + [downloadTask resume]; + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:5.0]]; + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + XCTAssertTrue(delegate.URLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytesCalled); + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testDelegateURLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; + [downloadTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled); + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1.0]]; + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + }]; + [instrument deregisterInstrumentors]; +} + +/** Tests that even if a delegate doesn't implement a method, we add it to the delegate class. */ +- (void)testDelegateUnimplementedURLSessionTaskDidCompleteWithError { + FPRNSURLSessionTestDelegate *delegate = [[FPRNSURLSessionTestDelegate alloc] init]; + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:delegate + delegateQueue:nil]; + XCTAssertTrue([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:self.testServer.serverURL]; + [downloadTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + }]; + [instrument deregisterInstrumentors]; +} + +#pragma mark - Testing proxy delegate method wrapping + +/** Tests that the delegate class is instrumented once when its wrapped with NSProxy. */ +- (void)testProxyDelegateSwizzlesDelegateOnce { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + [NSURLSession sessionWithConfiguration:configuration delegate:proxyDelegate delegateQueue:nil]; + [NSURLSession sessionWithConfiguration:configuration delegate:proxyDelegate delegateQueue:nil]; + XCTAssertEqual(instrument.delegateInstrument.classInstrumentors.count, 1); + XCTAssertEqual(instrument.delegateInstrument.instrumentedClasses.count, 1); + XCTAssertTrue( + [instrument.delegateInstrument.instrumentedClasses containsObject:[delegate class]]); + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionTaskDidCompleteWithError { + [self.testServer stop]; + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + // This request needs to fail. + NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"http://nonurl"]]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURLSessionTask *task; + @autoreleasepool { + task = [session dataTaskWithRequest:request]; + [task resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:task]); + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:5.0]]; + } + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:task]); + XCTAssertTrue(delegate.URLSessionTaskDidCompleteWithErrorCalled); + [instrument deregisterInstrumentors]; + [self.testServer start]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionTaskDidSendBodyDataTotalBytesSentTotalBytesExpectedToSend { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testUpload"]; + NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL]; + request.HTTPMethod = @"POST"; + + NSBundle *bundle = [FPRTestUtils getBundle]; + NSURL *fileURL = [bundle URLForResource:@"smallDownloadFile" withExtension:@""]; + NSURLSessionUploadTask *uploadTask = [session uploadTaskWithRequest:request fromFile:fileURL]; + [uploadTask resume]; + + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:uploadTask]); + FPRNetworkTrace *networkTrace = [FPRNetworkTrace networkTraceFromObject:uploadTask]; + + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionTaskDidSendBodyDataTotalBytesSentTotalBytesExpectedCalled); + XCTAssert(networkTrace.requestSize > 0); + XCTAssert( + [networkTrace + timeIntervalBetweenCheckpointState:FPRNetworkTraceCheckpointStateInitiated + andState:FPRNetworkTraceCheckpointStateRequestCompleted] > 0); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:uploadTask]); + }]; + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandler { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testRedirect"]; + NSURLRequest *request = [NSURLRequest requestWithURL:URL]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURLSessionTask *task = [session dataTaskWithRequest:request]; + [task resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:task]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue( + delegate.URLSessionTaskWillPerformHTTPRedirectionNewRequestCompletionHandlerCalled); + }]; + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionDataTaskDidReceiveData { + FPRNSURLSessionInstrument *instrument; + NSURLSessionDataTask *dataTask; + @autoreleasepool { + instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; + dataTask = [session dataTaskWithURL:URL]; + [dataTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveDataCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + }]; + } + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through. */ +- (void)testProxyDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { + FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; + [instrument registerInstrumentors]; + FPRNSURLSessionCompleteTestDelegate *delegate = + [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; + NSURLSessionConfiguration *configuration = + [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration + delegate:proxyDelegate + delegateQueue:nil]; + NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testDownload"]; + NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; + [downloadTask resume]; + XCTAssertNotNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, + GCDWebServerResponse *_Nonnull response) { + XCTAssertTrue(delegate.URLSessionDownloadTaskDidFinishDownloadingToURLCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:downloadTask]); + }]; + [instrument deregisterInstrumentors]; +} + /** Tests that the called delegate selector is wrapped and calls through. */ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { FPRNSURLSessionInstrument *instrument; @@ -565,6 +799,7 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertTrue(delegate.URLSessionTaskDidCompleteWithErrorCalled); XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; } @@ -572,15 +807,17 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { } /** Tests that the called delegate selector is wrapped and calls through. */ -- (void)testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { +- (void)testProxyDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; [instrument registerInstrumentors]; FPRNSURLSessionTestDownloadDelegate *delegate = [[FPRNSURLSessionTestDownloadDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:delegate + delegate:proxyDelegate delegateQueue:nil]; NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; @@ -592,15 +829,18 @@ - (void)testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { } /** Tests that the called delegate selector is wrapped and calls through. */ -- (void)testDelegateURLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite { +- (void) + testProxyDelegateURLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite { FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; [instrument registerInstrumentors]; FPRNSURLSessionCompleteTestDelegate *delegate = [[FPRNSURLSessionCompleteTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:delegate + delegate:proxyDelegate delegateQueue:nil]; NSURL *URL = [self.testServer.serverURL URLByAppendingPathComponent:@"testBigDownload"]; NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:URL]; @@ -616,15 +856,17 @@ - (void)testDelegateURLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalByte } /** Tests that even if a delegate doesn't implement a method, we add it to the delegate class. */ -- (void)testDelegateUnimplementedURLSessionTaskDidCompleteWithError { +- (void)testProxyDelegateUnimplementedURLSessionTaskDidCompleteWithError { FPRNSURLSessionTestDelegate *delegate = [[FPRNSURLSessionTestDelegate alloc] init]; + FPRNSURLSessionDelegateProxy *proxyDelegate = + [[FPRNSURLSessionDelegateProxy alloc] initWithDelegate:delegate]; FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; [instrument registerInstrumentors]; NSURLSessionConfiguration *configuration = [NSURLSessionConfiguration defaultSessionConfiguration]; XCTAssertFalse([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); NSURLSession *session = [NSURLSession sessionWithConfiguration:configuration - delegate:delegate + delegate:proxyDelegate delegateQueue:nil]; XCTAssertTrue([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]); NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:self.testServer.serverURL];