Refactor FCM processing to improve use of foreground services.

app/src/main/AndroidManifest.xml

@@ -706,7 +706,9 @@
 
     <service android:name=".service.GenericForegroundService"/>
 
-    <service android:name=".gcm.FcmFetchService" />
+    <service android:name=".gcm.FcmFetchBackgroundService" />
+
+    <service android:name=".gcm.FcmFetchForegroundService" />
 
     <service android:name=".gcm.FcmReceiveService">
         <intent-filter>

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

@@ -0,0 +1,33 @@
+package org.thoughtcrime.securesms.gcm;
+
+import android.app.Service;
+import android.content.Intent;
+import android.os.IBinder;
+
+import androidx.annotation.Nullable;
+
+import org.signal.core.util.logging.Log;
+
+/**
+ * Works with {@link FcmFetchManager} to exists as a service that will keep the app process running in the background while we fetch messages.
+ */
+public class FcmFetchBackgroundService extends Service {
+
+  private static final String TAG = Log.tag(FcmFetchBackgroundService.class);
+
+  @Override
+  public int onStartCommand(Intent intent, int flags, int startId) {
+    return START_STICKY;
+  }
+
+  @Override
+  public void onDestroy() {
+    Log.i(TAG, "onDestroy()");
+  }
+
+  @Override
+  public @Nullable IBinder onBind(Intent intent) {
+    return null;
+  }
+}
+

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

@@ -0,0 +1,44 @@
+package org.thoughtcrime.securesms.gcm
+
+import android.app.PendingIntent
+import android.app.Service
+import android.content.Intent
+import android.os.IBinder
+import androidx.core.app.NotificationCompat
+import org.signal.core.util.logging.Log
+import org.thoughtcrime.securesms.MainActivity
+import org.thoughtcrime.securesms.R
+import org.thoughtcrime.securesms.notifications.NotificationChannels
+import org.thoughtcrime.securesms.notifications.NotificationIds
+
+/**
+ * Works with {@link FcmFetchManager} to exists as a service that will keep the app process running in the foreground while we fetch messages.
+ */
+class FcmFetchForegroundService : Service() {
+
+  private val TAG = Log.tag(FcmFetchForegroundService::class.java)
+
+  override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
+    startForeground(
+      NotificationIds.FCM_FETCH,
+      NotificationCompat.Builder(this, NotificationChannels.OTHER)
+        .setSmallIcon(R.drawable.ic_notification)
+        .setContentTitle(getString(R.string.BackgroundMessageRetriever_checking_for_messages))
+        .setCategory(NotificationCompat.CATEGORY_SERVICE)
+        .setProgress(0, 0, true)
+        .setContentIntent(PendingIntent.getActivity(this, 0, MainActivity.clearTop(this), 0))
+        .setVibrate(longArrayOf(0))
+        .build()
+    )
+
+    return START_STICKY
+  }
+
+  override fun onDestroy() {
+    Log.i(TAG, "onDestroy()")
+  }
+
+  override fun onBind(intent: Intent?): IBinder? {
+    return null
+  }
+}

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

@@ -0,0 +1,91 @@
+package org.thoughtcrime.securesms.gcm
+
+import android.content.Context
+import android.content.Intent
+import android.os.Build
+import androidx.core.content.ContextCompat
+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.PushNotificationReceiveJob
+import org.thoughtcrime.securesms.messages.RestStrategy
+import org.thoughtcrime.securesms.util.concurrent.SerialMonoLifoExecutor
+
+/**
+ * Our goals with FCM processing are as follows:
+ * (1) Ensure some service is active for the duration of the fetch and processing stages.
+ * (2) Do not make unnecessary network requests.
+ *
+ * To fulfill goal 1, this class will not stop the services until there is no more running
+ * requests.
+ *
+ * To fulfill goal 2, this class  will not enqueue a fetch if there are already 2 active fetches
+ * (or rather, 1 active and 1 waiting, since we use a single thread executor).
+ *
+ * Unfortunately we can't do this all in [FcmReceiveService] because it won't let us process
+ * the next FCM message until [FcmReceiveService.onMessageReceived] returns,
+ * but as soon as that method returns, it could also destroy the service. By not letting us control
+ * when the service is destroyed, we can't accomplish both goals within that service.
+ */
+object FcmFetchManager {
+
+  private val TAG = Log.tag(FcmFetchManager::class.java)
+
+  private val EXECUTOR = SerialMonoLifoExecutor(SignalExecutors.UNBOUNDED)
+
+  @Volatile
+  private var activeCount = 0
+
+  @JvmStatic
+  fun enqueue(context: Context, foreground: Boolean) {
+    synchronized(this) {
+      if (foreground) {
+        Log.i(TAG, "Starting in the foreground.")
+        ContextCompat.startForegroundService(context, Intent(context, FcmFetchForegroundService::class.java))
+      } 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")
+      }
+    }
+  }
+
+  private fun fetch(context: Context) {
+    retrieveMessages(context)
+
+    synchronized(this) {
+      activeCount--
+
+      if (activeCount <= 0) {
+        Log.i(TAG, "No more active. Stopping.")
+        context.stopService(Intent(context, FcmFetchForegroundService::class.java))
+        context.stopService(Intent(context, FcmFetchBackgroundService::class.java))
+      }
+    }
+  }
+
+  @JvmStatic
+  fun retrieveMessages(context: Context) {
+    val success = ApplicationDependencies.getBackgroundMessageRetriever().retrieveMessages(context, RestStrategy(), RestStrategy())
+
+    if (success) {
+      Log.i(TAG, "Successfully retrieved messages.")
+    } else {
+      if (Build.VERSION.SDK_INT >= 26) {
+        Log.w(TAG, "[API ${Build.VERSION.SDK_INT}] Failed to retrieve messages. Scheduling on the system JobScheduler (API " + Build.VERSION.SDK_INT + ").")
+        FcmJobService.schedule(context)
+      } else {
+        Log.w(TAG, "[API ${Build.VERSION.SDK_INT}] Failed to retrieve messages. Scheduling on JobManager (API " + Build.VERSION.SDK_INT + ").")
+        ApplicationDependencies.getJobManager().add(PushNotificationReceiveJob())
+      }
+    }
+  }
+}

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

@@ -1,142 +0,0 @@
-package org.thoughtcrime.securesms.gcm;
-
-import android.app.PendingIntent;
-import android.app.Service;
-import android.content.Context;
-import android.content.Intent;
-import android.os.Build;
-import android.os.IBinder;
-
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-import androidx.core.app.NotificationCompat;
-
-import com.google.firebase.messaging.RemoteMessage;
-
-import org.signal.core.util.ThreadUtil;
-import org.signal.core.util.concurrent.SignalExecutors;
-import org.signal.core.util.logging.Log;
-import org.thoughtcrime.securesms.MainActivity;
-import org.thoughtcrime.securesms.R;
-import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
-import org.thoughtcrime.securesms.jobs.PushNotificationReceiveJob;
-import org.thoughtcrime.securesms.messages.BackgroundMessageRetriever;
-import org.thoughtcrime.securesms.messages.RestStrategy;
-import org.thoughtcrime.securesms.notifications.NotificationChannels;
-import org.thoughtcrime.securesms.notifications.NotificationIds;
-import org.thoughtcrime.securesms.service.GenericForegroundService;
-import org.thoughtcrime.securesms.service.NotificationController;
-import org.thoughtcrime.securesms.util.concurrent.SerialMonoLifoExecutor;
-
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicReference;
-
-/**
- * This service does the actual network fetch in response to an FCM message.
- *
- * Our goals with FCM processing are as follows:
- * (1) Ensure some service is active for the duration of the fetch and processing stages.
- * (2) Do not make unnecessary network requests.
- *
- * To fulfill goal 1, this service will not call {@link #stopSelf()} until there is no more running
- * requests.
- *
- * To fulfill goal 2, this service will not enqueue a fetch if there are already 2 active fetches
- * (or rather, 1 active and 1 waiting, since we use a single thread executor).
- *
- * Unfortunately we can't do this all in {@link FcmReceiveService} because it won't let us process
- * the next FCM message until {@link FcmReceiveService#onMessageReceived(RemoteMessage)} returns,
- * but as soon as that method returns, it could also destroy the service. By not letting us control
- * when the service is destroyed, we can't accomplish both goals within that service.
- */
-public class FcmFetchService extends Service {
-
-  private static final String TAG = Log.tag(FcmFetchService.class);
-
-  static final String KEY_FOREGROUND = "is_foreground";
-
-  private static final SerialMonoLifoExecutor EXECUTOR = new SerialMonoLifoExecutor(SignalExecutors.UNBOUNDED);
-
-  private final AtomicInteger                           activeCount          = new AtomicInteger(0);
-  private final AtomicReference<NotificationController> foregroundController = new AtomicReference<>();
-
-  public static @NonNull Intent buildIntent(@NonNull Context context, boolean foreground) {
-    Intent intent = new Intent(context, FcmFetchService.class);
-    intent.putExtra(KEY_FOREGROUND, foreground);
-    return intent;
-  }
-
-  @Override
-  public int onStartCommand(Intent intent, int flags, int startId) {
-    boolean performedReplace = EXECUTOR.enqueue(this::fetch);
-
-    if (performedReplace) {
-      Log.i(TAG, "Already have one running and one enqueued. Ignoring.");
-    } else {
-      int count = activeCount.incrementAndGet();
-      Log.i(TAG, "Incrementing active count to " + count);
-    }
-
-    synchronized (foregroundController) {
-      boolean useForeground = intent.getBooleanExtra(KEY_FOREGROUND, false);
-      boolean hasController = foregroundController.get() != null;
-      
-      if (useForeground && !hasController) {
-        Log.i(TAG, "Launching in the foreground.");
-        NotificationController controller = GenericForegroundService.startForegroundTask(this, getString(R.string.BackgroundMessageRetriever_checking_for_messages), NotificationChannels.OTHER);
-        controller.setIndeterminateProgress();
-        foregroundController.set(controller);
-      } else {
-        Log.i(TAG, "Launching in the background. (useForeground: " + useForeground + ", hasController: " + hasController + ")");
-      }
-    }
-
-    return START_NOT_STICKY;
-  }
-
-  @Override
-  public void onDestroy() {
-    Log.i(TAG, "onDestroy()");
-  }
-
-  @Override
-  public @Nullable IBinder onBind(Intent intent) {
-    return null;
-  }
-
-  private void fetch() {
-    retrieveMessages(this);
-
-    if (activeCount.decrementAndGet() == 0) {
-      Log.d(TAG, "No more active. Stopping.");
-      stopSelf();
-
-      synchronized (foregroundController) {
-        NotificationController activeController = foregroundController.get();
-        if (activeController != null) {
-          Log.d(TAG, "Stopping foreground notification.");
-          activeController.close();
-          foregroundController.set(null);
-        }
-      }
-    }
-  }
-
-  static void retrieveMessages(@NonNull Context context) {
-    BackgroundMessageRetriever retriever = ApplicationDependencies.getBackgroundMessageRetriever();
-    boolean                    success   = retriever.retrieveMessages(context, new RestStrategy(), new RestStrategy());
-
-    if (success) {
-      Log.i(TAG, "Successfully retrieved messages.");
-    } else {
-      if (Build.VERSION.SDK_INT >= 26) {
-        Log.w(TAG, "Failed to retrieve messages. Scheduling on the system JobScheduler (API " + Build.VERSION.SDK_INT + ").");
-        FcmJobService.schedule(context);
-      } else {
-        Log.w(TAG, "Failed to retrieve messages. Scheduling on JobManager (API " + Build.VERSION.SDK_INT + ").");
-        ApplicationDependencies.getJobManager().add(new PushNotificationReceiveJob());
-      }
-    }
-  }
-}
-

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

@@ -80,15 +80,17 @@ public class FcmReceiveService extends FirebaseMessagingService {
   private static void handleReceivedNotification(Context context, @Nullable RemoteMessage remoteMessage) {
     try {
       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));
+
       if (FeatureFlags.useFcmForegroundService() && Build.VERSION.SDK_INT >= 31 && remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH && timeSinceLastRefresh > FCM_FOREGROUND_INTERVAL) {
-        context.startService(FcmFetchService.buildIntent(context, true));
+        FcmFetchManager.enqueue(context, true);
         SignalStore.misc().setLastFcmForegroundServiceTime(System.currentTimeMillis());
       } else {
-        context.startService(FcmFetchService.buildIntent(context, false));
+        FcmFetchManager.enqueue(context, false);
       }
     } catch (Exception e) {
       Log.w(TAG, "Failed to start service. Falling back to legacy approach.", e);
-      FcmFetchService.retrieveMessages(context);
+      FcmFetchManager.retrieveMessages(context);
     }
   }
 

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

@@ -95,7 +95,7 @@ public final class FeatureFlags {
   private static final String USE_AEC3                          = "android.calling.useAec3";
   private static final String PAYMENTS_COUNTRY_BLOCKLIST        = "android.payments.blocklist";
   private static final String PNP_CDS                           = "android.pnp.cds";
-  private static final String USE_FCM_FOREGROUND_SERVICE        = "android.useFcmForegroundService.2";
+  private static final String USE_FCM_FOREGROUND_SERVICE        = "android.useFcmForegroundService.3";
   private static final String STORIES_AUTO_DOWNLOAD_MAXIMUM     = "android.stories.autoDownloadMaximum";
   private static final String GIFT_BADGES                       = "android.giftBadges";