Manage scan requests in MediaRouter2Manager

This is a cherrypick of commit d9ac018756.

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

@@ -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();
+            }
         }
     }
 

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<CallbackRecord> mCallbackRecords = new CopyOnWriteArrayList<>();
 
@@ -155,20 +156,17 @@ public final class MediaRouter2Manager {
     }
 
     /**
-     * Starts scanning remote routes.
+     * Registers a request to scan for remote routes.
-     * <p>
+     *
-     * Route discovery can happen even when the {@link #startScan()} is not called.
+     * <p>Increases the count of active scanning requests. When the count transitions from zero to
-     * This is because the scanning could be started before by other apps.
+     * one, sends a request to the system server to start scanning.
-     * Therefore, calling this method after calling {@link #stopScan()} does not necessarily mean
+     *
-     * that the routes found before are removed and added again.
+     * <p>Clients must {@link #unregisterScanRequest() unregister their scan requests} when scanning
-     * <p>
+     * is no longer needed, to avoid unnecessary resource usage.
-     * Use {@link Callback} to get the route related events.
-     * <p>
-     * @see #stopScan()
      */
-    public void startScan() {
+    public void registerScanRequest() {
-        Client client = getOrCreateClient();
+        if (mScanRequestCount.getAndIncrement() == 0) {
-        if (client != null) {
+            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.
+     * Unregisters a scan request made by {@link #registerScanRequest()}.
-     * <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.
      *
-     * @see #startScan()
+     * <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.
+     *
+     * @throws IllegalStateException If called while there are no active scan requests.
      */
-    public void stopScan() {
+    public void unregisterScanRequest() {
-        Client client = getOrCreateClient();
+        if (mScanRequestCount.updateAndGet(
-        if (client != null) {
+                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) {

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

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();
     }
 
     /**