diff --git a/webtrit_callkeep_android/android/build.gradle b/webtrit_callkeep_android/android/build.gradle index b5ccdc96..5ee9bd00 100644 --- a/webtrit_callkeep_android/android/build.gradle +++ b/webtrit_callkeep_android/android/build.gradle @@ -68,6 +68,14 @@ android { main.java.srcDirs += 'src/main/kotlin' test.java.srcDirs += 'src/test/kotlin' } + + testOptions { + unitTests { + // Robolectric needs the library's own resources (notification strings/icons) + // to build service notifications in unit tests. + includeAndroidResources = true + } + } } kotlin { diff --git a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/managers/NotificationManager.kt b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/managers/NotificationManager.kt index ccc14398..3521d82c 100644 --- a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/managers/NotificationManager.kt +++ b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/managers/NotificationManager.kt @@ -46,7 +46,7 @@ class NotificationManager { if (activeCalls.isNotEmpty()) { val activeCallsBundles = activeCalls.map { it.toBundle() } val intent = Intent(context, ActiveCallService::class.java) - intent.putExtra("metadata", ArrayList(activeCallsBundles)) + intent.putExtra(ActiveCallService.EXTRA_CALLS_METADATA, ArrayList(activeCallsBundles)) context.startService(intent) } else { context.stopService(Intent(context, ActiveCallService::class.java)) diff --git a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallService.kt b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallService.kt index 772e447b..65d782f7 100644 --- a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallService.kt +++ b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallService.kt @@ -35,34 +35,95 @@ class ActiveCallService : Service() { ): Int { // Handle the hangup action from the notification if (NotificationAction.Decline.action == intent?.action) { - hungUpCall() + hungUpCall(intent.extras?.let(CallMetadata::fromBundleOrNull), startId) return START_NOT_STICKY } callsMetadata = intent - ?.parcelableArrayList("metadata") + ?.parcelableArrayList(EXTRA_CALLS_METADATA) ?.map { CallMetadata.fromBundle(it) } ?.toMutableList() ?: mutableListOf() activeCallNotificationBuilder.setCallsMetaData(callsMetadata) val notification = activeCallNotificationBuilder.build() - startForegroundServiceCompat( - this, - ActiveCallNotificationBuilder.NOTIFICATION_ID, - notification, - getForegroundServiceTypes(callsMetadata), - ) + // startForeground must be called even when there are no calls to show: the service may + // have been (re)started as foreground and skipping the promotion would kill the process + // with ForegroundServiceDidNotStartInTimeException. + // + // On Android 14+ the MICROPHONE type is rejected with SecurityException when the + // promotion happens from the background - which is exactly the START_STICKY restart + // after a process kill. Fall back to the phone-call type (not while-in-use restricted) + // so the promotion, and the empty-metadata guard below, run instead of crash-looping + // the restart. + try { + startForegroundServiceCompat( + this, + ActiveCallNotificationBuilder.NOTIFICATION_ID, + notification, + getForegroundServiceTypes(callsMetadata), + ) + } catch (e: SecurityException) { + Log.w(TAG, "onStartCommand: typed promotion rejected, falling back to phone-call type", e) + startForegroundServiceCompat( + this, + ActiveCallNotificationBuilder.NOTIFICATION_ID, + notification, + ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL, + ) + } + + if (callsMetadata.isEmpty()) { + // Empty metadata means a START_STICKY restart delivered a null intent after the + // process was killed. NotificationManager tracks active calls in its own static + // list, so nothing will ever stop this orphaned instance - an ongoing notification + // left here would be undismissable until the user force-stops the app. + // + // A call leg may have survived in :callkeep_core (Telecom keeps that process alive + // via its own binding), and this restart is the last signal the main process gets + // about it: tear the connection services down so the device is not left stuck in a + // zombie Telecom call with no UI to end it. Must run before stopForeground - while + // this service is still foreground the startService toward the connection services + // is exempt from background-start restrictions. + Log.w(TAG, "onStartCommand: no calls metadata (null-intent restart), tearing down and stopping self") + tearDownAndStop(startId) + return START_NOT_STICKY + } return START_STICKY } - private fun hungUpCall() = - callsMetadata.firstOrNull()?.let { - CallkeepCore.instance.startHungUpCall(it) - } ?: CallkeepCore.instance.tearDownService() + private fun hungUpCall( + intentMetadata: CallMetadata?, + startId: Int, + ) { + // The Decline PendingIntent carries the tapped call's bundle in its extras. When the tap + // creates a fresh service instance (process death cleared callsMetadata), that is the + // only way left to hang up the specific call instead of tearing everything down. + val call = callsMetadata.firstOrNull() ?: intentMetadata + if (call != null) { + CallkeepCore.instance.startHungUpCall(call) + } else { + // Hang up tapped on a notification with no known calls (re-posted by a null-intent + // restart). + Log.w(TAG, "hungUpCall: no calls metadata, tearing down and stopping self") + tearDownAndStop(startId) + } + } + + // Tears down the connection services (a call leg may have survived in :callkeep_core), + // removes the notification and stops this instance. tearDownService only tears down the + // connection services and does not stop this one, so the notification has to be removed + // here. stopSelf(startId), not stopSelf(): a newer start with real metadata may already + // be queued behind the current command (the recovering app re-posting its call list), and + // an unconditional stop would cancel it together with its foreground notification. + private fun tearDownAndStop(startId: Int) { + CallkeepCore.instance.tearDownService() + stopForeground(STOP_FOREGROUND_REMOVE) + stopSelf(startId) + } private fun getForegroundServiceTypes(callsMetadata: List): Int? = when { @@ -102,4 +163,12 @@ class ActiveCallService : Service() { stopForeground(STOP_FOREGROUND_REMOVE) super.onDestroy() } + + companion object { + // Intent extra carrying the ArrayList of active calls; the writing side is + // NotificationManager.upsertActiveCallsService. + const val EXTRA_CALLS_METADATA = "metadata" + + private const val TAG = "ActiveCallService" + } } diff --git a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/incoming_call/IncomingCallService.kt b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/incoming_call/IncomingCallService.kt index f445e711..9a82593f 100644 --- a/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/incoming_call/IncomingCallService.kt +++ b/webtrit_callkeep_android/android/src/main/kotlin/com/webtrit/callkeep/services/services/incoming_call/IncomingCallService.kt @@ -280,9 +280,9 @@ class IncomingCallService : return START_NOT_STICKY } - private fun performAnswerCall(metadata: CallMetadata): Int { + // Called from onConnectionEvent only, never from onStartCommand - no start mode to return. + private fun performAnswerCall(metadata: CallMetadata) { callLifecycleHandler.performAnswerCall(metadata) - return START_STICKY } // Launches the service with the LAUNCH action and cancels the timeout diff --git a/webtrit_callkeep_android/android/src/test/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallServiceRestartTest.kt b/webtrit_callkeep_android/android/src/test/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallServiceRestartTest.kt new file mode 100644 index 00000000..370cd3d1 --- /dev/null +++ b/webtrit_callkeep_android/android/src/test/kotlin/com/webtrit/callkeep/services/services/active_call/ActiveCallServiceRestartTest.kt @@ -0,0 +1,139 @@ +package com.webtrit.callkeep.services.services.active_call + +import android.app.Service +import android.content.Context +import android.content.Intent +import android.os.Build +import android.os.Bundle +import androidx.test.core.app.ApplicationProvider +import com.webtrit.callkeep.models.CallMetadata +import com.webtrit.callkeep.models.NotificationAction +import com.webtrit.callkeep.notifications.ActiveCallNotificationBuilder +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +/** + * Unit tests for the [ActiveCallService] empty-metadata guard. + * + * A START_STICKY restart after a process kill delivers a null intent, leaving + * [ActiveCallService] with no calls metadata. NotificationManager tracks active calls in its + * own static list (reset by the process death), so nothing external ever stops such an + * orphaned instance: without the guard its ongoing FGS notification stays in the shade, + * cannot be swiped away, and the Hang up action has no visible effect. + * + * The guard: after satisfying the startForeground contract, an instance with no calls + * tears down the connection services (a call leg may have survived in :callkeep_core), + * removes its notification and stops itself; the empty branch of hungUpCall does the same + * on top of the existing tearDownService fallback. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.UPSIDE_DOWN_CAKE]) +class ActiveCallServiceRestartTest { + private val context: Context = ApplicationProvider.getApplicationContext() + + private fun buildService(): ActiveCallService = Robolectric.buildService(ActiveCallService::class.java).create().get() + + private fun metadataIntent(vararg callIds: String): Intent = + Intent(context, ActiveCallService::class.java).apply { + putParcelableArrayListExtra( + ActiveCallService.EXTRA_CALLS_METADATA, + ArrayList(callIds.map { CallMetadata(callId = it).toBundle() }), + ) + } + + private fun declineIntent(metadata: CallMetadata? = null): Intent = + Intent(context, ActiveCallService::class.java).apply { + action = NotificationAction.Decline.action + metadata?.toBundle()?.let { putExtras(it) } + } + + private fun assertStoppedAndNotificationRemoved(service: ActiveCallService) { + val shadow = shadowOf(service) + assertTrue("service must stop itself", shadow.isStoppedBySelf) + assertTrue("foreground must be stopped", shadow.isForegroundStopped) + assertTrue("notification must be removed", shadow.notificationShouldRemoved) + } + + @Test + fun `null-intent restart stops self and removes the notification`() { + val service = buildService() + + val result = service.onStartCommand(null, 0, 1) + + assertEquals(Service.START_NOT_STICKY, result) + assertStoppedAndNotificationRemoved(service) + } + + @Test + fun `start with an explicitly empty metadata list stops self`() { + val service = buildService() + + val result = service.onStartCommand(metadataIntent(), 0, 1) + + assertEquals(Service.START_NOT_STICKY, result) + assertStoppedAndNotificationRemoved(service) + } + + @Test + fun `start with calls metadata stays foreground and sticky`() { + val service = buildService() + + val result = service.onStartCommand(metadataIntent("call-1"), 0, 1) + + assertEquals(Service.START_STICKY, result) + val shadow = shadowOf(service) + assertFalse("service must keep running", shadow.isStoppedBySelf) + assertFalse("foreground must stay", shadow.isForegroundStopped) + assertEquals(ActiveCallNotificationBuilder.NOTIFICATION_ID, shadow.lastForegroundNotificationId) + } + + @Test + fun `hang up with no known calls stops self and removes the notification`() { + // Decline tapped on a notification re-posted by a null-intent restart: callsMetadata + // is empty, tearDownService alone does not stop this service. + val service = buildService() + + val result = service.onStartCommand(declineIntent(), 0, 1) + + assertEquals(Service.START_NOT_STICKY, result) + assertStoppedAndNotificationRemoved(service) + } + + @Test + fun `hang up with metadata only in the intent extras hangs up that call and keeps the service`() { + // Fresh instance created by the notification's Decline PendingIntent after process + // death: the callsMetadata field is empty, but the intent extras carry the tapped + // call's bundle (getHungUpCallIntent puts it there). The hangup must be routed to + // that call instead of falling into the tear-everything-down branch. + val service = buildService() + + val result = service.onStartCommand(declineIntent(CallMetadata(callId = "call-1")), 0, 1) + + assertEquals(Service.START_NOT_STICKY, result) + val shadow = shadowOf(service) + assertFalse("hangup must be routed, not torn down", shadow.isStoppedBySelf) + assertFalse("no notification to remove on a fresh instance", shadow.isForegroundStopped) + } + + @Test + fun `hang up with a known call keeps the service running`() { + // The normal path: the hangup is routed to the connection service; this service is + // stopped later by NotificationManager once the connection reports disconnect. + val service = buildService() + service.onStartCommand(metadataIntent("call-1"), 0, 1) + + val result = service.onStartCommand(declineIntent(), 0, 2) + + assertEquals(Service.START_NOT_STICKY, result) + val shadow = shadowOf(service) + assertFalse("service must keep running until the connection disconnects", shadow.isStoppedBySelf) + assertFalse("foreground must stay", shadow.isForegroundStopped) + } +}