From 9afeb206fcbb823d691564addabbc4c3d21c1d58 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 10 May 2022 15:11:47 -0400 Subject: [PATCH] Refactor FCM processing to improve use of foreground services. --- app/src/main/AndroidManifest.xml | 4 +- .../gcm/FcmFetchBackgroundService.java | 33 ++++ .../gcm/FcmFetchForegroundService.kt | 44 ++++++ .../securesms/gcm/FcmFetchManager.kt | 91 +++++++++++ .../securesms/gcm/FcmFetchService.java | 142 ------------------ .../securesms/gcm/FcmReceiveService.java | 8 +- .../securesms/util/FeatureFlags.java | 2 +- 7 files changed, 177 insertions(+), 147 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchBackgroundService.java create mode 100644 app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt create mode 100644 app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt delete mode 100644 app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchService.java diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index d3e49396e..4f5e6d02a 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -706,7 +706,9 @@ - + + + diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchBackgroundService.java b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchBackgroundService.java new file mode 100644 index 000000000..9d62ac2e8 --- /dev/null +++ b/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; + } +} + diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchForegroundService.kt new file mode 100644 index 000000000..18e4503c9 --- /dev/null +++ b/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 + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt new file mode 100644 index 000000000..1448a355c --- /dev/null +++ b/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()) + } + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchService.java b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchService.java deleted file mode 100644 index ea9bca759..000000000 --- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchService.java +++ /dev/null @@ -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 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()); - } - } - } -} - 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 48257816e..808dc07d2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmReceiveService.java +++ b/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); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index ce075257b..c09cb7b00 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/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";