Fix NSURLSession delegate instrumentation for NSProxy delegates #14478 - #15869
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
Ready for review!!! |
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where NSURLSession delegate callbacks were not being delivered when the delegate was an NSProxy. The changes introduce a new registerProxy method in FPRObjectInstrumentor and FPRProxyObjectHelper to correctly identify and instrument the real delegate behind the proxy. This ensures that optional delegate methods are properly queried and invoked. The test suite has been updated with new proxy-specific tests to verify the fix.
|
Hey there @tejasd Thank you so much for the review! I have addressed your comments and made changes! |
|
Comments addressed @tejasd |
2 participants
Discussion
Fixes #14478. With Performance instrumentation enabled, NSURLSession delegate callbacks were not delivered when the delegate was an NSProxy (e.g. forwarding to a real delegate). The SDK was instrumenting the proxy instead of the underlying delegate, so optional methods like
URLSession:dataTask:didReceiveResponse:completionHandler:were never queried or invoked. This change detects proxy delegates via[delegate isProxy], usesFPRProxyObjectHelperto find the real delegate from the proxy’s ivars, and instruments that object instead.Testing
Existing Firebase Performance tests pass. Added/updated tests:
FPRNSURLSessionDelegateProxytest helper and tests that use it to verify all delegate methods (includingdidReceiveResponse:completionHandler:anddidResumeAtOffset:expectedTotalBytes:) are forwarded and instrumented when the session delegate is an NSProxy.testDelegateURLSessionDownloadDidReceiveResponseCompletionHandlerandtestProxyDelegateURLSessionDownloadDidReceiveResponseCompletionHandlerfor the optional data-task response delegate.testDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytesandtestProxyDelegateURLSessionDownloadTaskDidResumeAtOffsetExpectedTotalBytesuse the delegate’s cancel-and-resume flow;FPRNSURLSessionTestDownloadDelegatenow uses a per-instancehasCancelledOnceflag instead of a staticdispatch_once_tso both tests run correctly.API Changes