diff options
7 files changed, 107 insertions, 174 deletions
diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index d8995b419f50..891ab45c7a6e 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -48,6 +48,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -118,6 +119,7 @@ public final class MediaRouter2 { private final Map<String, RoutingController> mNonSystemRoutingControllers = new ArrayMap<>(); private final AtomicInteger mNextRequestId = new AtomicInteger(1); + private final AtomicBoolean mIsScanning = new AtomicBoolean(/* initialValue= */ false); final Handler mHandler; @@ -234,7 +236,9 @@ public final class MediaRouter2 { @RequiresPermission(Manifest.permission.MEDIA_CONTENT_CONTROL) public void startScan() { if (isSystemRouter()) { - sManager.startScan(); + if (!mIsScanning.getAndSet(true)) { + sManager.registerScanRequest(); + } } } @@ -260,7 +264,9 @@ public final class MediaRouter2 { @RequiresPermission(Manifest.permission.MEDIA_CONTENT_CONTROL) public void stopScan() { if (isSystemRouter()) { - sManager.stopScan(); + if (mIsScanning.getAndSet(false)) { + sManager.unregisterScanRequest(); + } } } diff --git a/media/java/android/media/MediaRouter2Manager.java b/media/java/android/media/MediaRouter2Manager.java index c84f5b09f166..d79740c2fdff 100644 --- a/media/java/android/media/MediaRouter2Manager.java +++ b/media/java/android/media/MediaRouter2Manager.java @@ -79,9 +79,11 @@ public final class MediaRouter2Manager { final String mPackageName; private final Context mContext; - @GuardedBy("sLock") - private Client mClient; + + private final Client mClient; + private final IMediaRouterService mMediaRouterService; + private final AtomicInteger mScanRequestCount = new AtomicInteger(/* initialValue= */ 0); final Handler mHandler; final CopyOnWriteArrayList<CallbackRecord> mCallbackRecords = new CopyOnWriteArrayList<>(); @@ -119,7 +121,12 @@ public final class MediaRouter2Manager { .getSystemService(Context.MEDIA_SESSION_SERVICE); mPackageName = mContext.getPackageName(); mHandler = new Handler(context.getMainLooper()); - mHandler.post(this::getOrCreateClient); + mClient = new Client(); + try { + mMediaRouterService.registerManager(mClient, mPackageName); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); + } } /** @@ -155,22 +162,18 @@ public final class MediaRouter2Manager { } /** - * Starts scanning remote routes. - * <p> - * Route discovery can happen even when the {@link #startScan()} is not called. - * This is because the scanning could be started before by other apps. - * Therefore, calling this method after calling {@link #stopScan()} does not necessarily mean - * that the routes found before are removed and added again. - * <p> - * Use {@link Callback} to get the route related events. - * <p> - * @see #stopScan() + * Registers a request to scan for remote routes. + * + * <p>Increases the count of active scanning requests. When the count transitions from zero to + * one, sends a request to the system server to start scanning. + * + * <p>Clients must {@link #unregisterScanRequest() unregister their scan requests} when scanning + * is no longer needed, to avoid unnecessary resource usage. */ - public void startScan() { - Client client = getOrCreateClient(); - if (client != null) { + public void registerScanRequest() { + if (mScanRequestCount.getAndIncrement() == 0) { try { - mMediaRouterService.startScan(client); + mMediaRouterService.startScan(mClient); } catch (RemoteException ex) { throw ex.rethrowFromSystemServer(); } @@ -178,23 +181,26 @@ public final class MediaRouter2Manager { } /** - * Stops scanning remote routes to reduce resource consumption. - * <p> - * Route discovery can be continued even after this method is called. - * This is because the scanning is only turned off when all the apps stop scanning. - * Therefore, calling this method does not necessarily mean the routes are removed. - * Also, for the same reason it does not mean that {@link Callback#onRoutesAdded(List)} - * is not called afterwards. - * <p> - * Use {@link Callback} to get the route related events. + * Unregisters a scan request made by {@link #registerScanRequest()}. + * + * <p>Decreases the count of active scanning requests. When the count transitions from one to + * zero, sends a request to the system server to stop scanning. * - * @see #startScan() + * @throws IllegalStateException If called while there are no active scan requests. */ - public void stopScan() { - Client client = getOrCreateClient(); - if (client != null) { + public void unregisterScanRequest() { + if (mScanRequestCount.updateAndGet( + count -> { + if (count == 0) { + throw new IllegalStateException( + "No active scan requests to unregister."); + } else { + return --count; + } + }) + == 0) { try { - mMediaRouterService.stopScan(client); + mMediaRouterService.stopScan(mClient); } catch (RemoteException ex) { throw ex.rethrowFromSystemServer(); } @@ -358,8 +364,7 @@ public final class MediaRouter2Manager { @Nullable public RoutingSessionInfo getSystemRoutingSession(@Nullable String packageName) { try { - return mMediaRouterService.getSystemSessionInfoForPackage( - getOrCreateClient(), packageName); + return mMediaRouterService.getSystemSessionInfoForPackage(mClient, packageName); } catch (RemoteException ex) { throw ex.rethrowFromSystemServer(); } @@ -423,15 +428,11 @@ public final class MediaRouter2Manager { */ @NonNull public List<RoutingSessionInfo> getRemoteSessions() { - Client client = getOrCreateClient(); - if (client != null) { - try { - return mMediaRouterService.getRemoteSessions(client); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + return mMediaRouterService.getRemoteSessions(mClient); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } - return Collections.emptyList(); } /** @@ -514,14 +515,11 @@ public final class MediaRouter2Manager { return; } - Client client = getOrCreateClient(); - if (client != null) { - try { - int requestId = mNextRequestId.getAndIncrement(); - mMediaRouterService.setRouteVolumeWithManager(client, requestId, route, volume); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + int requestId = mNextRequestId.getAndIncrement(); + mMediaRouterService.setRouteVolumeWithManager(mClient, requestId, route, volume); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -543,15 +541,12 @@ public final class MediaRouter2Manager { return; } - Client client = getOrCreateClient(); - if (client != null) { - try { - int requestId = mNextRequestId.getAndIncrement(); - mMediaRouterService.setSessionVolumeWithManager( - client, requestId, sessionInfo.getId(), volume); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + int requestId = mNextRequestId.getAndIncrement(); + mMediaRouterService.setSessionVolumeWithManager( + mClient, requestId, sessionInfo.getId(), volume); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -808,15 +803,12 @@ public final class MediaRouter2Manager { return; } - Client client = getOrCreateClient(); - if (client != null) { - try { - int requestId = mNextRequestId.getAndIncrement(); - mMediaRouterService.selectRouteWithManager( - client, requestId, sessionInfo.getId(), route); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + int requestId = mNextRequestId.getAndIncrement(); + mMediaRouterService.selectRouteWithManager( + mClient, requestId, sessionInfo.getId(), route); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -850,15 +842,12 @@ public final class MediaRouter2Manager { return; } - Client client = getOrCreateClient(); - if (client != null) { - try { - int requestId = mNextRequestId.getAndIncrement(); - mMediaRouterService.deselectRouteWithManager( - client, requestId, sessionInfo.getId(), route); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + int requestId = mNextRequestId.getAndIncrement(); + mMediaRouterService.deselectRouteWithManager( + mClient, requestId, sessionInfo.getId(), route); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -875,15 +864,11 @@ public final class MediaRouter2Manager { public void releaseSession(@NonNull RoutingSessionInfo sessionInfo) { Objects.requireNonNull(sessionInfo, "sessionInfo must not be null"); - Client client = getOrCreateClient(); - if (client != null) { - try { - int requestId = mNextRequestId.getAndIncrement(); - mMediaRouterService.releaseSessionWithManager( - client, requestId, sessionInfo.getId()); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + int requestId = mNextRequestId.getAndIncrement(); + mMediaRouterService.releaseSessionWithManager(mClient, requestId, sessionInfo.getId()); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -896,14 +881,11 @@ public final class MediaRouter2Manager { @NonNull MediaRoute2Info route) { int requestId = createTransferRequest(session, route); - Client client = getOrCreateClient(); - if (client != null) { - try { - mMediaRouterService.transferToRouteWithManager( - client, requestId, session.getId(), route); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + mMediaRouterService.transferToRouteWithManager( + mClient, requestId, session.getId(), route); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -916,14 +898,11 @@ public final class MediaRouter2Manager { int requestId = createTransferRequest(oldSession, route); - Client client = getOrCreateClient(); - if (client != null) { - try { - mMediaRouterService.requestCreateSessionWithManager( - client, requestId, oldSession, route); - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } + try { + mMediaRouterService.requestCreateSessionWithManager( + mClient, requestId, oldSession, route); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); } } @@ -967,22 +946,6 @@ public final class MediaRouter2Manager { sessionInfo.getOwnerPackageName()); } - private Client getOrCreateClient() { - synchronized (sLock) { - if (mClient != null) { - return mClient; - } - Client client = new Client(); - try { - mMediaRouterService.registerManager(client, mPackageName); - mClient = client; - return client; - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); - } - } - } - /** * Interface for receiving events about media routing changes. */ diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java index b4aad9df64a7..4086dec99218 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java @@ -39,6 +39,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import android.Manifest; @@ -121,7 +122,7 @@ public class MediaRouter2ManagerTest { MediaRouter2ManagerTestActivity.startActivity(mContext); mManager = MediaRouter2Manager.getInstance(mContext); - mManager.startScan(); + mManager.registerScanRequest(); mRouter2 = MediaRouter2.getInstance(mContext); // If we need to support thread pool executors, change this to thread pool executor. @@ -152,7 +153,7 @@ public class MediaRouter2ManagerTest { @After public void tearDown() { - mManager.stopScan(); + mManager.unregisterScanRequest(); // order matters (callbacks should be cleared at the last) releaseAllSessions(); @@ -818,6 +819,13 @@ public class MediaRouter2ManagerTest { assertFalse(failureLatch.await(WAIT_TIME_MS, TimeUnit.MILLISECONDS)); } + @Test + public void unregisterScanRequest_enforcesANonNegativeCount() { + mManager.unregisterScanRequest(); // One request was made in the test setup. + assertThrows(IllegalStateException.class, () -> mManager.unregisterScanRequest()); + mManager.registerScanRequest(); // So that the cleanup doesn't fail. + } + /** * Tests if getSelectableRoutes and getDeselectableRoutes filter routes based on * selected routes diff --git a/packages/SettingsLib/src/com/android/settingslib/media/InfoMediaManager.java b/packages/SettingsLib/src/com/android/settingslib/media/InfoMediaManager.java index 58c15eb8073c..7ec0fcdfeb64 100644 --- a/packages/SettingsLib/src/com/android/settingslib/media/InfoMediaManager.java +++ b/packages/SettingsLib/src/com/android/settingslib/media/InfoMediaManager.java @@ -96,14 +96,14 @@ public class InfoMediaManager extends MediaManager { public void startScan() { mMediaDevices.clear(); mRouterManager.registerCallback(mExecutor, mMediaRouterCallback); - mRouterManager.startScan(); + mRouterManager.registerScanRequest(); refreshDevices(); } @Override public void stopScan() { mRouterManager.unregisterCallback(mMediaRouterCallback); - mRouterManager.stopScan(); + mRouterManager.unregisterScanRequest(); } /** diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java index 5df0ca25a285..19b401d80600 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java @@ -224,15 +224,7 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback, Log.d(TAG, "No media controller for " + mPackageName); } } - if (mLocalMediaManager == null) { - if (DEBUG) { - Log.d(TAG, "No local media manager " + mPackageName); - } - return; - } mCallback = cb; - mLocalMediaManager.unregisterCallback(this); - mLocalMediaManager.stopScan(); mLocalMediaManager.registerCallback(this); mLocalMediaManager.startScan(); } @@ -254,10 +246,8 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback, if (mMediaController != null) { mMediaController.unregisterCallback(mCb); } - if (mLocalMediaManager != null) { - mLocalMediaManager.unregisterCallback(this); - mLocalMediaManager.stopScan(); - } + mLocalMediaManager.unregisterCallback(this); + mLocalMediaManager.stopScan(); synchronized (mMediaDevicesLock) { mCachedMediaDevices.clear(); mMediaDevices.clear(); @@ -661,10 +651,6 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback, return mLocalMediaManager.getCurrentConnectedDevice(); } - private MediaDevice getMediaDeviceById(String id) { - return mLocalMediaManager.getMediaDeviceById(new ArrayList<>(mMediaDevices), id); - } - boolean addDeviceToPlayMedia(MediaDevice device) { mMetricLogger.logInteractionExpansion(device); return mLocalMediaManager.addDeviceToPlayMedia(device); @@ -686,10 +672,6 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback, return mLocalMediaManager.getDeselectableMediaDevice(); } - void adjustSessionVolume(String sessionId, int volume) { - mLocalMediaManager.adjustSessionVolume(sessionId, volume); - } - void adjustSessionVolume(int volume) { mLocalMediaManager.adjustSessionVolume(volume); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputBaseDialogTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputBaseDialogTest.java index eb8ecae305bc..9be201e99b2b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputBaseDialogTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputBaseDialogTest.java @@ -241,46 +241,29 @@ public class MediaOutputBaseDialogTest extends SysuiTestCase { } @Test - public void onStart_isBroadcasting_verifyRegisterLeBroadcastServiceCallBack() { + public void whenBroadcasting_verifyLeBroadcastServiceCallBackIsRegisteredAndUnregistered() { when(mLocalBluetoothProfileManager.getLeAudioBroadcastProfile()).thenReturn( mLocalBluetoothLeBroadcast); mIsBroadcasting = true; mMediaOutputBaseDialogImpl.onStart(); - verify(mLocalBluetoothLeBroadcast).registerServiceCallBack(any(), any()); - } - - @Test - public void onStart_notBroadcasting_noRegisterLeBroadcastServiceCallBack() { - when(mLocalBluetoothProfileManager.getLeAudioBroadcastProfile()).thenReturn( - mLocalBluetoothLeBroadcast); - mIsBroadcasting = false; - - mMediaOutputBaseDialogImpl.onStart(); - - verify(mLocalBluetoothLeBroadcast, never()).registerServiceCallBack(any(), any()); - } - - @Test - public void onStart_isBroadcasting_verifyUnregisterLeBroadcastServiceCallBack() { - when(mLocalBluetoothProfileManager.getLeAudioBroadcastProfile()).thenReturn( - mLocalBluetoothLeBroadcast); - mIsBroadcasting = true; mMediaOutputBaseDialogImpl.onStop(); - verify(mLocalBluetoothLeBroadcast).unregisterServiceCallBack(any()); } @Test - public void onStop_notBroadcasting_noUnregisterLeBroadcastServiceCallBack() { + public void + whenNotBroadcasting_verifyLeBroadcastServiceCallBackIsNotRegisteredOrUnregistered() { when(mLocalBluetoothProfileManager.getLeAudioBroadcastProfile()).thenReturn( mLocalBluetoothLeBroadcast); mIsBroadcasting = false; + mMediaOutputBaseDialogImpl.onStart(); mMediaOutputBaseDialogImpl.onStop(); + verify(mLocalBluetoothLeBroadcast, never()).registerServiceCallBack(any(), any()); verify(mLocalBluetoothLeBroadcast, never()).unregisterServiceCallBack(any()); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputControllerTest.java index 465654ed585f..cb31fde26bf2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputControllerTest.java @@ -171,15 +171,6 @@ public class MediaOutputControllerTest extends SysuiTestCase { } @Test - public void start_LocalMediaManagerIsNull_verifyNotStartScan() { - mMediaOutputController.mLocalMediaManager = null; - mMediaOutputController.start(mCb); - - verify(mLocalMediaManager, never()).registerCallback(mMediaOutputController); - verify(mLocalMediaManager, never()).startScan(); - } - - @Test public void stop_verifyLocalMediaManagerDeinit() { mMediaOutputController.start(mCb); reset(mLocalMediaManager); |