From 2be074c097351fa94ebf7df5b8dc3d69f108afe1 Mon Sep 17 00:00:00 2001 From: Santiago Seifert Date: Tue, 27 Sep 2022 11:22:53 +0000 Subject: [PATCH 1/2] Use UserSwitchObserver to detect switches in the running user The current implementation relies on listening for Intent.ACTION_USER_SWITCHED, which can take more than 30 seconds to propagate, making UserSwitchObserver a more reliable option. Bug: 242188673 Test: atest mediaroutertest CtsMediaBetterTogetherTestCases Test: Manually tested casting on secondary users. Test: Pending CL to verify route availability on secondary users as part of the linked bug. Change-Id: I67765a671bce590495b73ef7df6bf72ba3c9f03a Merged-In: I67765a671bce590495b73ef7df6bf72ba3c9f03a --- .../server/media/MediaRouter2ServiceImpl.java | 5 +-- .../server/media/MediaRouterService.java | 40 ++++++++++++------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index e27cbeaab139f..1e23d16328eec 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -601,10 +601,9 @@ class MediaRouter2ServiceImpl { } } - //TODO(b/136703681): Review this is handling multi-user properly. - void switchUser() { + // TODO(b/136703681): Review this is handling multi-user properly. + void switchUser(int userId) { synchronized (mLock) { - int userId = ActivityManager.getCurrentUser(); if (mCurrentUserId != userId) { final int oldUserId = mCurrentUserId; mCurrentUserId = userId; // do this first diff --git a/services/core/java/com/android/server/media/MediaRouterService.java b/services/core/java/com/android/server/media/MediaRouterService.java index e61f55381b0c7..2893b85962fdb 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -17,7 +17,9 @@ package com.android.server.media; import android.annotation.NonNull; +import android.annotation.RequiresPermission; import android.app.ActivityManager; +import android.app.UserSwitchObserver; import android.bluetooth.BluetoothA2dp; import android.bluetooth.BluetoothDevice; import android.content.BroadcastReceiver; @@ -217,18 +219,27 @@ public final class MediaRouterService extends IMediaRouterService.Stub context.registerReceiverAsUser(mReceiver, UserHandle.ALL, intentFilter, null, null); } - public void systemRunning() { - IntentFilter filter = new IntentFilter(Intent.ACTION_USER_SWITCHED); - mContext.registerReceiver(new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (intent.getAction().equals(Intent.ACTION_USER_SWITCHED)) { - switchUser(); - } - } - }, filter); - - switchUser(); + /** + * Initializes the MediaRouter service. + * + * @throws RemoteException If an error occurs while registering the {@link UserSwitchObserver}. + */ + @RequiresPermission( + anyOf = { + "android.permission.INTERACT_ACROSS_USERS", + "android.permission.INTERACT_ACROSS_USERS_FULL" + }) + public void systemRunning() throws RemoteException { + ActivityManager.getService() + .registerUserSwitchObserver( + new UserSwitchObserver() { + @Override + public void onUserSwitchComplete(int newUserId) { + switchUser(newUserId); + } + }, + TAG); + switchUser(ActivityManager.getCurrentUser()); } @Override @@ -702,9 +713,8 @@ public final class MediaRouterService extends IMediaRouterService.Stub } } - void switchUser() { + void switchUser(int userId) { synchronized (mLock) { - int userId = ActivityManager.getCurrentUser(); if (mCurrentUserId != userId) { final int oldUserId = mCurrentUserId; mCurrentUserId = userId; // do this first @@ -721,7 +731,7 @@ public final class MediaRouterService extends IMediaRouterService.Stub } } } - mService2.switchUser(); + mService2.switchUser(userId); } void clientDied(ClientRecord clientRecord) { From cf655c6693be868776641f453b31a91275b58cbe Mon Sep 17 00:00:00 2001 From: Santiago Seifert Date: Wed, 28 Sep 2022 10:22:22 +0000 Subject: [PATCH 2/2] Add support for profiles in MediaRouterService Before this change, only the currently active user is tracked by the service's internal state. After this change, also the profiles of the current active user are tracked. Test: atest CtsMediaBetterTogetherTestCases Bug: 242188673 Bug: 136703681 Change-Id: If17dad36461a5877678cd0cd0e2fceee7591a718 Merged-In: If17dad36461a5877678cd0cd0e2fceee7591a718 --- .../server/media/MediaRouter2ServiceImpl.java | 59 +++++++++----- .../server/media/MediaRouterService.java | 77 ++++++++++++------- 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 1e23d16328eec..350aa6b2f686e 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -58,6 +58,8 @@ import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.function.pooled.PooledLambda; +import com.android.server.LocalServices; +import com.android.server.pm.UserManagerInternal; import java.io.PrintWriter; import java.lang.ref.WeakReference; @@ -87,6 +89,7 @@ class MediaRouter2ServiceImpl { private static final int PACKAGE_IMPORTANCE_FOR_DISCOVERY = IMPORTANCE_FOREGROUND_SERVICE; private final Context mContext; + private final UserManagerInternal mUserManagerInternal; private final Object mLock = new Object(); final AtomicInteger mNextRouterOrManagerId = new AtomicInteger(1); final ActivityManager mActivityManager; @@ -99,7 +102,7 @@ class MediaRouter2ServiceImpl { @GuardedBy("mLock") private final ArrayMap mAllManagerRecords = new ArrayMap<>(); @GuardedBy("mLock") - private int mCurrentUserId = -1; + private int mCurrentActiveUserId = -1; private final ActivityManager.OnUidImportanceListener mOnUidImportanceListener = (uid, importance) -> { @@ -125,12 +128,13 @@ class MediaRouter2ServiceImpl { } }; - MediaRouter2ServiceImpl(Context context) { + /* package */ MediaRouter2ServiceImpl(Context context) { mContext = context; mActivityManager = mContext.getSystemService(ActivityManager.class); mActivityManager.addOnUidImportanceListener(mOnUidImportanceListener, PACKAGE_IMPORTANCE_FOR_DISCOVERY); mPowerManager = mContext.getSystemService(PowerManager.class); + mUserManagerInternal = LocalServices.getService(UserManagerInternal.class); IntentFilter screenOnOffIntentFilter = new IntentFilter(); screenOnOffIntentFilter.addAction(ACTION_SCREEN_ON); @@ -601,24 +605,23 @@ class MediaRouter2ServiceImpl { } } - // TODO(b/136703681): Review this is handling multi-user properly. - void switchUser(int userId) { + /* package */ void updateRunningUserAndProfiles(int newActiveUserId) { synchronized (mLock) { - if (mCurrentUserId != userId) { - final int oldUserId = mCurrentUserId; - mCurrentUserId = userId; // do this first - - UserRecord oldUser = mUserRecords.get(oldUserId); - if (oldUser != null) { - oldUser.mHandler.sendMessage( - obtainMessage(UserHandler::stop, oldUser.mHandler)); - disposeUserIfNeededLocked(oldUser); // since no longer current user - } - - UserRecord newUser = mUserRecords.get(userId); - if (newUser != null) { - newUser.mHandler.sendMessage( - obtainMessage(UserHandler::start, newUser.mHandler)); + if (mCurrentActiveUserId != newActiveUserId) { + mCurrentActiveUserId = newActiveUserId; + for (int i = 0; i < mUserRecords.size(); i++) { + int userId = mUserRecords.keyAt(i); + UserRecord userRecord = mUserRecords.valueAt(i); + if (isUserActiveLocked(userId)) { + // userId corresponds to the active user, or one of its profiles. We + // ensure the associated structures are initialized. + userRecord.mHandler.sendMessage( + obtainMessage(UserHandler::start, userRecord.mHandler)); + } else { + userRecord.mHandler.sendMessage( + obtainMessage(UserHandler::stop, userRecord.mHandler)); + disposeUserIfNeededLocked(userRecord); + } } } } @@ -636,11 +639,21 @@ class MediaRouter2ServiceImpl { } } + /** + * Returns {@code true} if the given {@code userId} corresponds to the active user or a profile + * of the active user, returns {@code false} otherwise. + */ + @GuardedBy("mLock") + private boolean isUserActiveLocked(int userId) { + return mUserManagerInternal.getProfileParentId(userId) == mCurrentActiveUserId; + } + //////////////////////////////////////////////////////////////// //// ***Locked methods related to MediaRouter2 //// - Should have @NonNull/@Nullable on all arguments //////////////////////////////////////////////////////////////// + @GuardedBy("mLock") private void registerRouter2Locked(@NonNull IMediaRouter2 router, int uid, int pid, @NonNull String packageName, int userId, boolean hasConfigureWifiDisplayPermission, boolean hasModifyAudioRoutingPermission) { @@ -668,6 +681,7 @@ class MediaRouter2ServiceImpl { userRecord.mHandler, routerRecord)); } + @GuardedBy("mLock") private void unregisterRouter2Locked(@NonNull IMediaRouter2 router, boolean died) { RouterRecord routerRecord = mAllRouterRecords.remove(router.asBinder()); if (routerRecord == null) { @@ -890,6 +904,7 @@ class MediaRouter2ServiceImpl { return sessionInfos; } + @GuardedBy("mLock") private void registerManagerLocked(@NonNull IMediaRouter2Manager manager, int uid, int pid, @NonNull String packageName, int userId) { final IBinder binder = manager.asBinder(); @@ -1124,13 +1139,14 @@ class MediaRouter2ServiceImpl { //// - Should have @NonNull/@Nullable on all arguments //////////////////////////////////////////////////////////// + @GuardedBy("mLock") private UserRecord getOrCreateUserRecordLocked(int userId) { UserRecord userRecord = mUserRecords.get(userId); if (userRecord == null) { userRecord = new UserRecord(userId); mUserRecords.put(userId, userRecord); userRecord.init(); - if (userId == mCurrentUserId) { + if (isUserActiveLocked(userId)) { userRecord.mHandler.sendMessage( obtainMessage(UserHandler::start, userRecord.mHandler)); } @@ -1138,12 +1154,13 @@ class MediaRouter2ServiceImpl { return userRecord; } + @GuardedBy("mLock") private void disposeUserIfNeededLocked(@NonNull UserRecord userRecord) { // If there are no records left and the user is no longer current then go ahead // and purge the user record and all of its associated state. If the user is current // then leave it alone since we might be connected to a route or want to query // the same route information again soon. - if (userRecord.mUserId != mCurrentUserId + if (!isUserActiveLocked(userRecord.mUserId) && userRecord.mRouterRecords.isEmpty() && userRecord.mManagerRecords.isEmpty()) { if (DEBUG) { diff --git a/services/core/java/com/android/server/media/MediaRouterService.java b/services/core/java/com/android/server/media/MediaRouterService.java index 2893b85962fdb..440178a1e667e 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -63,7 +63,9 @@ import android.util.SparseArray; import android.util.TimeUtils; import com.android.internal.util.DumpUtils; +import com.android.server.LocalServices; import com.android.server.Watchdog; +import com.android.server.pm.UserManagerInternal; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -104,9 +106,10 @@ public final class MediaRouterService extends IMediaRouterService.Stub // State guarded by mLock. private final Object mLock = new Object(); + private final UserManagerInternal mUserManagerInternal; private final SparseArray mUserRecords = new SparseArray<>(); private final ArrayMap mAllClientRecords = new ArrayMap<>(); - private int mCurrentUserId = -1; + private int mCurrentActiveUserId = -1; private final IAudioService mAudioService; private final AudioPlayerStateMonitor mAudioPlayerStateMonitor; private final Handler mHandler = new Handler(); @@ -132,6 +135,7 @@ public final class MediaRouterService extends IMediaRouterService.Stub mBluetoothA2dpRouteId = res.getString(com.android.internal.R.string.bluetooth_a2dp_audio_route_id); + mUserManagerInternal = LocalServices.getService(UserManagerInternal.class); mAudioService = IAudioService.Stub.asInterface( ServiceManager.getService(Context.AUDIO_SERVICE)); mAudioPlayerStateMonitor = AudioPlayerStateMonitor.getInstance(context); @@ -235,11 +239,11 @@ public final class MediaRouterService extends IMediaRouterService.Stub new UserSwitchObserver() { @Override public void onUserSwitchComplete(int newUserId) { - switchUser(newUserId); + updateRunningUserAndProfiles(newUserId); } }, TAG); - switchUser(ActivityManager.getCurrentUser()); + updateRunningUserAndProfiles(ActivityManager.getCurrentUser()); } @Override @@ -459,7 +463,7 @@ public final class MediaRouterService extends IMediaRouterService.Stub pw.println("MEDIA ROUTER SERVICE (dumpsys media_router)"); pw.println(); pw.println("Global state"); - pw.println(" mCurrentUserId=" + mCurrentUserId); + pw.println(" mCurrentUserId=" + mCurrentActiveUserId); synchronized (mLock) { final int count = mUserRecords.size(); @@ -713,25 +717,31 @@ public final class MediaRouterService extends IMediaRouterService.Stub } } - void switchUser(int userId) { + /** + * Starts all {@link UserRecord user records} associated with the active user (whose ID is + * {@code newActiveUserId}) or the active user's profiles. + * + *

All other records are stopped, and those without associated client records are removed. + */ + private void updateRunningUserAndProfiles(int newActiveUserId) { synchronized (mLock) { - if (mCurrentUserId != userId) { - final int oldUserId = mCurrentUserId; - mCurrentUserId = userId; // do this first - - UserRecord oldUser = mUserRecords.get(oldUserId); - if (oldUser != null) { - oldUser.mHandler.sendEmptyMessage(UserHandler.MSG_STOP); - disposeUserIfNeededLocked(oldUser); // since no longer current user - } - - UserRecord newUser = mUserRecords.get(userId); - if (newUser != null) { - newUser.mHandler.sendEmptyMessage(UserHandler.MSG_START); + if (mCurrentActiveUserId != newActiveUserId) { + mCurrentActiveUserId = newActiveUserId; + for (int i = 0; i < mUserRecords.size(); i++) { + int userId = mUserRecords.keyAt(i); + UserRecord userRecord = mUserRecords.valueAt(i); + if (isUserActiveLocked(userId)) { + // userId corresponds to the active user, or one of its profiles. We + // ensure the associated structures are initialized. + userRecord.mHandler.sendEmptyMessage(UserHandler.MSG_START); + } else { + userRecord.mHandler.sendEmptyMessage(UserHandler.MSG_STOP); + disposeUserIfNeededLocked(userRecord); + } } } } - mService2.switchUser(userId); + mService2.updateRunningUserAndProfiles(newActiveUserId); } void clientDied(ClientRecord clientRecord) { @@ -786,8 +796,10 @@ public final class MediaRouterService extends IMediaRouterService.Stub clientRecord.mGroupId = groupId; if (groupId != null) { userRecord.addToGroup(groupId, clientRecord); - userRecord.mHandler.obtainMessage(UserHandler.MSG_NOTIFY_GROUP_ROUTE_SELECTED, groupId) - .sendToTarget(); + userRecord + .mHandler + .obtainMessage(UserHandler.MSG_NOTIFY_GROUP_ROUTE_SELECTED, groupId) + .sendToTarget(); } } @@ -873,9 +885,13 @@ public final class MediaRouterService extends IMediaRouterService.Stub clientRecord.mUserRecord.mClientGroupMap.get(clientRecord.mGroupId); if (group != null) { group.mSelectedRouteId = routeId; - clientRecord.mUserRecord.mHandler.obtainMessage( - UserHandler.MSG_NOTIFY_GROUP_ROUTE_SELECTED, clientRecord.mGroupId) - .sendToTarget(); + clientRecord + .mUserRecord + .mHandler + .obtainMessage( + UserHandler.MSG_NOTIFY_GROUP_ROUTE_SELECTED, + clientRecord.mGroupId) + .sendToTarget(); } } } @@ -907,7 +923,7 @@ public final class MediaRouterService extends IMediaRouterService.Stub if (DEBUG) { Slog.d(TAG, userRecord + ": Initialized"); } - if (userRecord.mUserId == mCurrentUserId) { + if (isUserActiveLocked(userRecord.mUserId)) { userRecord.mHandler.sendEmptyMessage(UserHandler.MSG_START); } } @@ -917,8 +933,7 @@ public final class MediaRouterService extends IMediaRouterService.Stub // and purge the user record and all of its associated state. If the user is current // then leave it alone since we might be connected to a route or want to query // the same route information again soon. - if (userRecord.mUserId != mCurrentUserId - && userRecord.mClientRecords.isEmpty()) { + if (!isUserActiveLocked(userRecord.mUserId) && userRecord.mClientRecords.isEmpty()) { if (DEBUG) { Slog.d(TAG, userRecord + ": Disposed"); } @@ -927,6 +942,14 @@ public final class MediaRouterService extends IMediaRouterService.Stub } } + /** + * Returns {@code true} if the given {@code userId} corresponds to the active user or a profile + * of the active user, returns {@code false} otherwise. + */ + private boolean isUserActiveLocked(int userId) { + return mUserManagerInternal.getProfileParentId(userId) == mCurrentActiveUserId; + } + private void initializeClientLocked(ClientRecord clientRecord) { if (DEBUG) { Slog.d(TAG, clientRecord + ": Registered");