From 2fd0d11db336f40b7bc2ccffbea3c42e6e0f784f Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Fri, 25 Apr 2025 14:48:31 +0800 Subject: [PATCH] Pipe the satellite network opt-in UIDs down to ConnectivityService Flag: EXEMPT flagged with a kill switch Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.connectivity.SatelliteAccessControllerTest \ --update-device Bug: 410731080 Change-Id: I76a57a3c42659b594cd541c55e0e179062461085 --- .../SatelliteAccessController.java | 55 +++++++----- .../SatelliteAccessControllerTest.kt | 90 +++++++++++++------ 2 files changed, 96 insertions(+), 49 deletions(-) diff --git a/service/src/com/android/server/connectivity/SatelliteAccessController.java b/service/src/com/android/server/connectivity/SatelliteAccessController.java index b1a104cee1..baf146f734 100644 --- a/service/src/com/android/server/connectivity/SatelliteAccessController.java +++ b/service/src/com/android/server/connectivity/SatelliteAccessController.java @@ -43,6 +43,8 @@ import java.util.Set; import java.util.concurrent.Executor; import java.util.function.BiConsumer; +import javax.annotation.CheckReturnValue; + /** * Tracks the uid of all the default messaging application which are role_sms role and * satellite_communication permission complaint and requests ConnectivityService to create multi @@ -145,6 +147,8 @@ public class SatelliteAccessController { mSupportConstrainedDataSatelliteOptIn = mDeps.supportConstrainedDataSatelliteOptIn(c); } + // TODO: Rename to updateSatelliteSmsRoleUidListCache since Opt-In apps are also + // fallback uids. private Set updateSatelliteNetworkFallbackUidListCache(List packageNames, @NonNull UserHandle userHandle) { Set fallbackUids = new ArraySet<>(); @@ -229,11 +233,13 @@ public class SatelliteAccessController { mergedSatelliteNetworkFallbackUidCache.addAll( mAllUsersSatelliteNetworkFallbackUidCache.valueAt(i)); } - Log.i(TAG, "merged uid list for multi layer request : " - + mergedSatelliteNetworkFallbackUidCache); + Log.i(TAG, "SmsRoleUids: " + mergedSatelliteNetworkFallbackUidCache + + " Opt-InUids:" + mSatelliteDataOptimizedUids); // trigger multiple layer request for satellite network fallback of multi user uids - mCallback.accept(mergedSatelliteNetworkFallbackUidCache, Set.of()); + final ArraySet optimizedApps = new ArraySet(mSatelliteDataOptimizedUids); + optimizedApps.removeAll(mergedSatelliteNetworkFallbackUidCache); + mCallback.accept(mergedSatelliteNetworkFallbackUidCache, optimizedApps); } public void start() { @@ -241,12 +247,14 @@ public class SatelliteAccessController { mDefaultMessageRoleListener.register(); } - private void updateSatelliteFallbackUidListOnUserRemoval(int userIdRemoved) { + @CheckReturnValue + private boolean updateSatelliteFallbackUidListOnUserRemoval(int userIdRemoved) { Log.i(TAG, "user id removed:" + userIdRemoved); if (mAllUsersSatelliteNetworkFallbackUidCache.contains(userIdRemoved)) { mAllUsersSatelliteNetworkFallbackUidCache.remove(userIdRemoved); - reportSatelliteNetworkFallbackUids(); + return true; // Changed. } + return false; // Unchanged. } /** @@ -269,7 +277,10 @@ public class SatelliteAccessController { getSatelliteDataOptimizedAppsForUser(apps); Log.i(TAG, "Add SatelliteDataOptimizedApps + for user " + userHandle + ": " + satelliteDataOptimizedAppsForUser); - mSatelliteDataOptimizedUids.addAll(satelliteDataOptimizedAppsForUser); + if (satelliteDataOptimizedAppsForUser.size() > 0) { + mSatelliteDataOptimizedUids.addAll(satelliteDataOptimizedAppsForUser); + reportSatelliteNetworkFallbackUids(); + } } /** @@ -278,9 +289,17 @@ public class SatelliteAccessController { * @param userHandle The integer userHandle of the removed user. See {@link #EXTRA_USER_HANDLE}. */ public void onUserRemoved(@NonNull UserHandle userHandle) { - updateSatelliteFallbackUidListOnUserRemoval(userHandle.getIdentifier()); + final boolean smsRoleUidsChanged = + updateSatelliteFallbackUidListOnUserRemoval(userHandle.getIdentifier()); + final boolean mDataOptimizedUidChanged; if (mSupportConstrainedDataSatelliteOptIn) { - removeSatelliteDataOptimizedAppsForUser(userHandle.getIdentifier()); + mDataOptimizedUidChanged = + removeSatelliteDataOptimizedUidsForUser(userHandle.getIdentifier()); + } else { + mDataOptimizedUidChanged = false; + } + if (smsRoleUidsChanged || mDataOptimizedUidChanged) { + reportSatelliteNetworkFallbackUids(); } } @@ -293,6 +312,7 @@ public class SatelliteAccessController { public void onPackageAdded(@NonNull final String packageName, final int uid) { if (mSupportConstrainedDataSatelliteOptIn && isSatelliteDataOptimizedApp(packageName)) { mSatelliteDataOptimizedUids.add(uid); + reportSatelliteNetworkFallbackUids(); } } @@ -317,18 +337,7 @@ public class SatelliteAccessController { // If the loop completes without returning, it means no other // satellite-optimized app shares the UID. mSatelliteDataOptimizedUids.remove(uid); - } - - /** Checks if the given UID is in the set of satellite data optimized UIDs. */ - @VisibleForTesting - public boolean isSatelliteDataOptimizedUid(int uid) { - return mSatelliteDataOptimizedUids.contains(uid); - } - - /** Gets the number of satellite data optimized UIDs being tracked. */ - @VisibleForTesting - public int getSatelliteDataOptimizedUidCount() { - return mSatelliteDataOptimizedUids.size(); + reportSatelliteNetworkFallbackUids(); } @NonNull @@ -351,7 +360,9 @@ public class SatelliteAccessController { } } - private void removeSatelliteDataOptimizedAppsForUser(int userIdToRemove) { - mSatelliteDataOptimizedUids.removeIf(uid -> uid / PER_USER_RANGE == userIdToRemove); + // Return true if changed. + @CheckReturnValue + private boolean removeSatelliteDataOptimizedUidsForUser(int userIdToRemove) { + return mSatelliteDataOptimizedUids.removeIf(uid -> uid / PER_USER_RANGE == userIdToRemove); } } diff --git a/tests/unit/java/com/android/server/connectivity/SatelliteAccessControllerTest.kt b/tests/unit/java/com/android/server/connectivity/SatelliteAccessControllerTest.kt index 0191b3dad9..dc1aaca197 100644 --- a/tests/unit/java/com/android/server/connectivity/SatelliteAccessControllerTest.kt +++ b/tests/unit/java/com/android/server/connectivity/SatelliteAccessControllerTest.kt @@ -38,9 +38,6 @@ import com.android.testutils.com.android.testutils.SetFeatureFlagsRule.FeatureFl import java.util.concurrent.CompletableFuture import java.util.concurrent.Executor import java.util.function.BiConsumer -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test @@ -50,6 +47,7 @@ import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.doReturn +import org.mockito.Mockito.inOrder import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.times @@ -355,8 +353,7 @@ class SatelliteAccessControllerTest { fun testSatelliteOptimizedUids_onPackageAdded() { mockIsSatelliteDataOptimizedApp(TEST_PACKAGE1, true) mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 1) + verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) } @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) @@ -364,8 +361,7 @@ class SatelliteAccessControllerTest { fun testSatelliteOptimizedUids_onPackageAdded_ignoresIfNotSatelliteOptimized() { mockIsSatelliteDataOptimizedApp(TEST_PACKAGE1, false) mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) - assertFalse(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) + verify(mCallback, never()).accept(any(), any()) } @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) @@ -375,11 +371,9 @@ class SatelliteAccessControllerTest { mockGetPackagesForUid(TEST_UID1, arrayOf(TEST_PACKAGE1)) mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 1) + verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) mSatelliteAccessController.onPackageRemoved(TEST_PACKAGE1, TEST_UID1) - assertFalse(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) + verify(mCallback).accept(emptySet(), emptySet()) } @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) @@ -390,18 +384,16 @@ class SatelliteAccessControllerTest { mockGetPackagesForUid(TEST_UID1, arrayOf(TEST_PACKAGE1, TEST_PACKAGE2)) // Verify uid is not removed if there is still another package shares the same uid. + val inOrder = inOrder(mCallback) mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 1) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) mSatelliteAccessController.onPackageRemoved(TEST_PACKAGE1, TEST_UID1) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 1) + inOrder.verifyNoMoreInteractions() // Verify uid is removed if there is no other package with shared uid. mockGetPackagesForUid(TEST_UID1, null) mSatelliteAccessController.onPackageRemoved(TEST_PACKAGE2, TEST_UID1) - assertFalse(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) + inOrder.verify(mCallback).accept(emptySet(), emptySet()) } @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) @@ -412,22 +404,22 @@ class SatelliteAccessControllerTest { val packageInfo1 = makePackageInfo(TEST_PACKAGE1, TEST_UID1) val packageInfo2 = makePackageInfo(TEST_PACKAGE2, TEST_UID2) + val inOrder = inOrder(mCallback) mSatelliteAccessController .onUserAddedWithInstalledPackageList(PRIMARY_USER_HANDLE, listOf(packageInfo1)) mSatelliteAccessController .onUserAddedWithInstalledPackageList(SECONDARY_USER_HANDLE, listOf(packageInfo2)) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID2)) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1, TEST_UID2)) mSatelliteAccessController.onUserRemoved(SECONDARY_USER_HANDLE) // Verify that the app associated with the non-removed user is not removed. - assertTrue(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID1)) - // Verify that the app associated with the removed user is removed. - assertFalse(mSatelliteAccessController.isSatelliteDataOptimizedUid(TEST_UID2)) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) mSatelliteAccessController.onUserRemoved(PRIMARY_USER_HANDLE) // Verify everything is removed. - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) + inOrder.verify(mCallback).accept(emptySet(), emptySet()) + inOrder.verifyNoMoreInteractions() } @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN, enabled = false) @@ -439,12 +431,56 @@ class SatelliteAccessControllerTest { // Verify nothing changes and nothing crashes. mSatelliteAccessController .onUserAddedWithInstalledPackageList(PRIMARY_USER_HANDLE, listOf(packageInfo1)) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) mSatelliteAccessController.onPackageRemoved(TEST_PACKAGE1, TEST_UID1) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) mSatelliteAccessController.onUserRemoved(PRIMARY_USER_HANDLE) - assertEquals(mSatelliteAccessController.satelliteDataOptimizedUidCount, 0) + verify(mCallback, never()).accept(any(), any()) + } + + @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) + @Test + fun testSatelliteOptimizedUids_withSmsRoleUids() { + startSatelliteAccessController() + // Set SMS_APP1 under primary user as a sms-role Uid. + doReturn(listOf(SMS_APP1)) + .`when`(mDeps).getRoleHoldersAsUser(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + mRoleHolderChangedListener.onRoleHoldersChanged(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + verify(mCallback).accept(setOf(SMS_APP_ID1.toUid(PRIMARY_USER)), emptySet()) + + // Mock another opt-in uid, verify they both reported via the callback. + mockIsSatelliteDataOptimizedApp(TEST_PACKAGE1, true) + mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) + verify(mCallback).accept(setOf(SMS_APP_ID1.toUid(PRIMARY_USER)), setOf(TEST_UID1)) + } + + @FeatureFlag(name = CONSTRAINED_DATA_SATELLITE_OPTIN) + @Test + fun testSatelliteOptimizedUids_withSmsRoleUids_overlappedUid() { + startSatelliteAccessController() + val smsUid = SMS_APP_ID1.toUid(PRIMARY_USER) + + val inOrder = inOrder(mCallback) + // Mock opt-in uids, verify they both reported via the callback. + // However, one opt-in uid is a messaging app and will surprise us later. + mockIsSatelliteDataOptimizedApp(TEST_PACKAGE1, true) + mockIsSatelliteDataOptimizedApp(SMS_APP1, true) + mSatelliteAccessController.onPackageAdded(TEST_PACKAGE1, TEST_UID1) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1)) + mSatelliteAccessController.onPackageAdded(SMS_APP1, smsUid) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1, smsUid)) + + // Set SMS_APP1 as a sms-role Uid. + // Verify the sms-role Uid is excluded from the opt-in Uid list. + doReturn(listOf(SMS_APP1)) + .`when`(mDeps).getRoleHoldersAsUser(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + mRoleHolderChangedListener.onRoleHoldersChanged(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + inOrder.verify(mCallback).accept(setOf(smsUid), setOf(TEST_UID1)) + + // Unset SMS_APP1 as the sms-role Uid. + // Verify the sms-role Uid is included to the opt-in Uid list again. + doReturn(emptyList()) + .`when`(mDeps).getRoleHoldersAsUser(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + mRoleHolderChangedListener.onRoleHoldersChanged(RoleManager.ROLE_SMS, PRIMARY_USER_HANDLE) + inOrder.verify(mCallback).accept(emptySet(), setOf(TEST_UID1, smsUid)) } }