From 9dd419fc917a2b41d99057392d0f424c60ca4c38 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Wed, 17 Apr 2024 00:51:27 -0400
Subject: [PATCH] always catch exceptions from startForeground

Change-Id: I6dd49161a96bf3d828346f362ae6714bf5684404
---
 .../ring/service/CallNotificationService.kt   | 101 ++++++++++--------
 .../main/java/cx/ring/service/SyncService.kt  |  14 ++-
 .../cx/ring/services/DataTransferService.kt   |  22 ++--
 3 files changed, 82 insertions(+), 55 deletions(-)

diff --git a/jami-android/app/src/main/java/cx/ring/service/CallNotificationService.kt b/jami-android/app/src/main/java/cx/ring/service/CallNotificationService.kt
index 31ab58b43..1011d38f5 100644
--- a/jami-android/app/src/main/java/cx/ring/service/CallNotificationService.kt
+++ b/jami-android/app/src/main/java/cx/ring/service/CallNotificationService.kt
@@ -17,6 +17,7 @@
 package cx.ring.service
 
 import android.Manifest
+import android.annotation.SuppressLint
 import android.app.Notification
 import android.app.Service
 import android.content.Intent
@@ -24,6 +25,7 @@ import android.content.pm.PackageManager
 import android.content.pm.ServiceInfo
 import android.os.Build
 import android.os.IBinder
+import android.util.Log
 import cx.ring.services.NotificationServiceImpl
 import dagger.hilt.android.AndroidEntryPoint
 import net.jami.services.NotificationService
@@ -32,9 +34,18 @@ import javax.inject.Inject
 @AndroidEntryPoint
 class CallNotificationService : Service() {
 
+    private fun PackageManager.hasPermission(permission: String, pn: String = packageName): Boolean =
+        checkPermission(permission, pn) == PackageManager.PERMISSION_GRANTED
+
+    private fun PackageManager.hasPermissions(vararg permissions: String): Boolean {
+        val pn = packageName
+        return permissions.all { checkPermission(it, pn) == PackageManager.PERMISSION_GRANTED }
+    }
+
     @Inject
     lateinit var mNotificationService: NotificationService
 
+    @SuppressLint("ForegroundServiceType")
     override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int {
         super.onStartCommand(intent, flags, startId)
         if (ACTION_START == intent.action) {
@@ -42,49 +53,52 @@ class CallNotificationService : Service() {
             val notification = mNotificationService.showCallNotification(intent.getIntExtra(NotificationService.KEY_NOTIFICATION_ID, -1)) as Notification?
             val startScreenshare = intent.getBooleanExtra(NotificationService.KEY_SCREENSHARE, false)
             if (notification != null) {
-                // Since API 34, screen sharing (media projection)
-                // should not be specified before user grants permission.
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
-                    val pm = packageManager
-                    val name = packageName
-                    val cameraServiceType = if (pm.checkPermission(Manifest.permission.FOREGROUND_SERVICE_CAMERA, name) ==
-                            PackageManager.PERMISSION_GRANTED) ServiceInfo.FOREGROUND_SERVICE_TYPE_CAMERA else 0
-                    val microphoneServiceType = if (pm.checkPermission(Manifest.permission.FOREGROUND_SERVICE_MICROPHONE, name) ==
-                            PackageManager.PERMISSION_GRANTED) ServiceInfo.FOREGROUND_SERVICE_TYPE_MICROPHONE else 0
-                    val callServiceType = if (pm.checkPermission(Manifest.permission.FOREGROUND_SERVICE_PHONE_CALL, name) ==
-                            PackageManager.PERMISSION_GRANTED) ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL else 0
-                    val screenShareType = if (startScreenshare && pm.checkPermission(Manifest.permission.FOREGROUND_SERVICE_MEDIA_PROJECTION, name) ==
-                        PackageManager.PERMISSION_GRANTED) ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION else 0
-                    startForeground(
-                        NotificationServiceImpl.NOTIF_CALL_ID,
-                        notification,
-                        callServiceType
-                                or microphoneServiceType
-                                or cameraServiceType
-                                or screenShareType
-                    )
-                    // Since API 30, microphone and camera should be specified for app to use them.
-                } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R)
-                    startForeground(
-                        NotificationServiceImpl.NOTIF_CALL_ID,
-                        notification,
-                        ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL
-                                or ServiceInfo.FOREGROUND_SERVICE_TYPE_MICROPHONE
-                                or ServiceInfo.FOREGROUND_SERVICE_TYPE_CAMERA
-                                or (if (startScreenshare) ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION else 0)
-                    )
-                // Since API 29, should specify foreground service type.
-                else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
-                    startForeground(
-                        NotificationServiceImpl.NOTIF_CALL_ID,
-                        notification,
-                        ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL
-                                or ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION
-                    )
-                else // Before API 29, just start foreground service.
-                    startForeground(NotificationServiceImpl.NOTIF_CALL_ID, notification)
-                if (startScreenshare && confId != null)
-                    mNotificationService.startPendingScreenshare(confId)
+                try {
+                    // Since API 34, foreground services
+                    // should not be specified before user grants permission.
+                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
+                        val pm = packageManager
+                        val cameraServiceType = if (pm.hasPermissions(Manifest.permission.FOREGROUND_SERVICE_CAMERA, Manifest.permission.CAMERA))
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_CAMERA else 0
+                        val microphoneServiceType = if (pm.hasPermissions(Manifest.permission.FOREGROUND_SERVICE_MICROPHONE, Manifest.permission.RECORD_AUDIO))
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_MICROPHONE else 0
+                        val callServiceType = if (pm.hasPermission(Manifest.permission.FOREGROUND_SERVICE_PHONE_CALL))
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL else 0
+                        val screenShareType = if (startScreenshare && pm.hasPermission(Manifest.permission.FOREGROUND_SERVICE_MEDIA_PROJECTION))
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION else 0
+                        startForeground(
+                            NotificationServiceImpl.NOTIF_CALL_ID,
+                            notification,
+                            callServiceType
+                                    or microphoneServiceType
+                                    or cameraServiceType
+                                    or screenShareType
+                        )
+                        // Since API 30, microphone and camera should be specified for app to use them.
+                    } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R)
+                        startForeground(
+                            NotificationServiceImpl.NOTIF_CALL_ID,
+                            notification,
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL
+                                    or ServiceInfo.FOREGROUND_SERVICE_TYPE_MICROPHONE
+                                    or ServiceInfo.FOREGROUND_SERVICE_TYPE_CAMERA
+                                    or (if (startScreenshare) ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION else 0)
+                        )
+                    // Since API 29, should specify foreground service type.
+                    else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
+                        startForeground(
+                            NotificationServiceImpl.NOTIF_CALL_ID,
+                            notification,
+                            ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL
+                                    or ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION
+                        )
+                    else // Before API 29, just start foreground service.
+                        startForeground(NotificationServiceImpl.NOTIF_CALL_ID, notification)
+                    if (startScreenshare && confId != null)
+                        mNotificationService.startPendingScreenshare(confId)
+                } catch (e: Exception) {
+                    Log.e(TAG, "Failed to start foreground service", e)
+                }
             }
         } else if (ACTION_STOP == intent.action) {
             stopForeground(STOP_FOREGROUND_REMOVE)
@@ -97,6 +111,7 @@ class CallNotificationService : Service() {
     override fun onBind(intent: Intent): IBinder? = null
 
     companion object {
+        private const val TAG = "CallNotificationService"
         const val ACTION_START = "START"
         const val ACTION_STOP = "STOP"
     }
diff --git a/jami-android/app/src/main/java/cx/ring/service/SyncService.kt b/jami-android/app/src/main/java/cx/ring/service/SyncService.kt
index 9a1f4a7dc..8138bddc8 100644
--- a/jami-android/app/src/main/java/cx/ring/service/SyncService.kt
+++ b/jami-android/app/src/main/java/cx/ring/service/SyncService.kt
@@ -24,6 +24,7 @@ import android.content.pm.ServiceInfo
 import android.os.Build
 import android.os.Handler
 import android.os.IBinder
+import android.util.Log
 import androidx.core.app.NotificationCompat
 import cx.ring.R
 import cx.ring.application.JamiApplication
@@ -68,10 +69,14 @@ class SyncService : Service() {
                     .setContentIntent(PendingIntent.getActivity(applicationContext, mRandom.nextInt(), contentIntent, ContentUri.immutable()))
                     .build()
             }
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
-                startForeground(NOTIF_SYNC_SERVICE_ID, notification!!, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC)
-            else
-                startForeground(NOTIF_SYNC_SERVICE_ID, notification)
+            try {
+                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
+                    startForeground(NOTIF_SYNC_SERVICE_ID, notification!!, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC)
+                else
+                    startForeground(NOTIF_SYNC_SERVICE_ID, notification)
+            } catch (e: Exception) {
+                Log.e(TAG, "Error in startForeground", e)
+            }
             if (serviceUsers == 0) {
                 JamiApplication.instance?.startDaemon(this)
             }
@@ -103,6 +108,7 @@ class SyncService : Service() {
     override fun onBind(intent: Intent): IBinder? = null
 
     companion object {
+        const val TAG = "SyncService"
         const val NOTIF_SYNC_SERVICE_ID = 1004
         const val ACTION_START = "startService"
         const val ACTION_STOP = "stopService"
diff --git a/jami-android/app/src/main/java/cx/ring/services/DataTransferService.kt b/jami-android/app/src/main/java/cx/ring/services/DataTransferService.kt
index cead3e628..35d2d9e4e 100644
--- a/jami-android/app/src/main/java/cx/ring/services/DataTransferService.kt
+++ b/jami-android/app/src/main/java/cx/ring/services/DataTransferService.kt
@@ -49,20 +49,26 @@ class DataTransferService : Service() {
         val action = intent.action
         if (ACTION_START == action) {
             serviceNotifications.add(notificationId)
-            val notification = mNotificationService.getDataTransferNotification(notificationId) as Notification
+            val notification = mNotificationService.getDataTransferNotification(notificationId) as Notification? ?: return START_NOT_STICKY
             // Log.w(TAG, "Updating notification " + intent);
             if (!started) {
                 Log.w(TAG, "starting transfer service $intent")
                 serviceNotificationId = notificationId
                 started = true
             }
-            if (notificationId == serviceNotificationId) {
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
-                    startForeground(NOTIF_FILE_SERVICE_ID, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC)
-                else
-                    startForeground(NOTIF_FILE_SERVICE_ID, notification)
-            } else {
-                notificationManager.notify(notificationId, notification)
+            try {
+                if (notificationId == serviceNotificationId) {
+                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
+                        startForeground(NOTIF_FILE_SERVICE_ID, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC)
+                    else
+                        startForeground(NOTIF_FILE_SERVICE_ID, notification)
+                } else {
+                    notificationManager.notify(notificationId, notification)
+                }
+            } catch (e: Exception) {
+                Log.e(TAG, "Failed to update notification", e)
+                stopSelfResult(startId)
+                started = false
             }
         } else if (ACTION_STOP == action) {
             serviceNotifications.remove(notificationId)
-- 
GitLab