diff options
| author | 2022-10-05 01:26:21 +0000 | |
|---|---|---|
| committer | 2022-10-05 01:26:21 +0000 | |
| commit | 106f08490ce8e6aec9e5b78767e0de4e42fdb710 (patch) | |
| tree | 04ae352f084108b92beb24b03778cd84e88dd146 | |
| parent | cb077acdbd27d3b456676992c62e9fcaed310088 (diff) | |
| parent | 37f4b300c66fe893f039a38b56b5deb1b6e0c6a3 (diff) | |
Merge "UserSwitcherController impl uses UserInteractor" into tm-qpr-dev am: 37f4b300c6
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20090976
Change-Id: I1a3d139feb426db437533f92c1278218d0eace02
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
5 files changed, 262 insertions, 77 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt index 4932a6544d05..16926566105c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt @@ -17,12 +17,21 @@ package com.android.systemui.statusbar.policy +import android.content.Context import android.content.Intent import android.view.View +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.flags.FeatureFlags import com.android.systemui.flags.Flags +import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor +import com.android.systemui.plugins.ActivityStarter import com.android.systemui.qs.user.UserSwitchDialogController import com.android.systemui.user.data.source.UserRecord +import com.android.systemui.user.domain.interactor.GuestUserInteractor +import com.android.systemui.user.domain.interactor.UserInteractor +import com.android.systemui.user.legacyhelper.data.LegacyUserDataHelper +import com.android.systemui.user.legacyhelper.ui.LegacyUserUiHelper import dagger.Lazy import java.io.PrintWriter import java.lang.ref.WeakReference @@ -30,11 +39,17 @@ import javax.inject.Inject import kotlinx.coroutines.flow.Flow /** Implementation of [UserSwitcherController]. */ +@SysUISingleton class UserSwitcherControllerImpl @Inject constructor( - private val flags: FeatureFlags, + @Application private val applicationContext: Context, + flags: FeatureFlags, @Suppress("DEPRECATION") private val oldImpl: Lazy<UserSwitcherControllerOldImpl>, + private val userInteractorLazy: Lazy<UserInteractor>, + private val guestUserInteractorLazy: Lazy<GuestUserInteractor>, + private val keyguardInteractorLazy: Lazy<KeyguardInteractor>, + private val activityStarter: ActivityStarter, ) : UserSwitcherController { private val useInteractor: Boolean = @@ -42,15 +57,21 @@ constructor( !flags.isEnabled(Flags.USER_INTERACTOR_AND_REPO_USE_CONTROLLER) private val _oldImpl: UserSwitcherControllerOldImpl get() = oldImpl.get() + private val userInteractor: UserInteractor by lazy { userInteractorLazy.get() } + private val guestUserInteractor: GuestUserInteractor by lazy { guestUserInteractorLazy.get() } + private val keyguardInteractor: KeyguardInteractor by lazy { keyguardInteractorLazy.get() } - private fun notYetImplemented(): Nothing { - error("Not yet implemented!") + private val callbackCompatMap = + mutableMapOf<UserSwitcherController.UserSwitchCallback, UserInteractor.UserCallback>() + + private fun notSupported(): Nothing { + error("Not supported in the new implementation!") } override val users: ArrayList<UserRecord> get() = if (useInteractor) { - notYetImplemented() + userInteractor.userRecords.value } else { _oldImpl.users } @@ -58,15 +79,13 @@ constructor( override val isSimpleUserSwitcher: Boolean get() = if (useInteractor) { - notYetImplemented() + userInteractor.isSimpleUserSwitcher } else { _oldImpl.isSimpleUserSwitcher } override fun init(view: View) { - if (useInteractor) { - notYetImplemented() - } else { + if (!useInteractor) { _oldImpl.init(view) } } @@ -74,7 +93,7 @@ constructor( override val currentUserRecord: UserRecord? get() = if (useInteractor) { - notYetImplemented() + userInteractor.selectedUserRecord.value } else { _oldImpl.currentUserRecord } @@ -82,7 +101,14 @@ constructor( override val currentUserName: String? get() = if (useInteractor) { - notYetImplemented() + currentUserRecord?.let { + LegacyUserUiHelper.getUserRecordName( + context = applicationContext, + record = it, + isGuestUserAutoCreated = userInteractor.isGuestUserAutoCreated, + isGuestUserResetting = userInteractor.isGuestUserResetting, + ) + } } else { _oldImpl.currentUserName } @@ -92,7 +118,7 @@ constructor( dialogShower: UserSwitchDialogController.DialogShower? ) { if (useInteractor) { - notYetImplemented() + userInteractor.selectUser(userId) } else { _oldImpl.onUserSelected(userId, dialogShower) } @@ -101,7 +127,7 @@ constructor( override val isAddUsersFromLockScreenEnabled: Flow<Boolean> get() = if (useInteractor) { - notYetImplemented() + notSupported() } else { _oldImpl.isAddUsersFromLockScreenEnabled } @@ -109,7 +135,7 @@ constructor( override val isGuestUserAutoCreated: Boolean get() = if (useInteractor) { - notYetImplemented() + userInteractor.isGuestUserAutoCreated } else { _oldImpl.isGuestUserAutoCreated } @@ -117,7 +143,7 @@ constructor( override val isGuestUserResetting: Boolean get() = if (useInteractor) { - notYetImplemented() + userInteractor.isGuestUserResetting } else { _oldImpl.isGuestUserResetting } @@ -126,7 +152,7 @@ constructor( dialogShower: UserSwitchDialogController.DialogShower?, ) { if (useInteractor) { - notYetImplemented() + notSupported() } else { _oldImpl.createAndSwitchToGuestUser(dialogShower) } @@ -134,7 +160,7 @@ constructor( override fun showAddUserDialog(dialogShower: UserSwitchDialogController.DialogShower?) { if (useInteractor) { - notYetImplemented() + notSupported() } else { _oldImpl.showAddUserDialog(dialogShower) } @@ -142,23 +168,31 @@ constructor( override fun startSupervisedUserActivity() { if (useInteractor) { - notYetImplemented() + notSupported() } else { _oldImpl.startSupervisedUserActivity() } } override fun onDensityOrFontScaleChanged() { - if (useInteractor) { - notYetImplemented() - } else { + if (!useInteractor) { _oldImpl.onDensityOrFontScaleChanged() } } override fun addAdapter(adapter: WeakReference<BaseUserSwitcherAdapter>) { if (useInteractor) { - notYetImplemented() + userInteractor.addCallback( + object : UserInteractor.UserCallback { + override fun isEvictable(): Boolean { + return adapter.get() == null + } + + override fun onUserStateChanged() { + adapter.get()?.notifyDataSetChanged() + } + } + ) } else { _oldImpl.addAdapter(adapter) } @@ -169,7 +203,11 @@ constructor( dialogShower: UserSwitchDialogController.DialogShower?, ) { if (useInteractor) { - notYetImplemented() + if (LegacyUserDataHelper.isUser(record)) { + userInteractor.selectUser(record.resolveId()) + } else { + userInteractor.executeAction(LegacyUserDataHelper.toUserActionModel(record)) + } } else { _oldImpl.onUserListItemClicked(record, dialogShower) } @@ -177,7 +215,10 @@ constructor( override fun removeGuestUser(guestUserId: Int, targetUserId: Int) { if (useInteractor) { - notYetImplemented() + userInteractor.removeGuestUser( + guestUserId = guestUserId, + targetUserId = targetUserId, + ) } else { _oldImpl.removeGuestUser(guestUserId, targetUserId) } @@ -189,7 +230,7 @@ constructor( forceRemoveGuestOnExit: Boolean ) { if (useInteractor) { - notYetImplemented() + userInteractor.exitGuestUser(guestUserId, targetUserId, forceRemoveGuestOnExit) } else { _oldImpl.exitGuestUser(guestUserId, targetUserId, forceRemoveGuestOnExit) } @@ -197,7 +238,7 @@ constructor( override fun schedulePostBootGuestCreation() { if (useInteractor) { - notYetImplemented() + guestUserInteractor.onDeviceBootCompleted() } else { _oldImpl.schedulePostBootGuestCreation() } @@ -206,14 +247,14 @@ constructor( override val isKeyguardShowing: Boolean get() = if (useInteractor) { - notYetImplemented() + keyguardInteractor.isKeyguardShowing() } else { _oldImpl.isKeyguardShowing } override fun startActivity(intent: Intent) { if (useInteractor) { - notYetImplemented() + activityStarter.startActivity(intent, /* dismissShade= */ false) } else { _oldImpl.startActivity(intent) } @@ -221,7 +262,7 @@ constructor( override fun refreshUsers(forcePictureLoadForId: Int) { if (useInteractor) { - notYetImplemented() + userInteractor.refreshUsers() } else { _oldImpl.refreshUsers(forcePictureLoadForId) } @@ -229,7 +270,14 @@ constructor( override fun addUserSwitchCallback(callback: UserSwitcherController.UserSwitchCallback) { if (useInteractor) { - notYetImplemented() + val interactorCallback = + object : UserInteractor.UserCallback { + override fun onUserStateChanged() { + callback.onUserSwitched() + } + } + callbackCompatMap[callback] = interactorCallback + userInteractor.addCallback(interactorCallback) } else { _oldImpl.addUserSwitchCallback(callback) } @@ -237,7 +285,10 @@ constructor( override fun removeUserSwitchCallback(callback: UserSwitcherController.UserSwitchCallback) { if (useInteractor) { - notYetImplemented() + val interactorCallback = callbackCompatMap.remove(callback) + if (interactorCallback != null) { + userInteractor.removeCallback(interactorCallback) + } } else { _oldImpl.removeUserSwitchCallback(callback) } @@ -245,7 +296,7 @@ constructor( override fun dump(pw: PrintWriter, args: Array<out String>) { if (useInteractor) { - notYetImplemented() + userInteractor.dump(pw) } else { _oldImpl.dump(pw, args) } diff --git a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/GuestUserInteractor.kt b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/GuestUserInteractor.kt index 27748128a557..07e5cf9d9df2 100644 --- a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/GuestUserInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/GuestUserInteractor.kt @@ -173,7 +173,7 @@ constructor( } /** Removes the guest user from the device. */ - private suspend fun remove( + suspend fun remove( @UserIdInt guestUserId: Int, @UserIdInt targetUserId: Int, showDialog: (ShowDialogRequestModel) -> Unit, diff --git a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt index e6bb9bcbc264..a84238c1559a 100644 --- a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt @@ -52,8 +52,7 @@ import com.android.systemui.user.legacyhelper.data.LegacyUserDataHelper import com.android.systemui.user.shared.model.UserActionModel import com.android.systemui.user.shared.model.UserModel import com.android.systemui.util.kotlin.pairwise -import java.util.Collections -import java.util.WeakHashMap +import java.io.PrintWriter import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -70,6 +69,9 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext /** Encapsulates business logic to interact with user data and systems. */ @@ -96,7 +98,9 @@ constructor( * Defines interface for classes that can be notified when the state of users on the device is * changed. */ - fun interface UserCallback { + interface UserCallback { + /** Returns `true` if this callback can be cleaned-up. */ + fun isEvictable(): Boolean = false /** Notifies that the state of users on the device has changed. */ fun onUserStateChanged() } @@ -110,7 +114,8 @@ constructor( com.android.internal.R.string.config_supervisedUserCreationPackage ) - private val callbacks = Collections.newSetFromMap(WeakHashMap<UserCallback, Boolean>()) + private val callbackMutex = Mutex() + private val callbacks = mutableSetOf<UserCallback>() /** List of current on-device users to select from. */ val users: Flow<List<UserModel>> @@ -263,6 +268,7 @@ constructor( } ) } + .onEach { notifyCallbacks() } .stateIn( scope = applicationScope, started = SharingStarted.Eagerly, @@ -307,30 +313,6 @@ constructor( error("Not supported in the old implementation!") } - fun addCallback(callback: UserCallback) { - callbacks.add(callback) - } - - fun removeCallback(callback: UserCallback) { - callbacks.remove(callback) - } - - fun onDialogShown() { - _dialogShowRequests.value = null - } - - fun onDialogDismissed() { - _dialogDismissRequests.value = null - } - - private fun showDialog(request: ShowDialogRequestModel) { - _dialogShowRequests.value = request - } - - private fun dismissDialog() { - _dialogDismissRequests.value = Unit - } - init { if (isNewImpl) { refreshUsersScheduler.refreshIfNotPaused() @@ -364,6 +346,46 @@ constructor( } } + fun addCallback(callback: UserCallback) { + applicationScope.launch { callbackMutex.withLock { callbacks.add(callback) } } + } + + fun removeCallback(callback: UserCallback) { + applicationScope.launch { callbackMutex.withLock { callbacks.remove(callback) } } + } + + fun refreshUsers() { + refreshUsersScheduler.refreshIfNotPaused() + } + + fun onDialogShown() { + _dialogShowRequests.value = null + } + + fun onDialogDismissed() { + _dialogDismissRequests.value = null + } + + fun dump(pw: PrintWriter) { + pw.println("UserInteractor state:") + pw.println(" lastSelectedNonGuestUserId=${repository.lastSelectedNonGuestUserId}") + + val users = userRecords.value.filter { it.info != null } + pw.println(" userCount=${userRecords.value.count { LegacyUserDataHelper.isUser(it) }}") + for (i in users.indices) { + pw.println(" ${users[i]}") + } + + val actions = userRecords.value.filter { it.info == null } + pw.println(" actionCount=${userRecords.value.count { !LegacyUserDataHelper.isUser(it) }}") + for (i in actions.indices) { + pw.println(" ${actions[i]}") + } + + pw.println("isSimpleUserSwitcher=$isSimpleUserSwitcher") + pw.println("isGuestUserAutoCreated=$isGuestUserAutoCreated") + } + fun onDeviceBootCompleted() { guestUserInteractor.onDeviceBootCompleted() } @@ -460,6 +482,60 @@ constructor( } } + fun exitGuestUser( + @UserIdInt guestUserId: Int, + @UserIdInt targetUserId: Int, + forceRemoveGuestOnExit: Boolean, + ) { + guestUserInteractor.exit( + guestUserId = guestUserId, + targetUserId = targetUserId, + forceRemoveGuestOnExit = forceRemoveGuestOnExit, + showDialog = this::showDialog, + dismissDialog = this::dismissDialog, + switchUser = this::switchUser, + ) + } + + fun removeGuestUser( + @UserIdInt guestUserId: Int, + @UserIdInt targetUserId: Int, + ) { + applicationScope.launch { + guestUserInteractor.remove( + guestUserId = guestUserId, + targetUserId = targetUserId, + ::showDialog, + ::dismissDialog, + ::selectUser, + ) + } + } + + private fun showDialog(request: ShowDialogRequestModel) { + _dialogShowRequests.value = request + } + + private fun dismissDialog() { + _dialogDismissRequests.value = Unit + } + + private fun notifyCallbacks() { + applicationScope.launch { + callbackMutex.withLock { + val iterator = callbacks.iterator() + while (iterator.hasNext()) { + val callback = iterator.next() + if (!callback.isEvictable()) { + callback.onUserStateChanged() + } else { + iterator.remove() + } + } + } + } + } + private suspend fun toRecord( userInfo: UserInfo, selectedUserId: Int, @@ -498,21 +574,6 @@ constructor( ) } - private fun exitGuestUser( - @UserIdInt guestUserId: Int, - @UserIdInt targetUserId: Int, - forceRemoveGuestOnExit: Boolean, - ) { - guestUserInteractor.exit( - guestUserId = guestUserId, - targetUserId = targetUserId, - forceRemoveGuestOnExit = forceRemoveGuestOnExit, - showDialog = this::showDialog, - dismissDialog = this::dismissDialog, - switchUser = this::switchUser, - ) - } - private fun switchUser(userId: Int) { // TODO(b/246631653): track jank and lantecy like in the old impl. refreshUsersScheduler.pause() @@ -533,7 +594,7 @@ constructor( dismissDialog() val selectedUserId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1) if (previousUserInfo?.id != selectedUserId) { - callbacks.forEach { it.onUserStateChanged() } + notifyCallbacks() restartSecondaryService(selectedUserId) } if (guestUserInteractor.isGuestUserAutoCreated) { diff --git a/packages/SystemUI/src/com/android/systemui/user/legacyhelper/data/LegacyUserDataHelper.kt b/packages/SystemUI/src/com/android/systemui/user/legacyhelper/data/LegacyUserDataHelper.kt index 8f6662f7b192..137de1544b2d 100644 --- a/packages/SystemUI/src/com/android/systemui/user/legacyhelper/data/LegacyUserDataHelper.kt +++ b/packages/SystemUI/src/com/android/systemui/user/legacyhelper/data/LegacyUserDataHelper.kt @@ -83,6 +83,21 @@ object LegacyUserDataHelper { ) } + fun toUserActionModel(record: UserRecord): UserActionModel { + check(!isUser(record)) + + return when { + record.isAddUser -> UserActionModel.ADD_USER + record.isAddSupervisedUser -> UserActionModel.ADD_SUPERVISED_USER + record.isGuest -> UserActionModel.ENTER_GUEST_MODE + else -> error("Not a known action: $record") + } + } + + fun isUser(record: UserRecord): Boolean { + return record.info != null + } + private fun getEnforcedAdmin( context: Context, selectedUserId: Int, diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/GuestUserInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/GuestUserInteractorTest.kt index 6b4c9ed38b47..120bf791c462 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/GuestUserInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/GuestUserInteractorTest.kt @@ -289,15 +289,73 @@ class GuestUserInteractorTest : SysuiTestCase() { verifyDidNotExit() } + @Test + fun `remove - returns to target user`() = + runBlocking(IMMEDIATE) { + whenever(manager.markGuestForDeletion(anyInt())).thenReturn(true) + repository.setSelectedUserInfo(GUEST_USER_INFO) + + val targetUserId = NON_GUEST_USER_INFO.id + underTest.remove( + guestUserId = GUEST_USER_INFO.id, + targetUserId = targetUserId, + showDialog = showDialog, + dismissDialog = dismissDialog, + switchUser = switchUser, + ) + + verify(manager).markGuestForDeletion(GUEST_USER_INFO.id) + verify(manager).removeUser(GUEST_USER_INFO.id) + verify(switchUser).invoke(targetUserId) + } + + @Test + fun `remove - selected different from guest user - do nothing`() = + runBlocking(IMMEDIATE) { + whenever(manager.markGuestForDeletion(anyInt())).thenReturn(true) + repository.setSelectedUserInfo(NON_GUEST_USER_INFO) + + underTest.remove( + guestUserId = GUEST_USER_INFO.id, + targetUserId = 123, + showDialog = showDialog, + dismissDialog = dismissDialog, + switchUser = switchUser, + ) + + verifyDidNotRemove() + } + + @Test + fun `remove - selected is actually not a guest user - do nothing`() = + runBlocking(IMMEDIATE) { + whenever(manager.markGuestForDeletion(anyInt())).thenReturn(true) + repository.setSelectedUserInfo(NON_GUEST_USER_INFO) + + underTest.remove( + guestUserId = NON_GUEST_USER_INFO.id, + targetUserId = 123, + showDialog = showDialog, + dismissDialog = dismissDialog, + switchUser = switchUser, + ) + + verifyDidNotRemove() + } + private fun setAllowedToAdd(isAllowed: Boolean = true) { whenever(deviceProvisionedController.isDeviceProvisioned).thenReturn(isAllowed) whenever(devicePolicyManager.isDeviceManaged).thenReturn(!isAllowed) } private fun verifyDidNotExit() { + verifyDidNotRemove() verify(manager, never()).getUserInfo(anyInt()) - verify(manager, never()).markGuestForDeletion(anyInt()) verify(uiEventLogger, never()).log(any()) + } + + private fun verifyDidNotRemove() { + verify(manager, never()).markGuestForDeletion(anyInt()) verify(showDialog, never()).invoke(any()) verify(dismissDialog, never()).invoke() verify(switchUser, never()).invoke(anyInt()) |