Skip to content

Commit 889ab61

Browse files
fix: Restore swizzling category and consolidate all recent changes
This commit addresses several critical fixes and consolidates all recent updates for the iOS App Delegate handling mechanism: 1. **Restored Swizzling Mechanism:** - Re-added the `UIApplication(FirebaseAppDelegateSwizzling)` category and its `+load` method to `app/src/util_ios.mm`. This was inadvertently lost during a previous operation and is essential for swizzling `[UIApplication setDelegate:]` with `Firebase_setDelegate`. 2. **Core Logic (already in working tree, confirmed):** - `Firebase_setDelegate` correctly tracks multiple unique delegate classes, includes a superclass check, and executes persistent pending blocks for genuinely new delegate classes. - `RunOnAppDelegateClasses` executes blocks for all currently known delegates and queues blocks for future new delegates. - `ClassMethodImplementationCache::ReplaceOrAddMethod` includes an idempotency check to prevent re-swizzling if a method already has the target IMP. 3. **Documentation (`Jules.md`):** - Learnings from this refactoring are integrated into relevant existing sections. - The document is formatted with 80-character line wrapping, and a note regarding this convention is included. This commit aims to bring the `refactor-forEachAppDelegateClass-ios` branch to its fully intended functional state, including all logic fixes, restorations, and documentation updates.
1 parent 170323c commit 889ab61

File tree

2 files changed

+99
-25
lines changed

2 files changed

+99
-25
lines changed

Jules.md

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Introduction
22

3+
> **Note on Document Formatting:** This document (`Jules.md`) should be
4+
> maintained with lines word-wrapped to a maximum of 80 characters to ensure
5+
> readability across various editors and terminals.
6+
37
This document provides context and guidance for AI agents (like Jules) when
48
making changes to the Firebase C++ SDK repository. It covers essential
59
information about the repository's structure, setup, testing procedures, API
@@ -34,8 +38,8 @@ instructions for your specific platform.
3438
* **Android SDK & NDK**: Required for building Android libraries. `sdkmanager`
3539
can be used for installation. CMake for Android (version 3.10.2
3640
recommended) is also needed.
37-
* **(Windows Only) Strings**: From Microsoft Sysinternals, required for Android
38-
builds on Windows.
41+
* **(Windows Only) Strings**: From Microsoft Sysinternals, required for
42+
Android builds on Windows.
3943
* **Cocoapods**: Required for building iOS or tvOS libraries.
4044

4145
## Building the SDK
@@ -74,9 +78,10 @@ generated in each library's build directory (e.g.,
7478

7579
### Desktop Platform Setup Details
7680

77-
When setting up for desktop, if you are using an iOS `GoogleService-Info.plist`
78-
file, convert it to the required `google-services-desktop.json` using the
79-
script: `python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist`
81+
When setting up for desktop, if you are using an iOS
82+
`GoogleService-Info.plist` file, convert it to the required
83+
`google-services-desktop.json` using the script:
84+
`python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist`
8085
(run this from the script's directory, ensuring the plist file is accessible).
8186

8287
The desktop SDK searches for configuration files in the current working
@@ -175,8 +180,9 @@ Database).
175180
parameters if not relying on a `google-services.json` or
176181
`GoogleService-Info.plist` file.
177182
2. **Service Instances**: Once `firebase::App` is initialized, you generally
178-
obtain instances of specific Firebase services using a static `GetInstance()`
179-
method on the service's class, passing the `firebase::App` object.
183+
obtain instances of specific Firebase services using a static
184+
`GetInstance()` method on the service's class, passing the `firebase::App`
185+
object.
180186
* Examples for services like Auth, Database, Storage, Firestore:
181187
* `firebase::auth::Auth* auth = firebase::auth::Auth::GetAuth(app, &init_result);`
182188
* `firebase::database::Database* database = firebase::database::Database::GetInstance(app, &init_result);`
@@ -192,8 +198,8 @@ Database).
192198
called as global functions within the `firebase::analytics` namespace,
193199
rather than on an instance object obtained via `GetInstance()`.
194200
Refer to the specific product's header file for its exact
195-
initialization mechanism if it deviates from the common `GetInstance(app, ...)`
196-
pattern.
201+
initialization mechanism if it deviates from the common
202+
`GetInstance(app, ...)` pattern.
197203

198204
### Asynchronous Operations: `firebase::Future<T>`
199205

@@ -205,8 +211,8 @@ where `T` is the type of the expected result.
205211
`kFutureStatusInvalid`.
206212
* **Getting Results**: Once `future.status() == kFutureStatusComplete`:
207213
* Check for errors: `future.error()`. A value of `0` (e.g.,
208-
`firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`)
209-
usually indicates success.
214+
`firebase::auth::kAuthErrorNone`,
215+
`firebase::database::kErrorNone`) usually indicates success.
210216
* Get the error message: `future.error_message()`.
211217
* Get the result: `future.result()`. This returns a pointer to the result
212218
object of type `T`. The result is only valid if `future.error()`
@@ -218,8 +224,8 @@ where `T` is the type of the expected result.
218224

219225
### Core Classes and Operations (Examples from Auth and Database)
220226

221-
While each Firebase product has its own specific classes, the following examples
222-
illustrate common API patterns:
227+
While each Firebase product has its own specific classes, the following
228+
examples illustrate common API patterns:
223229

224230
* **`firebase::auth::Auth`**: The main entry point for Firebase
225231
Authentication.
@@ -305,7 +311,12 @@ API documentation.
305311
as mentioned in `CONTRIBUTING.md`.
306312
* **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` to
307313
format your code before committing.
308-
* **Naming Precision for Dynamic Systems**: Function names should precisely reflect their behavior, especially in systems with dynamic or asynchronous interactions. For example, a function that processes a list of items should be named differently from one that operates on a single, specific item captured asynchronously. Regularly re-evaluate function names as requirements evolve to maintain clarity.
314+
* **Naming Precision for Dynamic Systems**: Function names should precisely
315+
reflect their behavior, especially in systems with dynamic or asynchronous
316+
interactions. For example, a function that processes a list of items should
317+
be named differently from one that operates on a single, specific item
318+
captured asynchronously. Regularly re-evaluate function names as
319+
requirements evolve to maintain clarity.
309320

310321
## Comments
311322

@@ -329,8 +340,9 @@ API documentation.
329340
* **Check `Future` status and errors**: Always check `future.status()` and
330341
`future.error()` before attempting to use `future.result()`.
331342
* A common success code is `0` (e.g.,
332-
`firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`).
333-
Other specific error codes are defined per module (e.g.,
343+
`firebase::auth::kAuthErrorNone`,
344+
`firebase::database::kErrorNone`). Other specific error codes are
345+
defined per module (e.g.,
334346
`firebase::auth::kAuthErrorUserNotFound`).
335347
* **Callback error parameters**: When using listeners or other callbacks,
336348
always check the provided error code and message before processing the
@@ -363,7 +375,13 @@ API documentation.
363375
otherwise ensuring the `Future` completes its course, particularly for
364376
operations with side effects or those that allocate significant backend
365377
resources.
366-
* **Lifecycle of Queued Callbacks/Blocks**: If blocks or callbacks are queued to be run upon an asynchronous event (e.g., an App Delegate class being set or a Future completing), clearly define and document their lifecycle. Determine if they are one-shot (cleared after first execution) or persistent (intended to run for multiple or future events). This impacts how associated data and the blocks themselves are stored and cleared, preventing memory leaks or unexpected multiple executions.
378+
* **Lifecycle of Queued Callbacks/Blocks**: If blocks or callbacks are queued
379+
to be run upon an asynchronous event (e.g., an App Delegate class being set
380+
or a Future completing), clearly define and document their lifecycle.
381+
Determine if they are one-shot (cleared after first execution) or
382+
persistent (intended to run for multiple or future events). This impacts
383+
how associated data and the blocks themselves are stored and cleared,
384+
preventing memory leaks or unexpected multiple executions.
367385

368386
## Immutability
369387

@@ -399,10 +417,29 @@ API documentation.
399417
integration, it can occasionally be a factor to consider when debugging app
400418
delegate behavior or integrating with other libraries that also perform
401419
swizzling.
402-
When implementing or interacting with swizzling, especially for App Delegate methods like `[UIApplication setDelegate:]`:
403-
* Be highly aware that `setDelegate:` can be called multiple times with different delegate class instances, including proxy classes from other libraries (e.g., GUL - Google Utilities). Swizzling logic must be robust against being invoked multiple times for the same effective method on the same class or on classes in a hierarchy. An idempotency check (i.e., if the method's current IMP is already the target swizzled IMP, do nothing more for that specific swizzle attempt) in any swizzling utility can prevent issues like recursion.
404-
* When tracking unique App Delegate classes (e.g., for applying hooks or callbacks via swizzling), consider the class hierarchy. If a superclass has already been processed, processing a subclass for the same inherited methods might be redundant or problematic. A strategy to check if a newly set delegate is a subclass of an already processed delegate can prevent such issues.
405-
* For code that runs very early in the application lifecycle on iOS/macOS (e.g., `+load` methods, static initializers involved in swizzling), prefer using `NSLog` directly over custom logging frameworks if there's any uncertainty about whether the custom framework is fully initialized, to avoid crashes during logging itself.
420+
When implementing or interacting with swizzling, especially for App Delegate
421+
methods like `[UIApplication setDelegate:]`:
422+
* Be highly aware that `setDelegate:` can be called multiple times
423+
with different delegate class instances, including proxy classes
424+
from other libraries (e.g., GUL - Google Utilities). Swizzling
425+
logic must be robust against being invoked multiple times for the
426+
same effective method on the same class or on classes in a
427+
hierarchy. An idempotency check (i.e., if the method's current IMP
428+
is already the target swizzled IMP, do nothing more for that
429+
specific swizzle attempt) in any swizzling utility can prevent
430+
issues like recursion.
431+
* When tracking unique App Delegate classes (e.g., for applying hooks
432+
or callbacks via swizzling), consider the class hierarchy. If a
433+
superclass has already been processed, processing a subclass for
434+
the same inherited methods might be redundant or problematic. A
435+
strategy to check if a newly set delegate is a subclass of an
436+
already processed delegate can prevent such issues.
437+
* For code that runs very early in the application lifecycle on
438+
iOS/macOS (e.g., `+load` methods, static initializers involved in
439+
swizzling), prefer using `NSLog` directly over custom logging
440+
frameworks if there's any uncertainty about whether the custom
441+
framework is fully initialized, to avoid crashes during logging
442+
itself.
406443

407444
## Class and File Structure
408445

@@ -468,9 +505,9 @@ management:
468505
module, but the fundamental responsibility for creation and deletion in
469506
typical scenarios lies with the Pimpl class itself.
470507

471-
It's crucial to correctly implement all these aspects (constructors, destructor,
472-
copy/move operators) when dealing with raw pointer Pimpls to prevent memory
473-
leaks, dangling pointers, or double deletions.
508+
It's crucial to correctly implement all these aspects (constructors,
509+
destructor, copy/move operators) when dealing with raw pointer Pimpls to
510+
prevent memory leaks, dangling pointers, or double deletions.
474511

475512
## Namespace Usage
476513

@@ -582,4 +619,8 @@ practices detailed in `Jules.md`.
582619
tests as described in the 'Testing' section of `Jules.md`.
583620
* **Commit Messages**: Follow standard commit message guidelines. A brief
584621
summary line, followed by a more detailed explanation if necessary.
585-
* **Tool Limitations & Path Specificity**: If codebase search tools (like `grep` or recursive `ls`) are limited or unavailable, and initial attempts to locate files/modules based on common directory structures are unsuccessful, explicitly ask for more specific paths rather than assuming a module doesn't exist or contains no relevant code.
622+
* **Tool Limitations & Path Specificity**: If codebase search tools (like
623+
`grep` or recursive `ls`) are limited or unavailable, and initial attempts
624+
to locate files/modules based on common directory structures are
625+
unsuccessful, explicitly ask for more specific paths rather than assuming a
626+
module doesn't exist or contains no relevant code.

app/src/util_ios.mm

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,39 @@ - (BOOL)application:(UIApplication *)application
158158
#endif // defined(__IPHONE_12_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_12_0
159159
@end
160160

161+
@implementation UIApplication (FirebaseAppDelegateSwizzling)
162+
163+
+ (void)load {
164+
static dispatch_once_t onceToken;
165+
dispatch_once(&onceToken, ^{
166+
Class uiApplicationClass = [UIApplication class];
167+
SEL originalSelector = @selector(setDelegate:);
168+
Method originalMethod = class_getInstanceMethod(uiApplicationClass, originalSelector);
169+
170+
if (!originalMethod) {
171+
NSLog(@"Firebase Error: Original [UIApplication setDelegate:] method not found for swizzling.");
172+
return;
173+
}
174+
175+
// Replace the original method's implementation with Firebase_setDelegate
176+
// and store the original IMP.
177+
IMP previousImp = method_setImplementation(originalMethod, (IMP)Firebase_setDelegate);
178+
if (previousImp) {
179+
g_original_setDelegate_imp = previousImp;
180+
NSLog(@"Firebase: Successfully swizzled [UIApplication setDelegate:] and stored original IMP.");
181+
} else {
182+
// This would be unusual - method_setImplementation replacing a NULL IMP,
183+
// or method_setImplementation itself failed (though it doesn't typically return NULL on failure,
184+
// it might return the new IMP or the old one depending on versions/runtime).
185+
// More robustly, g_original_setDelegate_imp should be checked before use.
186+
// For now, this logging indicates if previousImp was unexpectedly nil.
187+
NSLog(@"Firebase Error: Swizzled [UIApplication setDelegate:], but original IMP was NULL (or method_setImplementation failed to return the previous IMP).");
188+
}
189+
});
190+
}
191+
192+
@end
193+
161194
namespace firebase {
162195
namespace util {
163196

0 commit comments

Comments
 (0)