Repository navigation
Conversation
…eceiver firebase-messaging reports the dismissal of a notification it displayed with a broadcast that has the action messages arrive with (com.google.android.c2dm.intent.RECEIVE), its NOTIFICATION_DISMISS intent wrapped in the "wrapped_intent" extra. FlutterFirebaseMessagingReceiver read that broadcast as a message, so every dismissal reached onMessage, or onBackgroundMessage, as an empty RemoteMessage. The receiver now ignores a broadcast whose "wrapped_intent" extra is an Intent, the check Firebase's own CloudMessagingReceiver makes before it handles a broadcast as a message. Fixes firebase#18897 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request prevents dismissed notification events from being incorrectly processed as incoming data messages by the FlutterFirebaseMessagingReceiver. It introduces a helper method isNotificationEvent to detect these events and ignore them, accompanied by unit and integration tests. The feedback suggests adding a null check for the intent parameter in isNotificationEvent to ensure robust defensive programming and prevent potential NullPointerExceptions.
| static boolean isNotificationEvent(Intent intent) { | ||
| return intent.getParcelableExtra(EXTRA_WRAPPED_INTENT) instanceof Intent; | ||
| } |
There was a problem hiding this comment.
To ensure robust defensive programming, consider adding a null check for the intent parameter before calling getParcelableExtra. Although intent is typically non-null when called from onReceive, adding a guard clause prevents potential NullPointerExceptions if the method is called directly in other contexts (such as unit tests).
static boolean isNotificationEvent(Intent intent) {
if (intent == null) {
return false;
}
return intent.getParcelableExtra(EXTRA_WRAPPED_INTENT) instanceof Intent;
}References
- Enforce defensive programming by ensuring appropriate null checks exist before object property accesses for languages that support nullable references.
|
Superseded by #18899: the same change, opened again because this PR's commit listed a co-author who can't sign the CLA. |
Description
On Android, when the user dismisses a notification that the Firebase SDK displayed (swipes it away, or taps "Clear all"), the app receives an empty
RemoteMessage: no message ID, no notification, no data. It reachesonMessagewhile the app is in the foreground, andonBackgroundMessageotherwise. An app that draws its own notification for foreground messages shows a blank one. Steps to reproduce are in #18897.Cause. firebase-messaging (25.1.3, and 26.0.1 from BoM 35.1.0) gives a notification it displays with analytics enabled (
google.c.a.e) a delete intent: a broadcast with thecom.google.android.c2dm.intent.RECEIVEaction, sent to the app's package, with the realcom.google.firebase.messaging.NOTIFICATION_DISMISSintent in thewrapped_intentextra (CommonNotificationBuilder.createDeleteIntent). In our tests, messages sent through the HTTP v1 API hadgoogle.c.a.ewhether or not they setfcm_options.analytics_label. Since firebase/firebase-android-sdk#5410 (October 2023), that broadcast is addressed by action and package instead of toFirebaseInstanceIdReceiverby name, so it reaches every receiver for that action,FlutterFirebaseMessagingReceiverincluded.Firebase's own receiver,
CloudMessagingReceiver(the base ofFirebaseInstanceIdReceiver), checkswrapped_intentfirst and handles such a broadcast as a notification event (it logs thenotification_dismissanalytics event), never as a message.FlutterFirebaseMessagingReceiverhas no such check, so it buildsnew RemoteMessage(intent.getExtras())from the dismissal.Change.
FlutterFirebaseMessagingReceiver.onReceiveignores a broadcast whosewrapped_intentextra is anIntent, the same checkCloudMessagingReceivermakes. Firebase's own handling is unchanged:FirebaseInstanceIdReceiverstill receives the broadcast and logs the dismissal.Tests.
FlutterFirebaseMessagingReceiverTest: a dismissal is a notification event; a message, and awrapped_intentthat is not anIntent, are not.tests/remote_notificationschannel can now hand the receiver the dismissal broadcast before the messages (dismissalFirst), and a dismissed notification does not reach onMessage checks that only the message arrives. Before the change it failed:./gradlew :firebase_messaging:testDebugUnitTestfromtests/android(11 passed), andintegration_test/remote_notification_delivery_test.darton an Android 17 emulator (the 4 Android tests passed; the iOS ones are skipped there).broadcast received for a notification event, not a message. Ignored., nothing reachedonMessage, and the "no onBackgroundMessage handler has been registered" warnings stopped. Without the change, each state got one empty message per dismissed notification.Prior art. React Native Firebase has the same receiver and had the same bug (invertase/react-native-firebase#8040, closed as stale). invertase/react-native-firebase#9105 (July 2026) fixed it by ignoring broadcasts without
google.message_id, or with amessage_typeother thangcm. This PR uses the narrower check fromCloudMessagingReceiver, so it only drops what Firebase itself never treats as a message.When the app's process is not running, Android still starts it to deliver the dismissal broadcast. The plugin can't avoid that; only addressing the delete intent to Firebase's receiver by name again would.
Related Issues
RemoteMessage, without a reproduction).Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR. (dart analyze packages/firebase_messaging/firebase_messaging: no issues.)Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
🤖 Generated with Claude Code