Cleanup FetchPushForegroundService (#6577)

* Rename `PushHandlingWakeLock` to `FetchPushForegroundServiceManager`. Move the start/stop logic from `FetchPushForegroundService.Companion` to it.

* Add more tests using Robolectric.

* Remove `FeatureFlags.SyncNotificationsWithWorkManager` and associated code: this should have been removed in one of the previous refactors, since we don't have the 2 ways to sync notifications anymore, everything uses the `WorkManager`

---------

Co-authored-by: Benoit Marty <benoit@matrix.org>
This commit is contained in:
Jorge Martin Espinosa 2026-04-20 16:03:12 +02:00 committed by GitHub
parent 8853f160e2
commit f80a140cf9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 329 additions and 215 deletions

View file

@ -13,8 +13,6 @@ import dev.zacsweers.metro.SingleIn
import io.element.android.features.call.api.CallType
import io.element.android.features.call.api.ElementCallEntryPoint
import io.element.android.libraries.di.annotations.AppCoroutineScope
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
@ -32,7 +30,6 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv
import io.element.android.libraries.push.impl.push.MutableBatteryOptimizationStore
import io.element.android.libraries.push.impl.push.OnNotifiableEventReceived
import io.element.android.libraries.push.impl.push.OnRedactedEventReceived
import io.element.android.libraries.push.impl.push.SyncOnNotifiableEvent
import io.element.android.libraries.pushstore.api.UserPushStoreFactory
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
@ -60,8 +57,6 @@ class DefaultNotificationResultProcessor(
private val userPushStoreFactory: UserPushStoreFactory,
private val onRedactedEventReceived: OnRedactedEventReceived,
private val onNotifiableEventReceived: OnNotifiableEventReceived,
private val featureFlagService: FeatureFlagService,
private val syncOnNotifiableEvent: SyncOnNotifiableEvent,
private val elementCallEntryPoint: ElementCallEntryPoint,
private val notificationChannels: NotificationChannels,
@AppCoroutineScope private val coroutineScope: CoroutineScope,
@ -215,10 +210,6 @@ class DefaultNotificationResultProcessor(
if (nonRingingCallEvents.isNotEmpty()) {
onNotifiableEventReceived.onNotifiableEventsReceived(nonRingingCallEvents)
}
if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncNotificationsWithWorkManager)) {
syncOnNotifiableEvent(results.keys.toList())
}
}
private suspend fun handleRingingCallEvent(notifiableEvent: NotifiableRingingCallEvent) {

View file

@ -0,0 +1,95 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.push.impl.push
import android.app.ActivityManager
import android.content.Context
import android.content.Context.ACTIVITY_SERVICE
import android.content.Context.POWER_SERVICE
import android.content.Intent
import android.os.Build
import android.os.PowerManager
import androidx.core.content.ContextCompat
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.di.annotations.ApplicationContext
import io.element.android.libraries.push.api.push.FetchPushForegroundServiceManager
import kotlinx.coroutines.delay
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withTimeoutOrNull
import timber.log.Timber
import kotlin.time.Duration.Companion.seconds
@ContributesBinding(AppScope::class)
@SingleIn(AppScope::class)
class DefaultFetchPushForegroundServiceManager(
@ApplicationContext private val context: Context,
) : FetchPushForegroundServiceManager {
private val stopMutex = Mutex()
override fun start(): Boolean {
Timber.d("Acquiring wakelock for push handling, starting service.")
// Don't start the foreground service if the device is already awake
val powerManager = context.getSystemService(POWER_SERVICE) as PowerManager
if (powerManager.isInteractive) {
Timber.d("Device is already in an interactive state, no need to start FetchPushForegroundService")
return false
}
val intent = Intent(context, FetchPushForegroundService::class.java)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
runCatchingExceptions { ContextCompat.startForegroundService(context, intent) }
.onFailure { throwable ->
Timber.e(throwable, "Failed to start FetchPushForegroundService, notifications may take longer than usual to sync")
}
} else {
context.startService(intent)
}
return true
}
override suspend fun stop(): Boolean {
Timber.d("Releasing wakelock used for push handling, stopping service.")
return stopMutex.withLock {
val runningServiceInfo = getRunningServiceInfo(context)
if (runningServiceInfo != null) {
val intent = Intent(context, FetchPushForegroundService::class.java)
// If it's still not running in foreground, it means the service is still starting,
// so we delay the stop to give it time to start and be set as foreground, otherwise we can crash
// with `ForegroundServiceDidNotStartInTimeException`.
var isInForeground = runningServiceInfo.foreground
withTimeoutOrNull(5.seconds) {
while (!isInForeground) {
delay(50)
val updatedServiceInfo = getRunningServiceInfo(context)
if (updatedServiceInfo == null) {
Timber.d("FetchPushForegroundService is no longer running, no need to stop it.")
return@withTimeoutOrNull
}
isInForeground = updatedServiceInfo.foreground == true
}
} ?: Timber.w("FetchPushForegroundService did not start in foreground after 5s, stopping it anyway.")
context.stopService(intent)
} else {
false
}
}
}
@Suppress("DEPRECATION")
private fun getRunningServiceInfo(context: Context): ActivityManager.RunningServiceInfo? {
val activityManager = context.getSystemService(ACTIVITY_SERVICE) as ActivityManager
return activityManager.getRunningServices(Int.MAX_VALUE)
.firstOrNull { it.service.className == FetchPushForegroundService::class.java.name }
}
}

View file

@ -1,33 +0,0 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.push.impl.push
import android.content.Context
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.di.annotations.ApplicationContext
import io.element.android.libraries.push.api.push.PushHandlingWakeLock
import timber.log.Timber
import kotlin.time.Duration
@ContributesBinding(AppScope::class)
@SingleIn(AppScope::class)
class DefaultPushHandlingWakeLock(
@ApplicationContext private val context: Context,
) : PushHandlingWakeLock {
override fun lock(time: Duration) {
Timber.d("Acquiring wakelock for push handling, starting service.")
FetchPushForegroundService.startIfNeeded(context)
}
override suspend fun unlock() {
Timber.d("Releasing wakelock used for push handling.")
FetchPushForegroundService.stop(context)
}
}

View file

@ -7,32 +7,27 @@
package io.element.android.libraries.push.impl.push
import android.app.ActivityManager
import android.app.Service
import android.content.Context
import android.content.Intent
import android.content.pm.ServiceInfo
import android.os.Build
import android.os.IBinder
import android.os.PowerManager
import androidx.core.app.NotificationCompat
import androidx.core.app.ServiceCompat
import dev.zacsweers.metro.Inject
import io.element.android.libraries.architecture.bindings
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.designsystem.utils.CommonDrawables
import io.element.android.libraries.di.annotations.AppCoroutineScope
import io.element.android.libraries.push.api.push.PushHandlingWakeLock
import io.element.android.libraries.push.impl.di.PushBindings
import io.element.android.libraries.push.impl.notifications.channels.NotificationChannels
import io.element.android.libraries.ui.strings.CommonStrings
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withTimeoutOrNull
import timber.log.Timber
import kotlin.time.Duration.Companion.minutes
import kotlin.time.Duration.Companion.seconds
private const val NOTIFICATION_ID = 1001
@ -48,7 +43,6 @@ class FetchPushForegroundService : Service() {
}
@Inject lateinit var notificationChannels: NotificationChannels
@Inject lateinit var pushHandlingWakeLock: PushHandlingWakeLock
@Inject @AppCoroutineScope lateinit var coroutineScope: CoroutineScope
private val wakelock: PowerManager.WakeLock by lazy {
@ -78,8 +72,13 @@ class FetchPushForegroundService : Service() {
// Try to start the service in foreground. This can fail, even in cases where it's supposed to work according to the docs.
// In those cases we catch the exception and handle the failure so we don't try to start the wakelock or stop the service
// from running in foreground later.
val serviceType = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE
} else {
0
}
runCatchingExceptions {
startForeground(NOTIFICATION_ID, notificationCompat)
ServiceCompat.startForeground(this, NOTIFICATION_ID, notificationCompat, serviceType)
}
.onSuccess {
isOnForeground = true
@ -116,7 +115,7 @@ class FetchPushForegroundService : Service() {
override fun stopService(intent: Intent?): Boolean {
if (isOnForeground) {
wakelock.release()
stopForeground(STOP_FOREGROUND_REMOVE)
ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE)
}
return super.stopService(intent)
@ -131,64 +130,7 @@ class FetchPushForegroundService : Service() {
Timber.d("onTimeoutAction, calledByTheSystem: $calledByTheSystem, isOnForeground: $isOnForeground")
if (isOnForeground) {
Timber.d("Wakelock timeout reached, stopping FetchPushForegroundService")
coroutineScope.launch { pushHandlingWakeLock.unlock() }
}
}
companion object {
private val stopMutex = Mutex()
fun startIfNeeded(context: Context) {
// Don't start the foreground service if the device is already awake
val powerManager = context.getSystemService(POWER_SERVICE) as PowerManager
if (powerManager.isInteractive) return
start(context)
}
fun start(context: Context) {
val intent = Intent(context, FetchPushForegroundService::class.java)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
runCatchingExceptions { context.startForegroundService(intent) }
.onFailure { throwable ->
Timber.e(
throwable,
"Failed to start FetchPushForegroundService, notifications may take longer than usual to sync"
)
}
} else {
context.startService(intent)
}
}
suspend fun stop(context: Context) = stopMutex.withLock {
val runningServiceInfo = getRunningServiceInfo(context)
if (runningServiceInfo != null) {
val intent = Intent(context, FetchPushForegroundService::class.java)
// If it's still not running in foreground, it means the service is still starting,
// so we delay the stop to give it time to start and be set as foreground, otherwise we can crash
// with `ForegroundServiceDidNotStartInTimeException`.
var isInForeground = runningServiceInfo.foreground
withTimeoutOrNull(5.seconds) {
while (!isInForeground) {
delay(50)
val updatedServiceInfo = getRunningServiceInfo(context)
if (updatedServiceInfo == null) {
Timber.d("FetchPushForegroundService is no longer running, no need to stop it.")
return@withTimeoutOrNull
}
isInForeground = updatedServiceInfo.foreground == true
}
} ?: Timber.w("FetchPushForegroundService did not start in foreground after 5s, stopping it anyway.")
context.stopService(intent)
}
}
@Suppress("DEPRECATION")
private fun getRunningServiceInfo(context: Context): ActivityManager.RunningServiceInfo? {
val activityManager = context.getSystemService(ACTIVITY_SERVICE) as ActivityManager
return activityManager.getRunningServices(Int.MAX_VALUE)
.firstOrNull { it.service.className == FetchPushForegroundService::class.java.name }
stopSelf()
}
}
}

View file

@ -25,7 +25,7 @@ import io.element.android.libraries.matrix.api.auth.SessionRestorationException
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.exception.ClientException
import io.element.android.libraries.matrix.api.exception.isNetworkError
import io.element.android.libraries.push.api.push.PushHandlingWakeLock
import io.element.android.libraries.push.api.push.FetchPushForegroundServiceManager
import io.element.android.libraries.push.impl.db.PushRequest
import io.element.android.libraries.push.impl.history.PushHistoryService
import io.element.android.libraries.push.impl.notifications.NotifiableEventResolver
@ -58,7 +58,7 @@ class FetchPendingNotificationsWorker(
private val resultProcessor: NotificationResultProcessor,
private val analyticsService: AnalyticsService,
private val systemClock: SystemClock,
private val pushHandlingWakeLock: PushHandlingWakeLock,
private val fetchPushForegroundServiceManager: FetchPushForegroundServiceManager,
) : CoroutineWorker(context, params) {
override suspend fun doWork(): Result {
Timber.d("FetchNotificationsWorker started")
@ -67,7 +67,8 @@ class FetchPendingNotificationsWorker(
inputData.getString(SyncPendingNotificationsRequestBuilder.SESSION_ID)?.let(::SessionId)
}.getOrNull() ?: return Result.failure()
pushHandlingWakeLock.unlock()
// We can stop the foreground service and unlock the wakelock, since the work is now running and the device should be kept awake
fetchPushForegroundServiceManager.stop()
// Fetch pending requests in the last 24 hours
val fetchSince = Instant.fromEpochMilliseconds(systemClock.epochMillis()).minus(1.days)

View file

@ -10,7 +10,6 @@ package io.element.android.libraries.push.impl.notifications
import com.google.common.truth.Truth.assertThat
import io.element.android.features.call.api.CallType
import io.element.android.features.call.test.FakeElementCallEntryPoint
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
@ -34,7 +33,6 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv
import io.element.android.libraries.push.impl.push.FakeMutableBatteryOptimizationStore
import io.element.android.libraries.push.impl.push.FakeOnNotifiableEventReceived
import io.element.android.libraries.push.impl.push.FakeOnRedactedEventReceived
import io.element.android.libraries.push.impl.push.SyncOnNotifiableEvent
import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory
import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
import io.element.android.tests.testutils.lambda.any
@ -289,8 +287,6 @@ class DefaultNotificationResultProcessorTest {
userPushStoreFactory: FakeUserPushStoreFactory = FakeUserPushStoreFactory(),
onRedactedEventReceived: (List<ResolvedPushEvent.Redaction>) -> Unit = {},
onNotifiableEventsReceived: (List<NotifiableEvent>) -> Unit = {},
featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(),
syncOnNotifiableEvent: SyncOnNotifiableEvent = {},
elementCallEntryPoint: FakeElementCallEntryPoint = FakeElementCallEntryPoint(),
notificationChannels: FakeNotificationChannels = FakeNotificationChannels(),
coroutineScope: CoroutineScope = backgroundScope,
@ -301,8 +297,6 @@ class DefaultNotificationResultProcessorTest {
userPushStoreFactory = userPushStoreFactory,
onRedactedEventReceived = FakeOnRedactedEventReceived(onRedactedEventReceived),
onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventsReceived),
featureFlagService = featureFlagService,
syncOnNotifiableEvent = syncOnNotifiableEvent,
elementCallEntryPoint = elementCallEntryPoint,
notificationChannels = notificationChannels,
coroutineScope = coroutineScope,

View file

@ -0,0 +1,134 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.push.impl.push
import android.app.ActivityManager
import android.content.ComponentName
import android.content.Context.ACTIVITY_SERVICE
import android.content.Context.POWER_SERVICE
import android.os.PowerManager
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.async
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.withTimeout
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.Shadows
import org.robolectric.shadows.ShadowActivityManager
import org.robolectric.shadows.ShadowPowerManager
import kotlin.time.Duration.Companion.seconds
@RunWith(AndroidJUnit4::class)
class DefaultFetchPushForegroundServiceManagerTest {
@Test
fun `start should start the service if the device is not interactive`() {
val manager = createDefaultFetchPushForegroundServiceManager()
getShadowPowerManager().turnScreenOn(false)
assertThat(manager.start()).isTrue()
}
@Test
fun `start won't start the service if the device is interactive`() {
val manager = createDefaultFetchPushForegroundServiceManager()
getShadowPowerManager().turnScreenOn(true)
assertThat(manager.start()).isFalse()
}
@Test
fun `stop will stop the service if it's running`() = runTest {
val manager = createDefaultFetchPushForegroundServiceManager()
// Start the service first
getShadowPowerManager().turnScreenOn(false)
manager.start()
getShadowActivityManager().setServices(
listOf(
ActivityManager.RunningServiceInfo().apply {
service = ComponentName(InstrumentationRegistry.getInstrumentation().context, FetchPushForegroundService::class.java)
foreground = true
}
)
)
assertThat(manager.stop()).isTrue()
}
@Test
fun `stop will eventually stop the service once it's on foreground`() = runTest {
val manager = createDefaultFetchPushForegroundServiceManager()
// Start the service first
getShadowPowerManager().turnScreenOn(false)
manager.start()
// The service is started, but not yet in foreground
getShadowActivityManager().setServices(
listOf(
ActivityManager.RunningServiceInfo().apply {
service = ComponentName(InstrumentationRegistry.getInstrumentation().context, FetchPushForegroundService::class.java)
foreground = false
}
)
)
// We call stop, which won't stop the service yet since it's not in foreground
val future = async { manager.stop() }
// Then we set the service as running in foreground, which should allow the stop to complete
getShadowActivityManager().setServices(
listOf(
ActivityManager.RunningServiceInfo().apply {
service = ComponentName(InstrumentationRegistry.getInstrumentation().context, FetchPushForegroundService::class.java)
foreground = true
}
)
)
val stopped = withTimeout(5.seconds) { future.await() }
assertThat(stopped).isTrue()
}
@Test
fun `stop will not stop the service if it's stopped`() = runTest {
val manager = createDefaultFetchPushForegroundServiceManager()
// Set some fake running service data, even if the service is not really running
getShadowActivityManager().setServices(
listOf(
ActivityManager.RunningServiceInfo().apply {
service = ComponentName(InstrumentationRegistry.getInstrumentation().context, FetchPushForegroundService::class.java)
foreground = true
}
)
)
// Since the service was not really running, it was not stopped
assertThat(manager.stop()).isFalse()
}
private fun createDefaultFetchPushForegroundServiceManager() = DefaultFetchPushForegroundServiceManager(
context = InstrumentationRegistry.getInstrumentation().context,
)
private fun getShadowPowerManager(): ShadowPowerManager {
val powerManager = InstrumentationRegistry.getInstrumentation().context.getSystemService(POWER_SERVICE) as PowerManager
return Shadows.shadowOf(powerManager)
}
private fun getShadowActivityManager(): ShadowActivityManager {
val activityManager = InstrumentationRegistry.getInstrumentation().context.getSystemService(ACTIVITY_SERVICE) as ActivityManager
return Shadows.shadowOf(activityManager)
}
}

View file

@ -28,7 +28,7 @@ import io.element.android.libraries.push.impl.notifications.FakeNotificationResu
import io.element.android.libraries.push.impl.notifications.fixtures.aPushRequest
import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent
import io.element.android.libraries.push.impl.push.SyncOnNotifiableEvent
import io.element.android.libraries.push.test.push.FakePushHandlingWakeLock
import io.element.android.libraries.push.test.push.FakeFetchPushForegroundServiceManager
import io.element.android.libraries.workmanager.api.WorkManagerRequestBuilder
import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory
import io.element.android.services.analytics.test.FakeAnalyticsService
@ -239,7 +239,7 @@ class FetchPendingNotificationWorkerTest {
pushHistoryService: FakePushHistoryService = FakePushHistoryService(),
resultProcessor: FakeNotificationResultProcessor = FakeNotificationResultProcessor(),
systemClock: FakeSystemClock = FakeSystemClock(),
pushHandlingWakeLock: FakePushHandlingWakeLock = FakePushHandlingWakeLock(),
pushHandlingWakeLock: FakeFetchPushForegroundServiceManager = FakeFetchPushForegroundServiceManager(),
) = FetchPendingNotificationsWorker(
params = createWorkerParams(workDataOf("session_id" to input)),
context = InstrumentationRegistry.getInstrumentation().context,
@ -250,7 +250,7 @@ class FetchPendingNotificationWorkerTest {
pushHistoryService = pushHistoryService,
resultProcessor = resultProcessor,
systemClock = systemClock,
pushHandlingWakeLock = pushHandlingWakeLock,
fetchPushForegroundServiceManager = pushHandlingWakeLock,
)
private fun TestScope.createWorkerParams(