Skip to content

Issue receiving onMessageReceived from secondary process using Cloud Messaging #4270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ow-ro opened this issue Nov 1, 2022 · 9 comments
Closed

Comments

@ow-ro
Copy link

ow-ro commented Nov 1, 2022

We are seeing an issue with cloud messaging using a sepearate process for our CustomFCMListenerService, FirebaseMessagingService and FirebaseInstanceIdReceiver. We are not seeing our messages forwarded on from FirebaseMessagingService to our onMessageReceived in our CustomFCMListenerService class. After checking, it seems this is because we are failing the DisplayNotification.isAppForeground check, which would direct the message to onMessageReceived. Seems to be because its checking the current process priority is IMPORTANCE_FOREGROUND, which fails.

Is this required or could any/the main process (from getRunningAppProcesses) be used for this check? In this use case, our main process exists with IMPORTANCE_FOREGROUND so the app should be considered foregrounded

This offloading of the FCM to a separate process has reduced our Broadcast of Intent { act=com.google.android.c2dm.intent.RECEIVE cmp=com.wemesh.android/com.google.firebase.iid.FirebaseInstanceIdReceiver } and similar ANRs significantly from thousands a day to dozens. So ideally we want to keep this change. Example issues in this repo related to these ANRs: here, here and here

image

Using FCM 23.1.0

Secondary process manifest configuration:

<service android:name=".cloudmessaging.CustomFCMListenerService"
            android:exported="false"
            android:process=":fcm">
            <intent-filter>
                <action android:name="com.google.firebase.MESSAGING_EVENT" />
            </intent-filter>
</service>
<service
            android:name="com.google.firebase.messaging.FirebaseMessagingService"
            android:exported="false"
            android:process=":fcm"
            tools:node="replace">

            <intent-filter android:priority="-500">
                <action android:name="com.google.firebase.MESSAGING_EVENT" />
            </intent-filter>
</service>
<receiver
            android:name="com.google.firebase.iid.FirebaseInstanceIdReceiver"
            android:exported="true"
            android:permission="com.google.android.c2dm.permission.SEND"
            android:process=":fcm"
            tools:node="replace">

            <intent-filter>
                <action android:name="com.google.android.c2dm.intent.RECEIVE" />
            </intent-filter>
</receiver>
@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@argzdev
Copy link
Contributor

argzdev commented Nov 3, 2022

Thanks for putting the time and effort in reporting this, @ow-ro. I'm glad you're able to lessen your ANR issues with this setup. But before I raise this up to our engineering team, I'm sorry, but I don't seem to quite understand this completely. Could you explain this again to me?

Is this required

Maybe, but I'll ask our engineers if this is something required.

or could any/the main process (from getRunningAppProcesses) be used for this check?

Not sure what you mean by this, can you explain this further?

In this use case, our main process exists with IMPORTANCE_FOREGROUND so the app should be considered foregrounded

If I understand correctly, if your main process has "IMPORTANCE_FOREGROUND", shouldn't it pass the return process.importance == RunningAppProcessInfo.IMPORTANCE_FOREGROUND;, or is somehow being bypassed?

@ow-ro
Copy link
Author

ow-ro commented Nov 3, 2022

Hi @argzdev

Sure:

or could any/the main process (from getRunningAppProcesses) be used for this check?

getRunningAppProcesses, in our case, returns 2 processes when the app is foregrounded, the main app process and the secondary process (I'll refer to that as :fcm) used to handle cloud messaging notifications.

If I understand correctly, if your main process has "IMPORTANCE_FOREGROUND", shouldn't it pass the return process.importance == RunningAppProcessInfo.IMPORTANCE_FOREGROUND;, or is somehow being bypassed?

Because of the if (process.id == pid) check in the loop, where pid is the current process id (so in our case, :fcm), it doesn't do the IMPORTANCE_FOREGROUND check for our main process, only our :fcm process.

image

Let me know if you need anything else

@gsakakihara
Copy link
Contributor

Running FCM from a secondary process is not officially supported, so we may not be able to make a lot of changes for that use case, but just to check, you are sending FCM notifications that you want delivered as a data message to your onMessageReceived() callback when your app is in the foreground? Instead it's always posting it as an Android notification? I think the issue is that getRunningAppProcesses() can return all running processes, so if it didn't check for the current process, it could pick up any other app running in the foreground (at least it did in older versions of Android, more recent ones may be limited to just the app's processes now).

@ow-ro
Copy link
Author

ow-ro commented Nov 8, 2022

Hi @gsakakihara

you are sending FCM notifications that you want delivered as a data message to your onMessageReceived() callback when your app is in the foreground? Instead it's always posting it as an Android notification?

Yes, that's correct

it could pick up any other app running in the foreground (at least it did in older versions of Android, more recent ones may be limited to just the app's processes now)

Context is passed in to the DisplayNotification, so could the package name be checked against the process name to account for our case, without other app's processes being checked?

Say our package name is com.test.owro, with our 2 process config with app foregrounded, getRunningAppProcesses would show the main process with processName com.test.owro and our fcm process with processName com.test.owro:fcm

if (process.pid == pid) {
     return process.importance == RunningAppProcessInfo.IMPORTANCE_FOREGROUND;
}
if (process.pid == pid || process.processName.equals(context.getPackageName()))) {
     return process.importance == RunningAppProcessInfo.IMPORTANCE_FOREGROUND;
}

@gsakakihara
Copy link
Contributor

https://developer.android.com/guide/topics/manifest/activity-element?hl=en#proc

One potential issue with your change is that the process name can be specified by the app, so it's not necessarily the app's package name. This probably isn't an issue for false negatives (where the app has specified a different process name) since it is better than the current behavior and if an app has that issue, they can always use the default process name to fix it. It may be a problem for false positives (where another app has set their process name to your app's package name) since the other app's process could cause it to incorrectly identify your app as being in the foreground. I'm not sure if possible for an app to do that or, if so, whether it's something we would block the change on, but we would need to investigate it before we make any changes.

@argzdev
Copy link
Contributor

argzdev commented Jan 16, 2023

Just bumping up this thread. Could you confirm if this answers your questions, @ow-ro?

@google-oss-bot
Copy link
Contributor

Hey @ow-ro. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@google-oss-bot
Copy link
Contributor

Since there haven't been any recent updates here, I am going to close this issue.

@ow-ro if you're still experiencing this problem and want to continue the discussion just leave a comment here and we are happy to re-open this.

@firebase firebase locked and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants