Enable conditional opt out of automatically posting notifications to system tray

[sfuqua]

This change adds an extension point to FirebaseMessagingService such that an application can avoid the default behavior where a display message (an FCM message with a "notification" payload), received while the app is in the background, will automatically post a default-styled Android notification to the system tray without ever calling back into app code.

This is a stab at addressing longstanding community feedback about how onMessageReceived is not called for display messages when the app is backgrounded, which can be a point of confusion and frustration:

#46
#1807
Fixes #2639 I believe
firebase/quickstart-android#4
https://stackoverflow.com/q/37711082/244184
https://stackoverflow.com/q/37358462/244184

It is desirable as a developer to be able to opt out of this default behavior on a per-notification basis.
This allows an app to leverage the default, convenient behavior "for free" until there's time to customize the display with rich behavior using Android platform features. When that happens, the developer can use this new API to opt-out of the default handling and own their own display based on the message payload.

The proposed API change is a single new overrideable method on FirebaseMessagingService, called shouldUseDefaultNotificationBehavior - default implementation is just "return true".

If this method is overridden to return false for a given RemoteMessage, the current notification logic will be bypassed and the message will fall-through to onMessageReceived - the same way "data messages" are handled and the same way display messages while the app is foregrounded get handled.

I tested these changes on:

• an Android 7/SDK 24 emulator
• a physical Pixel 6 (Android 12/SDK 31)
• a Wear OS 2 (Android 9/SDK 28) emulator

Behavior is as expected - when my delegate returns false, the default notification is suppressed and onMessageReceived is called, as desired. When the delegate returns true, I get default SDK behavior.

API considerations

• Backwards compatible, maintain current behavior by default but allow "progressive enhancement" by the app
• Leverage existing public types (RemoteMessage) instead of exposing the full FCM Intent to the app
• Aiming to be as surgical as possible here to unblock developer scenarios while not overhauling the FCM client API surface

Given the history in this space and comments from Firebase folks on linked issues (especially #46) - I feel like I must be missing something on the why of the current SDK design, so looking forward to feedback here! Is there a scenario where this doesn't work and the OS doesn't invoke the SDK, or a reason Google/Firebase would not be okay allowing an app to bypass the current SDK logic for background notifications to configure its own display?

[google-oss-bot]

Hi @sfuqua. Thanks for your PR.

I'm waiting for a firebase member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sfuqua]

at the moment the change will bypass this MessagingAnalytics call, but I think that's probably okay?

if someone uses this new API to bypass default notification logic, the implication is that it should be treated identically to a standard data message (including for analytics)

[sfuqua]

@samtstern While the review process kicks off, I'm hoping you might be able to elaborate on some past comments you've made in linked issues, specifically:

#46 (comment)

Is there any reason to not provide a means to customize the notifications in the background?

This is a technical restriction that's outside the scope of the Android SDK. It's really an OS-level decision. Remember, the FCM message is received by the Google Play services process and your process is not always notified.

I am going to close this issue as there's nothing that can be done in this repo to fix it. However I will forward on your feedback to the FCM team in case they decided to revisit their decisions on background processing in the future.

In my testing (at least as far back as API level 24) and in inspecting code in this repo, the logic in question is owned by this SDK - I feel like I must be missing something as pertains to Play services and how the OS handles incoming messages and passes them off to userland via the com.google.android.c2dm.intent.RECEIVE action used internally by this SDK's BroadcastReceiver.

As far as I can tell this is a developer scenario that maybe used to work with GCM (when the app implemented a BroadcastReceiver for that intent themselves) and is now blocked by the current Firebase SDK design, but I can't tell if that's been intentional or just a gap - I can see some folks mentioning similar behavior with GCM:

googlesamples/google-services#166
https://stackoverflow.com/q/35480573/244184

However I am not convinced that this is actual Android OS behavior - I suspect this was an implementation detail of GcmListenerService, the same as it is an implementation detail of FirebaseMessagingService, and that an app has always been free to write their own BroadcastReceiver and handle the push notification Intent however they like - so I don't understand the intention of blocking the behavior in the SDK but am hoping someone can shine some light here.

quick edit - can confirm by adding a dependency on an old version of GCM to my test app and inspecting the decompiled GcmListenerService.class in Android Studio that it has all the same SDK logic we're talking about in Firebase - the SDK was responsible for inspecting the payload and invoking onMessageReceived (or not) and displaying a local notification (or not).

[sfuqua]

Hi @gsakakihara - I'm hoping you or someone else from either the Android or Firebase team can chime in on this PR.

It's a pretty scoped change that unblocks a lot of developer power, but I've seen a lot of confusing history in this space in other issues and it's not clear to me if there's actually Android platform-level issues involved in this scenario, or if it's purely a design choice of the GCM and FCM user-land libraries.

I understand an API change like this will require careful scrutiny from the team, but at a fundamental level, is there something objectionable about this sort of approach to the problem?

See my previous comment with research/questions in this space: #3492 (comment)

[sfuqua]

Hi @gsakakihara - this PR has been in limbo for several months. I don't expect to merge it as-is and I understand an API review is not trivial. However I'm very interested in a deeper understanding of the issues at play here and whether an approach like this would work for the Firebase and Android teams, or whether I'm fundamentally misunderstanding how push notifications work at an Android OS level.

There have been a lot of comments about how "display notifications" are handled automatically, but that seems to be untrue given that both the Firebase and GCM SDKs (which are app code, not special system code, and get invoked as standard BroadcastReceivers) both have logic in place to display prefab notifications that call standard, public Android Notification and NotificationChannel APIs.

If some part of the Android OS is automatically displaying notifications and it's not an SDK issue (in which case, why does the SDK have code to display notifications?), a PR like this one would obviously not be useful.

As a developer the current notification dichotomy on Android is very frustrating, and from the outside looking in it's not clear why it was designed the way that it was. If an "escape hatch" like the one introduced in this PR is possible it would make my life and the lives of many other Android developers much easier.

[suhaibkazi]

As per tests i conducted and internal logs, this workaround is the least intrusive and still allows for normal behaviour to take place as required.

[eddiecubed]

+1

Can we resolve the conversations in the PR to have this merged?

[gsakakihara]

The FCM team has previously looked into allowing developers more control over when and how notifications are displayed by the FCM SDK, but this doesn't fit into the current roadmap at this time. The best solution for now is probably to use a custom build of the SDK with a change like this that will allow you to override the SDK's automatic handling of notifications.

[sfuqua]

It might be futile at this point, but I'll offer that I'm frustrated that it took a year and a half to get any comment on this change, and that it has been completely impossible over many years to get the FCM team to offer a straight answer around the automatic behavior of notifications in the SDK.

The SDK's current behavior makes for one of the most perplexing "official" APIs I have ever used on any platform.

FWIW, it is very difficult to use a custom build of the FCM SDK with the rest of the Firebase suite. I spent quite a while trying to get an equivalent of npm's patch-package working where I could tamper with your AAR and overwrite just a couple lines of code, or the implementation of a single class, but it's not straightforward. As a result, I think the workaround of extending FirebaseMessageService with a hacky override of handleIntent is really the only maintainable thing that works today.

[RikScheffer]

would be very useful to have this