Start data sync foreground service task when FCM notification is received

[link2xt]

Currently FCM notifications are only used to start the application if it is in background so it can download the messages:

deltachat-android/src/gplay/java/org/thoughtcrime/securesms/notifications/FcmReceiveService.java

Lines 95 to 101 in 56717d3

	@Override
	public void onMessageReceived(@NonNull RemoteMessage remoteMessage) {
	Log.i(TAG, "FCM push notification received");
	// the app is running (again) now and fetching and notifications should be processed as usual.
	// to support accounts that do not send PUSH notifications and for simplicity,
	// we just let the app run as long as possible.
	}

Notification is shown only once core downloads a new message from IMAP and decides that notification should be shown. It is done this way to prevent showing notifications for read receipts as the server does not know for encrypted messages if the message is read receipt or not.

Users however report that on some Android phones application fails to connect immediately after receiving FCM notification and only downloads the messages when user opens the application. Because of this FCM notifications effectively do nothing, application starts in the background but cannot download messages. This seems to happen at least on some stock Samsung phones. See https://support.delta.chat/t/push-notifications-not-working-with-chatmail-server/3245/5. It should also be possible to replicate on vanilla Android by going to App info -> Mobile data & Wi-Fi and switching off "Background data" or configuring non-existing proxy.

In this case it would be nice to be able to show a local notification saying something like "Failed to connect". This can be done by checking connectivity status when FCM notification is received and if it is DC_CONNECTIVITY_NOT_CONNECTED show a notification "Failed to connect". Normally this should not happen because if we can connect to FCM, we should at least be in the "connecting" state in Delta Chat unless OS blocks internet connectivity.

As this may be annoying to receive these notifications constantly on OS that don't allow fixing this behavior, it should be possible to disable the option and it should suggest disable this in the notification if it is enabled by default.

[link2xt]

I have noticed that Signal shows a notification saying "Checking for messages…" when FCM notification arrives and it downloads messages. Apparently starting foreground task and showing such notification helps getting access to the network at least on some phones.

We should do the same, start a foreground task when notification arrives and this task should display connectivity status and display failure if connectivity does not become "connected" after some timeout.

[iequidoo]

I have WhatsApp on my phone and it seems that it does the same. At least i see this "You may have new messages" when a bus with wi-fi passes by me. But i don't remember that it shows any failures. A user may have a poor connection quite a long time, so i'm not sure we should scare users with errors.

[r10s]

shouldn't we go for dc_accounts_background_fetch() as on iOS, instead of just hoping that starting the app is good enough? cc @link2xt

iirc, dc_accounts_background_fetch() somehow does maybe_network and whatever is needed to perform a fetch - and can be called unconditionally. on iOS, things seem to work fine with that so far.

and yes, before dc_accounts_background_fetch() is called, we can add a permanent notification saying "Checking for messages..." that is removed just afterwards (and probably not really visible in the vast majority of cases).

in case these two things are not sufficient, we can iterate

still wondering, if there is a degradation somewhere, iirc, maybe due to the changed API version, android does things differently and kills app faster

[link2xt]

dc_accounts_background_fetch() goes over all accounts, pauses I/O (essentially closing existing connections) and then starts separate connections to do the minimum amount of work to download messages. This will unnecessary kill existing connections because unlike iOS we do not stop I/O when we go to background.

Just calling maybe_network() (we don't do it currently and I think should) and showing a notification with connectivity status is a smaller change.

If we really need to know when background fetch finishes, then we need to change the core to reuse existing connections rather than just pausing I/O. On iOS it is only fine because most of the time I/O is stopped anyway.

[adbenitez]

we should probably also ask for disabling battery optimizations for the app even if push notifications are are available, I always felt not doing so could bring potential issues

[link2xt]

we should probably also ask for disabling battery optimizations for the app even if push notifications are are available, I always felt not doing so could bring potential issues

Signal seems to only do this if you have no Play Services it seems, so should not be necessary. But maybe this is why they have to show foreground task notification.

At least one recent report from Android 12 had ignoreBatteryOptimizations=false, but at the same time the log clearly showed that messages were downloaded in background after receiving FCM notification so I don't know what was going on there.

[link2xt]

I opened a related issue regarding Android >=13 where it is possible to deny notifications on the first start: #3305

[link2xt]

Foreground task should be of dataSync type, this is what Signal and Conversations use:
https://developer.android.com/develop/background-work/services/fg-service-types#data-sync

We seem to have a "remote messaging" service (https://developer.android.com/develop/background-work/services/fg-service-types#remote-messaging), this should probably also be changed to dataSync.

"Remote messaging" is basically what WhatsApp does when you connect web version to the phone: https://support.google.com/googleplay/android-developer/answer/13392821
It is about relaying messages via user device, not about messaging someone remotely. "Data sync" is for uploading and downloading messages, e.g. uploading to SMTP and downloading from IMAP.

[link2xt]

Seems data sync tasks can run for 6 hours:
https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/services/core/java/com/android/server/am/ActivityManagerConstants.java;l=1105;drc=bb81654f47334f1c5339596e0b8db7e79fc09b55

Other tasks like remote messaging get only 3 minutes.

[adbenitez]

Just calling maybe_network() (we don't do it currently and I think should) and showing a notification with connectivity status is a smaller change.

well showing a "Checking for messages…" permanent notifications should be easy the question is how/when to remove it, because there is not clear "sync/fetch messages" blocking call in core api to know when core finished fetching, unless we use dc_accounts_background_fetch() as @r10s suggested, I guess

BTW, it sounds really odd to me that the app would need to show a permanent notification to get network access, it still counts as being in background so if background activity is blocked that will not solve this problem, also in some part you said that "the log clearly showed that messages were downloaded in background after receiving FCM notification" so it is not clear to me what exactly a "Checking for messages…" permanent notification would solve, the issue is about sometimes the connection being unavailable at the time of trying to check so in that case the "you may have new messages" normal notification that just opens the app on tapping sounds better

[r10s]

thanks a lot for the heads up, @link2xt and @adbenitez

the question is how/when to remove it

indeed, this is also unclear to me if we do not use dc_accounts_background_fetch().

before dc_accounts_background_fetch() was introduced, iOS watched for DC_EVENT_CONNECTIVITY_CHANGED and checked isAllWorkDone(). it would feel odd to go that way again, however, as we introduced dc_accounts_background_fetch() to solve error-prone DC_EVENT_CONNECTIVITY_CHANGED/isAllWorkDone(). or is there some other way? cc @link2xt

BTW, it sounds really odd to me that the app would need to show a permanent notification to get network access

imu, the permanent notification is not to get network access, but to stay active - and this makes some sense, iirc, there is just no background activity allowed on newer androids without showing a permanent notification. that signal/whatsapp show one as well points into that direction.

anyway, worth a try. the notification as such is a simple change, we are using it already on various places (with dataSync-flag iirc). if it does not solve the issue, we can iterate :)

[link2xt]

BTW, it sounds really odd to me that the app would need to show a permanent notification to get network access, it still counts as being in background

But starting a foreground task makes the app considered to be in the foreground, right? Otherwise if the app has background access disabled it cannot download messages when receiving FCM notification at all.

[link2xt]

If it's easier, use dc_accounts_background_fetch(). It is just not optimized for the case when we already have existing connections as it does not reuse them but closes existing connections and opens a new one just to fetch messages now. We will then have to fix this in the core before the next release.

dc_accounts_all_work_done() is even already removed: deltachat/deltachat-core-rust#5384

[r10s]

But starting a foreground task makes the app considered to be in the foreground, right? Otherwise if the app has background access disabled it cannot download messages when receiving FCM notification at all.

i have to re-dive into that again, but iirc, one needs to add a "visible permanent notification" for work needed to be done in the background - this is eg. also the case when doing an export and leave the app's activities, eg. by going to android's home screen - the export should be continued in background then.

iirc, androids/googles idea is that every bit running is somehow visible to the user. but the wording foreground/background is confusing here, maybe also in our own code :)