Skip to content

Commit 3b35d96

Browse files
jmartinespjmartinesp
authored
Fix ringing calls not stopping when the other user cancels the call (#4613)
* Fix ringing calls not being automatically canceled This will keep the sync active while the user is screening an incoming call, allowing receiving a call cancellation event, which will terminate the incoming call ringing early. * Add extra logs to help debugging ringing call issues.
1 parent 75cb315 commit 3b35d96

File tree

10 files changed

+112
-17
lines changed

10 files changed

+112
-17
lines changed

‎appnav/src/main/kotlin/io/element/android/appnav/di/SyncOrchestrator.kt‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,21 @@ class SyncOrchestrator @AssistedInject constructor(
7878
internal fun observeStates() = coroutineScope.launch {
7979
Timber.tag(tag).d("start observing the app and network state")
8080

81+
val isAppActiveFlow = combine(
82+
appForegroundStateService.isInForeground,
83+
appForegroundStateService.isInCall,
84+
appForegroundStateService.isSyncingNotificationEvent,
85+
appForegroundStateService.hasRingingCall,
86+
) { isInForeground, isInCall, isSyncingNotificationEvent, hasRingingCall ->
87+
isInForeground || isInCall || isSyncingNotificationEvent || hasRingingCall
88+
}
89+
8190
combine(
8291
// small debounce to avoid spamming startSync when the state is changing quickly in case of error.
8392
syncService.syncState.debounce(100.milliseconds),
8493
networkMonitor.connectivity,
85-
appForegroundStateService.isInForeground,
86-
appForegroundStateService.isInCall,
87-
appForegroundStateService.isSyncingNotificationEvent,
88-
) { syncState, networkState, isInForeground, isInCall, isSyncingNotificationEvent ->
89-
val isAppActive = isInForeground || isInCall || isSyncingNotificationEvent
94+
isAppActiveFlow,
95+
) { syncState, networkState, isAppActive ->
9096
val isNetworkAvailable = networkState == NetworkStatus.Connected
9197

9298
Timber.tag(tag).d("isAppActive=$isAppActive, isNetworkAvailable=$isNetworkAvailable")

‎appnav/src/test/kotlin/io/element/android/appnav/SyncOrchestratorTest.kt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,50 @@ class SyncOrchestratorTest {
236236
stopSyncRecorder.assertions().isCalledOnce()
237237
}
238238

239+
@Test
240+
fun `when the app was in background and we have an incoming ringing call, a sync will be started`() = runTest {
241+
val startSyncRecorder = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
242+
val stopSyncRecorder = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
243+
val syncService = FakeSyncService(initialSyncState = SyncState.Idle).apply {
244+
startSyncLambda = startSyncRecorder
245+
stopSyncLambda = stopSyncRecorder
246+
}
247+
val networkMonitor = FakeNetworkMonitor(initialStatus = NetworkStatus.Connected)
248+
val appForegroundStateService = FakeAppForegroundStateService(
249+
initialForegroundValue = false,
250+
initialIsSyncingNotificationEventValue = false,
251+
initialHasRingingCall = false,
252+
)
253+
val syncOrchestrator = createSyncOrchestrator(
254+
syncService = syncService,
255+
networkMonitor = networkMonitor,
256+
appForegroundStateService = appForegroundStateService,
257+
)
258+
259+
// We start observing
260+
syncOrchestrator.observeStates()
261+
262+
// Advance the time to make sure the orchestrator has had time to start processing the inputs
263+
advanceTimeBy(100.milliseconds)
264+
265+
// Start sync was never called
266+
startSyncRecorder.assertions().isNeverCalled()
267+
268+
// Now we receive a ringing call
269+
appForegroundStateService.updateHasRingingCall(true)
270+
271+
// Start sync will be called shortly after
272+
advanceTimeBy(1.milliseconds)
273+
startSyncRecorder.assertions().isCalledOnce()
274+
275+
// If the sync is running and the ringing call notification is now over, the sync stops after a delay
276+
syncService.emitSyncState(SyncState.Running)
277+
appForegroundStateService.updateHasRingingCall(false)
278+
279+
advanceTimeBy(10.seconds)
280+
stopSyncRecorder.assertions().isCalledOnce()
281+
}
282+
239283
@Test
240284
fun `when the app is in foreground, we sync for a notification and a call is ongoing, the sync will only stop when all conditions are false`() = runTest {
241285
val startSyncRecorder = lambdaRecorder<Result<Unit>> { Result.success(Unit) }

‎features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import io.element.android.libraries.matrix.api.MatrixClientProvider
2626
import io.element.android.libraries.push.api.notifications.ForegroundServiceType
2727
import io.element.android.libraries.push.api.notifications.NotificationIdProvider
2828
import io.element.android.libraries.push.api.notifications.OnMissedCallNotificationHandler
29+
import io.element.android.services.appnavstate.api.AppForegroundStateService
2930
import kotlinx.coroutines.CoroutineScope
3031
import kotlinx.coroutines.ExperimentalCoroutinesApi
3132
import kotlinx.coroutines.Job
@@ -87,7 +88,9 @@ class DefaultActiveCallManager @Inject constructor(
8788
private val notificationManagerCompat: NotificationManagerCompat,
8889
private val matrixClientProvider: MatrixClientProvider,
8990
private val defaultCurrentCallService: DefaultCurrentCallService,
91+
private val appForegroundStateService: AppForegroundStateService,
9092
) : ActiveCallManager {
93+
private val tag = "DefaultActiveCallManager"
9194
private var timedOutCallJob: Job? = null
9295

9396
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@@ -106,9 +109,11 @@ class DefaultActiveCallManager @Inject constructor(
106109

107110
override suspend fun registerIncomingCall(notificationData: CallNotificationData) {
108111
mutex.withLock {
112+
appForegroundStateService.updateHasRingingCall(true)
113+
Timber.tag(tag).d("Received incoming call for room id: ${notificationData.roomId}")
109114
if (activeCall.value != null) {
110115
displayMissedCallNotification(notificationData)
111-
Timber.w("Already have an active call, ignoring incoming call: $notificationData")
116+
Timber.tag(tag).w("Already have an active call, ignoring incoming call: $notificationData")
112117
return
113118
}
114119
activeCall.value = ActiveCall(
@@ -129,6 +134,7 @@ class DefaultActiveCallManager @Inject constructor(
129134

130135
// Acquire a wake lock to keep the device awake during the incoming call, so we can process the room info data
131136
if (activeWakeLock?.isHeld == false) {
137+
Timber.tag(tag).d("Acquiring partial wakelock")
132138
activeWakeLock.acquire(ElementCallConfig.RINGING_CALL_DURATION_SECONDS * 1000L)
133139
}
134140
}
@@ -139,10 +145,13 @@ class DefaultActiveCallManager @Inject constructor(
139145
*/
140146
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
141147
suspend fun incomingCallTimedOut(displayMissedCallNotification: Boolean) = mutex.withLock {
148+
Timber.tag(tag).d("Incoming call timed out")
149+
142150
val previousActiveCall = activeCall.value ?: return
143151
val notificationData = (previousActiveCall.callState as? CallState.Ringing)?.notificationData ?: return
144152
activeCall.value = null
145153
if (activeWakeLock?.isHeld == true) {
154+
Timber.tag(tag).d("Releasing partial wakelock after timeout")
146155
activeWakeLock.release()
147156
}
148157

@@ -155,11 +164,12 @@ class DefaultActiveCallManager @Inject constructor(
155164

156165
override suspend fun hungUpCall(callType: CallType) = mutex.withLock {
157166
if (activeCall.value?.callType != callType) {
158-
Timber.w("Call type $callType does not match the active call type, ignoring")
167+
Timber.tag(tag).w("Call type $callType does not match the active call type, ignoring")
159168
return
160169
}
161170
cancelIncomingCallNotification()
162171
if (activeWakeLock?.isHeld == true) {
172+
Timber.tag(tag).d("Releasing partial wakelock after hang up")
163173
activeWakeLock.release()
164174
}
165175
timedOutCallJob?.cancel()
@@ -169,6 +179,7 @@ class DefaultActiveCallManager @Inject constructor(
169179
override suspend fun joinedCall(callType: CallType) = mutex.withLock {
170180
cancelIncomingCallNotification()
171181
if (activeWakeLock?.isHeld == true) {
182+
Timber.tag(tag).d("Releasing partial wakelock after joining call")
172183
activeWakeLock.release()
173184
}
174185
timedOutCallJob?.cancel()
@@ -181,6 +192,7 @@ class DefaultActiveCallManager @Inject constructor(
181192

182193
@SuppressLint("MissingPermission")
183194
private suspend fun showIncomingCallNotification(notificationData: CallNotificationData) {
195+
Timber.tag(tag).d("Displaying ringing call notification")
184196
val notification = ringingCallNotificationCreator.createNotification(
185197
sessionId = notificationData.sessionId,
186198
roomId = notificationData.roomId,
@@ -204,10 +216,13 @@ class DefaultActiveCallManager @Inject constructor(
204216
}
205217

206218
private fun cancelIncomingCallNotification() {
219+
appForegroundStateService.updateHasRingingCall(false)
220+
Timber.tag(tag).d("Ringing call notification cancelled")
207221
notificationManagerCompat.cancel(NotificationIdProvider.getForegroundServiceNotificationId(ForegroundServiceType.INCOMING_CALL))
208222
}
209223

210224
private fun displayMissedCallNotification(notificationData: CallNotificationData) {
225+
Timber.tag(tag).d("Displaying missed call notification")
211226
coroutineScope.launch {
212227
onMissedCallNotificationHandler.addMissedCallNotification(
213228
sessionId = notificationData.sessionId,
@@ -227,14 +242,14 @@ class DefaultActiveCallManager @Inject constructor(
227242
.flatMapLatest { activeCall ->
228243
val callType = activeCall.callType as CallType.RoomCall
229244
// Get a flow of updated `hasRoomCall` and `activeRoomCallParticipants` values for the room
230-
matrixClientProvider.getOrRestore(callType.sessionId).getOrNull()
231-
?.getRoom(callType.roomId)
232-
?.roomInfoFlow
233-
?.map {
234-
Timber.d("Has room call status changed for ringing call: ${it.hasRoomCall}")
235-
it.hasRoomCall to (callType.sessionId in it.activeRoomCallParticipants)
236-
}
237-
?: flowOf()
245+
val room = matrixClientProvider.getOrRestore(callType.sessionId).getOrNull()?.getRoom(callType.roomId) ?: run {
246+
Timber.tag(tag).d("Couldn't find room for incoming call: $activeCall")
247+
return@flatMapLatest flowOf()
248+
}
249+
room.roomInfoFlow.map {
250+
Timber.tag(tag).d("Has room call status changed for ringing call: ${it.hasRoomCall}")
251+
it.hasRoomCall to (callType.sessionId in it.activeRoomCallParticipants)
252+
}
238253
}
239254
// We only want to check if the room active call status changes
240255
.distinctUntilChanged()

‎features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultActiveCallManagerTest.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import io.element.android.libraries.push.api.notifications.NotificationIdProvide
3535
import io.element.android.libraries.push.test.notifications.FakeImageLoaderHolder
3636
import io.element.android.libraries.push.test.notifications.FakeOnMissedCallNotificationHandler
3737
import io.element.android.libraries.push.test.notifications.push.FakeNotificationBitmapLoader
38+
import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
3839
import io.element.android.tests.testutils.lambda.lambdaRecorder
3940
import io.element.android.tests.testutils.lambda.value
4041
import io.mockk.mockk
@@ -323,5 +324,6 @@ class DefaultActiveCallManagerTest {
323324
notificationManagerCompat = notificationManagerCompat,
324325
matrixClientProvider = matrixClientProvider,
325326
defaultCurrentCallService = DefaultCurrentCallService(),
327+
appForegroundStateService = FakeAppForegroundStateService(),
326328
)
327329
}

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/CallNotificationEventResolver.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import io.element.android.libraries.push.impl.R
1717
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
1818
import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent
1919
import io.element.android.services.toolbox.api.strings.StringProvider
20+
import timber.log.Timber
2021
import javax.inject.Inject
2122

2223
/**
@@ -69,6 +70,9 @@ class DefaultCallNotificationEventResolver @Inject constructor(
6970
senderAvatarUrl = senderAvatarUrl,
7071
)
7172
} else {
73+
val now = System.currentTimeMillis()
74+
val elapsed = now - timestamp
75+
Timber.d("Event $eventId is call notify but should not ring: $timestamp vs $now ($elapsed ms elapsed), notify: ${content.type}")
7276
// Create a simple message notification event
7377
buildNotifiableMessageEvent(
7478
sessionId = sessionId,

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ class DefaultNotifiableEventResolver @Inject constructor(
9292
return notificationData.flatMap {
9393
if (it == null) {
9494
Timber.tag(loggerTag.value).d("No notification data found for event $eventId")
95-
return@flatMap Result.failure(ResolvingException("Unable to resolve event"))
95+
return@flatMap Result.failure(ResolvingException("Unable to resolve event $eventId"))
9696
} else {
97+
Timber.tag(loggerTag.value).d("Found notification item for $eventId")
9798
it.asNotifiableEvent(client, sessionId)
9899
}
99100
}

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,12 @@ class DefaultPushHandler @Inject constructor(
133133
is ResolvedPushEvent.Event -> {
134134
when (val notifiableEvent = resolvedPushEvent.notifiableEvent) {
135135
is NotifiableRingingCallEvent -> {
136+
Timber.tag(loggerTag.value).d("Notifiable event ${pushData.eventId} is ringing call: $notifiableEvent")
136137
onNotifiableEventReceived.onNotifiableEventReceived(notifiableEvent)
137138
handleRingingCallEvent(notifiableEvent)
138139
}
139140
else -> {
141+
Timber.tag(loggerTag.value).d("Notifiable event ${pushData.eventId} is normal event: $notifiableEvent")
140142
val userPushStore = userPushStoreFactory.getOrCreate(userId)
141143
val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first()
142144
if (areNotificationsEnabled) {

‎services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/AppForegroundStateService.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ interface AppForegroundStateService {
1818
*/
1919
val isInForeground: StateFlow<Boolean>
2020

21+
/**
22+
* Updates to whether the app is active because an incoming ringing call is happening will be emitted here.
23+
*/
24+
val hasRingingCall: StateFlow<Boolean>
25+
2126
/**
2227
* Updates to whether the app is in an active call or not will be emitted here.
2328
*/
@@ -38,6 +43,11 @@ interface AppForegroundStateService {
3843
*/
3944
fun updateIsInCallState(isInCall: Boolean)
4045

46+
/**
47+
* Update the 'has ringing call' state.
48+
*/
49+
fun updateHasRingingCall(hasRingingCall: Boolean)
50+
4151
/**
4252
* Update the active state for the syncing notification event flow.
4353
*/

‎services/appnavstate/impl/src/main/kotlin/io/element/android/services/appnavstate/impl/DefaultAppForegroundStateService.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class DefaultAppForegroundStateService : AppForegroundStateService {
1717
override val isInForeground = MutableStateFlow(false)
1818
override val isInCall = MutableStateFlow(false)
1919
override val isSyncingNotificationEvent = MutableStateFlow(false)
20+
override val hasRingingCall = MutableStateFlow(false)
2021

2122
private val appLifecycle: Lifecycle by lazy { ProcessLifecycleOwner.get().lifecycle }
2223

@@ -28,6 +29,10 @@ class DefaultAppForegroundStateService : AppForegroundStateService {
2829
this.isInCall.value = isInCall
2930
}
3031

32+
override fun updateHasRingingCall(hasRingingCall: Boolean) {
33+
this.hasRingingCall.value = hasRingingCall
34+
}
35+
3136
override fun updateIsSyncingNotificationEvent(isSyncingNotificationEvent: Boolean) {
3237
this.isSyncingNotificationEvent.value = isSyncingNotificationEvent
3338
}

‎services/appnavstate/test/src/main/kotlin/io/element/android/services/appnavstate/test/FakeAppForegroundStateService.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ import kotlinx.coroutines.flow.MutableStateFlow
1313
class FakeAppForegroundStateService(
1414
initialForegroundValue: Boolean = true,
1515
initialIsInCallValue: Boolean = false,
16-
initialIsSyncingNotificationEventValue: Boolean = false
16+
initialIsSyncingNotificationEventValue: Boolean = false,
17+
initialHasRingingCall: Boolean = false,
1718
) : AppForegroundStateService {
1819
override val isInForeground = MutableStateFlow(initialForegroundValue)
1920
override val isInCall = MutableStateFlow(initialIsInCallValue)
2021
override val isSyncingNotificationEvent = MutableStateFlow(initialIsSyncingNotificationEventValue)
22+
override val hasRingingCall = MutableStateFlow(initialHasRingingCall)
2123

2224
override fun startObservingForeground() {
2325
// No-op
@@ -34,4 +36,8 @@ class FakeAppForegroundStateService(
3436
override fun updateIsSyncingNotificationEvent(isSyncingNotificationEvent: Boolean) {
3537
this.isSyncingNotificationEvent.value = isSyncingNotificationEvent
3638
}
39+
40+
override fun updateHasRingingCall(hasRingingCall: Boolean) {
41+
this.hasRingingCall.value = hasRingingCall
42+
}
3743
}

0 commit comments

Comments
 (0)