From 2c4c44d195a092e81ef2cc33a233830740c45454 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Wed, 25 Jan 2023 13:24:13 -0800 Subject: [PATCH] Quick affordances crash fix for secondary users. This CL fixes a few issues that are making lock screen customization and lock screen shortcuts unavailable for secondary users. The attached bug occurs because we're querying state off the main thread. The first part of the CL modifies our content provider to move its querying onto the main thread while blocking the calling thread (which is a background thread). After fixing that, several problems emerged. When we query for affordances, we iterate through each affordance configuration and check its "picker state". For the camera and flashlight affordances, this was problematic. The camera affordance had a complex dependnecy chain that ended up relying on DevicePolicyManager which was not being set until after the CoreStartable for CoreSurfacesImpl ran its start() method, which doesn't happen at all for secondary users. I worked around that issue by simplifying the use-case in the camera affordance to just directly check whether the device has camera hardware. If an admin doesn't allow it, I presume that opening the camera app will fail. The flashlight affordance triggered a broadcast to be sent for FLASHLIGHT_CHANGED. The problem is that this is a protected broadcast designed to prevent non-framework apps from sending it; which also means that our secondary user process does not have the necessary security permission to send it, causing the SystemUI process to crash. I removed the broadcast sending. This was not needed by the flashlight affordance or by the quick setting or by the smart space "turn off flashlight" link. Also, the unit test for the controller passes. Fix: 265245824 Test: unit tests still pass, some were updated to pass Test: manully verified the wallpaper picker loads and shows all affordance options for both primary and secondary users Test: manually verified that selecting any of the affordance options in wallpaper picker produces fully working experiences for the lock screen shortcuts (tested all on both primary and secondary user) Test: manually verified that flashlight toggles on and off correctly from the lock screen shortcut, smart space, and quick settings tile, for primary and secondary users alike Change-Id: Ifb2b47e7d521c67378b2c64f6f32f863319a5b5d --- .../docs/device-entry/quickaffordance.md | 4 ++++ .../systemui/keyguard/CustomizationProvider.kt | 17 +++++++++++------ .../CameraQuickAffordanceConfig.kt | 5 +++-- .../policy/FlashlightControllerImpl.java | 2 -- .../keyguard/CustomizationProviderTest.kt | 2 ++ .../CameraQuickAffordanceConfigTest.kt | 8 ++++++-- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/SystemUI/docs/device-entry/quickaffordance.md b/packages/SystemUI/docs/device-entry/quickaffordance.md index ccb35fac36f2f..79d5718efe0db 100644 --- a/packages/SystemUI/docs/device-entry/quickaffordance.md +++ b/packages/SystemUI/docs/device-entry/quickaffordance.md @@ -52,6 +52,10 @@ A picker experience may: * Unselect an already-selected quick affordance from a slot * Unselect all already-selected quick affordances from a slot +## Testing +* Add a unit test for your implementation of `KeyguardQuickAffordanceConfig` +* Manually verify that your implementation works in multi-user environments from both the main user and a secondary user + ## Debugging To see the current state of the system, you can run `dumpsys`: diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/CustomizationProvider.kt b/packages/SystemUI/src/com/android/systemui/keyguard/CustomizationProvider.kt index 482138e6c277a..680c504d50fce 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/CustomizationProvider.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/CustomizationProvider.kt @@ -31,10 +31,12 @@ import android.os.Bundle import android.util.Log import com.android.systemui.SystemUIAppComponentFactoryBase import com.android.systemui.SystemUIAppComponentFactoryBase.ContextAvailableCallback +import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.keyguard.domain.interactor.KeyguardQuickAffordanceInteractor import com.android.systemui.keyguard.ui.preview.KeyguardRemotePreviewManager import com.android.systemui.shared.customization.data.content.CustomizationProviderContract as Contract import javax.inject.Inject +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.runBlocking class CustomizationProvider : @@ -42,6 +44,7 @@ class CustomizationProvider : @Inject lateinit var interactor: KeyguardQuickAffordanceInteractor @Inject lateinit var previewManager: KeyguardRemotePreviewManager + @Inject @Main lateinit var mainDispatcher: CoroutineDispatcher private lateinit var contextAvailableCallback: ContextAvailableCallback @@ -138,12 +141,14 @@ class CustomizationProvider : selectionArgs: Array?, sortOrder: String?, ): Cursor? { - return when (uriMatcher.match(uri)) { - MATCH_CODE_ALL_AFFORDANCES -> runBlocking { queryAffordances() } - MATCH_CODE_ALL_SLOTS -> querySlots() - MATCH_CODE_ALL_SELECTIONS -> runBlocking { querySelections() } - MATCH_CODE_ALL_FLAGS -> queryFlags() - else -> null + return runBlocking(mainDispatcher) { + when (uriMatcher.match(uri)) { + MATCH_CODE_ALL_AFFORDANCES -> queryAffordances() + MATCH_CODE_ALL_SLOTS -> querySlots() + MATCH_CODE_ALL_SELECTIONS -> querySelections() + MATCH_CODE_ALL_FLAGS -> queryFlags() + else -> null + } } } diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfig.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfig.kt index f6e6d6b7dc1b0..5a9f7752277ec 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfig.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfig.kt @@ -19,6 +19,7 @@ package com.android.systemui.keyguard.data.quickaffordance import android.app.StatusBarManager import android.content.Context +import android.content.pm.PackageManager import com.android.systemui.R import com.android.systemui.animation.Expandable import com.android.systemui.camera.CameraGestureHelper @@ -26,7 +27,6 @@ import com.android.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Application -import com.android.systemui.statusbar.StatusBarState import dagger.Lazy import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -37,6 +37,7 @@ class CameraQuickAffordanceConfig @Inject constructor( @Application private val context: Context, + private val packageManager: PackageManager, private val cameraGestureHelper: Lazy, ) : KeyguardQuickAffordanceConfig { @@ -79,6 +80,6 @@ constructor( } private fun isLaunchable(): Boolean { - return cameraGestureHelper.get().canCameraGestureBeLaunched(StatusBarState.KEYGUARD) + return packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_ANY) } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java index 9946b4b9ecaaa..5dcafb37f57e0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java @@ -17,7 +17,6 @@ package com.android.systemui.statusbar.policy; import android.annotation.WorkerThread; -import android.content.Intent; import android.content.pm.PackageManager; import android.hardware.camera2.CameraAccessException; import android.hardware.camera2.CameraCharacteristics; @@ -255,7 +254,6 @@ public class FlashlightControllerImpl implements FlashlightController { setTorchMode(enabled); mSecureSettings.putInt(Settings.Secure.FLASHLIGHT_AVAILABLE, 1); mSecureSettings.putInt(Secure.FLASHLIGHT_ENABLED, enabled ? 1 : 0); - mBroadcastSender.sendBroadcast(new Intent(ACTION_FLASHLIGHT_CHANGED)); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/CustomizationProviderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/CustomizationProviderTest.kt index c0af0cb4089e7..fb54d6d6b2d43 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/CustomizationProviderTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/CustomizationProviderTest.kt @@ -62,6 +62,7 @@ import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test @@ -184,6 +185,7 @@ class CustomizationProviderTest : SysuiTestCase() { mainDispatcher = testDispatcher, backgroundHandler = backgroundHandler, ) + underTest.mainDispatcher = UnconfinedTestDispatcher() underTest.attachInfoForTesting( context, diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfigTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfigTest.kt index 8da4eae2f64a0..58cdec447cc61 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfigTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/data/quickaffordance/CameraQuickAffordanceConfigTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.keyguard.data.quickaffordance import android.app.StatusBarManager import android.content.Context +import android.content.pm.PackageManager import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.camera.CameraGestureHelper @@ -32,7 +33,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.mockito.Mock -import org.mockito.Mockito.anyInt import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations @@ -43,16 +43,19 @@ class CameraQuickAffordanceConfigTest : SysuiTestCase() { @Mock private lateinit var cameraGestureHelper: CameraGestureHelper @Mock private lateinit var context: Context + @Mock private lateinit var packageManager: PackageManager private lateinit var underTest: CameraQuickAffordanceConfig @Before fun setUp() { MockitoAnnotations.initMocks(this) + setLaunchable(true) underTest = CameraQuickAffordanceConfig( context, + packageManager, ) { cameraGestureHelper } @@ -86,6 +89,7 @@ class CameraQuickAffordanceConfigTest : SysuiTestCase() { } private fun setLaunchable(isLaunchable: Boolean) { - whenever(cameraGestureHelper.canCameraGestureBeLaunched(anyInt())).thenReturn(isLaunchable) + whenever(packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_ANY)) + .thenReturn(isLaunchable) } }