Refactor FcmFetchManager to make foreground service clearer.

app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt

@@ -11,6 +11,7 @@ import org.signal.core.util.PendingIntentFlags
 import org.signal.core.util.logging.Log
 import org.thoughtcrime.securesms.MainActivity
 import org.thoughtcrime.securesms.R
+import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil
 import org.thoughtcrime.securesms.notifications.NotificationChannels
 import org.thoughtcrime.securesms.notifications.NotificationIds
 import org.thoughtcrime.securesms.util.WakeLockUtil
@@ -27,6 +28,7 @@ class FcmFetchForegroundService : Service() {
 
     private const val WAKELOCK_TAG = "FcmForegroundService"
     private const val KEY_STOP_SELF = "stop_self"
+    private const val MAX_BLOCKING_TIME_MS = 500L
 
     private val WAKELOCK_TIMEOUT = FcmFetchManager.WEBSOCKET_DRAIN_TIMEOUT
 
@@ -36,11 +38,76 @@ class FcmFetchForegroundService : Service() {
      * The safest thing to do is to just tell it to start so it can call [startForeground] and then stop itself.
      * Fun.
      */
-    fun buildStopIntent(context: Context): Intent {
+    private fun buildStopIntent(context: Context): Intent {
       return Intent(context, FcmFetchForegroundService::class.java).apply {
         putExtra(KEY_STOP_SELF, true)
       }
     }
+
+    enum class State {
+      STOPPED,
+      STARTED,
+      STOPPING,
+      RESTARTING
+    }
+
+    private var foregroundServiceState: State = State.STOPPED
+
+    fun startServiceIfNecessary(context: Context) {
+      synchronized(this) {
+        when (foregroundServiceState) {
+          State.STOPPING -> foregroundServiceState = State.RESTARTING
+          State.STOPPED -> {
+            foregroundServiceState = try {
+              startForegroundFetchService(context)
+              State.STARTED
+            } catch (e: IllegalStateException) {
+              Log.e(TAG, "Failed to start foreground service", e)
+              State.STOPPED
+            }
+          }
+          else -> Log.i(TAG, "Already started foreground service")
+        }
+      }
+    }
+
+    fun stopServiceIfNecessary(context: Context) {
+      synchronized(this) {
+        when (foregroundServiceState) {
+          State.STARTED -> {
+            foregroundServiceState = State.STOPPING
+            try {
+              context.startService(buildStopIntent(context))
+            } catch (e: IllegalStateException) {
+              Log.w(TAG, "Failed to stop the foreground service, assuming already stopped", e)
+              foregroundServiceState = State.STOPPED
+            }
+          }
+          State.RESTARTING -> foregroundServiceState = State.STOPPED
+          else -> Log.i(TAG, "No service to stop")
+        }
+      }
+    }
+
+    private fun onServiceDestroyed(context: Context) {
+      synchronized(this) {
+        Log.i(TAG, "Fcm fetch service destroyed")
+        when (foregroundServiceState) {
+          State.RESTARTING -> {
+            foregroundServiceState = State.STOPPED
+            Log.i(TAG, "Restarting service.")
+            startServiceIfNecessary(context)
+          }
+          else -> {
+            foregroundServiceState = State.STOPPED
+          }
+        }
+      }
+    }
+
+    private fun startForegroundFetchService(context: Context) {
+      ForegroundServiceUtil.startWhenCapableOrThrow(context, Intent(context, FcmFetchForegroundService::class.java), MAX_BLOCKING_TIME_MS)
+    }
   }
 
   override fun onCreate() {
@@ -81,7 +148,7 @@ class FcmFetchForegroundService : Service() {
   override fun onDestroy() {
     Log.i(TAG, "onDestroy()")
     WakeLockUtil.release(wakeLock, WAKELOCK_TAG)
-    FcmFetchManager.onDestroyForegroundFetchService()
+    onServiceDestroyed(this)
 
     wakeLock = null
   }

app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt

@@ -3,11 +3,9 @@ package org.thoughtcrime.securesms.gcm
 import android.content.Context
 import android.content.Intent
 import android.os.Build
-import android.os.PowerManager
 import org.signal.core.util.concurrent.SignalExecutors
 import org.signal.core.util.logging.Log
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies
-import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil
 import org.thoughtcrime.securesms.jobs.PushNotificationReceiveJob
 import org.thoughtcrime.securesms.messages.WebSocketStrategy
 import org.thoughtcrime.securesms.util.SignalLocalMetrics
@@ -33,7 +31,6 @@ import kotlin.time.Duration.Companion.minutes
 object FcmFetchManager {
 
   private val TAG = Log.tag(FcmFetchManager::class.java)
-  private const val MAX_BLOCKING_TIME_MS = 500L
   private val EXECUTOR = SerialMonoLifoExecutor(SignalExecutors.UNBOUNDED)
 
   val WEBSOCKET_DRAIN_TIMEOUT = 5.minutes.inWholeMilliseconds
@@ -41,57 +38,22 @@ object FcmFetchManager {
   @Volatile
   private var activeCount = 0
 
-  @Volatile
+  /**
-  private var startedForeground = false
+   * @return True if a service was successfully started, otherwise false.
-
+   */
-  @Volatile
+  @JvmStatic
-  private var stoppingForeground = false
+  fun startBackgroundService(context: Context) {
-
+    Log.i(TAG, "Starting in the background.")
-  @Volatile
+    context.startService(Intent(context, FcmFetchBackgroundService::class.java))
-  private var startForegroundOnDestroy = false
+  }
-
-  private var wakeLock: PowerManager.WakeLock? = null
 
   /**
    * @return True if a service was successfully started, otherwise false.
    */
   @JvmStatic
-  fun enqueue(context: Context, foreground: Boolean): Boolean {
+  fun startForegroundService(context: Context) {
-    synchronized(this) {
-      try {
-        if (foreground) {
     Log.i(TAG, "Starting in the foreground.")
-          if (!startedForeground) {
+    FcmFetchForegroundService.startServiceIfNecessary(context)
-            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")
-          }
-        } else {
-          Log.i(TAG, "Starting in the background.")
-          context.startService(Intent(context, FcmFetchBackgroundService::class.java))
-        }
-
-        val performedReplace = EXECUTOR.enqueue { fetch(context) }
-
-        if (performedReplace) {
-          Log.i(TAG, "Already have one running and one enqueued. Ignoring.")
-        } else {
-          activeCount++
-          Log.i(TAG, "Incrementing active count to $activeCount")
-        }
-      } catch (e: Exception) {
-        Log.w(TAG, "Failed to start service!", e)
-        return false
-      }
-    }
-
-    return true
   }
 
   private fun fetch(context: Context) {
@@ -109,37 +71,20 @@ object FcmFetchManager {
       if (activeCount <= 0) {
         Log.i(TAG, "No more active. Stopping.")
         context.stopService(Intent(context, FcmFetchBackgroundService::class.java))
-
+        FcmFetchForegroundService.stopServiceIfNecessary(context)
-        if (startedForeground) {
-          try {
-            context.startService(FcmFetchForegroundService.buildStopIntent(context))
-            stoppingForeground = true
-          } catch (e: IllegalStateException) {
-            Log.w(TAG, "Failed to stop the foreground notification!", e)
-          }
-
-          startedForeground = false
-        }
       }
     }
   }
 
   @JvmStatic
-  fun tryLegacyFallback(context: Context) {
+  fun enqueueFetch(context: Context) {
     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) }
-
       if (performedReplace) {
-        Log.i(TAG, "Legacy fallback: already have one running and one enqueued. Ignoring.")
+        Log.i(TAG, "Already have one running and one enqueued. Ignoring.")
       } else {
         activeCount++
-        Log.i(TAG, "Legacy fallback: Incrementing active count to $activeCount")
+        Log.i(TAG, "Incrementing active count to $activeCount")
       }
     }
   }
@@ -162,18 +107,4 @@ object FcmFetchManager {
 
     return success
   }
-
-  fun onDestroyForegroundFetchService() {
-    synchronized(this) {
-      stoppingForeground = false
-      if (startForegroundOnDestroy) {
-        startForegroundOnDestroy = false
-        try {
-          enqueue(ApplicationDependencies.getApplication(), true)
-        } catch (e: Exception) {
-          Log.e(TAG, "Failed to restart foreground service after onDestroy")
-        }
-      }
-    }
-  }
 }

app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java

@@ -15,18 +15,14 @@ import org.thoughtcrime.securesms.jobs.FcmRefreshJob;
 import org.thoughtcrime.securesms.jobs.SubmitRateLimitPushChallengeJob;
 import org.thoughtcrime.securesms.keyvalue.SignalStore;
 import org.thoughtcrime.securesms.registration.PushChallengeRequest;
-import org.thoughtcrime.securesms.util.FeatureFlags;
 import org.thoughtcrime.securesms.util.NetworkUtil;
 
 import java.util.Locale;
-import java.util.concurrent.TimeUnit;
 
 public class FcmReceiveService extends FirebaseMessagingService {
 
   private static final String TAG = Log.tag(FcmReceiveService.class);
 
-  private static final long FCM_FOREGROUND_INTERVAL = TimeUnit.MINUTES.toMillis(3);
-
   @Override
   public void onMessageReceived(RemoteMessage remoteMessage) {
     Log.i(TAG, String.format(Locale.US,
@@ -78,37 +74,21 @@ public class FcmReceiveService extends FirebaseMessagingService {
   }
 
   private static void handleReceivedNotification(Context context, @Nullable RemoteMessage remoteMessage) {
-    boolean enqueueSuccessful = false;
-    boolean highPriority = false;
     try {
-      highPriority = remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH;
+      boolean 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, RemoteMessagePriority: %s", Build.VERSION.SDK_INT, remoteMessage != null ? remoteMessage.getPriority() : "n/a"));
 
-      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));
+      if (highPriority) {
-
+        FcmFetchManager.startForegroundService(context);
-      if (highPriority && FeatureFlags.useFcmForegroundService()) {
+      } else if (Build.VERSION.SDK_INT < 26) {
-        enqueueSuccessful = FcmFetchManager.enqueue(context, true);
+        FcmFetchManager.startBackgroundService(context);
-        SignalStore.misc().setLastFcmForegroundServiceTime(System.currentTimeMillis());
-      } else if (highPriority && Build.VERSION.SDK_INT >= 31 && timeSinceLastRefresh > FCM_FOREGROUND_INTERVAL) {
-        enqueueSuccessful = FcmFetchManager.enqueue(context, true);
-        SignalStore.misc().setLastFcmForegroundServiceTime(System.currentTimeMillis());
-      } else if (highPriority || Build.VERSION.SDK_INT < 26 || remoteMessage == null) {
-        enqueueSuccessful = FcmFetchManager.enqueue(context, false);
       }
     } catch (Exception e) {
       Log.w(TAG, "Failed to start service.", e);
-      enqueueSuccessful = false;
     }
 
-    if (!enqueueSuccessful) {
+    FcmFetchManager.enqueueFetch(context);
-      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);
-    }
   }
 
   private static void handleRegistrationPushChallenge(@NonNull String challenge) {

app/src/main/java/org/thoughtcrime/securesms/keyvalue/MiscellaneousValues.java

@@ -29,7 +29,6 @@ public final class MiscellaneousValues extends SignalStoreValues {
   private static final String LAST_GV2_PROFILE_CHECK_TIME    = "misc.last_gv2_profile_check_time";
   private static final String CDS_TOKEN                      = "misc.cds_token";
   private static final String CDS_BLOCKED_UNTIL              = "misc.cds_blocked_until";
-  private static final String LAST_FCM_FOREGROUND_TIME       = "misc.last_fcm_foreground_time";
   private static final String LAST_FOREGROUND_TIME           = "misc.last_foreground_time";
   private static final String PNI_INITIALIZED_DEVICES        = "misc.pni_initialized_devices";
   private static final String SMS_PHASE_1_START_MS           = "misc.sms_export.phase_1_start.3";
@@ -217,14 +216,6 @@ public final class MiscellaneousValues extends SignalStoreValues {
     return getLong(CDS_BLOCKED_UNTIL, 0);
   }
 
-  public long getLastFcmForegroundServiceTime() {
-    return getLong(LAST_FCM_FOREGROUND_TIME, 0);
-  }
-
-  public void setLastFcmForegroundServiceTime(long time) {
-    putLong(LAST_FCM_FOREGROUND_TIME, time);
-  }
-
   public long getLastForegroundTime() {
     return getLong(LAST_FOREGROUND_TIME, 0);
   }

app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java

@@ -86,7 +86,6 @@ public final class FeatureFlags {
   private static final String USE_AEC3                          = "android.calling.useAec3";
   private static final String PAYMENTS_COUNTRY_BLOCKLIST        = "global.payments.disabledRegions";
   public  static final String PHONE_NUMBER_PRIVACY              = "android.pnp";
-  private static final String USE_FCM_FOREGROUND_SERVICE        = "android.useFcmForegroundService.4";
   private static final String STORIES_AUTO_DOWNLOAD_MAXIMUM     = "android.stories.autoDownloadMaximum";
   private static final String TELECOM_MANUFACTURER_ALLOWLIST    = "android.calling.telecomAllowList";
   private static final String TELECOM_MODEL_BLOCKLIST           = "android.calling.telecomModelBlockList";
@@ -145,7 +144,6 @@ public final class FeatureFlags {
       USE_HARDWARE_AEC_IF_OLD,
       USE_AEC3,
       PAYMENTS_COUNTRY_BLOCKLIST,
-      USE_FCM_FOREGROUND_SERVICE,
       STORIES_AUTO_DOWNLOAD_MAXIMUM,
       TELECOM_MANUFACTURER_ALLOWLIST,
       TELECOM_MODEL_BLOCKLIST,
@@ -220,7 +218,6 @@ public final class FeatureFlags {
       USE_HARDWARE_AEC_IF_OLD,
       USE_AEC3,
       PAYMENTS_COUNTRY_BLOCKLIST,
-      USE_FCM_FOREGROUND_SERVICE,
       TELECOM_MANUFACTURER_ALLOWLIST,
       TELECOM_MODEL_BLOCKLIST,
       CAMERAX_MODEL_BLOCKLIST,
@@ -488,11 +485,6 @@ public final class FeatureFlags {
     return getBoolean(USE_AEC3, true);
   }
 
-  /** Whether or not we show a foreground service on every high-priority FCM push. */
-  public static boolean useFcmForegroundService() {
-    return getBoolean(USE_FCM_FOREGROUND_SERVICE, false);
-  }
-
   /**
    * Prefetch count for stories from a given user.
    */