Try handling ForegroundServiceStartNotAllowedException better (#6483)

* Try handling `ForegroundServiceStartNotAllowedException` better

The docs mention starting a foreground service when the app is on background is allowed when FCM receives a high priority notification, so we don't do it if the priority is not high.

Also, we handle the case where starting the foreground service fails so it doesn't crash the app.

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()
+
+        // 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,19 +114,22 @@ class FetchPushForegroundService : Service() {
     }
 
     override fun stopService(intent: Intent?): Boolean {
+        if (isOnForeground) {
             wakelock.release()
-
             stopForeground(STOP_FOREGROUND_REMOVE)
+        }
+
         return super.stopService(intent)
     }
 
     override fun onTimeout(startId: Int) {
         super.onTimeout(startId)
 
+        if (isOnForeground) {
             Timber.d("Wakelock timeout reached, stopping FetchPushForegroundService")
-
             coroutineScope.launch { pushHandlingWakeLock.unlock() }
         }
+    }
 
     companion object {
         private val stopMutex = Mutex()

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}")
 
+        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]}"
                     },
                 )
+                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()
                 }
             }

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<Duration, Unit> { _ -> }
+        val unlockLambda = lambdaRecorder<Unit> { }
+        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<String, Unit> { }