Skip to content

ANR on session start when sending client info is enabled #7605

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

Merged
merged 2 commits into from
Nov 18, 2022
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
1 change: 1 addition & 0 deletions changelog.d/7604.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ANR on session start when sending client info is enabled
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ class ActiveSessionHolder @Inject constructor(
}
?: sessionInitializer.tryInitialize(readCurrentSession = { activeSessionReference.get() }) { session ->
setActiveSession(session)
runBlocking {
configureAndStartSessionUseCase.execute(session, startSyncing = startSync)
}
configureAndStartSessionUseCase.execute(session, startSyncing = startSync)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import im.vector.app.core.extensions.startSyncing
import im.vector.app.core.notification.EnableNotificationsSettingUpdater
import im.vector.app.core.session.clientinfo.UpdateMatrixClientInfoUseCase
import im.vector.app.features.call.webrtc.WebRtcCallManager
import im.vector.app.features.session.coroutineScope
import im.vector.app.features.settings.VectorPreferences
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.sync.FilterService
import timber.log.Timber
Expand All @@ -36,7 +38,7 @@ class ConfigureAndStartSessionUseCase @Inject constructor(
private val enableNotificationsSettingUpdater: EnableNotificationsSettingUpdater,
) {

suspend fun execute(session: Session, startSyncing: Boolean = true) {
fun execute(session: Session, startSyncing: Boolean = true) {
Timber.i("Configure and start session for ${session.myUserId}. startSyncing: $startSyncing")
session.open()
session.filterService().setFilter(FilterService.FilterPreset.ElementFilter)
Expand All @@ -45,8 +47,10 @@ class ConfigureAndStartSessionUseCase @Inject constructor(
}
session.pushersService().refreshPushers()
webRtcCallManager.checkForProtocolsSupportIfNeeded()
if (vectorPreferences.isClientInfoRecordingEnabled()) {
updateMatrixClientInfoUseCase.execute(session)
session.coroutineScope.launch {
if (vectorPreferences.isClientInfoRecordingEnabled()) {
updateMatrixClientInfoUseCase.execute(session)
}
}
enableNotificationsSettingUpdater.onSessionsStarted(session)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.core.session

import im.vector.app.core.extensions.startSyncing
import im.vector.app.core.session.clientinfo.UpdateMatrixClientInfoUseCase
import im.vector.app.features.session.coroutineScope
import im.vector.app.test.fakes.FakeContext
import im.vector.app.test.fakes.FakeEnableNotificationsSettingUpdater
import im.vector.app.test.fakes.FakeSession
Expand All @@ -32,6 +33,7 @@ import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.unmockkAll
import io.mockk.verify
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Before
Expand All @@ -57,6 +59,7 @@ class ConfigureAndStartSessionUseCaseTest {
@Before
fun setup() {
mockkStatic("im.vector.app.core.extensions.SessionKt")
mockkStatic("im.vector.app.features.session.SessionCoroutineScopesKt")
}

@After
Expand All @@ -68,13 +71,15 @@ class ConfigureAndStartSessionUseCaseTest {
fun `given start sync needed and client info recording enabled when execute then it should be configured properly`() = runTest {
// Given
val fakeSession = givenASession()
every { fakeSession.coroutineScope } returns this
fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds()
coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) }
fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = true)
fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession)

// When
configureAndStartSessionUseCase.execute(fakeSession, startSyncing = true)
advanceUntilIdle()

// Then
verify { fakeSession.startSyncing(fakeContext.instance) }
Expand All @@ -88,13 +93,15 @@ class ConfigureAndStartSessionUseCaseTest {
fun `given start sync needed and client info recording disabled when execute then it should be configured properly`() = runTest {
// Given
val fakeSession = givenASession()
every { fakeSession.coroutineScope } returns this
fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds()
coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) }
fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = false)
fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession)

// When
configureAndStartSessionUseCase.execute(fakeSession, startSyncing = true)
advanceUntilIdle()

// Then
verify { fakeSession.startSyncing(fakeContext.instance) }
Expand All @@ -108,13 +115,15 @@ class ConfigureAndStartSessionUseCaseTest {
fun `given a session and no start sync needed when execute then it should be configured properly`() = runTest {
// Given
val fakeSession = givenASession()
every { fakeSession.coroutineScope } returns this
fakeWebRtcCallManager.givenCheckForProtocolsSupportIfNeededSucceeds()
coJustRun { fakeUpdateMatrixClientInfoUseCase.execute(any()) }
fakeVectorPreferences.givenIsClientInfoRecordingEnabled(isEnabled = true)
fakeEnableNotificationsSettingUpdater.givenOnSessionsStarted(fakeSession)

// When
configureAndStartSessionUseCase.execute(fakeSession, startSyncing = false)
advanceUntilIdle()

// Then
verify(inverse = true) { fakeSession.startSyncing(fakeContext.instance) }
Expand Down