Skip to content

Commit 44f6627

Browse files
Yasin Kilicderethestinger
Yasin Kilicdere
authored and
thestinger
committed
Make UserSwitchObserver.onBeforeUserSwitching oneway but still blocking.
Bug: 371536480 Test: atest UserControllerTest Test: atest UserTrackerImplTest Flag: EXEMPT bugfix (cherry picked from commit a3bd1e2) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f7efa779da5c59085b38cb73da61ef0d83b672b6) Merged-In: I6fd04b00ab768533df01a3eca613f388bf70e42a Change-Id: I6fd04b00ab768533df01a3eca613f388bf70e42a
1 parent a3c5baa commit 44f6627

File tree

7 files changed

+193
-112
lines changed

7 files changed

+193
-112
lines changed

‎core/java/android/app/IUserSwitchObserver.aidl‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ package android.app;
1919
import android.os.IRemoteCallback;
2020

2121
/** {@hide} */
22-
interface IUserSwitchObserver {
23-
void onBeforeUserSwitching(int newUserId);
24-
oneway void onUserSwitching(int newUserId, IRemoteCallback reply);
25-
oneway void onUserSwitchComplete(int newUserId);
26-
oneway void onForegroundProfileSwitch(int newProfileId);
27-
oneway void onLockedBootComplete(int newUserId);
22+
oneway interface IUserSwitchObserver {
23+
void onBeforeUserSwitching(int newUserId, IRemoteCallback reply);
24+
void onUserSwitching(int newUserId, IRemoteCallback reply);
25+
void onUserSwitchComplete(int newUserId);
26+
void onForegroundProfileSwitch(int newProfileId);
27+
void onLockedBootComplete(int newUserId);
2828
}

‎core/java/android/app/UserSwitchObserver.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ public UserSwitchObserver() {
3030
}
3131

3232
@Override
33-
public void onBeforeUserSwitching(int newUserId) throws RemoteException {}
33+
public void onBeforeUserSwitching(int newUserId, IRemoteCallback reply) throws RemoteException {
34+
if (reply != null) {
35+
reply.sendResult(null);
36+
}
37+
}
3438

3539
@Override
3640
public void onUserSwitching(int newUserId, IRemoteCallback reply) throws RemoteException {

‎packages/SystemUI/src/com/android/systemui/settings/UserTracker.kt‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,18 @@ interface UserTracker : UserContentResolverProvider, UserContextProvider {
6161
/** Callback for notifying of changes. */
6262
@WeaklyReferencedCallback
6363
interface Callback {
64-
/** Notifies that the current user will be changed. */
64+
/**
65+
* Same as {@link onBeforeUserSwitching(Int, Runnable)} but the callback will be called
66+
* automatically after the completion of this method.
67+
*/
6568
fun onBeforeUserSwitching(newUser: Int) {}
6669

70+
/** Notifies that the current user will be changed. */
71+
fun onBeforeUserSwitching(newUser: Int, resultCallback: Runnable) {
72+
onBeforeUserSwitching(newUser)
73+
resultCallback.run()
74+
}
75+
6776
/**
6877
* Same as {@link onUserChanging(Int, Context, Runnable)} but the callback will be called
6978
* automatically after the completion of this method.

‎packages/SystemUI/src/com/android/systemui/settings/UserTrackerImpl.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,9 @@ internal constructor(
195195
private fun registerUserSwitchObserver() {
196196
iActivityManager.registerUserSwitchObserver(
197197
object : UserSwitchObserver() {
198-
override fun onBeforeUserSwitching(newUserId: Int) {
198+
override fun onBeforeUserSwitching(newUserId: Int, reply: IRemoteCallback?) {
199199
handleBeforeUserSwitching(newUserId)
200+
reply?.sendResult(null)
200201
}
201202

202203
override fun onUserSwitching(newUserId: Int, reply: IRemoteCallback?) {
@@ -235,8 +236,7 @@ internal constructor(
235236
setUserIdInternal(newUserId)
236237

237238
notifySubscribers { callback, resultCallback ->
238-
callback.onBeforeUserSwitching(newUserId)
239-
resultCallback.run()
239+
callback.onBeforeUserSwitching(newUserId, resultCallback)
240240
}
241241
.await()
242242
}

‎packages/SystemUI/tests/src/com/android/systemui/settings/UserTrackerImplTest.kt‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class UserTrackerImplTest : SysuiTestCase() {
7676

7777
@Mock private lateinit var iActivityManager: IActivityManager
7878

79+
@Mock private lateinit var beforeUserSwitchingReply: IRemoteCallback
80+
7981
@Mock private lateinit var userSwitchingReply: IRemoteCallback
8082

8183
@Mock(stubOnly = true) private lateinit var dumpManager: DumpManager
@@ -199,9 +201,10 @@ class UserTrackerImplTest : SysuiTestCase() {
199201

200202
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
201203
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
202-
captor.value.onBeforeUserSwitching(newID)
204+
captor.value.onBeforeUserSwitching(newID, beforeUserSwitchingReply)
203205
captor.value.onUserSwitching(newID, userSwitchingReply)
204206
runCurrent()
207+
verify(beforeUserSwitchingReply).sendResult(any())
205208
verify(userSwitchingReply).sendResult(any())
206209

207210
verify(userManager).getProfiles(newID)
@@ -341,10 +344,11 @@ class UserTrackerImplTest : SysuiTestCase() {
341344

342345
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
343346
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
344-
captor.value.onBeforeUserSwitching(newID)
347+
captor.value.onBeforeUserSwitching(newID, beforeUserSwitchingReply)
345348
captor.value.onUserSwitching(newID, userSwitchingReply)
346349
runCurrent()
347350

351+
verify(beforeUserSwitchingReply).sendResult(any())
348352
verify(userSwitchingReply).sendResult(any())
349353
assertThat(callback.calledOnUserChanging).isEqualTo(1)
350354
assertThat(callback.lastUser).isEqualTo(newID)
@@ -395,7 +399,7 @@ class UserTrackerImplTest : SysuiTestCase() {
395399

396400
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
397401
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
398-
captor.value.onBeforeUserSwitching(newID)
402+
captor.value.onBeforeUserSwitching(newID, any())
399403
captor.value.onUserSwitchComplete(newID)
400404
runCurrent()
401405

@@ -453,8 +457,10 @@ class UserTrackerImplTest : SysuiTestCase() {
453457

454458
val captor = ArgumentCaptor.forClass(IUserSwitchObserver::class.java)
455459
verify(iActivityManager).registerUserSwitchObserver(capture(captor), anyString())
460+
captor.value.onBeforeUserSwitching(newID, beforeUserSwitchingReply)
456461
captor.value.onUserSwitching(newID, userSwitchingReply)
457462
runCurrent()
463+
verify(beforeUserSwitchingReply).sendResult(any())
458464
verify(userSwitchingReply).sendResult(any())
459465
captor.value.onUserSwitchComplete(newID)
460466

@@ -488,13 +494,19 @@ class UserTrackerImplTest : SysuiTestCase() {
488494
}
489495

490496
private class TestCallback : UserTracker.Callback {
497+
var calledOnBeforeUserChanging = 0
491498
var calledOnUserChanging = 0
492499
var calledOnUserChanged = 0
493500
var calledOnProfilesChanged = 0
494501
var lastUser: Int? = null
495502
var lastUserContext: Context? = null
496503
var lastUserProfiles = emptyList<UserInfo>()
497504

505+
override fun onBeforeUserSwitching(newUser: Int) {
506+
calledOnBeforeUserChanging++
507+
lastUser = newUser
508+
}
509+
498510
override fun onUserChanging(newUser: Int, userContext: Context) {
499511
calledOnUserChanging++
500512
lastUser = newUser

‎services/core/java/com/android/server/am/UserController.java‎

Lines changed: 101 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@
160160
import java.util.concurrent.TimeUnit;
161161
import java.util.concurrent.atomic.AtomicBoolean;
162162
import java.util.concurrent.atomic.AtomicInteger;
163+
import java.util.function.BiConsumer;
163164
import java.util.function.Consumer;
164165

165166
/**
@@ -1920,20 +1921,26 @@ private boolean startUserInternal(@UserIdInt int userId, int displayId,
19201921
return false;
19211922
}
19221923

1923-
mHandler.post(() -> startUserInternalOnHandler(userId, oldUserId, userStartMode,
1924-
unlockListener, callingUid, callingPid));
1924+
final Runnable continueStartUserInternal = () -> continueStartUserInternal(userInfo,
1925+
oldUserId, userStartMode, unlockListener, callingUid, callingPid);
1926+
if (foreground) {
1927+
mHandler.post(() -> dispatchOnBeforeUserSwitching(userId, () ->
1928+
mHandler.post(continueStartUserInternal)));
1929+
} else {
1930+
continueStartUserInternal.run();
1931+
}
19251932
} finally {
19261933
Binder.restoreCallingIdentity(ident);
19271934
}
19281935

19291936
return true;
19301937
}
19311938

1932-
private void startUserInternalOnHandler(int userId, int oldUserId, int userStartMode,
1939+
private void continueStartUserInternal(UserInfo userInfo, int oldUserId, int userStartMode,
19331940
IProgressListener unlockListener, int callingUid, int callingPid) {
19341941
final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
19351942
final boolean foreground = userStartMode == USER_START_MODE_FOREGROUND;
1936-
final UserInfo userInfo = getUserInfo(userId);
1943+
final int userId = userInfo.id;
19371944

19381945
boolean needStart = false;
19391946
boolean updateUmState = false;
@@ -1995,7 +2002,6 @@ private void startUserInternalOnHandler(int userId, int oldUserId, int userStart
19952002
// it should be moved outside, but for now it's not as there are many calls to
19962003
// external components here afterwards
19972004
updateProfileRelatedCaches();
1998-
dispatchOnBeforeUserSwitching(userId);
19992005
mInjector.getWindowManager().setCurrentUser(userId);
20002006
mInjector.reportCurWakefulnessUsageEvent();
20012007
// Once the internal notion of the active user has switched, we lock the device
@@ -2296,25 +2302,42 @@ private void dispatchForegroundProfileChanged(@UserIdInt int userId) {
22962302
mUserSwitchObservers.finishBroadcast();
22972303
}
22982304

2299-
private void dispatchOnBeforeUserSwitching(@UserIdInt int newUserId) {
2305+
private void dispatchOnBeforeUserSwitching(@UserIdInt int newUserId, Runnable onComplete) {
23002306
final TimingsTraceAndSlog t = new TimingsTraceAndSlog();
23012307
t.traceBegin("dispatchOnBeforeUserSwitching-" + newUserId);
2302-
final int observerCount = mUserSwitchObservers.beginBroadcast();
2303-
for (int i = 0; i < observerCount; i++) {
2304-
final String name = "#" + i + " " + mUserSwitchObservers.getBroadcastCookie(i);
2305-
t.traceBegin("onBeforeUserSwitching-" + name);
2308+
final AtomicBoolean isFirst = new AtomicBoolean(true);
2309+
startTimeoutForOnBeforeUserSwitching(isFirst, onComplete);
2310+
informUserSwitchObservers((observer, callback) -> {
23062311
try {
2307-
mUserSwitchObservers.getBroadcastItem(i).onBeforeUserSwitching(newUserId);
2312+
observer.onBeforeUserSwitching(newUserId, callback);
23082313
} catch (RemoteException e) {
2309-
// Ignore
2310-
} finally {
2311-
t.traceEnd();
2314+
// ignore
23122315
}
2313-
}
2314-
mUserSwitchObservers.finishBroadcast();
2316+
}, () -> {
2317+
if (isFirst.getAndSet(false)) {
2318+
onComplete.run();
2319+
}
2320+
}, "onBeforeUserSwitching");
23152321
t.traceEnd();
23162322
}
23172323

2324+
private void startTimeoutForOnBeforeUserSwitching(AtomicBoolean isFirst,
2325+
Runnable onComplete) {
2326+
final long timeout = getUserSwitchTimeoutMs();
2327+
mHandler.postDelayed(() -> {
2328+
if (isFirst.getAndSet(false)) {
2329+
String unresponsiveObservers;
2330+
synchronized (mLock) {
2331+
unresponsiveObservers = String.join(", ", mCurWaitingUserSwitchCallbacks);
2332+
}
2333+
Slogf.e(TAG, "Timeout on dispatchOnBeforeUserSwitching. These UserSwitchObservers "
2334+
+ "did not respond in " + timeout + "ms: " + unresponsiveObservers + ".");
2335+
onComplete.run();
2336+
}
2337+
}, timeout);
2338+
}
2339+
2340+
23182341
/** Called on handler thread */
23192342
@VisibleForTesting
23202343
void dispatchUserSwitchComplete(@UserIdInt int oldUserId, @UserIdInt int newUserId) {
@@ -2527,70 +2550,76 @@ void dispatchUserSwitch(final UserState uss, final int oldUserId, final int newU
25272550
t.traceBegin("dispatchUserSwitch-" + oldUserId + "-to-" + newUserId);
25282551

25292552
EventLog.writeEvent(EventLogTags.UC_DISPATCH_USER_SWITCH, oldUserId, newUserId);
2553+
uss.switching = true;
2554+
informUserSwitchObservers((observer, callback) -> {
2555+
try {
2556+
observer.onUserSwitching(newUserId, callback);
2557+
} catch (RemoteException e) {
2558+
// ignore
2559+
}
2560+
}, () -> {
2561+
synchronized (mLock) {
2562+
sendContinueUserSwitchLU(uss, oldUserId, newUserId);
2563+
}
2564+
}, "onUserSwitching");
2565+
t.traceEnd();
2566+
}
25302567

2568+
void informUserSwitchObservers(BiConsumer<IUserSwitchObserver, IRemoteCallback> consumer,
2569+
final Runnable onComplete, String trace) {
25312570
final int observerCount = mUserSwitchObservers.beginBroadcast();
2532-
if (observerCount > 0) {
2533-
final ArraySet<String> curWaitingUserSwitchCallbacks = new ArraySet<>();
2571+
if (observerCount == 0) {
2572+
onComplete.run();
2573+
mUserSwitchObservers.finishBroadcast();
2574+
return;
2575+
}
2576+
final ArraySet<String> curWaitingUserSwitchCallbacks = new ArraySet<>();
2577+
synchronized (mLock) {
2578+
mCurWaitingUserSwitchCallbacks = curWaitingUserSwitchCallbacks;
2579+
}
2580+
final AtomicInteger waitingCallbacksCount = new AtomicInteger(observerCount);
2581+
final long userSwitchTimeoutMs = getUserSwitchTimeoutMs();
2582+
final long dispatchStartedTime = SystemClock.elapsedRealtime();
2583+
for (int i = 0; i < observerCount; i++) {
2584+
final long dispatchStartedTimeForObserver = SystemClock.elapsedRealtime();
2585+
// Prepend with unique prefix to guarantee that keys are unique
2586+
final String name = "#" + i + " " + mUserSwitchObservers.getBroadcastCookie(i);
25342587
synchronized (mLock) {
2535-
uss.switching = true;
2536-
mCurWaitingUserSwitchCallbacks = curWaitingUserSwitchCallbacks;
2537-
}
2538-
final AtomicInteger waitingCallbacksCount = new AtomicInteger(observerCount);
2539-
final long userSwitchTimeoutMs = getUserSwitchTimeoutMs();
2540-
final long dispatchStartedTime = SystemClock.elapsedRealtime();
2541-
for (int i = 0; i < observerCount; i++) {
2542-
final long dispatchStartedTimeForObserver = SystemClock.elapsedRealtime();
2543-
try {
2544-
// Prepend with unique prefix to guarantee that keys are unique
2545-
final String name = "#" + i + " " + mUserSwitchObservers.getBroadcastCookie(i);
2588+
curWaitingUserSwitchCallbacks.add(name);
2589+
}
2590+
final IRemoteCallback callback = new IRemoteCallback.Stub() {
2591+
@Override
2592+
public void sendResult(Bundle data) throws RemoteException {
2593+
asyncTraceEnd(trace + "-" + name, 0);
25462594
synchronized (mLock) {
2547-
curWaitingUserSwitchCallbacks.add(name);
2548-
}
2549-
final IRemoteCallback callback = new IRemoteCallback.Stub() {
2550-
@Override
2551-
public void sendResult(Bundle data) throws RemoteException {
2552-
asyncTraceEnd("onUserSwitching-" + name, newUserId);
2553-
synchronized (mLock) {
2554-
long delayForObserver = SystemClock.elapsedRealtime()
2555-
- dispatchStartedTimeForObserver;
2556-
if (delayForObserver > LONG_USER_SWITCH_OBSERVER_WARNING_TIME_MS) {
2557-
Slogf.w(TAG, "User switch slowed down by observer " + name
2558-
+ ": result took " + delayForObserver
2559-
+ " ms to process.");
2560-
}
2561-
2562-
long totalDelay = SystemClock.elapsedRealtime()
2563-
- dispatchStartedTime;
2564-
if (totalDelay > userSwitchTimeoutMs) {
2565-
Slogf.e(TAG, "User switch timeout: observer " + name
2566-
+ "'s result was received " + totalDelay
2567-
+ " ms after dispatchUserSwitch.");
2568-
}
2569-
2570-
curWaitingUserSwitchCallbacks.remove(name);
2571-
// Continue switching if all callbacks have been notified and
2572-
// user switching session is still valid
2573-
if (waitingCallbacksCount.decrementAndGet() == 0
2574-
&& (curWaitingUserSwitchCallbacks
2575-
== mCurWaitingUserSwitchCallbacks)) {
2576-
sendContinueUserSwitchLU(uss, oldUserId, newUserId);
2577-
}
2578-
}
2595+
long delayForObserver = SystemClock.elapsedRealtime()
2596+
- dispatchStartedTimeForObserver;
2597+
if (delayForObserver > LONG_USER_SWITCH_OBSERVER_WARNING_TIME_MS) {
2598+
Slogf.w(TAG, "User switch slowed down by observer " + name
2599+
+ ": result took " + delayForObserver
2600+
+ " ms to process. " + trace);
25792601
}
2580-
};
2581-
asyncTraceBegin("onUserSwitching-" + name, newUserId);
2582-
mUserSwitchObservers.getBroadcastItem(i).onUserSwitching(newUserId, callback);
2583-
} catch (RemoteException e) {
2584-
// Ignore
2602+
long totalDelay = SystemClock.elapsedRealtime() - dispatchStartedTime;
2603+
if (totalDelay > userSwitchTimeoutMs) {
2604+
Slogf.e(TAG, "User switch timeout: observer " + name
2605+
+ "'s result was received " + totalDelay
2606+
+ " ms after dispatchUserSwitch. " + trace);
2607+
}
2608+
curWaitingUserSwitchCallbacks.remove(name);
2609+
// Continue switching if all callbacks have been notified and
2610+
// user switching session is still valid
2611+
if (waitingCallbacksCount.decrementAndGet() == 0
2612+
&& (curWaitingUserSwitchCallbacks
2613+
== mCurWaitingUserSwitchCallbacks)) {
2614+
onComplete.run();
2615+
}
2616+
}
25852617
}
2586-
}
2587-
} else {
2588-
synchronized (mLock) {
2589-
sendContinueUserSwitchLU(uss, oldUserId, newUserId);
2590-
}
2618+
};
2619+
asyncTraceBegin(trace + "-" + name, 0);
2620+
consumer.accept(mUserSwitchObservers.getBroadcastItem(i), callback);
25912621
}
25922622
mUserSwitchObservers.finishBroadcast();
2593-
t.traceEnd(); // end dispatchUserSwitch-
25942623
}
25952624

25962625
@GuardedBy("mLock")

0 commit comments

Comments
 (0)