Fix bug where switching users didn't dismiss UI.

There's a bug where, with the new implementation, when the user switches
to a different user on a phone, the user switcher dialog is not properly
dismissed.

This CL takes care of that by properly dismissing the DialogShower, if
one is passed in.

Fix: 251733467
Test: Tests included. Verified that the bug no longer reproduces after
the fix.

Change-Id: I053eec01051a863e375906c17a64932aa865df7e
This commit is contained in:
Alejandro Nijamkin
2022-10-10 12:20:12 -07:00
committed by Ale Nijamkin
parent 56223dafd0
commit dc6c303983
4 changed files with 41 additions and 16 deletions

View File

@@ -117,7 +117,7 @@ constructor(
dialogShower: UserSwitchDialogController.DialogShower?
) {
if (useInteractor) {
userInteractor.selectUser(userId)
userInteractor.selectUser(userId, dialogShower)
} else {
_oldImpl.onUserSelected(userId, dialogShower)
}
@@ -202,7 +202,7 @@ constructor(
dialogShower: UserSwitchDialogController.DialogShower?,
) {
if (useInteractor) {
userInteractor.onRecordSelected(record)
userInteractor.onRecordSelected(record, dialogShower)
} else {
_oldImpl.onUserListItemClicked(record, dialogShower)
}

View File

@@ -43,6 +43,7 @@ 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.statusbar.policy.UserSwitcherController
import com.android.systemui.telephony.domain.interactor.TelephonyInteractor
import com.android.systemui.user.data.repository.UserRepository
@@ -391,19 +392,23 @@ constructor(
}
/** Switches to the user or executes the action represented by the given record. */
fun onRecordSelected(record: UserRecord) {
fun onRecordSelected(
record: UserRecord,
dialogShower: UserSwitchDialogController.DialogShower? = null,
) {
if (LegacyUserDataHelper.isUser(record)) {
// It's safe to use checkNotNull around record.info because isUser only returns true
// if record.info is not null.
selectUser(checkNotNull(record.info).id)
selectUser(checkNotNull(record.info).id, dialogShower)
} else {
executeAction(LegacyUserDataHelper.toUserActionModel(record))
executeAction(LegacyUserDataHelper.toUserActionModel(record), dialogShower)
}
}
/** Switches to the user with the given user ID. */
fun selectUser(
newlySelectedUserId: Int,
dialogShower: UserSwitchDialogController.DialogShower? = null,
) {
if (isNewImpl) {
val currentlySelectedUserInfo = repository.getSelectedUserInfo()
@@ -439,22 +444,28 @@ constructor(
return
}
dialogShower?.dismiss()
switchUser(newlySelectedUserId)
} else {
controller.onUserSelected(newlySelectedUserId, /* dialogShower= */ null)
controller.onUserSelected(newlySelectedUserId, dialogShower)
}
}
/** Executes the given action. */
fun executeAction(action: UserActionModel) {
fun executeAction(
action: UserActionModel,
dialogShower: UserSwitchDialogController.DialogShower? = null,
) {
if (isNewImpl) {
when (action) {
UserActionModel.ENTER_GUEST_MODE ->
guestUserInteractor.createAndSwitchTo(
this::showDialog,
this::dismissDialog,
this::selectUser,
)
) { userId ->
selectUser(userId, dialogShower)
}
UserActionModel.ADD_USER -> {
val currentUser = repository.getSelectedUserInfo()
showDialog(
@@ -586,7 +597,7 @@ constructor(
}
private fun switchUser(userId: Int) {
// TODO(b/246631653): track jank and lantecy like in the old impl.
// TODO(b/246631653): track jank and latency like in the old impl.
refreshUsersScheduler.pause()
try {
activityManager.switchUser(userId)

View File

@@ -49,6 +49,7 @@ import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import org.mockito.ArgumentMatchers.anyBoolean
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
@SmallTest
@@ -81,8 +82,9 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
userRepository.setSelectedUserInfo(userInfos[0])
userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
underTest.onRecordSelected(UserRecord(info = userInfos[1]))
underTest.onRecordSelected(UserRecord(info = userInfos[1]), dialogShower)
verify(dialogShower).dismiss()
verify(activityManager).switchUser(userInfos[1].id)
Unit
}
@@ -108,9 +110,12 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
userRepository.setUserInfos(userInfos)
userRepository.setSelectedUserInfo(userInfos[0])
userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
val guestUserInfo = createUserInfo(id = 1337, name = "guest", isGuest = true)
whenever(manager.createGuest(any())).thenReturn(guestUserInfo)
underTest.onRecordSelected(UserRecord(isGuest = true))
underTest.onRecordSelected(UserRecord(isGuest = true), dialogShower)
verify(dialogShower).dismiss()
verify(manager).createGuest(any())
Unit
}
@@ -123,8 +128,9 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
userRepository.setSelectedUserInfo(userInfos[0])
userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
underTest.onRecordSelected(UserRecord(isAddSupervisedUser = true))
underTest.onRecordSelected(UserRecord(isAddSupervisedUser = true), dialogShower)
verify(dialogShower, never()).dismiss()
verify(activityStarter).startActivity(any(), anyBoolean())
}
@@ -392,10 +398,14 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
var dialogRequest: ShowDialogRequestModel? = null
val job = underTest.dialogShowRequests.onEach { dialogRequest = it }.launchIn(this)
underTest.selectUser(newlySelectedUserId = guestUserInfo.id)
underTest.selectUser(
newlySelectedUserId = guestUserInfo.id,
dialogShower = dialogShower,
)
assertThat(dialogRequest)
.isInstanceOf(ShowDialogRequestModel.ShowExitGuestDialog::class.java)
verify(dialogShower, never()).dismiss()
job.cancel()
}
@@ -411,10 +421,11 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
var dialogRequest: ShowDialogRequestModel? = null
val job = underTest.dialogShowRequests.onEach { dialogRequest = it }.launchIn(this)
underTest.selectUser(newlySelectedUserId = userInfos[0].id)
underTest.selectUser(newlySelectedUserId = userInfos[0].id, dialogShower = dialogShower)
assertThat(dialogRequest)
.isInstanceOf(ShowDialogRequestModel.ShowExitGuestDialog::class.java)
verify(dialogShower, never()).dismiss()
job.cancel()
}
@@ -428,10 +439,11 @@ class UserInteractorRefactoredTest : UserInteractorTest() {
var dialogRequest: ShowDialogRequestModel? = null
val job = underTest.dialogShowRequests.onEach { dialogRequest = it }.launchIn(this)
underTest.selectUser(newlySelectedUserId = userInfos[1].id)
underTest.selectUser(newlySelectedUserId = userInfos[1].id, dialogShower = dialogShower)
assertThat(dialogRequest).isNull()
verify(activityManager).switchUser(userInfos[1].id)
verify(dialogShower).dismiss()
job.cancel()
}

View File

@@ -27,6 +27,7 @@ import com.android.systemui.flags.Flags
import com.android.systemui.keyguard.data.repository.FakeKeyguardRepository
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.statusbar.policy.DeviceProvisionedController
import com.android.systemui.statusbar.policy.UserSwitcherController
import com.android.systemui.telephony.data.repository.FakeTelephonyRepository
@@ -46,6 +47,7 @@ abstract class UserInteractorTest : SysuiTestCase() {
@Mock protected lateinit var deviceProvisionedController: DeviceProvisionedController
@Mock protected lateinit var devicePolicyManager: DevicePolicyManager
@Mock protected lateinit var uiEventLogger: UiEventLogger
@Mock protected lateinit var dialogShower: UserSwitchDialogController.DialogShower
protected lateinit var underTest: UserInteractor