Improve reliability of launching FCM foreground service.

We were getting weird errors around the service not calling
startForeground() that I couldn't reproduce. I figure it must be
something with how we only sometimes start the FcmFetchService in the
foreground. So I think the safest thing to do is to just use
GenericForegroundService.
This commit is contained in:
Greyson Parrelli
2022-04-26 11:22:01 -04:00
committed by GitHub
parent 65372e547a
commit 797c02e893
4 changed files with 38 additions and 30 deletions

View File

@@ -24,9 +24,12 @@ 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.
@@ -54,7 +57,8 @@ public class FcmFetchService extends Service {
private static final SerialMonoLifoExecutor EXECUTOR = new SerialMonoLifoExecutor(SignalExecutors.UNBOUNDED);
private final AtomicInteger activeCount = new AtomicInteger(0);
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);
@@ -73,16 +77,18 @@ public class FcmFetchService extends Service {
Log.i(TAG, "Incrementing active count to " + count);
}
if (intent.getBooleanExtra(KEY_FOREGROUND, false)) {
Log.i(TAG, "Launching in the foreground.");
startForeground(NotificationIds.FCM_FETCH, new 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(new long[] { 0 })
.build());
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;
@@ -104,6 +110,15 @@ public class FcmFetchService extends Service {
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);
}
}
}
}

View File

@@ -17,6 +17,7 @@ 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 java.util.Locale;
import java.util.concurrent.TimeUnit;
@@ -79,8 +80,8 @@ public class FcmReceiveService extends FirebaseMessagingService {
private static void handleReceivedNotification(Context context, @Nullable RemoteMessage remoteMessage) {
try {
long timeSinceLastRefresh = System.currentTimeMillis() - SignalStore.misc().getLastFcmForegroundServiceTime();
if (Build.VERSION.SDK_INT >= 31 && remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH && timeSinceLastRefresh > FCM_FOREGROUND_INTERVAL) {
ContextCompat.startForegroundService(context, FcmFetchService.buildIntent(context, true));
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));
SignalStore.misc().setLastFcmForegroundServiceTime(System.currentTimeMillis());
} else {
context.startService(FcmFetchService.buildIntent(context, false));