From 51747b6c1ba49c9888694c12a85a4f10c84e0b0e Mon Sep 17 00:00:00 2001 From: Jesus Rojas Date: Thu, 19 Feb 2026 19:02:31 -0600 Subject: [PATCH 1/2] Fix NSURLSession delegate instrumentation for NSProxy delegates #14478 --- .../Instrumentation/FPRObjectInstrumentor.h | 6 + .../Instrumentation/FPRProxyObjectHelper.h | 11 + .../Instrumentation/FPRProxyObjectHelper.m | 11 + .../FPRNSURLConnectionDelegateInstrument.m | 10 + .../FPRNSURLSessionDelegateInstrument.m | 10 + .../Network/FPRNSURLSessionInstrument.m | 9 +- .../FPRNSURLSessionInstrumentTest.m | 333 ++++++++++++++++++ .../FPRNSURLSessionInstrumentTestDelegates.h | 9 + .../FPRNSURLSessionInstrumentTestDelegates.m | 22 +- 9 files changed, 410 insertions(+), 11 deletions(-) diff --git a/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h b/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h index b016c070ef8..47c6789593b 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..a6c8347089f 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..0f4666b2e3d 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/FPRNSURLConnectionDelegateInstrument.m b/FirebasePerformance/Sources/Instrumentation/Network/Delegates/FPRNSURLConnectionDelegateInstrument.m index a01fb2180a6..e9852c085b1 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(NSURLConnectionDelegate) + varFoundHandler:^(id ivar) { + [self registerClass:[ivar class]]; + [self registerObject: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 57394071199..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) { - [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 ffffbe17c17..ac358d31310 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -62,6 +62,39 @@ - (void)forwardInvocation:(NSInvocation *)invocation { @end +@interface FPRNSURLSessionDelegateProxy : NSProxy { + // The wrapped delegate object. + id _delegate; +} + +/** @return an instance of the delegate 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]; +} + +- (BOOL)respondsToSelector:(SEL)aSelector { + return [_delegate respondsToSelector:aSelector]; +} + +- (void)forwardInvocation:(NSInvocation *)invocation { + [invocation invokeWithTarget:_delegate]; +} + +@end + @interface FPRNSURLSessionInstrumentTest : FPRTestCase /** Test server to create connections to. */ @@ -483,6 +516,36 @@ - (void)testDelegateURLSessionDownloadTaskDidFinishDownloadingToURL { } /** Tests that the called delegate selector is wrapped and calls through. */ +- (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. + * The delegate cancels the download from didWriteData and resumes with resume data, which + * triggers didResumeAtOffset:expectedTotalBytes: on the same delegate. + */ - (void)testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytes { FPRNSURLSessionInstrument *instrument = [[FPRNSURLSessionInstrument alloc] init]; [instrument registerInstrumentors]; @@ -548,6 +611,276 @@ - (void)testDelegateUnimplementedURLSessionTaskDidCompleteWithError { [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]; + 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; + 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.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); + XCTAssertTrue(delegate.URLSessionTaskDidCompleteWithErrorCalled); + XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); + }]; + } + [instrument deregisterInstrumentors]; +} + +/** Tests that the called delegate selector is wrapped and calls through when delegate is a proxy. + * The delegate cancels the download from didWriteData and resumes with resume data, which + * triggers didResumeAtOffset:expectedTotalBytes: on the same delegate (via the proxy). + */ +- (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:proxyDelegate + 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) + 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:proxyDelegate + 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)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:proxyDelegate + 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 instance method wrapping /** Tests that dataTaskWithRequest: returns a non-nil object. */ diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.h index 722f508fa84..9bf53a3d12e 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; @@ -87,6 +93,9 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic) BOOL URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled; +/** Set to YES after the first cancel-and-resume has been triggered, preventing re-entry. */ +@property(nonatomic) BOOL hasCancelledOnce; + @end NS_ASSUME_NONNULL_END diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTestDelegates.m index 28525bd4967..02a2fd71ef2 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 @@ -124,14 +132,12 @@ - (void)URLSession:(NSURLSession *)session didWriteData:(int64_t)bytesWritten totalBytesWritten:(int64_t)totalBytesWritten totalBytesExpectedToWrite:(int64_t)totalBytesExpectedToWrite { - if (bytesWritten > 0) { - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - [downloadTask cancelByProducingResumeData:^(NSData *_Nullable resumeData) { - // Create a resumable download task with the cancelled data, and start it. - [[session downloadTaskWithResumeData:resumeData] resume]; - }]; - }); + if (bytesWritten > 0 && !self.hasCancelledOnce) { + self.hasCancelledOnce = YES; + [downloadTask cancelByProducingResumeData:^(NSData *_Nullable resumeData) { + // Create a resumable download task with the cancelled data, and start it. + [[session downloadTaskWithResumeData:resumeData] resume]; + }]; } self.URLSessionDownloadTaskDidWriteDataTotalBytesWrittenTotalBytesCalled = YES; } From 51cc90742aff40273149a2ae45472e0902e73936 Mon Sep 17 00:00:00 2001 From: Jesus Rojas Date: Tue, 24 Feb 2026 16:59:50 -0600 Subject: [PATCH 2/2] Adress comments and Update documentation --- FirebasePerformance/CHANGELOG.md | 3 +++ .../Sources/Instrumentation/FPRObjectInstrumentor.h | 5 +++-- .../Sources/Instrumentation/FPRProxyObjectHelper.h | 10 +++++----- .../Unit/Instruments/FPRNSURLSessionInstrumentTest.m | 1 - 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/FirebasePerformance/CHANGELOG.md b/FirebasePerformance/CHANGELOG.md index 42a452dbf35..557ac441e15 100644 --- a/FirebasePerformance/CHANGELOG.md +++ b/FirebasePerformance/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Fixed NSURLSession delegate instrumentation for NSProxy delegates. (#14478) + # 12.10.0 - [fixed] Fix a race condition by replacing `mstats()` with `malloc_zone_statistics()`. (#15501) - [fixed] Fixed a deadlock in Firebase Sessions where the main thread could diff --git a/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h b/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h index 47c6789593b..52096f8af16 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h +++ b/FirebasePerformance/Sources/Instrumentation/FPRObjectInstrumentor.h @@ -33,9 +33,10 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)registerObject:(id)object; -/** Registers an instance of the delegate class to be instrumented. +/** Registers an NSProxy instance that wraps a delegate object to be instrumented. * - * @param proxy The instance to instrument. + * @param proxy The NSProxy instance to instrument. This proxy should wrap another delegate object + * that will receive the forwarded method calls. */ - (void)registerProxy:(id)proxy; diff --git a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h index a6c8347089f..db37291801d 100644 --- a/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h +++ b/FirebasePerformance/Sources/Instrumentation/FPRProxyObjectHelper.h @@ -31,12 +31,12 @@ 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. +/** Registers a proxy object and runs the handler block when an ivar conforming to the given + * protocol is discovered. * - * @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. + * @param proxy The proxy object whose ivars will be iterated. + * @param protocol The protocol all ivars will be compared against. + * @param varFoundHandler The block to run when an ivar conformsToProtocol:protocol. */ + (void)registerProxyObject:(id)proxy forProtocol:(Protocol *)protocol diff --git a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m index ac358d31310..aec07193ca7 100644 --- a/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m +++ b/FirebasePerformance/Tests/Unit/Instruments/FPRNSURLSessionInstrumentTest.m @@ -798,7 +798,6 @@ - (void)testProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandler { [self waitAndRunBlockAfterResponse:^(id self, GCDWebServerRequest *_Nonnull request, GCDWebServerResponse *_Nonnull response) { XCTAssertTrue(delegate.URLSessionDataTaskDidReceiveResponseCompletionHandlerCalled); - XCTAssertTrue(delegate.URLSessionTaskDidCompleteWithErrorCalled); XCTAssertNil([FPRNetworkTrace networkTraceFromObject:dataTask]); }]; }