From 08c661bb14161d58caf31bfb4886e3fd3cf52a9b Mon Sep 17 00:00:00 2001
From: Clark <clark@signal.org>
Date: Wed, 14 Jun 2023 14:42:08 -0400
Subject: [PATCH] Fix foreground service start/stop race.

---
 .../gcm/FcmFetchForegroundService.kt          |  2 +-
 .../securesms/gcm/FcmFetchManager.kt          | 24 +++++++++++++++----
 .../securesms/gcm/FcmReceiveService.java      | 11 ++++++---
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt
index 0b577411a3..9b09606fbf 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt
@@ -47,7 +47,6 @@ class FcmFetchForegroundService : Service() {
     return if (intent != null && intent.getBooleanExtra(KEY_STOP_SELF, false)) {
       stopForeground(true)
       stopSelf()
-      FcmFetchManager.onForegroundFetchServiceStop()
       START_NOT_STICKY
     } else {
       START_STICKY
@@ -70,6 +69,7 @@ class FcmFetchForegroundService : Service() {
 
   override fun onDestroy() {
     Log.i(TAG, "onDestroy()")
+    FcmFetchManager.onDestroyForegroundFetchService()
   }
 
   override fun onBind(intent: Intent?): IBinder? {
diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt
index 2b756fd362..c0631d44b5 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt
@@ -42,6 +42,9 @@ object FcmFetchManager {
   @Volatile
   private var stoppingForeground = false
 
+  @Volatile
+  private var startForegroundOnDestroy = false
+
   /**
    * @return True if a service was successfully started, otherwise false.
    */
@@ -52,8 +55,13 @@ object FcmFetchManager {
         if (foreground) {
           Log.i(TAG, "Starting in the foreground.")
           if (!startedForeground) {
-            ForegroundServiceUtil.startWhenCapableOrThrow(context, Intent(context, FcmFetchForegroundService::class.java), MAX_BLOCKING_TIME_MS)
-            startedForeground = true
+            if (!stoppingForeground) {
+              ForegroundServiceUtil.startWhenCapableOrThrow(context, Intent(context, FcmFetchForegroundService::class.java), MAX_BLOCKING_TIME_MS)
+              startedForeground = true
+            } else {
+              Log.w(TAG, "Foreground service currently stopping, enqueuing a start after destroy")
+              startForegroundOnDestroy = true
+            }
           } else {
             Log.i(TAG, "Already started foreground service")
           }
@@ -108,6 +116,7 @@ object FcmFetchManager {
     synchronized(this) {
       if (startedForeground || stoppingForeground) {
         if (stoppingForeground) {
+          startForegroundOnDestroy = true
           Log.i(TAG, "Legacy fallback: foreground service is stopping, but trying to run in background anyways.")
         }
         val performedReplace = EXECUTOR.enqueue { fetch(context) }
@@ -142,10 +151,17 @@ object FcmFetchManager {
     }
   }
 
-  fun onForegroundFetchServiceStop() {
+  fun onDestroyForegroundFetchService() {
     synchronized(this) {
       stoppingForeground = false
-      Log.i(TAG, "Foreground service stopped successfully")
+      if (startForegroundOnDestroy) {
+        startForegroundOnDestroy = false
+        try {
+          enqueue(ApplicationDependencies.getApplication(), true)
+        } catch (e: Exception) {
+          Log.e(TAG, "Failed to restart foreground service after onDestroy")
+        }
+      }
     }
   }
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java
index 81fc837da2..51a6d6cd8f 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java
@@ -79,9 +79,10 @@ public class FcmReceiveService extends FirebaseMessagingService {
 
   private static void handleReceivedNotification(Context context, @Nullable RemoteMessage remoteMessage) {
     boolean enqueueSuccessful = false;
-
+    boolean highPriority = false;
     try {
-      boolean highPriority         = remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH;
+      highPriority = remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH;
+
       long    timeSinceLastRefresh = System.currentTimeMillis() - SignalStore.misc().getLastFcmForegroundServiceTime();
 
       Log.d(TAG, String.format(Locale.US, "[handleReceivedNotification] API: %s, FeatureFlag: %s, RemoteMessagePriority: %s, TimeSinceLastRefresh: %s ms", Build.VERSION.SDK_INT, FeatureFlags.useFcmForegroundService(), remoteMessage != null ? remoteMessage.getPriority() : "n/a", timeSinceLastRefresh));
@@ -101,7 +102,11 @@ public class FcmReceiveService extends FirebaseMessagingService {
     }
 
     if (!enqueueSuccessful) {
-      Log.w(TAG, "Unable to start service. Falling back to legacy approach.");
+      if (highPriority) {
+        Log.w(TAG, "Unable to start service though high priority push. Falling back to legacy approach.");
+      } else {
+        Log.d(TAG, "Low priority push, trying legacy fallback");
+      }
       FcmFetchManager.tryLegacyFallback(context);
     }
   }