Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: crash when starting foreground service for calling from background (WPB-11112) #3456

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@
android:foregroundServiceType="specialUse"/>

<service
android:name=".services.OngoingCallService"
android:name=".services.CallService"
android:exported="false"
android:foregroundServiceType="phoneCall|microphone" />
</application>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.wire.android.R
import com.wire.android.appLogger
import com.wire.android.util.dispatchers.DispatcherProvider
import com.wire.kalium.logic.data.call.Call
import com.wire.kalium.logic.data.call.CallStatus
import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.QualifiedID
Expand Down Expand Up @@ -62,7 +63,6 @@ class CallNotificationManager @Inject constructor(
private val notificationManager = NotificationManagerCompat.from(context)
private val scope = CoroutineScope(SupervisorJob() + dispatcherProvider.default())
private val incomingCallsForUsers = MutableStateFlow<Map<UserId, Call>>(mapOf())
private val outgoingCallForUsers = MutableStateFlow<Map<UserId, Call>>(mapOf())
private val reloadCallNotification = MutableSharedFlow<CallNotificationIds>()

init {
Expand All @@ -81,25 +81,6 @@ class CallNotificationManager @Inject constructor(
}
}
}
scope.launch {
outgoingCallForUsers
.map { it.entries.firstOrNull()?.toCallNotificationData() }
.distinctUntilChanged()
.reloadIfNeeded()
.collectLatest { outgoingCallData ->
if (outgoingCallData == null) {
hideOutgoingCallNotification()
} else {
appLogger.i("$TAG: showing outgoing call")
showOutgoingCallNotification(
outgoingCallData.copy(
conversationName = outgoingCallData.conversationName
?: context.getString(R.string.username_unavailable_label)
)
)
}
}
}
}

fun reloadIfNeeded(data: CallNotificationData): Flow<CallNotificationData> = reloadCallNotification
Expand All @@ -126,14 +107,6 @@ class CallNotificationManager @Inject constructor(
}
}

fun handleOutgoingCallNotifications(calls: List<Call>, userId: UserId) {
if (calls.isEmpty()) {
outgoingCallForUsers.update { it.filter { it.key != userId } }
} else {
outgoingCallForUsers.update { it.filter { it.key != userId } + (userId to calls.first()) }
}
}

fun hideAllNotifications() {
hideIncomingCallNotification()
}
Expand All @@ -149,11 +122,6 @@ class CallNotificationManager @Inject constructor(
notificationManager.cancel(NotificationIds.CALL_INCOMING_NOTIFICATION_ID.ordinal)
}

private fun hideOutgoingCallNotification() {
appLogger.i("$TAG: hiding outgoing call")
notificationManager.cancel(NotificationIds.CALL_OUTGOING_NOTIFICATION_ID.ordinal)
}

@SuppressLint("MissingPermission")
@VisibleForTesting
internal fun showIncomingCallNotification(data: CallNotificationData) {
Expand All @@ -165,17 +133,6 @@ class CallNotificationManager @Inject constructor(
)
}

@SuppressLint("MissingPermission")
@VisibleForTesting
internal fun showOutgoingCallNotification(data: CallNotificationData) {
appLogger.i("$TAG: showing outgoing call notification for user ${data.userId.toLogString()}")
val notification = builder.getOutgoingCallNotification(data)
notificationManager.notify(
NotificationIds.CALL_OUTGOING_NOTIFICATION_ID.ordinal,
notification
)
}

// Notifications

companion object {
Expand All @@ -199,7 +156,7 @@ class CallNotificationBuilder @Inject constructor(
fun getOutgoingCallNotification(data: CallNotificationData): Notification {
val userIdString = data.userId.toString()
val conversationIdString = data.conversationId.toString()
val channelId = NotificationConstants.getIncomingChannelId(data.userId)
val channelId = NotificationConstants.getOutgoingChannelId(data.userId)

return NotificationCompat.Builder(context, channelId)
.setPriority(NotificationCompat.PRIORITY_DEFAULT)
Expand Down Expand Up @@ -262,6 +219,7 @@ class CallNotificationBuilder @Inject constructor(
.setSmallIcon(R.drawable.notification_icon_small)
.setAutoCancel(true)
.setOngoing(true)
.setUsesChronometer(true)
.addAction(getHangUpCallAction(context, conversationIdString, userIdString))
.addAction(getOpenOngoingCallAction(context, conversationIdString))
.setFullScreenIntent(openOngoingCallPendingIntent(context, conversationIdString), true)
Expand All @@ -271,15 +229,15 @@ class CallNotificationBuilder @Inject constructor(
}

/**
* @return placeholder Notification for OngoingCall, that can be shown immediately after starting the Service
* @return placeholder Notification for CallService, that can be shown immediately after starting the Service
* (e.g. in [android.app.Service.onCreate]). It has no any [NotificationCompat.Action], on click - just opens the app.
* This notification should be replace by the user-specific notification (with corresponding [NotificationCompat.Action],
* [android.content.Intent] and title) once it's possible (e.g. in [android.app.Service.onStartCommand])
*/
fun getOngoingCallPlaceholderNotification(): Notification {
fun getCallServicePlaceholderNotification(): Notification {
val channelId = NotificationConstants.ONGOING_CALL_CHANNEL_ID
return NotificationCompat.Builder(context, channelId)
.setContentText(context.getString(R.string.notification_ongoing_call_content))
.setContentText(context.getString(R.string.notification_outgoing_call_tap_to_return))
.setPriority(NotificationCompat.PRIORITY_DEFAULT)
.setCategory(NotificationCompat.CATEGORY_CALL)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
Expand Down Expand Up @@ -325,6 +283,7 @@ data class CallNotificationData(
val conversationType: Conversation.Type,
val callerName: String?,
val callerTeamName: String?,
val callStatus: CallStatus
) {
constructor(userId: UserId, call: Call) : this(
userId,
Expand All @@ -333,6 +292,7 @@ data class CallNotificationData(
call.conversationType,
call.callerName,
call.callerTeamName,
call.status
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ object NotificationConstants {
// Notification IDs (has to be unique!)
enum class NotificationIds {
CALL_INCOMING_NOTIFICATION_ID,
CALL_OUTGOING_NOTIFICATION_ID,
CALL_ONGOING_NOTIFICATION_ID,
CALL_OUTGOING_ONGOING_NOTIFICATION_ID,
PERSISTENT_NOTIFICATION_ID,
MESSAGE_SYNC_NOTIFICATION_ID,
MIGRATION_NOTIFICATION_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.cancellable
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
Expand Down Expand Up @@ -162,7 +163,10 @@ class WireNotificationManager @Inject constructor(
val observeCallsJob = observeCallNotificationsOnceJob(userId)

appLogger.d("$TAG start syncing")
connectionPolicyManager.handleConnectionOnPushNotification(userId, STAY_ALIVE_TIME_ON_PUSH_MS)
connectionPolicyManager.handleConnectionOnPushNotification(
userId,
STAY_ALIVE_TIME_ON_PUSH_MS
)

observeMessagesJob?.cancel("$TAG checked the notifications once, canceling observing.")
observeCallsJob?.cancel("$TAG checked the calls once, canceling observing.")
Expand All @@ -179,7 +183,12 @@ class WireNotificationManager @Inject constructor(
appLogger.d("$TAG checking the notifications once, but notifications are already observed, no need to start a new job")
null
} else {
scope.launch { observeMessageNotifications(userId, MutableStateFlow(CurrentScreen.InBackground)) }
scope.launch {
observeMessageNotifications(
userId,
MutableStateFlow(CurrentScreen.InBackground)
)
}
}
}

Expand Down Expand Up @@ -241,7 +250,7 @@ class WireNotificationManager @Inject constructor(
appLogger.i("$TAG no Users -> hide all the notifications")
messagesNotificationManager.hideAllNotifications()
callNotificationManager.hideAllNotifications()
servicesManager.stopOngoingCallService()
servicesManager.stopCallService()

return
}
Expand All @@ -256,22 +265,19 @@ class WireNotificationManager @Inject constructor(
incomingCallsJob = scope.launch(dispatcherProvider.default()) {
observeIncomingCalls(userId)
},
outgoingCallJob = scope.launch(dispatcherProvider.default()) {
observeOutgoingCalls(userId)
},
messagesJob = scope.launch(dispatcherProvider.default()) {
observeMessageNotifications(userId, currentScreenState)
},
)
observingJobs.userJobs[userId] = jobs
}

// start observing ongoing calls for all users, but only if not yet started
if (observingJobs.ongoingCallJob.get().let { it == null || !it.isActive }) {
// start observing outgoing and ongoing calls for all users, but only if not yet started
if (observingJobs.outgoingOngoingCallJob.get().let { it == null || !it.isActive }) {
val job = scope.launch(dispatcherProvider.default()) {
observeOngoingCalls()
observeOutgoingOngoingCalls()
}
observingJobs.ongoingCallJob.set(job)
observingJobs.outgoingOngoingCallJob.set(job)
}
}

Expand Down Expand Up @@ -305,8 +311,16 @@ class WireNotificationManager @Inject constructor(
currentScreenState
.collect { screens ->
when (screens) {
is CurrentScreen.Conversation -> messagesNotificationManager.hideNotification(screens.id, userId)
is CurrentScreen.OtherUserProfile -> messagesNotificationManager.hideNotification(screens.id, userId)
is CurrentScreen.Conversation -> messagesNotificationManager.hideNotification(
screens.id,
userId
)

is CurrentScreen.OtherUserProfile -> messagesNotificationManager.hideNotification(
screens.id,
userId
)

else -> {}
}
}
Expand Down Expand Up @@ -338,15 +352,6 @@ class WireNotificationManager @Inject constructor(
}
}

private suspend fun observeOutgoingCalls(
userId: UserId
) {
appLogger.d("$TAG observing outgoing calls")
coreLogic.getSessionScope(userId).calls.observeOutgoingCall().collect {
callNotificationManager.handleOutgoingCallNotifications(it, userId)
}
}

/**
* Infinitely listen for the new Message notifications and show it.
* Can be used for listening for the Notifications when the app is running.
Expand Down Expand Up @@ -394,34 +399,51 @@ class WireNotificationManager @Inject constructor(
if (isBlockedByE2EIRequiredState.value) {
appLogger.d("$TAG notifications were skipped as E2EI is required")
} else {
messagesNotificationManager.handleNotification(newNotifications, userId, selfUserNameState.value)
messagesNotificationManager.handleNotification(
newNotifications,
userId,
selfUserNameState.value
)
}
markMessagesAsNotified(userId)
markConnectionAsNotified(userId)
}
}

/**
* Infinitely listen for the established calls of a current user and run OngoingCall foreground Service
* Infinitely listen for outgoing and established calls of a current user and run the call on foreground Service
* to show corresponding notification and do not lose a call.
*/
private suspend fun observeOngoingCalls() {
private suspend fun observeOutgoingOngoingCalls() {
coreLogic.getGlobalScope().session.currentSessionFlow()
.flatMapLatest {
if (it is CurrentSessionResult.Success && it.accountInfo.isValid()) {
coreLogic.getSessionScope(it.accountInfo.userId).calls.establishedCall()
.map { it.isNotEmpty() }
combine(
coreLogic.getSessionScope(it.accountInfo.userId).calls.establishedCall(),
coreLogic.getSessionScope(it.accountInfo.userId).calls.observeOutgoingCall()
) { establishedCalls, outgoingCalls ->
if (establishedCalls.isNotEmpty()) {
return@combine true
}
if (outgoingCalls.isNotEmpty()) {
return@combine true
}
return@combine false
}
} else {
flowOf(false)
}
}
.distinctUntilChanged()
.onCompletion {
servicesManager.stopOngoingCallService()
servicesManager.stopCallService()
}
.collect { isOngoingCall ->
if (isOngoingCall) servicesManager.startOngoingCallService()
else servicesManager.stopOngoingCallService()
.collect { isOnCall ->
if (isOnCall) {
servicesManager.startCallService()
} else {
servicesManager.stopCallService()
}
}
}

Expand All @@ -437,7 +459,10 @@ class WireNotificationManager @Inject constructor(
newNotifications
}

private suspend fun markMessagesAsNotified(userId: QualifiedID, conversationId: ConversationId? = null) {
private suspend fun markMessagesAsNotified(
userId: QualifiedID,
conversationId: ConversationId? = null
) {
val markNotified = conversationId?.let {
MarkMessagesAsNotifiedUseCase.UpdateTarget.SingleConversation(conversationId)
} ?: MarkMessagesAsNotifiedUseCase.UpdateTarget.AllConversations
Expand All @@ -446,7 +471,10 @@ class WireNotificationManager @Inject constructor(
.markMessagesAsNotified(markNotified)
}

private suspend fun markConnectionAsNotified(userId: QualifiedID?, connectionRequestUserId: QualifiedID? = null) {
private suspend fun markConnectionAsNotified(
userId: QualifiedID?,
connectionRequestUserId: QualifiedID? = null
) {
appLogger.d("$TAG markConnectionAsNotified")
userId?.let {
coreLogic.getSessionScope(it)
Expand Down Expand Up @@ -483,22 +511,20 @@ class WireNotificationManager @Inject constructor(
private data class UserObservingJobs(
val currentScreenJob: Job,
val incomingCallsJob: Job,
val outgoingCallJob: Job,
val messagesJob: Job,
) {
fun cancelAll() {
currentScreenJob.cancel()
incomingCallsJob.cancel()
outgoingCallJob.cancel()
messagesJob.cancel()
}

fun isAllActive(): Boolean =
currentScreenJob.isActive && incomingCallsJob.isActive && messagesJob.isActive && outgoingCallJob.isActive
currentScreenJob.isActive && incomingCallsJob.isActive && messagesJob.isActive
}

private data class ObservingJobs(
val ongoingCallJob: AtomicReference<Job?> = AtomicReference(),
val outgoingOngoingCallJob: AtomicReference<Job?> = AtomicReference(),
val userJobs: ConcurrentHashMap<QualifiedID, UserObservingJobs> = ConcurrentHashMap()
)

Expand Down
Loading
Loading