Merge changes I2e52ff67,I9f2b2d0d am: f11ccc456b

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1661226

Change-Id: Icd8b937b0fca9ac4d8f32c688aa8b325cf563f65

services/core/java/com/android/server/VcnManagementService.java

@@ -40,7 +40,6 @@ import android.net.vcn.IVcnManagementService;
 import android.net.vcn.IVcnStatusCallback;
 import android.net.vcn.IVcnUnderlyingNetworkPolicyListener;
 import android.net.vcn.VcnConfig;
-import android.net.vcn.VcnManager;
 import android.net.vcn.VcnManager.VcnErrorCode;
 import android.net.vcn.VcnManager.VcnStatusCode;
 import android.net.vcn.VcnUnderlyingNetworkPolicy;
@@ -532,11 +531,12 @@ public class VcnManagementService extends IVcnManagementService.Stub {
 
         if (mVcns.containsKey(subscriptionGroup)) {
             final Vcn vcn = mVcns.get(subscriptionGroup);
-            final boolean isActive = vcn.isActive();
+            final int status = vcn.getStatus();
             vcn.updateConfig(config);
 
+            // TODO(b/183174340): Remove this once opportunistic-safe-mode is supported
             // Only notify VcnStatusCallbacks if this VCN was previously in Safe Mode
-            if (!isActive) {
+            if (status == VCN_STATUS_CODE_SAFE_MODE) {
                 // TODO(b/181789060): invoke asynchronously after Vcn notifies through VcnCallback
                 notifyAllPermissionedStatusCallbacksLocked(
                         subscriptionGroup, VCN_STATUS_CODE_ACTIVE);
@@ -769,7 +769,7 @@ public class VcnManagementService extends IVcnManagementService.Stub {
             synchronized (mLock) {
                 final Vcn vcn = mVcns.get(subGrp);
                 if (vcn != null) {
-                    if (vcn.isActive()) {
+                    if (vcn.getStatus() == VCN_STATUS_CODE_ACTIVE) {
                         isVcnManagedNetwork = true;
                     }
 
@@ -880,20 +880,23 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                 // now that callback is registered, send it the VCN's current status
                 final VcnConfig vcnConfig = mConfigs.get(subGroup);
                 final Vcn vcn = mVcns.get(subGroup);
-                final int vcnStatus;
+                final int vcnStatus =
+                        vcn == null ? VCN_STATUS_CODE_NOT_CONFIGURED : vcn.getStatus();
+                final int resultStatus;
                 if (vcnConfig == null || !isCallbackPermissioned(cbInfo, subGroup)) {
-                    vcnStatus = VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED;
+                    resultStatus = VCN_STATUS_CODE_NOT_CONFIGURED;
                 } else if (vcn == null) {
-                    vcnStatus = VcnManager.VCN_STATUS_CODE_INACTIVE;
+                    resultStatus = VCN_STATUS_CODE_INACTIVE;
-                } else if (vcn.isActive()) {
+                } else if (vcnStatus == VCN_STATUS_CODE_ACTIVE
-                    vcnStatus = VcnManager.VCN_STATUS_CODE_ACTIVE;
+                        || vcnStatus == VCN_STATUS_CODE_SAFE_MODE) {
+                    resultStatus = vcnStatus;
                 } else {
-                    // TODO(b/181789060): create Vcn.getStatus() and Log.WTF() for unknown status
+                    Slog.wtf(TAG, "Unknown VCN status: " + vcnStatus);
-                    vcnStatus = VcnManager.VCN_STATUS_CODE_SAFE_MODE;
+                    resultStatus = VCN_STATUS_CODE_NOT_CONFIGURED;
                 }
 
                 try {
-                    cbInfo.mCallback.onVcnStatusChanged(vcnStatus);
+                    cbInfo.mCallback.onVcnStatusChanged(resultStatus);
                 } catch (RemoteException e) {
                     Slog.d(TAG, "VcnStatusCallback threw on VCN status change", e);
                 }

services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java

@@ -118,7 +118,7 @@ public class UnderlyingNetworkTracker {
         if (!mIsQuitting) {
             mRouteSelectionCallback = new RouteSelectionCallback();
             mConnectivityManager.requestBackgroundNetwork(
-                    getBaseNetworkRequestBuilder().build(), mHandler, mRouteSelectionCallback);
+                    getRouteSelectionRequest(), mHandler, mRouteSelectionCallback);
 
             mWifiBringupCallback = new NetworkBringupCallback();
             mConnectivityManager.requestBackgroundNetwork(
@@ -149,12 +149,48 @@ public class UnderlyingNetworkTracker {
         }
     }
 
-    private NetworkRequest getWifiNetworkRequest() {
+    /**
+     * Builds the Route selection request
+     *
+     * <p>This request is guaranteed to select carrier-owned, non-VCN underlying networks by virtue
+     * of a populated set of subIds as expressed in NetworkCapabilities#getSubIds(). Only carrier
+     * owned networks may be selected, as the request specifies only subIds in the VCN's
+     * subscription group, while the VCN networks are excluded by virtue of not having subIds set on
+     * the VCN-exposed networks.
+     */
+    private NetworkRequest getRouteSelectionRequest() {
         return getBaseNetworkRequestBuilder()
-                .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
+                .setSubIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup))
                 .build();
     }
 
+    /**
+     * Builds the WiFi bringup request
+     *
+     * <p>This request is built specifically to match only carrier-owned WiFi networks, but is also
+     * built to ONLY keep Carrier WiFi Networks alive (but never bring them up). This is a result of
+     * the WifiNetworkFactory not advertising a list of subIds, and therefore not accepting this
+     * request. As such, it will bind to a Carrier WiFi Network that has already been brought up,
+     * but will NEVER bring up a Carrier WiFi network itself.
+     */
+    private NetworkRequest getWifiNetworkRequest() {
+        return getBaseNetworkRequestBuilder()
+                .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
+                .setSubIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup))
+                .build();
+    }
+
+    /**
+     * Builds a Cellular bringup request for a given subId
+     *
+     * <p>This request is filed in order to ensure that the Telephony stack always has a
+     * NetworkRequest to bring up a VCN underlying cellular network. It is required in order to
+     * ensure that even when a VCN (appears as Cellular) satisfies the default request, Telephony
+     * will bring up additional underlying Cellular networks.
+     *
+     * <p>Since this request MUST make it to the TelephonyNetworkFactory, subIds are not specified
+     * in the NetworkCapabilities, but rather in the TelephonyNetworkSpecifier.
+     */
     private NetworkRequest getCellNetworkRequestForSubId(int subId) {
         return getBaseNetworkRequestBuilder()
                 .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
@@ -164,20 +200,13 @@ public class UnderlyingNetworkTracker {
 
     /**
      * Builds and returns a NetworkRequest builder common to all Underlying Network requests
-     *
-     * <p>This request is guaranteed to select carrier-owned, non-VCN underlying networks by virtue
-     * of a populated set of subIds as expressed in NetworkCapabilities#getSubIds(). Only carrier
-     * owned networks may be selected, as the request specifies only subIds in the VCN's
-     * subscription group, while the VCN networks are excluded by virtue of not having subIds set on
-     * the VCN-exposed networks.
      */
     private NetworkRequest.Builder getBaseNetworkRequestBuilder() {
         return new NetworkRequest.Builder()
                 .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
                 .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED)
                 .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
-                .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED)
+                .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
-                .setSubIds(mLastSnapshot.getAllSubIdsInGroup(mSubscriptionGroup));
     }
 
     /**

services/core/java/com/android/server/vcn/Vcn.java

@@ -17,6 +17,9 @@
 package com.android.server.vcn;
 
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED;
+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_SAFE_MODE;
 
 import static com.android.server.VcnManagementService.VDBG;
 
@@ -44,7 +47,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * Represents an single instance of a VCN.
@@ -137,17 +139,14 @@ public class Vcn extends Handler {
     @NonNull private TelephonySubscriptionSnapshot mLastSnapshot;
 
     /**
-     * Whether this Vcn instance is active and running.
+     * The current status of this Vcn instance
      *
-     * <p>The value will be {@code true} while running. It will be {@code false} if the VCN has been
+     * <p>The value will be {@link VCN_STATUS_CODE_ACTIVE} while all VcnGatewayConnections are in
-     * shut down or has entered safe mode.
+     * good standing, {@link VCN_STATUS_CODE_SAFE_MODE} if any VcnGatewayConnections are in safe
-     *
+     * mode, and {@link VCN_STATUS_CODE_INACTIVE} once a teardown has been commanded.
-     * <p>This AtomicBoolean is required in order to ensure consistency and correctness across
-     * multiple threads. Unlike the rest of the Vcn, this is queried synchronously on Binder threads
-     * from VcnManagementService, and therefore cannot rely on guarantees of running on the VCN
-     * Looper.
      */
-    private final AtomicBoolean mIsActive = new AtomicBoolean(true);
+    // Accessed from different threads, but always under lock in VcnManagementService
+    private volatile int mCurrentStatus = VCN_STATUS_CODE_ACTIVE;
 
     public Vcn(
             @NonNull VcnContext vcnContext,
@@ -199,9 +198,15 @@ public class Vcn extends Handler {
         sendMessageAtFrontOfQueue(obtainMessage(MSG_CMD_TEARDOWN));
     }
 
-    /** Synchronously checks whether this Vcn is active. */
+    /** Synchronously retrieves the current status code. */
-    public boolean isActive() {
+    public int getStatus() {
-        return mIsActive.get();
+        return mCurrentStatus;
+    }
+
+    /** Sets the status of this VCN */
+    @VisibleForTesting(visibility = Visibility.PRIVATE)
+    public void setStatus(int status) {
+        mCurrentStatus = status;
     }
 
     /** Get current Gateways for testing purposes */
@@ -217,12 +222,6 @@ public class Vcn extends Handler {
         return Collections.unmodifiableMap(new HashMap<>(mVcnGatewayConnections));
     }
 
-    /** Set whether this Vcn is active for testing purposes */
-    @VisibleForTesting(visibility = Visibility.PRIVATE)
-    public void setIsActive(boolean isActive) {
-        mIsActive.set(isActive);
-    }
-
     private class VcnNetworkRequestListener implements VcnNetworkProvider.NetworkRequestListener {
         @Override
         public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) {
@@ -264,7 +263,8 @@ public class Vcn extends Handler {
 
         mConfig = config;
 
-        if (mIsActive.getAndSet(true)) {
+        // TODO(b/183174340): Remove this once opportunistic safe mode is supported.
+        if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) {
             // VCN is already active - teardown any GatewayConnections whose configs have been
             // removed and get all current requests
             for (final Entry<VcnGatewayConnectionConfig, VcnGatewayConnection> entry :
@@ -288,11 +288,15 @@ public class Vcn extends Handler {
             // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be
             // satisfied start a new GatewayConnection)
             mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener);
-        } else {
+        } else if (mCurrentStatus == VCN_STATUS_CODE_SAFE_MODE) {
             // If this VCN was not previously active, it is exiting Safe Mode. Re-register the
             // request listener to get NetworkRequests again (and all cached requests).
             mVcnContext.getVcnNetworkProvider().registerListener(mRequestListener);
+        } else {
+            // Ignored; VCN was not active; config updates ignored.
+            return;
         }
+        mCurrentStatus = VCN_STATUS_CODE_ACTIVE;
     }
 
     private void handleTeardown() {
@@ -302,18 +306,20 @@ public class Vcn extends Handler {
             gatewayConnection.teardownAsynchronously();
         }
 
-        mIsActive.set(false);
+        mCurrentStatus = VCN_STATUS_CODE_INACTIVE;
     }
 
     private void handleEnterSafeMode() {
+        // TODO(b/183174340): Remove this once opportunistic-safe-mode is supported
         handleTeardown();
 
+        mCurrentStatus = VCN_STATUS_CODE_SAFE_MODE;
         mVcnCallback.onEnteredSafeMode();
     }
 
     private void handleNetworkRequested(
             @NonNull NetworkRequest request, int score, int providerId) {
-        if (!isActive()) {
+        if (mCurrentStatus != VCN_STATUS_CODE_ACTIVE) {
             Slog.v(getLogTag(), "Received NetworkRequest while inactive. Ignore for now");
             return;
         }
@@ -370,8 +376,8 @@ public class Vcn extends Handler {
         mVcnGatewayConnections.remove(config);
 
         // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be satisfied
-        // start a new GatewayConnection), but only if the Vcn is still active
+        // start a new GatewayConnection), but only if the Vcn is still alive
-        if (isActive()) {
+        if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) {
             mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener);
         }
     }
@@ -379,7 +385,7 @@ public class Vcn extends Handler {
     private void handleSubscriptionsChanged(@NonNull TelephonySubscriptionSnapshot snapshot) {
         mLastSnapshot = snapshot;
 
-        if (isActive()) {
+        if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) {
             for (VcnGatewayConnection gatewayConnection : mVcnGatewayConnections.values()) {
                 gatewayConnection.updateSubscriptionSnapshot(mLastSnapshot);
             }

tests/vcn/java/com/android/server/VcnManagementServiceTest.java

@@ -19,6 +19,8 @@ package com.android.server;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED;
 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.TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS;
 import static android.telephony.TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS;
 
@@ -695,7 +697,9 @@ public class VcnManagementServiceTest {
                 hasCarrierPrivileges);
 
         final Vcn vcn = startAndGetVcnInstance(subGrp);
-        doReturn(isVcnActive).when(vcn).isActive();
+        doReturn(isVcnActive ? VCN_STATUS_CODE_ACTIVE : VCN_STATUS_CODE_SAFE_MODE)
+                .when(vcn)
+                .getStatus();
 
         doReturn(true)
                 .when(mLocationPermissionChecker)

tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java

@@ -158,7 +158,7 @@ public class UnderlyingNetworkTrackerTest {
         for (final int subId : expectedSubIds) {
             verify(mConnectivityManager)
                     .requestBackgroundNetwork(
-                            eq(getCellRequestForSubId(subId, expectedSubIds)),
+                            eq(getCellRequestForSubId(subId)),
                             any(),
                             any(NetworkBringupCallback.class));
         }
@@ -189,30 +189,30 @@ public class UnderlyingNetworkTrackerTest {
     }
 
     private NetworkRequest getWifiRequest(Set<Integer> netCapsSubIds) {
-        return getExpectedRequestBase(netCapsSubIds)
+        return getExpectedRequestBase()
                 .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
+                .setSubIds(netCapsSubIds)
                 .build();
     }
 
-    private NetworkRequest getCellRequestForSubId(int subId, Set<Integer> netCapsSubIds) {
+    private NetworkRequest getCellRequestForSubId(int subId) {
-        return getExpectedRequestBase(netCapsSubIds)
+        return getExpectedRequestBase()
                 .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
                 .setNetworkSpecifier(new TelephonyNetworkSpecifier(subId))
                 .build();
     }
 
     private NetworkRequest getRouteSelectionRequest(Set<Integer> netCapsSubIds) {
-        return getExpectedRequestBase(netCapsSubIds).build();
+        return getExpectedRequestBase().setSubIds(netCapsSubIds).build();
     }
 
-    private NetworkRequest.Builder getExpectedRequestBase(Set<Integer> subIds) {
+    private NetworkRequest.Builder getExpectedRequestBase() {
         final NetworkRequest.Builder builder =
                 new NetworkRequest.Builder()
                         .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
                         .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED)
                         .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
-                        .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED)
+                        .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
-                        .setSubIds(subIds);
 
         return builder;
     }

tests/vcn/java/com/android/server/vcn/VcnTest.java

@@ -19,10 +19,12 @@ package com.android.server.vcn;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS;
+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_SAFE_MODE;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.argThat;
 import static org.mockito.Matchers.eq;
@@ -142,7 +144,7 @@ public class VcnTest {
         mTestLooper.dispatchAll();
     }
 
-    private void verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(boolean isActive) {
+    private void verifyUpdateSubscriptionSnapshotNotifiesGatewayConnections(int status) {
         final NetworkRequestListener requestListener = verifyAndGetRequestListener();
         startVcnGatewayWithCapabilities(requestListener, TEST_CAPS[0]);
 
@@ -152,25 +154,25 @@ public class VcnTest {
         final TelephonySubscriptionSnapshot updatedSnapshot =
                 mock(TelephonySubscriptionSnapshot.class);
 
-        mVcn.setIsActive(isActive);
+        mVcn.setStatus(status);
 
         mVcn.updateSubscriptionSnapshot(updatedSnapshot);
         mTestLooper.dispatchAll();
 
         for (final VcnGatewayConnection gateway : gatewayConnections) {
-            verify(gateway, isActive ? times(1) : never())
+            verify(gateway, status == VCN_STATUS_CODE_ACTIVE ? times(1) : never())
                     .updateSubscriptionSnapshot(eq(updatedSnapshot));
         }
     }
 
     @Test
     public void testSubscriptionSnapshotUpdatesVcnGatewayConnections() {
-        verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(true /* isActive */);
+        verifyUpdateSubscriptionSnapshotNotifiesGatewayConnections(VCN_STATUS_CODE_ACTIVE);
     }
 
     @Test
-    public void testSubscriptionSnapshotUpdatesVcnGatewayConnectionsWhileInactive() {
+    public void testSubscriptionSnapshotUpdatesVcnGatewayConnectionsInSafeMode() {
-        verifyUpdateSubscriptionSnapshotNotifiesConnectionGateways(false /* isActive */);
+        verifyUpdateSubscriptionSnapshotNotifiesGatewayConnections(VCN_STATUS_CODE_SAFE_MODE);
     }
 
     private void triggerVcnRequestListeners(NetworkRequestListener requestListener) {
@@ -201,7 +203,7 @@ public class VcnTest {
     private void verifySafeMode(
             NetworkRequestListener requestListener,
             Set<VcnGatewayConnection> expectedGatewaysTornDown) {
-        assertFalse(mVcn.isActive());
+        assertEquals(VCN_STATUS_CODE_SAFE_MODE, mVcn.getStatus());
         for (final VcnGatewayConnection gatewayConnection : expectedGatewaysTornDown) {
             verify(gatewayConnection).teardownAsynchronously();
         }
@@ -319,7 +321,7 @@ public class VcnTest {
 
         // Registered on start, then re-registered with new configs
         verify(mVcnNetworkProvider, times(2)).registerListener(eq(requestListener));
-        assertTrue(mVcn.isActive());
+        assertEquals(VCN_STATUS_CODE_ACTIVE, mVcn.getStatus());
         for (final int[] caps : TEST_CAPS) {
             // Expect each gateway connection created only on initial startup
             verify(mDeps)
@@ -334,7 +336,7 @@ public class VcnTest {
 
     @Test
     public void testIgnoreNetworkRequestWhileInactive() {
-        mVcn.setIsActive(false /* isActive */);
+        mVcn.setStatus(VCN_STATUS_CODE_INACTIVE);
 
         final NetworkRequestListener requestListener = verifyAndGetRequestListener();
         triggerVcnRequestListeners(requestListener);