Fix coroutine scope (#4820)

* Inject the session scope instead of the application scope where it's possible.

* Create AppCoroutineScope annotation to let developers explicitly choose the appropriate CoroutineScope when injecting one.
This commit is contained in:
Benoit Marty 2025-06-04 17:33:51 +02:00 committed by GitHub
parent 36c7c7ab9b
commit 5f191d9f9c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
58 changed files with 172 additions and 72 deletions

View file

@ -50,6 +50,7 @@ import io.element.android.libraries.architecture.inputs
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.designsystem.utils.OnLifecycleEvent
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.analytics.toAnalyticsViewRoom
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
@ -69,7 +70,8 @@ import kotlinx.coroutines.launch
class MessagesNode @AssistedInject constructor(
@Assisted buildContext: BuildContext,
@Assisted plugins: List<Plugin>,
private val coroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
private val room: BaseRoom,
private val analyticsService: AnalyticsService,
messageComposerPresenterFactory: MessageComposerPresenter.Factory,
@ -115,7 +117,7 @@ class MessagesNode @AssistedInject constructor(
super.onBuilt()
lifecycle.subscribe(
onCreate = {
coroutineScope.launch { analyticsService.capture(room.toAnalyticsViewRoom()) }
sessionCoroutineScope.launch { analyticsService.capture(room.toAnalyticsViewRoom()) }
},
onDestroy = {
mediaPlayer.close()

View file

@ -16,6 +16,7 @@ import dagger.assisted.AssistedInject
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runCatchingUpdatingState
import io.element.android.libraries.di.annotations.SessionCoroutineScope
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.timeline.TimelineProvider
@ -28,7 +29,8 @@ import kotlinx.coroutines.launch
class ForwardMessagesPresenter @AssistedInject constructor(
@Assisted eventId: String,
@Assisted private val timelineProvider: TimelineProvider,
private val appCoroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
) : Presenter<ForwardMessagesState> {
private val eventId: EventId = EventId(eventId)
@ -40,7 +42,7 @@ class ForwardMessagesPresenter @AssistedInject constructor(
private val forwardingActionState: MutableState<AsyncAction<List<RoomId>>> = mutableStateOf(AsyncAction.Uninitialized)
fun onRoomSelected(roomIds: List<RoomId>) {
appCoroutineScope.forwardEvent(eventId, roomIds.toPersistentList(), forwardingActionState)
sessionCoroutineScope.forwardEvent(eventId, roomIds.toPersistentList(), forwardingActionState)
}
@Composable

View file

@ -44,6 +44,7 @@ import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.core.mimetype.MimeTypes
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.matrix.api.core.UserId
@ -98,7 +99,8 @@ import io.element.android.libraries.core.mimetype.MimeTypes.Any as AnyMimeTypes
class MessageComposerPresenter @AssistedInject constructor(
@Assisted private val navigator: MessagesNavigator,
private val appCoroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
private val room: JoinedRoom,
private val mediaPickerProvider: PickerProvider,
private val featureFlagService: FeatureFlagService,
@ -200,7 +202,7 @@ class MessageComposerPresenter @AssistedInject constructor(
DisposableEffect(Unit) {
// Declare that the user is not typing anymore when the composer is disposed
onDispose {
appCoroutineScope.launch {
sessionCoroutineScope.launch {
if (sendTypingNotifications) {
room.typingNotice(false)
}
@ -236,12 +238,12 @@ class MessageComposerPresenter @AssistedInject constructor(
}
}
is MessageComposerEvents.SendMessage -> {
appCoroutineScope.sendMessage(
sessionCoroutineScope.sendMessage(
markdownTextEditorState = markdownTextEditorState,
richTextEditorState = richTextEditorState,
)
}
is MessageComposerEvents.SendUri -> appCoroutineScope.sendAttachment(
is MessageComposerEvents.SendUri -> sessionCoroutineScope.sendAttachment(
attachment = Attachment.Media(
localMedia = localMediaFactory.createFromUri(
uri = event.uri,
@ -338,7 +340,7 @@ class MessageComposerPresenter @AssistedInject constructor(
}
MessageComposerEvents.SaveDraft -> {
val draft = createDraftFromState(markdownTextEditorState, richTextEditorState)
appCoroutineScope.updateDraft(draft, isVolatile = false)
sessionCoroutineScope.updateDraft(draft, isVolatile = false)
}
}
}

View file

@ -38,6 +38,7 @@ import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.room.JoinedRoom
import io.element.android.libraries.matrix.api.room.powerlevels.canPinUnpin
import io.element.android.libraries.matrix.api.room.powerlevels.canRedactOther
@ -67,7 +68,8 @@ class PinnedMessagesListPresenter @AssistedInject constructor(
private val linkPresenter: Presenter<LinkState>,
private val snackbarDispatcher: SnackbarDispatcher,
@Assisted private val actionListPresenter: Presenter<ActionListState>,
private val appCoroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
private val analyticsService: AnalyticsService,
) : Presenter<PinnedMessagesListState> {
@AssistedFactory
@ -123,7 +125,7 @@ class PinnedMessagesListPresenter @AssistedInject constructor(
fun handleEvents(event: PinnedMessagesListEvents) {
when (event) {
is PinnedMessagesListEvents.HandleAction -> appCoroutineScope.handleTimelineAction(event.action, event.event)
is PinnedMessagesListEvents.HandleAction -> sessionCoroutineScope.handleTimelineAction(event.action, event.event)
}
}

View file

@ -38,6 +38,7 @@ import io.element.android.features.roomcall.api.RoomCallState
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.UniqueId
import io.element.android.libraries.matrix.api.room.JoinedRoom
@ -66,7 +67,8 @@ class TimelinePresenter @AssistedInject constructor(
timelineItemsFactoryCreator: TimelineItemsFactory.Creator,
private val room: JoinedRoom,
private val dispatchers: CoroutineDispatchers,
private val appScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
@Assisted private val navigator: MessagesNavigator,
private val redactedVoiceMessageManager: RedactedVoiceMessageManager,
private val sendPollResponseAction: SendPollResponseAction,
@ -135,7 +137,7 @@ class TimelinePresenter @AssistedInject constructor(
newEventState.value = NewEventState.None
}
Timber.d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
appScope.sendReadReceiptIfNeeded(
sessionCoroutineScope.sendReadReceiptIfNeeded(
firstVisibleIndex = event.firstIndex,
timelineItems = timelineItems,
lastReadReceiptId = lastReadReceiptId,
@ -145,13 +147,13 @@ class TimelinePresenter @AssistedInject constructor(
newEventState.value = NewEventState.None
}
}
is TimelineEvents.SelectPollAnswer -> appScope.launch {
is TimelineEvents.SelectPollAnswer -> sessionCoroutineScope.launch {
sendPollResponseAction.execute(
pollStartId = event.pollStartId,
answerId = event.answerId
)
}
is TimelineEvents.EndPoll -> appScope.launch {
is TimelineEvents.EndPoll -> sessionCoroutineScope.launch {
endPollAction.execute(
pollStartId = event.pollStartId,
)

View file

@ -8,6 +8,7 @@
package io.element.android.features.messages.impl.voicemessages.composer
import io.element.android.libraries.core.mimetype.MimeTypes
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.mediaplayer.api.MediaPlayer
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
@ -26,11 +27,12 @@ import javax.inject.Inject
* A media player for the voice message composer.
*
* @param mediaPlayer The [MediaPlayer] to use.
* @param coroutineScope
* @param sessionCoroutineScope
*/
class VoiceMessageComposerPlayer @Inject constructor(
private val mediaPlayer: MediaPlayer,
private val coroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
) {
companion object {
const val MIME_TYPE = MimeTypes.Ogg
@ -116,7 +118,7 @@ class VoiceMessageComposerPlayer @Inject constructor(
seekJob?.cancelAndJoin()
seekingTo.value = position
seekJob = coroutineScope.launch {
seekJob = sessionCoroutineScope.launch {
val mediaState = mediaPlayer.ensureMediaReady(mediaPath)
val duration = mediaState.duration ?: return@launch
val positionMs = (duration * position).toLong()

View file

@ -22,6 +22,7 @@ import androidx.lifecycle.Lifecycle
import im.vector.app.features.analytics.plan.Composer
import io.element.android.features.messages.api.MessageComposerContext
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.mediaupload.api.MediaSender
import io.element.android.libraries.permissions.api.PermissionsEvents
import io.element.android.libraries.permissions.api.PermissionsPresenter
@ -44,7 +45,8 @@ import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
class VoiceMessageComposerPresenter @Inject constructor(
private val appCoroutineScope: CoroutineScope,
@SessionCoroutineScope
private val sessionCoroutineScope: CoroutineScope,
private val voiceRecorder: VoiceRecorder,
private val analyticsService: AnalyticsService,
private val mediaSender: MediaSender,
@ -74,11 +76,11 @@ class VoiceMessageComposerPresenter @Inject constructor(
val onLifecycleEvent = { event: Lifecycle.Event ->
when (event) {
Lifecycle.Event.ON_PAUSE -> {
appCoroutineScope.finishRecording()
sessionCoroutineScope.finishRecording()
player.pause()
}
Lifecycle.Event.ON_DESTROY -> {
appCoroutineScope.cancelRecording()
sessionCoroutineScope.cancelRecording()
}
else -> {}
}
@ -145,7 +147,7 @@ class VoiceMessageComposerPresenter @Inject constructor(
isSending = true
player.pause()
analyticsService.captureComposerEvent()
appCoroutineScope.launch {
sessionCoroutineScope.launch {
val result = sendMessage(
file = finishedState.file,
mimeType = finishedState.mimeType,

View file

@ -97,6 +97,6 @@ class ForwardMessagesPresenterTest {
) = ForwardMessagesPresenter(
eventId = eventId.value,
timelineProvider = LiveTimelineProvider(fakeRoom),
appCoroutineScope = this,
sessionCoroutineScope = this,
)
}

View file

@ -1539,7 +1539,7 @@ class MessageComposerPresenterTest {
draftService: ComposerDraftService = FakeComposerDraftService(),
) = MessageComposerPresenter(
navigator = navigator,
appCoroutineScope = this,
sessionCoroutineScope = this,
room = room,
mediaPickerProvider = pickerProvider,
featureFlagService = featureFlagService,

View file

@ -337,7 +337,7 @@ class PinnedMessagesListPresenterTest {
actionListPresenter = { anActionListState() },
linkPresenter = { aLinkState() },
analyticsService = analyticsService,
appCoroutineScope = this,
sessionCoroutineScope = this,
)
}
}

View file

@ -727,7 +727,7 @@ class TimelinePresenterTest {
timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(),
room = room,
dispatchers = testCoroutineDispatchers(),
appScope = this,
sessionCoroutineScope = this,
navigator = messagesNavigator,
redactedVoiceMessageManager = redactedVoiceMessageManager,
endPollAction = endPollAction,