diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 7d7c8572c..5951f4496 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -888,7 +888,7 @@ - + diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackService.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackService.java index e341f8ad8..3239bbabb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackService.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackService.java @@ -16,7 +16,6 @@ import android.support.v4.media.session.PlaybackStateCompat; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.core.content.ContextCompat; import androidx.media.MediaBrowserServiceCompat; import com.google.android.exoplayer2.C; @@ -34,8 +33,10 @@ import org.thoughtcrime.securesms.database.MessageTable; import org.thoughtcrime.securesms.database.SignalDatabase; import org.thoughtcrime.securesms.database.model.MessageId; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil; import org.thoughtcrime.securesms.jobs.MultiDeviceViewedUpdateJob; import org.thoughtcrime.securesms.jobs.SendViewedReceiptJob; +import org.thoughtcrime.securesms.jobs.UnableToStartException; import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.service.KeyCachingService; @@ -275,9 +276,13 @@ public class VoiceNotePlaybackService extends MediaBrowserServiceCompat { @Override public void onNotificationPosted(int notificationId, Notification notification, boolean ongoing) { if (ongoing && !isForegroundService) { - ContextCompat.startForegroundService(getApplicationContext(), new Intent(getApplicationContext(), VoiceNotePlaybackService.class)); - startForeground(notificationId, notification); - isForegroundService = true; + try { + ForegroundServiceUtil.startWhenCapable(getApplicationContext(), new Intent(getApplicationContext(), VoiceNotePlaybackService.class)); + startForeground(notificationId, notification); + isForegroundService = true; + } catch (UnableToStartException e) { + Log.e(TAG, "Unable to start foreground service!", e); + } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherMigrationHelper.java b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherMigrationHelper.java index 98ea3f5af..3a457ccbe 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherMigrationHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherMigrationHelper.java @@ -22,6 +22,7 @@ import org.thoughtcrime.securesms.crypto.AttachmentSecretProvider; import org.thoughtcrime.securesms.crypto.MasterCipher; import org.thoughtcrime.securesms.crypto.MasterSecret; import org.thoughtcrime.securesms.crypto.MasterSecretUtil; +import org.thoughtcrime.securesms.jobs.UnableToStartException; import org.thoughtcrime.securesms.migrations.LegacyMigrationJob; import org.thoughtcrime.securesms.service.GenericForegroundService; import org.thoughtcrime.securesms.service.NotificationController; @@ -51,7 +52,7 @@ public class SQLCipherMigrationHelper { copyTable("recipient_preferences", legacyDb, modernDb, null); copyTable("group_receipts", legacyDb, modernDb, null); modernDb.setTransactionSuccessful(); - } catch (GenericForegroundService.UnableToStartException e) { + } catch (UnableToStartException e) { throw new IllegalStateException(e); } finally { modernDb.endTransaction(); @@ -176,7 +177,7 @@ public class SQLCipherMigrationHelper { AttachmentSecretProvider.getInstance(context).setClassicKey(context, masterSecret.getEncryptionKey().getEncoded(), masterSecret.getMacKey().getEncoded()); TextSecurePreferences.setNeedsSqlCipherMigration(context, false); modernDb.setTransactionSuccessful(); - } catch (GenericForegroundService.UnableToStartException e) { + } catch (UnableToStartException e) { throw new IllegalStateException(e); } finally { modernDb.endTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/exporter/SignalSmsExportService.kt b/app/src/main/java/org/thoughtcrime/securesms/exporter/SignalSmsExportService.kt index 81c4bea9e..34eb012c8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/exporter/SignalSmsExportService.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/exporter/SignalSmsExportService.kt @@ -3,7 +3,6 @@ package org.thoughtcrime.securesms.exporter import android.content.Context import android.content.Intent import androidx.core.app.NotificationCompat -import androidx.core.content.ContextCompat import app.cash.exhaustive.Exhaustive import org.signal.core.util.PendingIntentFlags import org.signal.smsexporter.ExportableMessage @@ -15,6 +14,7 @@ import org.thoughtcrime.securesms.database.model.MessageId import org.thoughtcrime.securesms.database.model.databaseprotos.MessageExportState import org.thoughtcrime.securesms.dependencies.ApplicationDependencies import org.thoughtcrime.securesms.exporter.flow.SmsExportActivity +import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil import org.thoughtcrime.securesms.notifications.NotificationChannels import org.thoughtcrime.securesms.notifications.NotificationIds import org.thoughtcrime.securesms.notifications.v2.NotificationPendingIntentHelper @@ -34,7 +34,7 @@ class SignalSmsExportService : SmsExportService() { fun start(context: Context, clearPreviousExportState: Boolean) { val intent = Intent(context, SignalSmsExportService::class.java) .apply { putExtra(CLEAR_PREVIOUS_EXPORT_STATE_EXTRA, clearPreviousExportState) } - ContextCompat.startForegroundService(context, intent) + ForegroundServiceUtil.startOrThrow(context, intent) } } 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 fa039b60c..7a8fc9d4c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/gcm/FcmFetchManager.kt @@ -3,10 +3,10 @@ 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.ForegroundServiceUtil import org.thoughtcrime.securesms.jobs.PushNotificationReceiveJob import org.thoughtcrime.securesms.messages.RestStrategy import org.thoughtcrime.securesms.util.concurrent.SerialMonoLifoExecutor @@ -48,7 +48,7 @@ object FcmFetchManager { try { if (foreground) { Log.i(TAG, "Starting in the foreground.") - ContextCompat.startForegroundService(context, Intent(context, FcmFetchForegroundService::class.java)) + ForegroundServiceUtil.startWhenCapableOrThrow(context, Intent(context, FcmFetchForegroundService::class.java)) startedForeground = true } else { Log.i(TAG, "Starting in the background.") diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java index 68f583eb1..a785db2dd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java @@ -30,7 +30,6 @@ import org.thoughtcrime.securesms.mms.MediaConstraints; import org.thoughtcrime.securesms.mms.MediaStream; import org.thoughtcrime.securesms.mms.MmsException; import org.thoughtcrime.securesms.mms.SentMediaQuality; -import org.thoughtcrime.securesms.service.GenericForegroundService; import org.thoughtcrime.securesms.service.NotificationController; import org.thoughtcrime.securesms.transport.UndeliverableMessageException; import org.thoughtcrime.securesms.util.BitmapDecodingException; @@ -205,7 +204,7 @@ public final class AttachmentCompressionJob extends BaseJob { return attachment; } - try (NotificationController notification = ForegroundUtil.requireForegroundTask(context, context.getString(R.string.AttachmentUploadJob_compressing_video_start))) { + try (NotificationController notification = ForegroundServiceUtil.startGenericTaskWhenCapable(context, context.getString(R.string.AttachmentUploadJob_compressing_video_start))) { notification.setIndeterminateProgress(); @@ -290,7 +289,7 @@ public final class AttachmentCompressionJob extends BaseJob { throw new UndeliverableMessageException("Failed to transcode and cannot skip due to editing", e); } } - } catch (GenericForegroundService.UnableToStartException | IOException | MmsException e) { + } catch (UnableToStartException | IOException | MmsException e) { throw new UndeliverableMessageException("Failed to transcode", e); } return attachment; diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java index bdb4fb3ab..88543f4f5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java @@ -25,7 +25,6 @@ import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint; import org.thoughtcrime.securesms.mms.PartAuthority; import org.thoughtcrime.securesms.net.NotPushRegisteredException; import org.thoughtcrime.securesms.recipients.Recipient; -import org.thoughtcrime.securesms.service.GenericForegroundService; import org.thoughtcrime.securesms.service.NotificationController; import org.thoughtcrime.securesms.util.MediaUtil; import org.whispersystems.signalservice.api.SignalServiceMessageSender; @@ -160,8 +159,8 @@ public final class AttachmentUploadJob extends BaseJob { private @Nullable NotificationController getNotificationForAttachment(@NonNull Attachment attachment) { if (attachment.getSize() >= FOREGROUND_LIMIT) { try { - return ForegroundUtil.requireForegroundTask(context, context.getString(R.string.AttachmentUploadJob_uploading_media)); - } catch (GenericForegroundService.UnableToStartException e) { + return ForegroundServiceUtil.startGenericTaskWhenCapable(context, context.getString(R.string.AttachmentUploadJob_uploading_media)); + } catch (UnableToStartException e) { Log.w(TAG, "Unable to start foreground service", e); return null; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundServiceUtil.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundServiceUtil.kt new file mode 100644 index 000000000..2b3bde2ab --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundServiceUtil.kt @@ -0,0 +1,166 @@ +package org.thoughtcrime.securesms.jobs + +import android.app.AlarmManager +import android.app.ForegroundServiceStartNotAllowedException +import android.app.PendingIntent +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.os.Build +import android.os.SystemClock +import androidx.core.content.ContextCompat +import org.signal.core.util.PendingIntentFlags +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies +import org.thoughtcrime.securesms.service.GenericForegroundService +import org.thoughtcrime.securesms.service.NotificationController +import org.thoughtcrime.securesms.util.ServiceUtil +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.locks.ReentrantLock +import kotlin.concurrent.withLock +import kotlin.time.Duration.Companion.minutes + +/** + * Helps start foreground services from the background. + */ +object ForegroundServiceUtil { + + private val TAG = Log.tag(ForegroundServiceUtil::class.java) + + private val updateMutex: ReentrantLock = ReentrantLock() + private var activeLatch: CountDownLatch? = null + + private val DEFAULT_TIMEOUT: Long = 1.minutes.inWholeMilliseconds + + /** + * A simple wrapper around [ContextCompat.startForegroundService], but makes the possible failure part of the contract by forcing the caller to handle the + * [UnableToStartException]. + */ + @JvmStatic + @Throws(UnableToStartException::class) + fun start(context: Context, intent: Intent) { + if (Build.VERSION.SDK_INT < 31) { + ContextCompat.startForegroundService(context, intent) + } else { + try { + ContextCompat.startForegroundService(context, intent) + } catch (e: IllegalStateException) { + if (e is ForegroundServiceStartNotAllowedException) { + Log.e(TAG, "Unable to start foreground service", e) + throw UnableToStartException(e) + } else { + throw e + } + } + } + } + + /** + * Literally just a wrapper around [ContextCompat.startForegroundService], but with a name to make the caller understand that this method could throw. + */ + @JvmStatic + fun startOrThrow(context: Context, intent: Intent) { + ContextCompat.startForegroundService(context, intent) + } + + /** + * Does its best to start a foreground service, including possibly blocking and waiting until we are able to. + * However, it is always possible that the attempt will fail, so be sure to handle the [UnableToStartException]. + * + * @param timeout The maximum time you're willing to wait to create the conditions for a foreground service to start. + */ + @JvmOverloads + @JvmStatic + @Throws(UnableToStartException::class) + fun startWhenCapable(context: Context, intent: Intent, timeout: Long = DEFAULT_TIMEOUT) { + try { + start(context, intent) + } catch (e: UnableToStartException) { + Log.w(TAG, "Failed to start normally. Blocking and then trying again.") + blockUntilCapable(context, timeout) + start(context, intent) + } + } + + /** + * Identical to [startWhenCapable], except in this case we just throw if we're unable to start the service. Should only be used if there's no good way + * to gracefully handle the exception (or if you know for sure it won't get thrown). + * + * @param timeout The maximum time you're willing to wait to create the conditions for a foreground service to start. + */ + @JvmOverloads + @JvmStatic + fun startWhenCapableOrThrow(context: Context, intent: Intent, timeout: Long = DEFAULT_TIMEOUT) { + try { + startWhenCapable(context, intent, timeout) + } catch (e: UnableToStartException) { + throw IllegalStateException(e) + } + } + + /** + * Does its best to start a foreground service with your task name, including possibly blocking and waiting until we are able to. + * However, it is always possible that the attempt will fail, so always handle the [UnableToStartException]. + */ + @Throws(UnableToStartException::class) + @JvmStatic + fun startGenericTaskWhenCapable(context: Context, task: String): NotificationController { + blockUntilCapable(context) + return GenericForegroundService.startForegroundTask(context, task) + } + + /** + * Waits until we're capable of starting a foreground service. + * @return True if you should expect to be able to start a foreground service, otherwise false. Please note that this isn't *guaranteed*, just what we believe + * given the information we have. + */ + private fun blockUntilCapable(context: Context, timeout: Long = DEFAULT_TIMEOUT): Boolean { + val alarmManager = ServiceUtil.getAlarmManager(context) + + if (Build.VERSION.SDK_INT < 31 || ApplicationDependencies.getAppForegroundObserver().isForegrounded) { + return true + } + + if (!alarmManager.canScheduleExactAlarms()) { + return false + } + + val latch: CountDownLatch? = updateMutex.withLock { + if (activeLatch == null) { + if (alarmManager.canScheduleExactAlarms()) { + activeLatch = CountDownLatch(1) + val pendingIntent = PendingIntent.getBroadcast(context, 0, Intent(context, Receiver::class.java), PendingIntentFlags.mutable()) + alarmManager.setExact(AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + 1000, pendingIntent) + } else { + Log.w(TAG, "Unable to schedule alarm") + } + } + activeLatch + } + + if (latch != null) { + try { + if (!latch.await(timeout, TimeUnit.MILLISECONDS)) { + Log.w(TAG, "Time ran out waiting for foreground") + return false + } + } catch (e: InterruptedException) { + Log.w(TAG, "Interrupted while waiting for foreground") + } + } + + return true + } + + class Receiver : BroadcastReceiver() { + override fun onReceive(context: Context?, intent: Intent?) { + updateMutex.withLock { + activeLatch?.countDown() + activeLatch = null + } + } + } +} + +class UnableToStartException(cause: Throwable) : Exception(cause) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundUtil.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundUtil.kt deleted file mode 100644 index 5f91f8ddd..000000000 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/ForegroundUtil.kt +++ /dev/null @@ -1,74 +0,0 @@ -package org.thoughtcrime.securesms.jobs - -import android.app.AlarmManager -import android.app.PendingIntent -import android.content.BroadcastReceiver -import android.content.Context -import android.content.Intent -import android.os.Build -import android.os.SystemClock -import org.signal.core.util.PendingIntentFlags -import org.signal.core.util.logging.Log -import org.thoughtcrime.securesms.dependencies.ApplicationDependencies -import org.thoughtcrime.securesms.service.GenericForegroundService -import org.thoughtcrime.securesms.service.NotificationController -import org.thoughtcrime.securesms.util.ServiceUtil -import java.util.concurrent.CountDownLatch -import java.util.concurrent.TimeUnit -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock - -/** - * Helps start foreground services from the background. - */ -object ForegroundUtil { - - private val TAG = Log.tag(ForegroundUtil::class.java) - - private val updateMutex: ReentrantLock = ReentrantLock() - private var activeLatch: CountDownLatch? = null - - @Throws(GenericForegroundService.UnableToStartException::class) - @JvmStatic - fun requireForegroundTask(context: Context, task: String): NotificationController { - val alarmManager = ServiceUtil.getAlarmManager(context) - - if (Build.VERSION.SDK_INT < 31 || ApplicationDependencies.getAppForegroundObserver().isForegrounded || !alarmManager.canScheduleExactAlarms()) { - return GenericForegroundService.startForegroundTask(context, task) - } - - val latch: CountDownLatch? = updateMutex.withLock { - if (activeLatch == null) { - if (alarmManager.canScheduleExactAlarms()) { - activeLatch = CountDownLatch(1) - val pendingIntent = PendingIntent.getBroadcast(context, 0, Intent(context, Receiver::class.java), PendingIntentFlags.mutable()) - alarmManager.setExact(AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + 1000, pendingIntent) - } else { - Log.w(TAG, "Unable to schedule alarm") - } - } - activeLatch - } - - if (latch != null) { - try { - if (!latch.await(1, TimeUnit.MINUTES)) { - Log.w(TAG, "Time ran out waiting for foreground") - } - } catch (e: InterruptedException) { - Log.w(TAG, "Interrupted while waiting for foreground") - } - } - - return GenericForegroundService.startForegroundTask(context, task) - } - - class Receiver : BroadcastReceiver() { - override fun onReceive(context: Context?, intent: Intent?) { - updateMutex.withLock { - activeLatch?.countDown() - activeLatch = null - } - } - } -} diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJob.java index fa76ae9a9..6e310d164 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJob.java @@ -165,7 +165,7 @@ public final class LocalBackupJob extends BaseJob { } BackupUtil.deleteOldBackups(); - } catch (GenericForegroundService.UnableToStartException e) { + } catch (UnableToStartException e) { Log.w(TAG, "This should not happen on API < 31"); throw new AssertionError(e); } finally { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJobApi29.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJobApi29.java index fdaed3aa1..bbd4b0e2d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJobApi29.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/LocalBackupJobApi29.java @@ -6,7 +6,6 @@ import android.net.Uri; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.annotation.RequiresApi; import androidx.documentfile.provider.DocumentFile; import androidx.documentfile.provider.DocumentFileHelper; @@ -101,7 +100,7 @@ public final class LocalBackupJobApi29 extends BaseJob { context.getString(R.string.LocalBackupJob_creating_signal_backup), NotificationChannels.BACKUPS, R.drawable.ic_signal_backup); - } catch (GenericForegroundService.UnableToStartException e) { + } catch (UnableToStartException e) { Log.w(TAG, "Unable to start foreground backup service, continuing without service"); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.java b/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.java index b0f876549..eca633098 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.java +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.java @@ -12,7 +12,6 @@ import android.os.IBinder; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.app.NotificationCompat; -import androidx.core.content.ContextCompat; import org.signal.core.util.ThreadUtil; import org.signal.core.util.concurrent.SignalExecutors; @@ -21,7 +20,9 @@ import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.jobmanager.impl.BackoffUtil; import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint; +import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil; import org.thoughtcrime.securesms.jobs.PushDecryptDrainedJob; +import org.thoughtcrime.securesms.jobs.UnableToStartException; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.messages.IncomingMessageProcessor.Processor; import org.thoughtcrime.securesms.notifications.NotificationChannels; @@ -82,7 +83,11 @@ public class IncomingMessageObserver { new MessageRetrievalThread().start(); if (!SignalStore.account().isFcmEnabled() || SignalStore.internalValues().isWebsocketModeForced()) { - ContextCompat.startForegroundService(context, new Intent(context, ForegroundService.class)); + try { + ForegroundServiceUtil.startWhenCapable(context, new Intent(context, ForegroundService.class)); + } catch (UnableToStartException e) { + Log.w(TAG, "Unable to start foreground service for websocket!", e); + } } ApplicationDependencies.getAppForegroundObserver().addListener(new AppForegroundObserver.Listener() { diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/GenericForegroundService.java b/app/src/main/java/org/thoughtcrime/securesms/service/GenericForegroundService.java index 3b99da9e7..9572333a0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/GenericForegroundService.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/GenericForegroundService.java @@ -1,6 +1,5 @@ package org.thoughtcrime.securesms.service; -import android.app.ForegroundServiceStartNotAllowedException; import android.app.PendingIntent; import android.app.Service; import android.content.Context; @@ -19,6 +18,8 @@ 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.jobs.UnableToStartException; import org.thoughtcrime.securesms.notifications.NotificationChannels; import org.whispersystems.signalservice.api.util.Preconditions; @@ -173,20 +174,7 @@ public final class GenericForegroundService extends Service { Log.i(TAG, String.format(Locale.US, "Starting foreground service (%s) id=%d", task, id)); - if (Build.VERSION.SDK_INT < 31) { - ContextCompat.startForegroundService(context, intent); - } else { - try { - ContextCompat.startForegroundService(context, intent); - } catch (IllegalStateException e) { - if (e instanceof ForegroundServiceStartNotAllowedException) { - Log.e(TAG, "Unable to start foreground service", e); - throw new UnableToStartException(e); - } else { - throw e; - } - } - } + ForegroundServiceUtil.start(context, intent); return new NotificationController(context, id); } @@ -197,7 +185,7 @@ public final class GenericForegroundService extends Service { intent.putExtra(EXTRA_ID, id); Log.i(TAG, String.format(Locale.US, "Stopping foreground service id=%d", id)); - ContextCompat.startForegroundService(context, intent); + ForegroundServiceUtil.startWhenCapableOrThrow(context, intent); } synchronized void replaceTitle(int id, @NonNull String title) { @@ -325,10 +313,4 @@ public final class GenericForegroundService extends Service { return GenericForegroundService.this; } } - - public static final class UnableToStartException extends Exception { - public UnableToStartException(Throwable cause) { - super(cause); - } - } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/WebRtcCallService.java b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/WebRtcCallService.java index 779b83b04..4809bebe1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/WebRtcCallService.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/WebRtcCallService.java @@ -17,11 +17,11 @@ import android.telephony.TelephonyManager; import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.core.content.ContextCompat; import org.signal.core.util.ThreadUtil; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.util.TelephonyUtil; @@ -58,6 +58,7 @@ public final class WebRtcCallService extends Service implements SignalAudioManag private static final int INVALID_NOTIFICATION_ID = -1; private static final long REQUEST_WEBSOCKET_STAY_OPEN_DELAY = TimeUnit.MINUTES.toMillis(1); + private static final long FOREGROUND_SERVICE_TIMEOUT = TimeUnit.SECONDS.toMillis(10); private final WebSocketKeepAliveTask webSocketKeepAliveTask = new WebSocketKeepAliveTask(); @@ -78,22 +79,22 @@ public final class WebRtcCallService extends Service implements SignalAudioManag .putExtra(EXTRA_UPDATE_TYPE, type) .putExtra(EXTRA_RECIPIENT_ID, recipientId); - ContextCompat.startForegroundService(context, intent); + ForegroundServiceUtil.startWhenCapableOrThrow(context, intent, FOREGROUND_SERVICE_TIMEOUT); } public static void denyCall(@NonNull Context context) { - ContextCompat.startForegroundService(context, denyCallIntent(context)); + ForegroundServiceUtil.startWhenCapableOrThrow(context, denyCallIntent(context), FOREGROUND_SERVICE_TIMEOUT); } public static void hangup(@NonNull Context context) { - ContextCompat.startForegroundService(context, hangupIntent(context)); + ForegroundServiceUtil.startWhenCapableOrThrow(context, hangupIntent(context), FOREGROUND_SERVICE_TIMEOUT); } public static void stop(@NonNull Context context) { Intent intent = new Intent(context, WebRtcCallService.class); intent.setAction(ACTION_STOP); - ContextCompat.startForegroundService(context, intent); + ForegroundServiceUtil.startWhenCapableOrThrow(context, intent, FOREGROUND_SERVICE_TIMEOUT); } public static @NonNull Intent denyCallIntent(@NonNull Context context) { @@ -108,7 +109,7 @@ public final class WebRtcCallService extends Service implements SignalAudioManag Intent intent = new Intent(context, WebRtcCallService.class); intent.setAction(ACTION_SEND_AUDIO_COMMAND) .putExtra(EXTRA_AUDIO_COMMAND, command); - ContextCompat.startForegroundService(context, intent); + ForegroundServiceUtil.startWhenCapableOrThrow(context, intent, FOREGROUND_SERVICE_TIMEOUT); } public static void changePowerButtonReceiver(@NonNull Context context, boolean register) { @@ -116,7 +117,7 @@ public final class WebRtcCallService extends Service implements SignalAudioManag intent.setAction(ACTION_CHANGE_POWER_BUTTON) .putExtra(EXTRA_ENABLED, register); - ContextCompat.startForegroundService(context, intent); + ForegroundServiceUtil.startWhenCapableOrThrow(context, intent, FOREGROUND_SERVICE_TIMEOUT); } @Override diff --git a/lintchecks/src/main/java/org/signal/lint/Registry.java b/lintchecks/src/main/java/org/signal/lint/Registry.java index 82f664aa4..3fdc026a3 100644 --- a/lintchecks/src/main/java/org/signal/lint/Registry.java +++ b/lintchecks/src/main/java/org/signal/lint/Registry.java @@ -19,7 +19,8 @@ public final class Registry extends IssueRegistry { AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE, BlockingGetDetector.UNSAFE_BLOCKING_GET, RecipientIdDatabaseDetector.RECIPIENT_ID_DATABASE_REFERENCE_ISSUE, - ThreadIdDatabaseDetector.THREAD_ID_DATABASE_REFERENCE_ISSUE); + ThreadIdDatabaseDetector.THREAD_ID_DATABASE_REFERENCE_ISSUE, + StartForegroundServiceDetector.START_FOREGROUND_SERVICE_ISSUE); } @Override diff --git a/lintchecks/src/main/java/org/signal/lint/StartForegroundServiceDetector.java b/lintchecks/src/main/java/org/signal/lint/StartForegroundServiceDetector.java new file mode 100644 index 000000000..7c021e91a --- /dev/null +++ b/lintchecks/src/main/java/org/signal/lint/StartForegroundServiceDetector.java @@ -0,0 +1,53 @@ +package org.signal.lint; + +import com.android.tools.lint.client.api.JavaEvaluator; +import com.android.tools.lint.detector.api.Category; +import com.android.tools.lint.detector.api.Detector; +import com.android.tools.lint.detector.api.Implementation; +import com.android.tools.lint.detector.api.Issue; +import com.android.tools.lint.detector.api.JavaContext; +import com.android.tools.lint.detector.api.LintFix; +import com.android.tools.lint.detector.api.Scope; +import com.android.tools.lint.detector.api.Severity; +import com.intellij.psi.PsiMethod; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.uast.UCallExpression; +import org.jetbrains.uast.UClass; +import org.jetbrains.uast.UExpression; + +import java.util.Arrays; +import java.util.List; + +@SuppressWarnings("UnstableApiUsage") +public final class StartForegroundServiceDetector extends Detector implements Detector.UastScanner { + + static final Issue START_FOREGROUND_SERVICE_ISSUE = Issue.create("StartForegroundServiceUsage", + "Starting a foreground service using ContextCompat.startForegroundService instead of ForegroundServiceUtil", + "Starting a foreground service may fail, and we should prefer our utils to make sure they're started correctly", + Category.MESSAGES, + 5, + Severity.ERROR, + new Implementation(StartForegroundServiceDetector.class, Scope.JAVA_FILE_SCOPE)); + + @Override + public List getApplicableMethodNames() { + return Arrays.asList("startForegroundService"); + } + + @Override + public void visitMethodCall(@NotNull JavaContext context, @NotNull UCallExpression call, @NotNull PsiMethod method) { + JavaEvaluator evaluator = context.getEvaluator(); + + if (context.getUastFile() != null && context.getUastFile().getClasses().stream().anyMatch(it -> "ForegroundServiceUtil".equals(it.getName()))) { + return; + } + + if (evaluator.isMemberInClass(method, "androidx.core.content.ContextCompat")) { + context.report(START_FOREGROUND_SERVICE_ISSUE, call, context.getLocation(call), "Using 'ContextCompat.startForegroundService' instead of a ForegroundServiceUtil"); + } else if (evaluator.isMemberInClass(method, "android.content.Context")) { + context.report(START_FOREGROUND_SERVICE_ISSUE, call, context.getLocation(call), "Using 'Context.startForegroundService' instead of a ForegroundServiceUtil"); + } + } +} \ No newline at end of file diff --git a/lintchecks/src/test/java/org/signal/lint/StartForegroundServiceDetectorTest.java b/lintchecks/src/test/java/org/signal/lint/StartForegroundServiceDetectorTest.java new file mode 100644 index 000000000..23c0d23a7 --- /dev/null +++ b/lintchecks/src/test/java/org/signal/lint/StartForegroundServiceDetectorTest.java @@ -0,0 +1,75 @@ +package org.signal.lint; + +import com.android.tools.lint.checks.infrastructure.TestFile; + +import org.junit.Test; + +import java.io.InputStream; +import java.util.Scanner; + +import static com.android.tools.lint.checks.infrastructure.TestFiles.java; +import static com.android.tools.lint.checks.infrastructure.TestFiles.kotlin; +import static com.android.tools.lint.checks.infrastructure.TestLintTask.lint; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("UnstableApiUsage") +public final class StartForegroundServiceDetectorTest { + + private static final TestFile contextCompatStub = java(readResourceAsString("ContextCompatStub.java")); + private static final TestFile contextStub = java(readResourceAsString("ContextStub.java")); + + @Test + public void contextCompatUsed() { + lint() + .files( + contextCompatStub, + java("package foo;\n" + + "import androidx.core.content.ContextCompat;\n" + + "public class Example {\n" + + " public void start() {\n" + + " ContextCompat.startForegroundService(context, new Intent());\n" + + " }\n" + + "}") + ) + .allowMissingSdk() + .issues(StartForegroundServiceDetector.START_FOREGROUND_SERVICE_ISSUE) + .run() + .expect("src/foo/Example.java:5: Error: Using 'ContextCompat.startForegroundService' instead of a ForegroundServiceUtil [StartForegroundServiceUsage]\n" + + " ContextCompat.startForegroundService(context, new Intent());\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings"); + } + + @Test + public void contextUsed() { + lint() + .files( + contextStub, + java("package foo;\n" + + "import android.content.Context;\n" + + "public class Example {\n" + + " Context context;\n" + + " public void start() {\n" + + " context.startForegroundService(new Intent());\n" + + " }\n" + + "}") + ) + .allowMissingSdk() + .issues(StartForegroundServiceDetector.START_FOREGROUND_SERVICE_ISSUE) + .run() + .expect("src/foo/Example.java:6: Error: Using 'Context.startForegroundService' instead of a ForegroundServiceUtil [StartForegroundServiceUsage]\n" + + " context.startForegroundService(new Intent());\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings"); + } + + + private static String readResourceAsString(String resourceName) { + InputStream inputStream = ClassLoader.getSystemClassLoader().getResourceAsStream(resourceName); + assertNotNull(inputStream); + Scanner scanner = new Scanner(inputStream).useDelimiter("\\A"); + assertTrue(scanner.hasNext()); + return scanner.next(); + } +} diff --git a/lintchecks/src/test/resources/ContextCompatStub.java b/lintchecks/src/test/resources/ContextCompatStub.java new file mode 100644 index 000000000..be0143012 --- /dev/null +++ b/lintchecks/src/test/resources/ContextCompatStub.java @@ -0,0 +1,6 @@ +package androidx.core.content; + +public class ContextCompat { + public static void startForegroundService(Context context, Intent intent) { + } +} diff --git a/lintchecks/src/test/resources/ContextStub.java b/lintchecks/src/test/resources/ContextStub.java new file mode 100644 index 000000000..10fc09ed3 --- /dev/null +++ b/lintchecks/src/test/resources/ContextStub.java @@ -0,0 +1,6 @@ +package android.content; + +public class Context { + public void startForegroundService(Intent intent) { + } +}