Refactor ForEachAppDelegateClass for iOS into a new function that swizzles [UIApplication setDelegate:] to obtain App Delegate classes. #1737
+149
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the `ForEachAppDelegateClass` function in `app/src/util_ios.mm`. Instead of scanning all runtime classes to find UIApplicationDelegate implementers, it now relies on method swizzling. The `[UIApplication setDelegate:]` method is swizzled at startup. When `setDelegate:` is called, the class of the actual application delegate is captured and stored globally (static to the .mm file). `ForEachAppDelegateClass` now uses this stored class. If it's called before `setDelegate:` has been invoked, the block passed to `ForEachAppDelegateClass` is queued. This queued block is then executed once the delegate is set via the swizzled `setDelegate:` method. This approach is more efficient and directly targets the actual App Delegate class used by the application. Key changes: - Added `Firebase_setDelegate` C function as the swizzled implementation. - Introduced `UIApplication(FirebaseAppDelegateSwizzling)` category with a `+load` method to perform the swizzling. - Uses `method_setImplementation` for swizzling and stores the original IMP. - Global static variables `g_app_delegate_class`, `g_original_setDelegate_imp`, and `g_pending_app_delegate_block` manage the state within `util_ios.mm`. - Modified `ForEachAppDelegateClass` to use the new mechanism and queue blocks if the delegate is not yet known.
jonsimantov
added
the
tests-requested: quick
github-actions
bot
added
tests: in-progress
⏳ Integration test in progress...Requested by @jonsimantov on commit f34a8d9 |
I replaced some logging calls with NSLog for better stability during early app startup. I also removed some unnecessary comments and unused include statements to keep things clean.
jonsimantov
added
the
tests-requested: quick
github-actions
bot
removed
the
tests-requested: quick
Modified `util_ios.mm` to support queueing multiple blocks (up to 8, defined by `MAX_PENDING_APP_DELEGATE_BLOCKS`) if `ForEachAppDelegateClass` is called before `[UIApplication setDelegate:]` is invoked. Changes include: - Replaced single pending block storage with a C array of block pointers and a counter (`g_pending_app_delegate_blocks` and `g_pending_block_count`). - `ForEachAppDelegateClass` now adds blocks to this array if the app delegate is not yet known. If the array is full, an error is logged and the block is discarded. - `Firebase_setDelegate` (the swizzled method) now iterates through all pending blocks in the array. If a valid delegate is being set, it executes each pending block. If the delegate is being set to nil, it clears all pending blocks. The array count is reset in both cases. - Added `#define MAX_PENDING_APP_DELEGATE_BLOCKS 8` for configurability.
github-actions
bot
added
the
tests: succeeded
firebase-workflow-trigger
bot
removed
the
tests: in-progress
- I removed extraneous developmental comments from app/src/util_ios.mm for better code clarity. - I updated a call site of firebase::util::RunOnAppDelegate (formerly ForEachAppDelegateClass) in messaging/src/ios/messaging.mm to use the new function name.
jonsimantov
added
the
tests-requested: quick
jonsimantov
commented
Jun 18, 2025
app/src/util_ios.mm
Outdated
| #define MAX_SEEN_DELEGATE_CLASSES 32 | ||
|
|
||
| static IMP g_original_setDelegate_imp = NULL; | ||
| // static Class g_app_delegate_class = nil; // Removed |
github-actions
bot
added
tests: in-progress
jonsimantov
commented
Jun 18, 2025
| @@ -286,7 +286,7 @@ @implementation UIApplication (FIRFBI) | |||
| + (void)load { | |||
| // C++ constructors may not be called yet so call NSLog rather than LogDebug. | |||
| NSLog(@"Loading UIApplication category for Firebase App"); | |||
| ::firebase::util::ForEachAppDelegateClass(^(Class clazz) { | |||
| ::firebase::util::RunOnAppDelegate(^(Class clazz) { // Renamed here | |||
There was a problem hiding this comment.
remove comment.
github-actions
bot
removed
the
tests: succeeded
jonsimantov
commented
Jun 18, 2025
app/src/util_ios.h
Outdated
| @@ -188,7 +188,7 @@ typedef BOOL ( | |||
| // Call the given block once for every Objective-C class that exists that | |||
| // implements the UIApplicationDelegate protocol (except for those in a | |||
| // blacklist we keep). | |||
| void ForEachAppDelegateClass(void (^block)(Class)); | |||
| void RunOnAppDelegate(void (^block)(Class)); | |||
There was a problem hiding this comment.
Maybe this should be renamed to RunOnAppDelegateClasses since it technically runs on the classes, not the delegate objects?
jonsimantov
commented
Jun 18, 2025
app/src/util_ios.h
Outdated
| @@ -188,7 +188,7 @@ typedef BOOL ( | |||
| // Call the given block once for every Objective-C class that exists that | |||
There was a problem hiding this comment.
This comment should be changed to reflect the new behavior - calling the block for every objective-C class that's passed into [UIApplication setDelegate:]
- I updated the documentation for RunOnAppDelegateClasses (formerly RunOnAppDelegate) in app/src/util_ios.h to accurately reflect its new behavior. - I renamed RunOnAppDelegate to RunOnAppDelegateClasses in all relevant locations (declaration, definition, internal logs, and call sites in invites and messaging modules) for clarity. - I removed the specified extraneous code comments from app/src/util_ios.mm and app/src/invites/ios/invites_ios_startup.mm.
jonsimantov
changed the title
I modified `ClassMethodImplementationCache::ReplaceOrAddMethod` in app/src/util_ios.mm to prevent re-swizzling a method if it's already swizzled with the target implementation. This is done by checking if the current method IMP is identical to the incoming IMP; if so, the function returns early. This resolves a recursive call issue observed when App Delegate hooks were applied multiple times to the same effective class via different GUL-proxied delegate instances. I also included a final cleanup of specified iterative code comments.
Modified `Firebase_setDelegate` in `app/src/util_ios.mm` to prevent
redundant processing for delegate classes that are subclasses of already
seen delegates.
- When `setDelegate:` is called with a `newClass`:
- It now first iterates through the superclasses of `newClass`. If any
superclass is found in the `g_seen_delegate_classes` list, `newClass`
is considered handled, and no further processing (adding to seen list
or running pending blocks for it) occurs.
- If no superclass is seen, it checks if `newClass` itself is already
seen. If so, it's skipped.
- If `newClass` is genuinely new (neither itself nor any superclass
already seen), it's added to `g_seen_delegate_classes`, and all
blocks from `g_pending_app_delegate_blocks` are executed for it.
- This addresses potential issues with third-party libraries (like GUL)
that might set their own delegate subclasses, ensuring our hooks
and blocks run appropriately.
- Includes cleanup of minor iterative comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit refactors the
ForEachAppDelegateClassfunction inapp/src/util_ios.mm. Instead of scanning all runtime classes to find UIApplicationDelegate implementers, it now relies on method swizzling.The
[UIApplication setDelegate:]method is swizzled at startup. WhensetDelegate:is called, the class of the actual application delegate is captured and stored globally (static to the .mm file).ForEachAppDelegateClassnow uses this stored class. If it's called beforesetDelegate:has been invoked, the block passed toForEachAppDelegateClassis queued. This queued block is then executed once the delegate is set via the swizzledsetDelegate:method.This approach is more efficient and directly targets the actual App Delegate class used by the application.
Key changes:
Firebase_setDelegateC function as the swizzled implementation.UIApplication(FirebaseAppDelegateSwizzling)category with a+loadmethod to perform the swizzling.method_setImplementationfor swizzling and stores the original IMP.g_app_delegate_class,g_original_setDelegate_imp, andg_pending_app_delegate_blockmanage the state withinutil_ios.mm.ForEachAppDelegateClassto use the new mechanism and queue blocks if the delegate is not yet known.Description
Testing
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.