diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/FetchPushForegroundService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/FetchPushForegroundService.kt index d7cc950b99..6dc6bdfa0e 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/FetchPushForegroundService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/FetchPushForegroundService.kt @@ -17,6 +17,7 @@ import android.os.PowerManager import androidx.core.app.NotificationCompat import dev.zacsweers.metro.Inject import io.element.android.libraries.architecture.bindings +import io.element.android.libraries.core.extensions.runCatchingExceptions import io.element.android.libraries.designsystem.utils.CommonDrawables import io.element.android.libraries.di.annotations.AppCoroutineScope import io.element.android.libraries.push.api.push.PushHandlingWakeLock @@ -57,6 +58,8 @@ class FetchPushForegroundService : Service() { } } + private var isOnForeground = false + override fun onCreate() { Timber.d("Creating FetchPushForegroundService") @@ -71,12 +74,32 @@ class FetchPushForegroundService : Service() { .setVibrate(longArrayOf(0)) .setSound(null) .build() - startForeground(NOTIFICATION_ID, notificationCompat) + + // Try to start the service in foreground. This can fail, even in cases where it's supposed to work according to the docs. + // In those cases we catch the exception and handle the failure so we don't try to start the wakelock or stop the service + // from running in foreground later. + runCatchingExceptions { + startForeground(NOTIFICATION_ID, notificationCompat) + } + .onSuccess { + isOnForeground = true + Timber.d("FetchPushForegroundService started in foreground successfully") + } + .onFailure { + isOnForeground = false + Timber.e(it, "Failed to start FetchPushForegroundService in foreground") + } super.onCreate() } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + if (!isOnForeground) { + Timber.w("FetchPushForegroundService is not running in foreground, stopping it to avoid crash") + stopSelf() + return START_NOT_STICKY + } + wakelock.acquire(wakelockTimeout) // The timeout is not automatic before Android 15, so we need to schedule it ourselves @@ -91,18 +114,21 @@ class FetchPushForegroundService : Service() { } override fun stopService(intent: Intent?): Boolean { - wakelock.release() + if (isOnForeground) { + wakelock.release() + stopForeground(STOP_FOREGROUND_REMOVE) + } - stopForeground(STOP_FOREGROUND_REMOVE) return super.stopService(intent) } override fun onTimeout(startId: Int) { super.onTimeout(startId) - Timber.d("Wakelock timeout reached, stopping FetchPushForegroundService") - - coroutineScope.launch { pushHandlingWakeLock.unlock() } + if (isOnForeground) { + Timber.d("Wakelock timeout reached, stopping FetchPushForegroundService") + coroutineScope.launch { pushHandlingWakeLock.unlock() } + } } companion object { diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingService.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingService.kt index 67da09f1ab..3961f1f591 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingService.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingService.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.pushproviders.firebase import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage +import com.google.firebase.messaging.RemoteMessage.PRIORITY_HIGH import dev.zacsweers.metro.Inject import io.element.android.libraries.architecture.bindings import io.element.android.libraries.core.log.logger.LoggerTag @@ -45,8 +46,11 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { override fun onMessageReceived(message: RemoteMessage) { Timber.tag(loggerTag.value).w("New Firebase message. Priority: ${message.priority}/${message.originalPriority}") - // Acquire wakelock to ensure the device stays awake while we handle the push and schedule and run the work - pushHandlingWakeLock.lock() + val isHighPriority = message.priority == PRIORITY_HIGH + if (isHighPriority) { + // Acquire wakelock to ensure the device stays awake while we handle the push and schedule and run the work + pushHandlingWakeLock.lock() + } coroutineScope.launch { val pushData = pushParser.parse(message.data) @@ -58,7 +62,9 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { "$it: ${message.data[it]}" }, ) - pushHandlingWakeLock.unlock() + if (isHighPriority) { + pushHandlingWakeLock.unlock() + } } else { val handled = pushHandler.handle( pushData = pushData, @@ -66,7 +72,7 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { ) // If we failed to handle the push, we should release the wakelock early to avoid keeping the device awake for too long. - if (!handled) { + if (!handled && isHighPriority) { pushHandlingWakeLock.unlock() } } diff --git a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingServiceTest.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingServiceTest.kt index 87ca74730c..798328e626 100644 --- a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingServiceTest.kt +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingServiceTest.kt @@ -96,6 +96,7 @@ class VectorFirebaseMessagingServiceTest { putString("event_id", AN_EVENT_ID.value) putString("room_id", A_ROOM_ID.value) putString("cs", A_SECRET) + putString("google.priority", "high") }, ) ) @@ -127,6 +128,7 @@ class VectorFirebaseMessagingServiceTest { putString("event_id", AN_EVENT_ID.value) putString("room_id", A_ROOM_ID.value) putString("cs", A_SECRET) + putString("google.priority", "high") }, ) ) @@ -141,6 +143,33 @@ class VectorFirebaseMessagingServiceTest { unlockLambda.assertions().isCalledOnce() } + @Test + fun `test pushHandler with a remote message with normal priority won't lock the wakelock`() = runTest { + val lockLambda = lambdaRecorder { _ -> } + val unlockLambda = lambdaRecorder { } + val vectorFirebaseMessagingService = createVectorFirebaseMessagingService( + pushHandler = FakePushHandler(handleResult = { _, _ -> false }), + pushHandlingWakeLock = FakePushHandlingWakeLock( + lock = lockLambda, + unlock = unlockLambda + ) + ) + vectorFirebaseMessagingService.onMessageReceived( + message = RemoteMessage( + Bundle().apply { + putString("event_id", AN_EVENT_ID.value) + putString("room_id", A_ROOM_ID.value) + putString("cs", A_SECRET) + putString("google.priority", "normal") + }, + ) + ) + + // The wakelock should not be locked + lockLambda.assertions().isNeverCalled() + unlockLambda.assertions().isNeverCalled() + } + @Test fun `test new token is forwarded to the handler`() = runTest { val lambda = lambdaRecorder { }