From b3ab0d1e63fbbad007bb2a7da2fd4e1206d86c40 Mon Sep 17 00:00:00 2001
From: Chalard Jean <jchalard@google.com>
Date: Mon, 21 May 2018 15:30:56 +0900
Subject: [PATCH] Unify behavior of various cases of "no underlying networks"

Before this change, VPNs having no underlying networks would be
marked as metered as the safe option, but VPNs having only
disconnected underlying networks would be marked as unmetered.
Fix this discrepancy.

Bug: 79748782
Test: runtest frameworks-net
Change-Id: I51c3badde29f43f692f383553bd98327d2da8dd1
---
 .../java/com/android/server/connectivity/Vpn.java | 15 +++++++++------
 .../android/server/ConnectivityServiceTest.java   |  6 ++----
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index dd82950fe0205..a919898385213 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -319,15 +319,12 @@ public class Vpn {
         boolean roaming = false;
         boolean congested = false;
 
-        if (ArrayUtils.isEmpty(underlyingNetworks)) {
-            // No idea what the underlying networks are; assume sane defaults
-            metered = true;
-            roaming = false;
-            congested = false;
-        } else {
+        boolean hadUnderlyingNetworks = false;
+        if (null != underlyingNetworks) {
             for (Network underlying : underlyingNetworks) {
                 final NetworkCapabilities underlyingCaps = cm.getNetworkCapabilities(underlying);
                 if (underlyingCaps == null) continue;
+                hadUnderlyingNetworks = true;
                 for (int underlyingType : underlyingCaps.getTransportTypes()) {
                     transportTypes = ArrayUtils.appendInt(transportTypes, underlyingType);
                 }
@@ -343,6 +340,12 @@ public class Vpn {
                 congested |= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_CONGESTED);
             }
         }
+        if (!hadUnderlyingNetworks) {
+            // No idea what the underlying networks are; assume sane defaults
+            metered = true;
+            roaming = false;
+            congested = false;
+        }
 
         caps.setTransportTypes(transportTypes);
         caps.setLinkDownstreamBandwidthKbps(downKbps);
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index cf7ab4294b986..220858081e934 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -4413,13 +4413,11 @@ public class ConnectivityServiceTest {
                 && 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.
+        // Disconnect wifi too. No underlying networks means this is now metered.
         mWiFiNetworkAgent.disconnect();
         vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
                 && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
-                && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
+                && !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
                 vpnNetworkAgent);
 
         mMockVpn.disconnect();