Fix: VPNs update caps upon underlying network disconnect.

Clean cherry-pick of ag/4083954

Bug: 79748782
Test: ConnectivityServiceTests still pass
Change-Id: I21e866c723099e5c3dee54ff13e830d44427fc7a
Merged-In: I12c948ebeb2b74290908f8320ff77220dc4a9fb9

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index ff30148..fe0f6d2 100644 (file)
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -2465,6 +2465,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 ensureNetworkTransitionWakelock(nai.name());
             }
             mLegacyTypeTracker.remove(nai, wasDefault);
+            if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) {
+                updateAllVpnsCapabilities();
+            }
             rematchAllNetworksAndRequests(null, 0);
             mLingerMonitor.noteDisconnect(nai);
             if (nai.created) {
@@ -3734,6 +3737,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    /**
+     * Ask all VPN objects to recompute and update their capabilities.
+     *
+     * When underlying networks change, VPNs may have to update capabilities to reflect things
+     * like the metered bit, their transports, and so on. This asks the VPN objects to update
+     * their capabilities, and as this will cause them to send messages to the ConnectivityService
+     * handler thread through their agent, this is asynchronous. When the capabilities objects
+     * are computed they will be up-to-date as they are computed synchronously from here and
+     * this is running on the ConnectivityService thread.
+     * TODO : Fix this and call updateCapabilities inline to remove out-of-order events.
+     */
+    private void updateAllVpnsCapabilities() {
+        synchronized (mVpns) {
+            for (int i = 0; i < mVpns.size(); i++) {
+                final Vpn vpn = mVpns.valueAt(i);
+                vpn.updateCapabilities();
+            }
+        }
+    }
+
     @Override
     public boolean updateLockdownVpn() {
         if (Binder.getCallingUid() != Process.SYSTEM_UID) {
@@ -4531,7 +4554,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
     // Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated.
     private final NetworkRequest mDefaultRequest;
- 
+
     // Request used to optionally keep mobile data active even when higher
     // priority networks like Wi-Fi are active.
     private final NetworkRequest mDefaultMobileDataRequest;
@@ -4891,12 +4914,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         if (!newNc.hasTransport(TRANSPORT_VPN)) {
             // Tell VPNs about updated capabilities, since they may need to
             // bubble those changes through.
-            synchronized (mVpns) {
-                for (int i = 0; i < mVpns.size(); i++) {
-                    final Vpn vpn = mVpns.valueAt(i);
-                    vpn.updateCapabilities();
-                }
-            }
+            updateAllVpnsCapabilities();
         }
     }
 

diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 572cb52..fef5417 100644 (file)
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -4401,24 +4401,22 @@ public class ConnectivityServiceTest {
                 && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                 vpnNetworkAgent);
 
-        if (false) { // TODO : reactivate this ; in the current state, the below fail.
-            // Disconnect cell. Receive update without even removing the dead network from the
-            // underlying networks – it's dead anyway. Not metered any more.
-            mCellNetworkAgent.disconnect();
-            vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
-                    && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
-                    && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
-                    vpnNetworkAgent);
-
-            // Disconnect wifi too. No underlying networks should mean this is now metered,
-            // unfortunately a discrepancy in the current implementation has this unmetered.
-            // TODO : fix this.
-            mWiFiNetworkAgent.disconnect();
-            vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
-                    && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
-                    && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
-                    vpnNetworkAgent);
-        }
+        // Disconnect cell. Receive update without even removing the dead network from the
+        // underlying networks – it's dead anyway. Not metered any more.
+        mCellNetworkAgent.disconnect();
+        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
+                && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
+                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
+                vpnNetworkAgent);
+
+        // Disconnect wifi too. No underlying networks should mean this is now metered,
+        // unfortunately a discrepancy in the current implementation has this unmetered.
+        // TODO : fix this.
+        mWiFiNetworkAgent.disconnect();
+        vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
+                && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
+                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
+                vpnNetworkAgent);
 
         mMockVpn.disconnect();
     }