diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 0c990ecfc827a..6a7afd90dc967 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -24,7 +24,6 @@ import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_INACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE; -import static android.telephony.SubscriptionManager.isValidSubscriptionId; import static com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import static com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionTrackerCallback; @@ -164,10 +163,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { @VisibleForTesting(visibility = Visibility.PRIVATE) static final String VCN_CONFIG_FILE = "/data/system/vcn/configs.xml"; - // TODO(b/176956496): Directly use CarrierServiceBindHelper.UNBIND_DELAY_MILLIS - @VisibleForTesting(visibility = Visibility.PRIVATE) - static final long CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS = TimeUnit.SECONDS.toMillis(30); - /* Binder context for this service */ @NonNull private final Context mContext; @NonNull private final Dependencies mDeps; @@ -372,12 +367,15 @@ public class VcnManagementService extends IVcnManagementService.Stub { /** Notifies the VcnManagementService that external dependencies can be set up. */ public void systemReady() { - mNetworkProvider.register(); - mContext.getSystemService(ConnectivityManager.class) - .registerNetworkCallback( - new NetworkRequest.Builder().clearCapabilities().build(), - mTrackingNetworkCallback); - mTelephonySubscriptionTracker.register(); + // Always run on the handler thread to ensure consistency. + mHandler.post(() -> { + mNetworkProvider.register(); + mContext.getSystemService(ConnectivityManager.class) + .registerNetworkCallback( + new NetworkRequest.Builder().clearCapabilities().build(), + mTrackingNetworkCallback); + mTelephonySubscriptionTracker.register(); + }); } private void enforcePrimaryUser() { @@ -471,22 +469,15 @@ public class VcnManagementService extends IVcnManagementService.Stub { if (!mVcns.containsKey(subGrp)) { startVcnLocked(subGrp, entry.getValue()); } - - // Cancel any scheduled teardowns for active subscriptions - mHandler.removeCallbacksAndMessages(mVcns.get(subGrp)); } } - // Schedule teardown of any VCN instances that have lost carrier privileges (after a - // delay) + // Schedule teardown of any VCN instances that have lost carrier privileges for (Entry entry : mVcns.entrySet()) { final ParcelUuid subGrp = entry.getKey(); final VcnConfig config = mConfigs.get(subGrp); final boolean isActiveSubGrp = isActiveSubGroup(subGrp, snapshot); - final boolean isValidActiveDataSubIdNotInVcnSubGrp = - isValidSubscriptionId(snapshot.getActiveDataSubscriptionId()) - && !isActiveSubGroup(subGrp, snapshot); // TODO(b/193687515): Support multiple VCNs active at the same time if (config == null @@ -496,31 +487,12 @@ public class VcnManagementService extends IVcnManagementService.Stub { final ParcelUuid uuidToTeardown = subGrp; final Vcn instanceToTeardown = entry.getValue(); - // TODO(b/193687515): Support multiple VCNs active at the same time - // If directly switching to a subscription not in the current group, - // teardown immediately to prevent other subscription's network from being - // outscored by the VCN. Otherwise, teardown after a delay to ensure that - // SIM profile switches do not trigger the VCN to cycle. - final long teardownDelayMs = - isValidActiveDataSubIdNotInVcnSubGrp - ? 0 - : CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS; - mHandler.postDelayed(() -> { - synchronized (mLock) { - // Guard against case where this is run after a old instance was - // torn down, and a new instance was started. Verify to ensure - // correct instance is torn down. This could happen as a result of a - // Carrier App manually removing/adding a VcnConfig. - if (mVcns.get(uuidToTeardown) == instanceToTeardown) { - stopVcnLocked(uuidToTeardown); + stopVcnLocked(uuidToTeardown); - // TODO(b/181789060): invoke asynchronously after Vcn notifies - // through VcnCallback - notifyAllPermissionedStatusCallbacksLocked( - uuidToTeardown, VCN_STATUS_CODE_INACTIVE); - } - } - }, instanceToTeardown, teardownDelayMs); + // TODO(b/181789060): invoke asynchronously after Vcn notifies + // through VcnCallback + notifyAllPermissionedStatusCallbacksLocked( + uuidToTeardown, VCN_STATUS_CODE_INACTIVE); } else { // If this VCN's status has not changed, update it with the new snapshot entry.getValue().updateSubscriptionSnapshot(mLastSnapshot); diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 7c7dc4d79e9ac..bb98bc0bab536 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -23,7 +23,6 @@ import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE; -import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static android.telephony.TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS; import static android.telephony.TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS; @@ -264,6 +263,7 @@ public class VcnManagementServiceTest { @Test public void testSystemReady() throws Exception { mVcnMgmtSvc.systemReady(); + mTestLooper.dispatchAll(); verify(mConnMgr).registerNetworkProvider(any(VcnNetworkProvider.class)); verify(mSubscriptionTracker).register(); @@ -475,10 +475,8 @@ public class VcnManagementServiceTest { mVcnMgmtSvc.addVcnUnderlyingNetworkPolicyListener(mMockPolicyListener); triggerSubscriptionTrackerCbAndGetSnapshot(null, Collections.emptySet()); - - // Verify teardown after delay - mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); mTestLooper.dispatchAll(); + verify(vcn).teardownAsynchronously(); verify(mMockPolicyListener).onPolicyChanged(); } @@ -504,92 +502,6 @@ public class VcnManagementServiceTest { assertEquals(0, mVcnMgmtSvc.getAllVcns().size()); } - /** - * Tests an intermediate state where carrier privileges are marked as lost before active data - * subId changes during a SIM ejection. - * - *

The expected outcome is that the VCN is torn down after a delay, as opposed to - * immediately. - */ - @Test - public void testTelephonyNetworkTrackerCallbackLostCarrierPrivilegesBeforeActiveDataSubChanges() - throws Exception { - setupActiveSubscription(TEST_UUID_2); - - final TelephonySubscriptionTrackerCallback cb = getTelephonySubscriptionTrackerCallback(); - final Vcn vcn = startAndGetVcnInstance(TEST_UUID_2); - - // Simulate privileges lost - triggerSubscriptionTrackerCbAndGetSnapshot( - TEST_SUBSCRIPTION_ID, - TEST_UUID_2, - Collections.emptySet(), - Collections.emptyMap(), - false /* hasCarrierPrivileges */); - - // Verify teardown after delay - mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); - mTestLooper.dispatchAll(); - verify(vcn).teardownAsynchronously(); - } - - @Test - public void testTelephonyNetworkTrackerCallbackSimSwitchesDoNotKillVcnInstances() - throws Exception { - setupActiveSubscription(TEST_UUID_2); - - final TelephonySubscriptionTrackerCallback cb = getTelephonySubscriptionTrackerCallback(); - final Vcn vcn = startAndGetVcnInstance(TEST_UUID_2); - - // Simulate SIM unloaded - triggerSubscriptionTrackerCbAndGetSnapshot( - INVALID_SUBSCRIPTION_ID, - null /* activeDataSubscriptionGroup */, - Collections.emptySet(), - Collections.emptyMap(), - false /* hasCarrierPrivileges */); - - // Simulate new SIM loaded right during teardown delay. - mTestLooper.moveTimeForward( - VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS / 2); - mTestLooper.dispatchAll(); - triggerSubscriptionTrackerCbAndGetSnapshot(TEST_UUID_2, Collections.singleton(TEST_UUID_2)); - - // Verify that even after the full timeout duration, the VCN instance is not torn down - mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); - mTestLooper.dispatchAll(); - verify(vcn, never()).teardownAsynchronously(); - } - - @Test - public void testTelephonyNetworkTrackerCallbackDoesNotKillNewVcnInstances() throws Exception { - setupActiveSubscription(TEST_UUID_2); - - final TelephonySubscriptionTrackerCallback cb = getTelephonySubscriptionTrackerCallback(); - final Vcn oldInstance = startAndGetVcnInstance(TEST_UUID_2); - - // Simulate SIM unloaded - triggerSubscriptionTrackerCbAndGetSnapshot(null, Collections.emptySet()); - - // Config cleared, SIM reloaded & config re-added right before teardown delay, staring new - // vcnInstance. - mTestLooper.moveTimeForward( - VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS / 2); - mTestLooper.dispatchAll(); - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2, TEST_PACKAGE_NAME); - triggerSubscriptionTrackerCbAndGetSnapshot(TEST_UUID_2, Collections.singleton(TEST_UUID_2)); - final Vcn newInstance = startAndGetVcnInstance(TEST_UUID_2); - - // Verify that new instance was different, and the old one was torn down - assertTrue(oldInstance != newInstance); - verify(oldInstance).teardownAsynchronously(); - - // Verify that even after the full timeout duration, the new VCN instance is not torn down - mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); - mTestLooper.dispatchAll(); - verify(newInstance, never()).teardownAsynchronously(); - } - @Test public void testPackageChangeListenerRegistered() throws Exception { verify(mMockContext).registerReceiver(any(BroadcastReceiver.class), argThat(filter -> { @@ -925,6 +837,8 @@ public class VcnManagementServiceTest { private void setupSubscriptionAndStartVcn( int subId, ParcelUuid subGrp, boolean isVcnActive, boolean hasCarrierPrivileges) { mVcnMgmtSvc.systemReady(); + mTestLooper.dispatchAll(); + triggerSubscriptionTrackerCbAndGetSnapshot( subGrp, Collections.singleton(subGrp), @@ -1020,6 +934,7 @@ public class VcnManagementServiceTest { private void setupTrackedCarrierWifiNetwork(NetworkCapabilities caps) { mVcnMgmtSvc.systemReady(); + mTestLooper.dispatchAll(); final ArgumentCaptor captor = ArgumentCaptor.forClass(NetworkCallback.class); @@ -1264,15 +1179,14 @@ public class VcnManagementServiceTest { true /* isActive */, true /* hasCarrierPrivileges */); - // VCN is currently active. Lose carrier privileges for TEST_PACKAGE and hit teardown - // timeout so the VCN goes inactive. + // VCN is currently active. Lose carrier privileges for TEST_PACKAGE so the VCN goes + // inactive. final TelephonySubscriptionSnapshot snapshot = triggerSubscriptionTrackerCbAndGetSnapshot( TEST_UUID_1, Collections.singleton(TEST_UUID_1), Collections.singletonMap(TEST_SUBSCRIPTION_ID, TEST_UUID_1), false /* hasCarrierPrivileges */); - mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); mTestLooper.dispatchAll(); // Giving TEST_PACKAGE privileges again will restart the VCN (which will indicate ACTIVE