wordpress-mobile / wordpress-mobile/WordPress-iOS
`handleNotification` can drop the remote-notification completion handler, throttling background push delivery
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 3.9k
- Forks
- 1.2k
- Avg merge
- 23h 51m
- Merged PRs (30d)
- 58
Description
Summary
PushNotificationsManager.handleNotification(_:userInteraction:completionHandler:) can return without ever invoking the OS-supplied fetchCompletionHandler. For a content-available (silent/background) push, the app then never signals iOS that it finished its background work — wasting the background assertion and, over repeated occurrences, leading iOS to throttle the app's future background/silent-push wakeups and background-fetch budget.
Pre-existing — not introduced by any recent change. Surfaced while reviewing #25643 (UIScene adoption), which makes windowless background launches a routinely-exercised path but does not touch this dispatch loop.
Root Cause
The OS entry point hands handleNotification a non-optional escaping handler (application(_:didReceiveRemoteNotification:fetchCompletionHandler:)). But handleNotification only calls it inside the four sub-handlers' success paths, with no terminal call when nothing handles the push — plus an early return on the badge path:
// Badge: Reset
guard let type = userInfo.string(forKey: Notification.typeKey), type != Notification.badgeResetValue else {
return // ← completionHandler never called
}
let handlers = [
handleSupportNotification,
handleAuthenticationNotification,
handleInactiveNotification,
handleBackgroundNotification
]
for handler in handlers {
if handler(userInfo, userInteraction, completionHandler) {
break
}
}
// ← if every handler returned false, completionHandler is never called
Trigger
A remote push delivered in the .background state with userInteraction == false that:
- has a
type(so it survives the badge-resetguard), - is not a Zendesk/support type →
handleSupportNotificationreturnsfalse, - is not an auth notification →
handleAuthenticationNotificationreturnsfalse, - →
handleInactiveNotificationreturnsfalse(.background && !userInteraction), - carries no
note_id→handleBackgroundNotificationreturnsfalse(it requiresNotification.identifierKey).
All four decline; completionHandler is never called. The badge-reset early-return at L208 is the same drop, though a badge-reset push isn't usually a content-available push the OS waits on.
In practice, real WP.com notification pushes (comment / like / follow / …) all carry note_id, so they land in handleBackgroundNotification and do call the handler. The drop therefore needs a malformed or future/unknown payload shape — latent and low-frequency, but unbounded by anything in the client.
Impact
- A wasted ~30s background assertion per occurrence (the OS waits out the watchdog, then suspends the app).
- iOS meters completion-handler reliability; repeated drops contribute to throttling of future silent-push delivery and background-fetch budget — i.e. degraded background responsiveness (share-extension upload completion, silent syncs).
No crash, no data loss — a robustness gap.
Fix
Guarantee the handler is called exactly once on every path:
guard let type = userInfo.string(forKey: Notification.typeKey), type != Notification.badgeResetValue else {
completionHandler?(.noData)
return
}
for handler in handlers {
if handler(userInfo, userInteraction, completionHandler) {
return // a handler took it (and already called completionHandler)
}
}
completionHandler?(.noData) // nothing handled it — still fulfil the OS contract
The return-on-handled keeps it single-call (no double invocation when a handler already called it).
Notes
- Confirmed pre-existing: the dispatch loop and badge-reset
returnare identical ontrunkand predate #25643. #25643's only change to this file (wideninghandleInactiveNotificationto accept.background && userInteraction) shrinks the set of dropped pushes — it neither introduces nor worsens this.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in WordPress/Classes/Utility/Notifications/PushNotificationsManager.swift around the badge-reset guard and handler dispatch loop. Trace each handler's completion-handler behavior, then ensure the OS-supplied handler is invoked exactly once for early returns and when no handler accepts the notification; done means every path fulfills the completion contract without double-calling it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ios, swift
- Domain
- mobile
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100