Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 28ef7ff2 authored by Alejandro Nijamkin's avatar Alejandro Nijamkin
Browse files

[flexiglass] Addresses ConcurrentModificationException

KeyguardBypassRepository depended on TunerService and was hitting it
from the background thread, probably causing a concurrent modification
if another system was writing to it from a different thread.

Since TunerService is deprecated, I changed KeyguardBypassRepository to
depend on secure settings instead.

Fix: 377907594
Test: manually verified face unlock correctly skips or doesn't fix the
lockscreen basaed on the setting
Test: unit tests updated
Flag: com.android.systemui.scene_container

Change-Id: I1f2cca79c871ba4b077a764a835a63abf8ff942d
parent 765f6a74
Loading
Loading
Loading
Loading
+12 −29
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package com.android.systemui.keyguard.data.repository
import android.annotation.IntDef
import android.content.res.Resources
import android.provider.Settings
import com.android.systemui.common.coroutine.ChannelExt.trySendWithFailureLogging
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.dagger.qualifiers.Main
@@ -27,13 +26,11 @@ import com.android.systemui.dump.DumpManager
import com.android.systemui.keyguard.shared.model.DevicePosture
import com.android.systemui.keyguard.shared.model.DevicePosture.UNKNOWN
import com.android.systemui.res.R
import com.android.systemui.tuner.TunerService
import com.android.systemui.util.kotlin.FlowDumperImpl
import com.android.systemui.util.settings.repository.UserAwareSecureSettingsRepository
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.callbackFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flowOf
@@ -48,7 +45,7 @@ constructor(
    biometricSettingsRepository: BiometricSettingsRepository,
    devicePostureRepository: DevicePostureRepository,
    dumpManager: DumpManager,
    private val tunerService: TunerService,
    secureSettingsRepository: UserAwareSecureSettingsRepository,
    @Background backgroundDispatcher: CoroutineDispatcher,
) : FlowDumperImpl(dumpManager) {

@@ -61,40 +58,26 @@ constructor(
        DevicePosture.toPosture(resources.getInteger(R.integer.config_face_auth_supported_posture))
    }

    private val dismissByDefault: Int by lazy {
        if (resources.getBoolean(com.android.internal.R.bool.config_faceAuthDismissesKeyguard)) {
            1
        } else {
            0
        }
    }

    private var bypassEnabledSetting: Flow<Boolean> =
        callbackFlow {
                val updateBypassSetting = { state: Boolean ->
                    trySendWithFailureLogging(state, TAG, "Error sending bypassSetting $state")
                }

                val tunable =
                    TunerService.Tunable { key, _ ->
                        updateBypassSetting(tunerService.getValue(key, dismissByDefault) != 0)
                    }

                updateBypassSetting(false)
                tunerService.addTunable(tunable, Settings.Secure.FACE_UNLOCK_DISMISSES_KEYGUARD)
                awaitClose { tunerService.removeTunable(tunable) }
            }
        secureSettingsRepository
            .boolSetting(
                name = Settings.Secure.FACE_UNLOCK_DISMISSES_KEYGUARD,
                defaultValue =
                    resources.getBoolean(
                        com.android.internal.R.bool.config_faceAuthDismissesKeyguard
                    ),
            )
            .flowOn(backgroundDispatcher)
            .dumpWhileCollecting("bypassEnabledSetting")

    val overrideFaceBypassSetting: Flow<Boolean> =
    private val overrideFaceBypassSetting: Flow<Boolean> =
        when (bypassOverride) {
            FACE_UNLOCK_BYPASS_ALWAYS -> flowOf(true)
            FACE_UNLOCK_BYPASS_NEVER -> flowOf(false)
            else -> bypassEnabledSetting
        }

    val isPostureAllowedForFaceAuth: Flow<Boolean> =
    private val isPostureAllowedForFaceAuth: Flow<Boolean> =
        when (configFaceAuthSupportedPosture) {
            UNKNOWN -> flowOf(true)
            else ->
+6 −0
Original line number Diff line number Diff line
@@ -85,6 +85,12 @@ abstract class UserAwareSettingsRepository(
        }
    }

    suspend fun setBoolean(name: String, value: Boolean) {
        withContext(bgContext) {
            userSettings.putBoolForUser(name, value, userRepository.getSelectedUserInfo().id)
        }
    }

    suspend fun getString(name: String): String? {
        return withContext(bgContext) {
            userSettings.getStringForUser(name, userRepository.getSelectedUserInfo().id)
+9 −33
Original line number Diff line number Diff line
@@ -34,45 +34,27 @@ import com.android.systemui.statusbar.policy.DevicePostureController.DEVICE_POST
import com.android.systemui.statusbar.policy.DevicePostureController.DEVICE_POSTURE_UNKNOWN
import com.android.systemui.statusbar.policy.devicePostureController
import com.android.systemui.testKosmos
import com.android.systemui.tuner.TunerService
import com.android.systemui.tuner.tunerService
import com.android.systemui.util.mockito.withArgCaptor
import com.android.systemui.util.settings.data.repository.userAwareSecureSettingsRepository
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.eq
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
import org.mockito.junit.MockitoJUnit
import org.mockito.junit.MockitoRule
import org.mockito.kotlin.whenever

@OptIn(ExperimentalCoroutinesApi::class)
@SmallTest
@RunWith(AndroidJUnit4::class)
@EnableSceneContainer
class KeyguardBypassRepositoryTest : SysuiTestCase() {
    @JvmField @Rule val mockito: MockitoRule = MockitoJUnit.rule()

    private lateinit var tunableCallback: TunerService.Tunable
    private lateinit var postureControllerCallback: DevicePostureController.Callback

    private val kosmos = testKosmos()
    private lateinit var underTest: KeyguardBypassRepository
    private val testScope = kosmos.testScope

    @Before
    fun setup() {
        MockitoAnnotations.initMocks(this)
    }

    // overrideFaceBypassSetting overridden to true
    // isFaceEnrolledAndEnabled true
    // isPostureAllowedForFaceAuth true/false on posture changes
@@ -148,24 +130,25 @@ class KeyguardBypassRepositoryTest : SysuiTestCase() {
            val bypassEnabled by collectLastValue(underTest.isBypassAvailable)
            runCurrent()
            postureControllerCallback = kosmos.devicePostureController.verifyCallback()
            tunableCallback = kosmos.tunerService.captureCallback()

            // Update face auth posture to match config
            postureControllerCallback.onPostureChanged(DEVICE_POSTURE_CLOSED)

            // FACE_UNLOCK_DISMISSES_KEYGUARD setting true
            whenever(kosmos.tunerService.getValue(eq(faceUnlockDismissesKeyguard), anyInt()))
                .thenReturn(1)
            tunableCallback.onTuningChanged(faceUnlockDismissesKeyguard, "")
            kosmos.userAwareSecureSettingsRepository.setBoolean(
                Settings.Secure.FACE_UNLOCK_DISMISSES_KEYGUARD,
                true,
            )

            runCurrent()
            // Assert bypass enabled
            assertThat(bypassEnabled).isTrue()

            // FACE_UNLOCK_DISMISSES_KEYGUARD setting false
            whenever(kosmos.tunerService.getValue(eq(faceUnlockDismissesKeyguard), anyInt()))
                .thenReturn(0)
            tunableCallback.onTuningChanged(faceUnlockDismissesKeyguard, "")
            kosmos.userAwareSecureSettingsRepository.setBoolean(
                Settings.Secure.FACE_UNLOCK_DISMISSES_KEYGUARD,
                false,
            )

            runCurrent()
            // Assert bypass not enabled
@@ -229,10 +212,3 @@ class KeyguardBypassRepositoryTest : SysuiTestCase() {
        private const val FACE_UNLOCK_BYPASS_NEVER = 2
    }
}

private const val faceUnlockDismissesKeyguard = Settings.Secure.FACE_UNLOCK_DISMISSES_KEYGUARD

private fun TunerService.captureCallback() =
    withArgCaptor<TunerService.Tunable> {
        verify(this@captureCallback).addTunable(capture(), eq(faceUnlockDismissesKeyguard))
    }
+2 −2
Original line number Diff line number Diff line
@@ -25,8 +25,8 @@ import com.android.systemui.res.R
import com.android.systemui.statusbar.policy.DevicePostureController
import com.android.systemui.statusbar.policy.DevicePostureController.DEVICE_POSTURE_CLOSED
import com.android.systemui.statusbar.policy.DevicePostureController.DEVICE_POSTURE_UNKNOWN
import com.android.systemui.tuner.tunerService
import com.android.systemui.util.mockito.withArgCaptor
import com.android.systemui.util.settings.data.repository.userAwareSecureSettingsRepository
import org.mockito.ArgumentMatchers.any
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
@@ -37,7 +37,7 @@ val Kosmos.keyguardBypassRepository: KeyguardBypassRepository by Fixture {
        biometricSettingsRepository,
        devicePostureRepository,
        dumpManager,
        tunerService,
        userAwareSecureSettingsRepository,
        testDispatcher,
    )
}