Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make UserTrackerImpl use UserSwitchObserver instead of broadcast.

Browse Source 
UserTrackerImp was listening to ACTION_USER_SWITCHED broadcast
to do stuff that makes changes on the screen for a user switch.
But since the broadcast is async (there is no way to tell back the
broadcaster that the reciver has finished it's work), many times those
UI changes were happening "after" the switch, whereas they should be
done "while" the switch is happening, i.e. when the screen is frozen
and UserSwithingDialog is visible.

In order to achive this, UserSwitchObserver's onUserSwitching method
should be used to do things while the screen is frozen for the user
switch.

This CL makes UserTrackerImpl use UserSwitchObserver instead of
ACTION_USER_SWITCHED and provides onUserChanging method (in addition
to already existing onUserChanged method) to it's subscribers. So that
the subscribers can decide when their code should run, during or after
the switch.

This CL also moves work done in UserRepository from onUserChanged to
onUserChanging because user pillar shown on top right of the tablet's
screen was updating after the user switch. Now it updates during the
switch, while the screen is frozen.

Bug: 249527131
Bug: 265068243
Test: Manual visual test by adding a 5s delay to ACTION_USER_SWITCHED
broadcast to make the jank consistent, and verifying the jank is gone
after the fix.
Test: atest UserTrackerImplTest
Test: atest UserTrackerImplReceiveTest
Test: atest UserRepositoryImplTest

Change-Id: Ica2be8cc434cea4bb526a51eeb8171765f461172
Merged-In: Ica2be8cc434cea4bb526a51eeb8171765f461172
This commit is contained in:
Yasin Kilicdere
2022-09-28 18:56:32 +01:00
parent 86be4a9dd0
commit 10b33a76fd
7 changed files with 127 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
packages/SystemUI/src/com/android/systemui/settings/UserTracker.kt
View File

@@ -62,12 +62,24 @@ interface UserTracker : UserContentResolverProvider, UserContextProvider {
fun removeCallback(callback: Callback)
/**
* Ćallback for notifying of changes.
* Callback for notifying of changes.
*/
interface Callback {
/**
* Notifies that the current user is being changed.
* Override this method to run things while the screen is frozen for the user switch.
* Please use {@link #onUserChanged} if the task doesn't need to push the unfreezing of the
* screen further. Please be aware that code executed in this callback will lengthen the
* user switch duration.
*/
@JvmDefault
fun onUserChanging(newUser: Int, userContext: Context) {}
/**
* Notifies that the current user has changed.
* Override this method to run things after the screen is unfrozen for the user switch.
* Please see {@link #onUserChanging} if you need to hide jank.
*/
@JvmDefault
fun onUserChanged(newUser: Int, userContext: Context) {}
@@ -78,4 +90,4 @@ interface UserTracker : UserContentResolverProvider, UserContextProvider {
@JvmDefault
fun onProfilesChanged(profiles: List<@JvmSuppressWildcards UserInfo>) {}
}
}
}

68
packages/SystemUI/src/com/android/systemui/settings/UserTrackerImpl.kt
View File

@@ -16,6 +16,8 @@
package com.android.systemui.settings
import android.app.IActivityManager
import android.app.UserSwitchObserver
import android.content.BroadcastReceiver
import android.content.ContentResolver
import android.content.Context
@@ -23,6 +25,7 @@ import android.content.Intent
import android.content.IntentFilter
import android.content.pm.UserInfo
import android.os.Handler
import android.os.IRemoteCallback
import android.os.UserHandle
import android.os.UserManager
import android.util.Log
@@ -34,6 +37,7 @@ import com.android.systemui.util.Assert
import java.io.PrintWriter
import java.lang.IllegalStateException
import java.lang.ref.WeakReference
import java.util.concurrent.CountDownLatch
import java.util.concurrent.Executor
import kotlin.properties.ReadWriteProperty
import kotlin.reflect.KProperty
@@ -56,6 +60,7 @@ import kotlin.reflect.KProperty
class UserTrackerImpl internal constructor(
private val context: Context,
private val userManager: UserManager,
private val iActivityManager: IActivityManager,
private val dumpManager: DumpManager,
private val backgroundHandler: Handler
) : UserTracker, Dumpable, BroadcastReceiver() {
@@ -107,7 +112,6 @@ class UserTrackerImpl internal constructor(
setUserIdInternal(startingUser)
val filter = IntentFilter().apply {
addAction(Intent.ACTION_USER_SWITCHED)
addAction(Intent.ACTION_USER_INFO_CHANGED)
// These get called when a managed profile goes in or out of quiet mode.
addAction(Intent.ACTION_MANAGED_PROFILE_AVAILABLE)
@@ -118,14 +122,13 @@ class UserTrackerImpl internal constructor(
}
context.registerReceiverForAllUsers(this, filter, null /* permission */, backgroundHandler)
registerUserSwitchObserver()
dumpManager.registerDumpable(TAG, this)
}
override fun onReceive(context: Context, intent: Intent) {
when (intent.action) {
Intent.ACTION_USER_SWITCHED -> {
handleSwitchUser(intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL))
}
Intent.ACTION_USER_INFO_CHANGED,
Intent.ACTION_MANAGED_PROFILE_AVAILABLE,
Intent.ACTION_MANAGED_PROFILE_UNAVAILABLE,
@@ -157,22 +160,43 @@ class UserTrackerImpl internal constructor(
return ctx to profiles
}
private fun registerUserSwitchObserver() {
iActivityManager.registerUserSwitchObserver(object : UserSwitchObserver() {
override fun onUserSwitching(newUserId: Int, reply: IRemoteCallback?) {
backgroundHandler.run {
handleUserSwitching(newUserId)
reply?.sendResult(null)
}
}
override fun onUserSwitchComplete(newUserId: Int) {
backgroundHandler.run {
handleUserSwitchComplete(newUserId)
}
}
}, TAG)
}
@WorkerThread
private fun handleSwitchUser(newUser: Int) {
private fun handleUserSwitching(newUserId: Int) {
Assert.isNotMainThread()
if (newUser == UserHandle.USER_NULL) {
Log.w(TAG, "handleSwitchUser - Couldn't get new id from intent")
return
}
if (newUser == userId) return
Log.i(TAG, "Switching to user $newUser")
val (ctx, profiles) = setUserIdInternal(newUser)
Log.i(TAG, "Switching to user $newUserId")
setUserIdInternal(newUserId)
notifySubscribers {
onUserChanged(newUser, ctx)
onProfilesChanged(profiles)
onUserChanging(newUserId, userContext)
}.await()
}
@WorkerThread
private fun handleUserSwitchComplete(newUserId: Int) {
Assert.isNotMainThread()
Log.i(TAG, "Switched to user $newUserId")
setUserIdInternal(newUserId)
notifySubscribers {
onUserChanged(newUserId, userContext)
onProfilesChanged(userProfiles)
}
}
@@ -201,17 +225,25 @@ class UserTrackerImpl internal constructor(
}
}
private inline fun notifySubscribers(crossinline action: UserTracker.Callback.() -> Unit) {
private inline fun notifySubscribers(
crossinline action: UserTracker.Callback.() -> Unit
): CountDownLatch {
val list = synchronized(callbacks) {
callbacks.toList()
}
val latch = CountDownLatch(list.size)
list.forEach {
if (it.callback.get() != null) {
it.executor.execute {
it.callback.get()?.action()
latch.countDown()
}
} else {
latch.countDown()
}
}
return latch
}
override fun dump(pw: PrintWriter, args: Array<out String>) {
@@ -258,4 +290,4 @@ private data class DataItem(
fun sameOrEmpty(other: UserTracker.Callback): Boolean {
return callback.get()?.equals(other) ?: true
}
}
}

5
packages/SystemUI/src/com/android/systemui/settings/dagger/MultiUserUtilsModule.java
View File

@@ -17,6 +17,7 @@
package com.android.systemui.settings.dagger;
import android.app.ActivityManager;
import android.app.IActivityManager;
import android.content.Context;
import android.os.Handler;
import android.os.UserManager;
@@ -57,11 +58,13 @@ public abstract class MultiUserUtilsModule {
static UserTracker provideUserTracker(
Context context,
UserManager userManager,
IActivityManager iActivityManager,
DumpManager dumpManager,
@Background Handler handler
) {
int startingUser = ActivityManager.getCurrentUser();
UserTrackerImpl tracker = new UserTrackerImpl(context, userManager, dumpManager, handler);
UserTrackerImpl tracker = new UserTrackerImpl(context, userManager, iActivityManager,
dumpManager, handler);
tracker.initialize(startingUser);
return tracker;
}

2
packages/SystemUI/src/com/android/systemui/user/data/repository/UserRepository.kt
View File

@@ -174,7 +174,7 @@ constructor(
val callback =
object : UserTracker.Callback {
override fun onUserChanged(newUser: Int, userContext: Context) {
override fun onUserChanging(newUser: Int, userContext: Context) {
send()
}

4
packages/SystemUI/tests/src/com/android/systemui/settings/UserTrackerImplReceiveTest.kt
View File

@@ -1,5 +1,6 @@
package com.android.systemui.settings
import android.app.IActivityManager
import android.content.Context
import android.content.Intent
import android.content.pm.UserInfo
@@ -51,6 +52,7 @@ class UserTrackerImplReceiveTest : SysuiTestCase() {
@Mock private lateinit var context: Context
@Mock private lateinit var userManager: UserManager
@Mock private lateinit var iActivityManager: IActivityManager
@Mock(stubOnly = true) private lateinit var dumpManager: DumpManager
@Mock(stubOnly = true) private lateinit var handler: Handler
@@ -67,7 +69,7 @@ class UserTrackerImplReceiveTest : SysuiTestCase() {
`when`(context.user).thenReturn(UserHandle.SYSTEM)
`when`(context.createContextAsUser(ArgumentMatchers.any(), anyInt())).thenReturn(context)
tracker = UserTrackerImpl(context, userManager, dumpManager, handler)
tracker = UserTrackerImpl(context, userManager, iActivityManager, dumpManager, handler)
}
@Test

63
packages/SystemUI/tests/src/com/android/systemui/settings/UserTrackerImplTest.kt
View File

@@ -16,11 +16,14 @@
package com.android.systemui.settings
import android.app.IActivityManager
import android.app.IUserSwitchObserver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.content.pm.UserInfo
import android.os.Handler
import android.os.IRemoteCallback
import android.os.UserHandle
import android.os.UserManager
import android.testing.AndroidTestingRunner
@@ -29,19 +32,20 @@ import com.android.systemui.SysuiTestCase
import com.android.systemui.dump.DumpManager
import com.android.systemui.util.mockito.capture
import com.google.common.truth.Truth.assertThat
import java.util.concurrent.Executor
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.ArgumentMatchers.any
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.anyString
import org.mockito.ArgumentMatchers.eq
import org.mockito.ArgumentMatchers.isNull
import org.mockito.Mock
import org.mockito.Mockito.`when`
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations
import java.util.concurrent.Executor
@SmallTest
@RunWith(AndroidTestingRunner::class)
@@ -51,6 +55,10 @@ class UserTrackerImplTest : SysuiTestCase() {
private lateinit var context: Context
@Mock
private lateinit var userManager: UserManager
@Mock
private lateinit var iActivityManager: IActivityManager
@Mock
private lateinit var userSwitchingReply: IRemoteCallback
@Mock(stubOnly = true)
private lateinit var dumpManager: DumpManager
@Mock(stubOnly = true)
@@ -76,7 +84,7 @@ class UserTrackerImplTest : SysuiTestCase() {
listOf(info)
}
tracker = UserTrackerImpl(context, userManager, dumpManager, handler)
tracker = UserTrackerImpl(context, userManager, iActivityManager, dumpManager, handler)
}
@Test
@@ -125,8 +133,7 @@ class UserTrackerImplTest : SysuiTestCase() {
verify(context).registerReceiverForAllUsers(
eq(tracker), capture(captor), isNull(), eq(handler))
with(captor.value) {
assertThat(countActions()).isEqualTo(7)
assertThat(hasAction(Intent.ACTION_USER_SWITCHED)).isTrue()
assertThat(countActions()).isEqualTo(6)
assertThat(hasAction(Intent.ACTION_USER_INFO_CHANGED)).isTrue()
assertThat(hasAction(Intent.ACTION_MANAGED_PROFILE_AVAILABLE)).isTrue()
assertThat(hasAction(Intent.ACTION_MANAGED_PROFILE_UNAVAILABLE)).isTrue()
@@ -158,8 +165,10 @@ class UserTrackerImplTest : SysuiTestCase() {
tracker.initialize(0)
val newID = 5
val intent = Intent(Intent.ACTION_USER_SWITCHED).putExtra(Intent.EXTRA_USER_HANDLE, newID)
tracker.onReceive(context, intent)
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
captor.value.onUserSwitching(newID, userSwitchingReply)
verify(userSwitchingReply).sendResult(any())
verify(userManager).getProfiles(newID)
@@ -271,6 +280,24 @@ class UserTrackerImplTest : SysuiTestCase() {
assertThat(callback.calledOnUserChanged).isEqualTo(0)
}
@Test
fun testCallbackCalledOnUserChanging() {
tracker.initialize(0)
val callback = TestCallback()
tracker.addCallback(callback, executor)
val newID = 5
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
captor.value.onUserSwitching(newID, userSwitchingReply)
verify(userSwitchingReply).sendResult(any())
assertThat(callback.calledOnUserChanging).isEqualTo(1)
assertThat(callback.lastUser).isEqualTo(newID)
assertThat(callback.lastUserContext?.userId).isEqualTo(newID)
}
@Test
fun testCallbackCalledOnUserChanged() {
tracker.initialize(0)
@@ -279,8 +306,9 @@ class UserTrackerImplTest : SysuiTestCase() {
val newID = 5
val intent = Intent(Intent.ACTION_USER_SWITCHED).putExtra(Intent.EXTRA_USER_HANDLE, newID)
tracker.onReceive(context, intent)
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
captor.value.onUserSwitchComplete(newID)
assertThat(callback.calledOnUserChanged).isEqualTo(1)
assertThat(callback.lastUser).isEqualTo(newID)
@@ -330,25 +358,36 @@ class UserTrackerImplTest : SysuiTestCase() {
tracker.addCallback(callback, executor)
tracker.removeCallback(callback)
val intent = Intent(Intent.ACTION_USER_SWITCHED).putExtra(Intent.EXTRA_USER_HANDLE, 5)
tracker.onReceive(context, intent)
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
captor.value.onUserSwitching(newID, userSwitchingReply)
verify(userSwitchingReply).sendResult(any())
captor.value.onUserSwitchComplete(newID)
val intentProfiles = Intent(Intent.ACTION_MANAGED_PROFILE_AVAILABLE)
.putExtra(Intent.EXTRA_USER, UserHandle.of(profileID))
tracker.onReceive(context, intentProfiles)
assertThat(callback.calledOnUserChanging).isEqualTo(0)
assertThat(callback.calledOnUserChanged).isEqualTo(0)
assertThat(callback.calledOnProfilesChanged).isEqualTo(0)
}
private class TestCallback : UserTracker.Callback {
var calledOnUserChanging = 0
var calledOnUserChanged = 0
var calledOnProfilesChanged = 0
var lastUser: Int? = null
var lastUserContext: Context? = null
var lastUserProfiles = emptyList<UserInfo>()
override fun onUserChanging(newUser: Int, userContext: Context) {
calledOnUserChanging++
lastUser = newUser
lastUserContext = userContext
}
override fun onUserChanged(newUser: Int, userContext: Context) {
calledOnUserChanged++
lastUser = newUser
@@ -360,4 +399,4 @@ class UserTrackerImplTest : SysuiTestCase() {
lastUserProfiles = profiles
}
}
}
}

5
packages/SystemUI/tests/utils/src/com/android/systemui/settings/FakeUserTracker.kt
View File

@@ -67,7 +67,10 @@ class FakeUserTracker(
_userHandle = UserHandle.of(_userId)
val copy = callbacks.toList()
copy.forEach { it.onUserChanged(_userId, userContext) }
copy.forEach {
it.onUserChanging(_userId, userContext)
it.onUserChanged(_userId, userContext)
}
}
fun onProfileChanged() {