From cbca646e970e3f2239ce8c72195588427028832d Mon Sep 17 00:00:00 2001 From: Santiago Seifert Date: Wed, 20 Jul 2022 13:46:25 +0000 Subject: [PATCH] Manage scan requests in MediaRouter2Manager This is a cherrypick of commit d9ac018756b3a52bd4c65d1c3ba7488c40b42381. Before this change, MediaRouter2Manager (singleton) does not protect clients against independent changes to the scanning state, which means one client can stop the scan while another needs scanning to go on. After this change, MediaRouter2Manager keeps count of the calls to request scanning, and will only request scanning from the system server while there is at least one active scan request (via registerScanRequest). This change does not affect MediaRouter2.start/stopScan. Bug: 232812007 Test: atest CtsMediaBetterTogetherTestCases Change-Id: I8d80872d7c55791b9a429180da1c68af50ae13b7 Merged-In: I8d80872d7c55791b9a429180da1c68af50ae13b7 --- media/java/android/media/MediaRouter2.java | 10 +++- .../android/media/MediaRouter2Manager.java | 54 ++++++++++--------- .../MediaRouter2ManagerTest.java | 12 ++++- .../settingslib/media/InfoMediaManager.java | 4 +- 4 files changed, 48 insertions(+), 32 deletions(-) diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index d8995b419f505..891ab45c7a6e4 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 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 c84f5b09f166a..24c117911f7c6 100644 --- a/media/java/android/media/MediaRouter2Manager.java +++ b/media/java/android/media/MediaRouter2Manager.java @@ -82,6 +82,7 @@ public final class MediaRouter2Manager { @GuardedBy("sLock") private Client mClient; private final IMediaRouterService mMediaRouterService; + private final AtomicInteger mScanRequestCount = new AtomicInteger(/* initialValue= */ 0); final Handler mHandler; final CopyOnWriteArrayList mCallbackRecords = new CopyOnWriteArrayList<>(); @@ -155,20 +156,17 @@ public final class MediaRouter2Manager { } /** - * Starts scanning remote routes. - *

- * 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. - *

- * Use {@link Callback} to get the route related events. - *

- * @see #stopScan() + * Registers a request to scan for remote routes. + * + *

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. + * + *

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) { + Client client = getOrCreateClient(); try { mMediaRouterService.startScan(client); } catch (RemoteException ex) { @@ -178,21 +176,25 @@ public final class MediaRouter2Manager { } /** - * Stops scanning remote routes to reduce resource consumption. - *

- * 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. - *

- * Use {@link Callback} to get the route related events. + * Unregisters a scan request made by {@link #registerScanRequest()}. * - * @see #startScan() + *

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. + * + * @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) { + Client client = getOrCreateClient(); try { mMediaRouterService.stopScan(client); } catch (RemoteException ex) { diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java index b4aad9df64a7d..4086dec99218d 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 58c15eb8073cb..7ec0fcdfeb640 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(); } /**