Merge changes I363dc391,I84786697,I8d80872d,I4d6aa4d7,I396d376a into tm-qpr-dev

* changes:
  Simplify MediaRouter2Manager.Client's lifecycle
  Remove unnecessary if checks
  Manage scan requests in MediaRouter2Manager
  Merge tests calling onStart and onStop
  Fix MediaOutputController resource management

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

@@ -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.
+     * 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) {
             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.
+     * 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) {
             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(
+            return mMediaRouterService.getSystemSessionInfoForPackage(mClient, packageName);
-                    getOrCreateClient(), packageName);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
@@ -423,16 +428,12 @@ public final class MediaRouter2Manager {
      */
     @NonNull
     public List<RoutingSessionInfo> getRemoteSessions() {
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
-                return mMediaRouterService.getRemoteSessions(client);
+            return mMediaRouterService.getRemoteSessions(mClient);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-        return Collections.emptyList();
-    }
 
     /**
      * Gets the list of all discovered routes.
@@ -514,16 +515,13 @@ public final class MediaRouter2Manager {
             return;
         }
 
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
             int requestId = mNextRequestId.getAndIncrement();
-                mMediaRouterService.setRouteVolumeWithManager(client, requestId, route, volume);
+            mMediaRouterService.setRouteVolumeWithManager(mClient, requestId, route, volume);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     /**
      * Requests a volume change for a routing session asynchronously.
@@ -543,17 +541,14 @@ public final class MediaRouter2Manager {
             return;
         }
 
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
             int requestId = mNextRequestId.getAndIncrement();
             mMediaRouterService.setSessionVolumeWithManager(
-                        client, requestId, sessionInfo.getId(), volume);
+                    mClient, requestId, sessionInfo.getId(), volume);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     void addRoutesOnHandler(List<MediaRoute2Info> routes) {
         synchronized (mRoutesLock) {
@@ -808,17 +803,14 @@ public final class MediaRouter2Manager {
             return;
         }
 
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
             int requestId = mNextRequestId.getAndIncrement();
             mMediaRouterService.selectRouteWithManager(
-                        client, requestId, sessionInfo.getId(), route);
+                    mClient, requestId, sessionInfo.getId(), route);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     /**
      * Deselects a route from the remote session. After a route is deselected, the media is
@@ -850,17 +842,14 @@ public final class MediaRouter2Manager {
             return;
         }
 
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
             int requestId = mNextRequestId.getAndIncrement();
             mMediaRouterService.deselectRouteWithManager(
-                        client, requestId, sessionInfo.getId(), route);
+                    mClient, requestId, sessionInfo.getId(), route);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     /**
      * Requests releasing a session.
@@ -875,17 +864,13 @@ 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(
+            mMediaRouterService.releaseSessionWithManager(mClient, requestId, sessionInfo.getId());
-                        client, requestId, sessionInfo.getId());
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     /**
      * Transfers the remote session to the given route.
@@ -896,16 +881,13 @@ 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);
+                    mClient, requestId, session.getId(), route);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     private void requestCreateSession(RoutingSessionInfo oldSession, MediaRoute2Info route) {
         if (TextUtils.isEmpty(oldSession.getClientPackageName())) {
@@ -916,16 +898,13 @@ public final class MediaRouter2Manager {
 
         int requestId = createTransferRequest(oldSession, route);
 
-        Client client = getOrCreateClient();
-        if (client != null) {
         try {
             mMediaRouterService.requestCreateSessionWithManager(
-                        client, requestId, oldSession, route);
+                    mClient, requestId, oldSession, route);
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
     }
-    }
 
     private int createTransferRequest(RoutingSessionInfo session, MediaRoute2Info route) {
         int requestId = mNextRequestId.getAndIncrement();
@@ -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.
      */

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

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

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

packages/SystemUI/tests/src/com/android/systemui/media/dialog/MediaOutputControllerTest.java

@@ -170,15 +170,6 @@ public class MediaOutputControllerTest extends SysuiTestCase {
         verify(mLocalMediaManager).startScan();
     }
 
-    @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);