Skip to content

Deliver callbacks to App Delegates that use fast forwarding - #247

Open
tetek wants to merge 6 commits into
google:mainfrom
tetek:fix/forwarding-target-app-delegate
Open

tetek wants to merge 6 commits into
google:mainfrom
tetek:fix/forwarding-target-app-delegate

Conversation

@tetek

@tetek tetek commented Sep 28, 2026 •

Copy link
Copy Markdown

Hey,

I've been tracking down an unwanted behaviour in a SwiftUI app using Firebase.
I set my AppDelegate with UIApplicationDelegateAdaptor.

After adding Firebase I stopped receiving:

    func application(
        _ application: UIApplication,
        didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data
    )

after calling UIApplication.shared.registerForRemoteNotifications().

It turns out UIApplicationDelegateAdaptor installs SwiftUI's own SwiftUI.AppDelegate as the
application delegate, which implements only a handful of selectors itself and forwards the rest to
the app's delegate via forwardingTargetForSelector:.

With the App Delegate Proxy active (the default), createSubclassWithObject: records the original
implementation using class_getInstanceMethod(realClass, sel), which is NULL for a forwarded
selector. Adding the donor method then makes message lookup succeed, so the runtime no longer
consults the forwarding target, and each donor's trailing if (realIMP) is false — the callback
reaches the interceptors but is never delivered to the app's delegate, with no diagnostic.

This isn't limited to APNS: application:continueUserActivity:restorationHandler: and
application:openURL:options: are lost the same way for any app using the adaptor together with a
GoogleUtilities-based SDK.

This PR makes each donor fall back to the forwarding target when the original implementation is
NULL, leaving behaviour unchanged for delegates that implement the selector themselves.
Seven tests cover this: forwarded APNS callbacks, continueUserActivity, interceptors still being
notified, return values, completion handlers, merging the fetch result with interceptors, and
a forwarding target that doesn't implement the selector.
Related: firebase/firebase-ios-sdk#13566, firebase/firebase-ios-sdk#14751,
firebase/firebase-ios-sdk#10417

Proxying a delegate that handles a selector via forwardingTargetForSelector:
instead of implementing it caused the callback to be dropped silently.

createSubclassWithObject: records the original implementation with
class_getInstanceMethod(realClass, sel), which is NULL for such a delegate.
Adding the donor method then makes message lookup succeed, so the runtime no
longer consults the forwarding target, and each donor's trailing
`if (realIMP)` is false. The message is neither delivered nor diagnosed.

This is the shape of the delegate SwiftUI installs for
UIApplicationDelegateAdaptor: SwiftUI.AppDelegate implements only a handful of
selectors -- didFinishLaunchingWithOptions:, configurationForConnecting:options:,
handleEventsForBackgroundURLSession:, respondsToSelector: and
forwardingTargetForSelector: -- and forwards the rest to the app's own delegate.
An app using that adaptor together with FirebaseMessaging or FirebaseAuth
therefore stops receiving the APNS callbacks on its own delegate, and any app
using a GoogleUtilities-based SDK stops receiving continueUserActivity: and
openURL:options:.

Each donor now falls back to the forwarding target when the original
implementation is NULL, so the behaviour is unchanged for delegates that
implement the selector themselves.
@ncooke3
ncooke3 requested a review from daymxn September 28, 2026 14:11

@daymxn daymxn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! There's a few things that need addressing before we move forward with it, but the core idea looks solid to me.

This PR should also include a changelog entry (see this file). Just add an # Unreleased section and add an entry there (eg; like this).

Comment thread GoogleUtilities/AppDelegateSwizzler/GULAppDelegateSwizzler.m Outdated
Comment thread GoogleUtilities/AppDelegateSwizzler/GULAppDelegateSwizzler.m Outdated
Comment thread GoogleUtilities/AppDelegateSwizzler/GULAppDelegateSwizzler.m Outdated
Comment thread GoogleUtilities/Tests/Unit/Swizzler/GULAppDelegateSwizzlerTest.m
@tetek

tetek commented Sep 29, 2026

Copy link
Copy Markdown
Author

Hey @daymxn, thanks for the review. All suggestions applied.

@tetek
tetek requested a review from daymxn September 29, 2026 09:53
Forward through NSInvocation rather than by messaging the target directly, so
that the APNS App Delegate selectors are not referenced as symbols. Referencing
them directly triggers an Apple review warning about a missing Push Notification
Entitlement even when the code never runs, which is why the surrounding code
builds those selectors from strings.

Adds tests for the value returned by a forwarded selector, for the completion
handlers of application:handleEventsForBackgroundURLSession:completionHandler:
and application:didReceiveRemoteNotification:fetchCompletionHandler:, and for a
forwarding delegate whose target does not implement the selector at all.

Adds the changelog entry.
@tetek
tetek force-pushed the fix/forwarding-target-app-delegate branch from 5a805af to 63f9c4f Compare September 30, 2026 07:22
@paulb777

paulb777 commented Oct 5, 2026

Copy link
Copy Markdown
Member

@tetek Based on the CI run, it looks the change doesn't build on macOS. Please address.

Fixes the macOS build: openURL:options:, handleEventsForBackgroundURLSession:
and the fetchCompletionHandler: variant aren't part of NSApplicationDelegate.
@tetek
tetek force-pushed the fix/forwarding-target-app-delegate branch from 3de1621 to bb9ec99 Compare October 5, 2026 14:27
@paulb777

paulb777 commented Oct 6, 2026

Copy link
Copy Markdown
Member

Thanks @tetek - I asked an agent to take a look and it came back with the following:

Thanks @tetek, and thanks for working through daymxn's feedback. I verified this against the real SwiftUI.AppDelegate with a minimal @UIApplicationDelegateAdaptor app on iOS 18.4, 26.5 and 27.0 simulators.

  • On main: after proxyOriginalDelegateIncludingAPNSMethods, the adaptor receives only handleEventsForBackgroundURLSession, which SwiftUI implements itself. The APNS callbacks, openURL:options: and continueUserActivity are dropped, both return NO, and the fetch completion reports .failed.
  • With this PR: all six reach the adaptor, both return YES, and the fetch result is .newData.

A few small things before merge:

  1. Dispatch group. donor_didReceiveRemoteNotification:fetchCompletionHandler: resolves the forwarding target twice: once to decide on dispatch_group_enter, and again inside forwardSelector:. If the second lookup returns nil, the group is never left, and the system completion handler never runs. Could you enter the group, call forwardSelector: once, and dispatch_group_leave if it returns nil?
  2. Test crash on regression. In testForwardingAppDelegateForwardsCompletionHandlers, if forwarding regresses, calling the stored completion handlers calls a nil block and crashes the test run. Please guard those calls, and consider asserting that the fetch result is UIBackgroundFetchResultNewData.
  3. Optional test. Add one with an interceptor registered plus a forwarding delegate for didReceiveRemoteNotification:fetchCompletionHandler:, checking that the original completion runs once with the merged result.
  4. PR description. Please update it (there are six tests now) and reference Swizzle breaks Application Delegates in Swift firebase/firebase-ios-sdk#13566, didRegister/didFailToRegisterForRemoteNotifications callbacks not called when FirebaseAuth is linked via SPM firebase/firebase-ios-sdk#14751 and SwiftUI and Objective-C inter-op with Swizzling firebase/firebase-ios-sdk#10417.
  5. Nit. Wrap the CHANGELOG entry at 80 columns like the other entries.

- Enter the callback group and call forwardSelector: once, leaving the group
  if nothing was forwarded, so the system completion handler always runs.
- Guard stored completion handlers against nil in tests and assert the
  forwarded fetch result.
- Add a test merging an interceptor's fetch result with a forwarding delegate.
- Wrap the CHANGELOG entry at 80 columns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants