From 350d7b427ea1daba67e19ba263017f54f0e09db5 Mon Sep 17 00:00:00 2001 From: Kyunglyul Hyun Date: Fri, 17 Apr 2020 23:09:08 +0900 Subject: [PATCH] Clear routes when media router manager has no callback This CL clears routes when the last callback of MediaRouter2Manager is unregistered. By doing this, we can ensure MediaRouter2Manager#getAvailableRoutes() returns correct routes during at least a single callback is registered. A test for the behavior is added as well. This CL also fixed a bug that unregistering a callback from MediaRouter2 disconnects it when multiple callback is used. Bug: 153515567 Test: atest mediaroutertest & cts test & manually using MediaRouter2Demo and Sample OutputSwitcher to see if forgotten BT device is correctly removed. (w/o this CL forgotten BT device remains) Change-Id: I31a5001115f1f163c8971bbd906516551b860252 --- media/java/android/media/MediaRouter2.java | 2 +- .../android/media/MediaRouter2Manager.java | 16 +++--- .../MediaRouter2ManagerTest.java | 52 ++++++++++++++++++- .../StubMediaRoute2ProviderService.java | 4 +- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index bd8fb96026562..cfe6db9fe3cf6 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -237,9 +237,9 @@ public final class MediaRouter2 { } catch (RemoteException ex) { Log.e(TAG, "Unable to unregister media router.", ex); } + mStub = null; } mShouldUpdateRoutes = true; - mStub = null; } } diff --git a/media/java/android/media/MediaRouter2Manager.java b/media/java/android/media/MediaRouter2Manager.java index b694fd059bfaf..3b570b60ff24d 100644 --- a/media/java/android/media/MediaRouter2Manager.java +++ b/media/java/android/media/MediaRouter2Manager.java @@ -147,14 +147,16 @@ public final class MediaRouter2Manager { } synchronized (sLock) { - if (mCallbackRecords.size() == 0 && mClient != null) { - try { - mMediaRouterService.unregisterManager(mClient); - } catch (RemoteException ex) { - Log.e(TAG, "Unable to unregister media router manager", ex); + if (mCallbackRecords.size() == 0) { + if (mClient != null) { + try { + mMediaRouterService.unregisterManager(mClient); + } catch (RemoteException ex) { + Log.e(TAG, "Unable to unregister media router manager", ex); + } + mClient = null; } - //TODO: clear mRoutes? - mClient = null; + mRoutes.clear(); mPreferredFeaturesMap.clear(); } } diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java index 6ca564fb34cc3..207534b36f958 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/MediaRouter2ManagerTest.java @@ -36,6 +36,7 @@ import static com.android.mediaroutertest.StubMediaRoute2ProviderService.VOLUME_ 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.assertTrue; import android.content.Context; @@ -160,6 +161,7 @@ public class MediaRouter2ManagerTest { }); MediaRoute2Info routeToRemove = routes.get(ROUTE_ID2); + assertNotNull(routeToRemove); StubMediaRoute2ProviderService sInstance = StubMediaRoute2ProviderService.getInstance(); @@ -171,6 +173,52 @@ public class MediaRouter2ManagerTest { assertTrue(addedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } + @Test + public void testGetRoutes_removedRoute_returnsCorrectRoutes() throws Exception { + CountDownLatch addedLatch = new CountDownLatch(1); + CountDownLatch removedLatch = new CountDownLatch(1); + + RouteCallback routeCallback = new RouteCallback() { + // Used to ensure the removed route is added. + @Override + public void onRoutesAdded(List routes) { + if (removedLatch.getCount() > 0) { + return; + } + addedLatch.countDown(); + } + + @Override + public void onRoutesRemoved(List routes) { + removedLatch.countDown(); + } + }; + + mRouter2.registerRouteCallback(mExecutor, routeCallback, + new RouteDiscoveryPreference.Builder(FEATURES_ALL, true).build()); + mRouteCallbacks.add(routeCallback); + + Map routes = waitAndGetRoutesWithManager(FEATURES_ALL); + MediaRoute2Info routeToRemove = routes.get(ROUTE_ID2); + assertNotNull(routeToRemove); + + StubMediaRoute2ProviderService sInstance = + StubMediaRoute2ProviderService.getInstance(); + assertNotNull(sInstance); + sInstance.removeRoute(ROUTE_ID2); + + // Wait until the route is removed. + assertTrue(removedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + + Map newRoutes = waitAndGetRoutesWithManager(FEATURES_ALL); + assertNull(newRoutes.get(ROUTE_ID2)); + + // Revert the removal. + sInstance.addRoute(routeToRemove); + assertTrue(addedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); + mRouter2.unregisterRouteCallback(routeCallback); + } + /** * Tests if we get proper routes for application that has special route feature. */ @@ -475,8 +523,8 @@ public class MediaRouter2ManagerTest { MediaRouter2Manager.Callback managerCallback = new MediaRouter2Manager.Callback() { @Override public void onRoutesAdded(List routes) { - for (int i = 0; i < routes.size(); i++) { - if (!routes.get(i).isSystemRoute()) { + for (MediaRoute2Info route : routes) { + if (!route.isSystemRoute()) { addedLatch.countDown(); break; } diff --git a/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java b/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java index 6d46ba582ddc1..4e398f26366ab 100644 --- a/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java +++ b/media/tests/MediaRouter/src/com/android/mediaroutertest/StubMediaRoute2ProviderService.java @@ -65,9 +65,9 @@ public class StubMediaRoute2ProviderService extends MediaRoute2ProviderService { public static final String ROUTE_NAME_VARIABLE_VOLUME = "Variable Volume Route"; public static final String FEATURE_SAMPLE = - "com.android.mediarouteprovider.FEATURE_SAMPLE"; + "com.android.mediaroutertest.FEATURE_SAMPLE"; public static final String FEATURE_SPECIAL = - "com.android.mediarouteprovider.FEATURE_SPECIAL"; + "com.android.mediaroutertest..FEATURE_SPECIAL"; Map mRoutes = new HashMap<>(); Map mRouteIdToSessionId = new HashMap<>();