Add getters to UnderlyingNetworkInfo

Address API review feedback, add getters to UnderlyingNetworkInfo
instead of exposing fields.

Instead of wasting memory by converting this into an array, have
migrateTun take a List<String>. In turn, tunAdjustmentInit should
also take a List<String>.

(cherry picked from ag/14211075)
Bug: 183972554
Test: atest android.net.UnderlyingNetworkInfoTest
Merged-In: Id59744097208d91298a25ef110ade91a9cf291a1
Change-Id: Id59744097208d91298a25ef110ade91a9cf291a1

8 files changed, 73 insertions, 51 deletions

diff --git a/core/api/module-lib-current.txt b/core/api/module-lib-current.txt
index 66ac74ac84d9..7069e7141a22 100644
--- a/core/api/module-lib-current.txt
+++ b/core/api/module-lib-current.txt
@@ -89,11 +89,11 @@ package android.net {
   public final class UnderlyingNetworkInfo implements android.os.Parcelable {
     ctor public UnderlyingNetworkInfo(int, @NonNull String, @NonNull java.util.List<java.lang.String>);
     method public int describeContents();
+    method @NonNull public String getIface();
+    method public int getOwnerUid();
+    method @NonNull public java.util.List<java.lang.String> getUnderlyingIfaces();
     method public void writeToParcel(@NonNull android.os.Parcel, int);
     field @NonNull public static final android.os.Parcelable.Creator<android.net.UnderlyingNetworkInfo> CREATOR;
-    field @NonNull public final String iface;
-    field public final int ownerUid;
-    field @NonNull public final java.util.List<java.lang.String> underlyingIfaces;
   }
 
   public class VpnManager {
diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java
index d42beae601ed..6ccbab7ed8ef 100644
--- a/core/java/android/net/NetworkStats.java
+++ b/core/java/android/net/NetworkStats.java
@@ -38,6 +38,7 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Predicate;
@@ -1423,11 +1424,11 @@ public final class NetworkStats implements Parcelable {
      * @hide
      */
     public void migrateTun(int tunUid, @NonNull String tunIface,
-            @NonNull String[] underlyingIfaces) {
+            @NonNull List<String> underlyingIfaces) {
         // Combined usage by all apps using VPN.
         final Entry tunIfaceTotal = new Entry();
         // Usage by VPN, grouped by its {@code underlyingIfaces}.
-        final Entry[] perInterfaceTotal = new Entry[underlyingIfaces.length];
+        final Entry[] perInterfaceTotal = new Entry[underlyingIfaces.size()];
         // Usage by VPN, summed across all its {@code underlyingIfaces}.
         final Entry underlyingIfacesTotal = new Entry();
 
@@ -1468,7 +1469,7 @@ public final class NetworkStats implements Parcelable {
      *     {@code underlyingIfaces}
      */
     private void tunAdjustmentInit(int tunUid, @NonNull String tunIface,
-            @NonNull String[] underlyingIfaces, @NonNull Entry tunIfaceTotal,
+            @NonNull List<String> underlyingIfaces, @NonNull Entry tunIfaceTotal,
             @NonNull Entry[] perInterfaceTotal, @NonNull Entry underlyingIfacesTotal) {
         final Entry recycle = new Entry();
         for (int i = 0; i < size; i++) {
@@ -1488,8 +1489,8 @@ public final class NetworkStats implements Parcelable {
 
             if (recycle.uid == tunUid) {
                 // Add up traffic through tunUid's underlying interfaces.
-                for (int j = 0; j < underlyingIfaces.length; j++) {
-                    if (Objects.equals(underlyingIfaces[j], recycle.iface)) {
+                for (int j = 0; j < underlyingIfaces.size(); j++) {
+                    if (Objects.equals(underlyingIfaces.get(j), recycle.iface)) {
                         perInterfaceTotal[j].add(recycle);
                         underlyingIfacesTotal.add(recycle);
                         break;
@@ -1515,12 +1516,12 @@ public final class NetworkStats implements Parcelable {
      *     underlyingIfaces}
      */
     private Entry[] addTrafficToApplications(int tunUid, @NonNull String tunIface,
-            @NonNull String[] underlyingIfaces, @NonNull Entry tunIfaceTotal,
+            @NonNull List<String> underlyingIfaces, @NonNull Entry tunIfaceTotal,
             @NonNull Entry[] perInterfaceTotal, @NonNull Entry underlyingIfacesTotal) {
         // Traffic that should be moved off of each underlying interface for tunUid (see
         // deductTrafficFromVpnApp below).
-        final Entry[] moved = new Entry[underlyingIfaces.length];
-        for (int i = 0; i < underlyingIfaces.length; i++) {
+        final Entry[] moved = new Entry[underlyingIfaces.size()];
+        for (int i = 0; i < underlyingIfaces.size(); i++) {
             moved[i] = new Entry();
         }
 
@@ -1582,8 +1583,8 @@ public final class NetworkStats implements Parcelable {
             }
             // In a second pass, distribute these values across interfaces in the proportion that
             // each interface represents of the total traffic of the underlying interfaces.
-            for (int j = 0; j < underlyingIfaces.length; j++) {
-                tmpEntry.iface = underlyingIfaces[j];
+            for (int j = 0; j < underlyingIfaces.size(); j++) {
+                tmpEntry.iface = underlyingIfaces.get(j);
                 tmpEntry.rxBytes = 0;
                 // Reset 'set' to correct value since it gets updated when adding debug info below.
                 tmpEntry.set = set[i];
@@ -1638,14 +1639,14 @@ public final class NetworkStats implements Parcelable {
 
     private void deductTrafficFromVpnApp(
             int tunUid,
-            @NonNull String[] underlyingIfaces,
+            @NonNull List<String> underlyingIfaces,
             @NonNull Entry[] moved) {
-        for (int i = 0; i < underlyingIfaces.length; i++) {
+        for (int i = 0; i < underlyingIfaces.size(); i++) {
             moved[i].uid = tunUid;
             // Add debug info
             moved[i].set = SET_DBG_VPN_OUT;
             moved[i].tag = TAG_NONE;
-            moved[i].iface = underlyingIfaces[i];
+            moved[i].iface = underlyingIfaces.get(i);
             moved[i].metered = METERED_ALL;
             moved[i].roaming = ROAMING_ALL;
             moved[i].defaultNetwork = DEFAULT_NETWORK_ALL;
@@ -1658,7 +1659,7 @@ public final class NetworkStats implements Parcelable {
             // METERED_NO, which should be the case as it comes directly from the /proc file.
             // We only blend in the roaming data after applying these adjustments, by checking the
             // NetworkIdentity of the underlying iface.
-            final int idxVpnBackground = findIndex(underlyingIfaces[i], tunUid, SET_DEFAULT,
+            final int idxVpnBackground = findIndex(underlyingIfaces.get(i), tunUid, SET_DEFAULT,
                             TAG_NONE, METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO);
             if (idxVpnBackground != -1) {
                 // Note - tunSubtract also updates moved[i]; whatever traffic that's left is removed
@@ -1666,7 +1667,7 @@ public final class NetworkStats implements Parcelable {
                 tunSubtract(idxVpnBackground, this, moved[i]);
             }
 
-            final int idxVpnForeground = findIndex(underlyingIfaces[i], tunUid, SET_FOREGROUND,
+            final int idxVpnForeground = findIndex(underlyingIfaces.get(i), tunUid, SET_FOREGROUND,
                             TAG_NONE, METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO);
             if (idxVpnForeground != -1) {
                 tunSubtract(idxVpnForeground, this, moved[i]);
diff --git a/core/java/android/net/UnderlyingNetworkInfo.java b/core/java/android/net/UnderlyingNetworkInfo.java
index 7bf923123910..459fdacef816 100644
--- a/core/java/android/net/UnderlyingNetworkInfo.java
+++ b/core/java/android/net/UnderlyingNetworkInfo.java
@@ -37,36 +37,56 @@ import java.util.Objects;
 @SystemApi(client = MODULE_LIBRARIES)
 public final class UnderlyingNetworkInfo implements Parcelable {
     /** The owner of this network. */
-    public final int ownerUid;
+    private final int mOwnerUid;
+
     /** The interface name of this network. */
     @NonNull
-    public final String iface;
+    private final String mIface;
+
     /** The names of the interfaces underlying this network. */
     @NonNull
-    public final List<String> underlyingIfaces;
+    private final List<String> mUnderlyingIfaces;
 
     public UnderlyingNetworkInfo(int ownerUid, @NonNull String iface,
             @NonNull List<String> underlyingIfaces) {
         Objects.requireNonNull(iface);
         Objects.requireNonNull(underlyingIfaces);
-        this.ownerUid = ownerUid;
-        this.iface = iface;
-        this.underlyingIfaces = Collections.unmodifiableList(new ArrayList<>(underlyingIfaces));
+        mOwnerUid = ownerUid;
+        mIface = iface;
+        mUnderlyingIfaces = Collections.unmodifiableList(new ArrayList<>(underlyingIfaces));
     }
 
     private UnderlyingNetworkInfo(@NonNull Parcel in) {
-        this.ownerUid = in.readInt();
-        this.iface = in.readString();
-        this.underlyingIfaces = new ArrayList<>();
-        in.readList(this.underlyingIfaces, null /*classLoader*/);
+        mOwnerUid = in.readInt();
+        mIface = in.readString();
+        List<String> underlyingIfaces = new ArrayList<>();
+        in.readList(underlyingIfaces, null /*classLoader*/);
+        mUnderlyingIfaces = Collections.unmodifiableList(underlyingIfaces);
+    }
+
+    /** Get the owner of this network. */
+    public int getOwnerUid() {
+        return mOwnerUid;
+    }
+
+    /** Get the interface name of this network. */
+    @NonNull
+    public String getIface() {
+        return mIface;
+    }
+
+    /** Get the names of the interfaces underlying this network. */
+    @NonNull
+    public List<String> getUnderlyingIfaces() {
+        return mUnderlyingIfaces;
     }
 
     @Override
     public String toString() {
         return "UnderlyingNetworkInfo{"
-                + "ownerUid=" + ownerUid
-                + ", iface='" + iface + '\''
-                + ", underlyingIfaces='" + underlyingIfaces.toString() + '\''
+                + "ownerUid=" + mOwnerUid
+                + ", iface='" + mIface + '\''
+                + ", underlyingIfaces='" + mUnderlyingIfaces.toString() + '\''
                 + '}';
     }
 
@@ -77,9 +97,9 @@ public final class UnderlyingNetworkInfo implements Parcelable {
 
     @Override
     public void writeToParcel(@NonNull Parcel dest, int flags) {
-        dest.writeInt(ownerUid);
-        dest.writeString(iface);
-        dest.writeList(underlyingIfaces);
+        dest.writeInt(mOwnerUid);
+        dest.writeString(mIface);
+        dest.writeList(mUnderlyingIfaces);
     }
 
     @NonNull
@@ -103,13 +123,13 @@ public final class UnderlyingNetworkInfo implements Parcelable {
         if (this == o) return true;
         if (!(o instanceof UnderlyingNetworkInfo)) return false;
         final UnderlyingNetworkInfo that = (UnderlyingNetworkInfo) o;
-        return ownerUid == that.ownerUid
-                && Objects.equals(iface, that.iface)
-                && Objects.equals(underlyingIfaces, that.underlyingIfaces);
+        return mOwnerUid == that.getOwnerUid()
+                && Objects.equals(mIface, that.getIface())
+                && Objects.equals(mUnderlyingIfaces, that.getUnderlyingIfaces());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(ownerUid, iface, underlyingIfaces);
+        return Objects.hash(mOwnerUid, mIface, mUnderlyingIfaces);
     }
 }
diff --git a/core/tests/benchmarks/src/android/net/NetworkStatsBenchmark.java b/core/tests/benchmarks/src/android/net/NetworkStatsBenchmark.java
index 3ebe1039d975..02998325708b 100644
--- a/core/tests/benchmarks/src/android/net/NetworkStatsBenchmark.java
+++ b/core/tests/benchmarks/src/android/net/NetworkStatsBenchmark.java
@@ -91,7 +91,7 @@ public class NetworkStatsBenchmark {
     public void timeMigrateTun(int reps) {
         for (int i = 0; i < reps; i++) {
             NetworkStats stats = mNetworkStats.clone();
-            stats.migrateTun(TUN_UID, TUN_IFACE, getVpnUnderlyingIfaces());
+            stats.migrateTun(TUN_UID, TUN_IFACE, Arrays.asList(getVpnUnderlyingIfaces()));
         }
     }
 
diff --git a/services/core/java/com/android/server/VpnManagerService.java b/services/core/java/com/android/server/VpnManagerService.java
index d756c1ffd00f..26ecee8f21ab 100644
--- a/services/core/java/com/android/server/VpnManagerService.java
+++ b/services/core/java/com/android/server/VpnManagerService.java
@@ -605,7 +605,7 @@ public class VpnManagerService extends IVpnManager.Stub {
             return null;
         } else {
             final UnderlyingNetworkInfo info = vpn.getUnderlyingNetworkInfo();
-            return (info == null || info.ownerUid != uid) ? null : vpn;
+            return (info == null || info.getOwnerUid() != uid) ? null : vpn;
         }
     }
 
diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java
index d042b882fee1..e7c0a50163da 100644
--- a/services/core/java/com/android/server/net/NetworkStatsFactory.java
+++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java
@@ -382,8 +382,8 @@ public class NetworkStatsFactory {
 
         // Migrate data usage over a VPN to the TUN network.
         for (UnderlyingNetworkInfo info : vpnArray) {
-            delta.migrateTun(info.ownerUid, info.iface,
-                    info.underlyingIfaces.toArray(new String[0]));
+            delta.migrateTun(info.getOwnerUid(), info.getIface(),
+                    info.getUnderlyingIfaces());
             // Filter out debug entries as that may lead to over counting.
             delta.filterDebugEntries();
         }
diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java
index 735fa7cf3751..23d5a7e5d5f8 100644
--- a/tests/net/java/android/net/NetworkStatsTest.java
+++ b/tests/net/java/android/net/NetworkStatsTest.java
@@ -50,6 +50,7 @@ import com.google.android.collect.Sets;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.Arrays;
 import java.util.HashSet;
 
 @RunWith(AndroidJUnit4.class)
@@ -616,7 +617,7 @@ public class NetworkStatsTest {
                 .insertEntry(underlyingIface, tunUid, SET_FOREGROUND, TAG_NONE, METERED_NO,
                         ROAMING_NO, DEFAULT_NETWORK_NO, 0L, 0L, 0L, 0L, 0L);
 
-        delta.migrateTun(tunUid, tunIface, new String[]{underlyingIface});
+        delta.migrateTun(tunUid, tunIface, Arrays.asList(underlyingIface));
         assertEquals(20, delta.size());
 
         // tunIface and TEST_IFACE entries are not changed.
@@ -697,7 +698,7 @@ public class NetworkStatsTest {
                 .insertEntry(underlyingIface, tunUid, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
                         DEFAULT_NETWORK_NO, 75500L, 37L, 130000L, 70L, 0L);
 
-        delta.migrateTun(tunUid, tunIface, new String[]{underlyingIface});
+        delta.migrateTun(tunUid, tunIface, Arrays.asList(underlyingIface));
         assertEquals(9, delta.size());
 
         // tunIface entries should not be changed.
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 9c797e403bad..0042c82d147e 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -5818,10 +5818,10 @@ public class ConnectivityServiceTest {
         if (vpnUid != null) {
             assertEquals("Should have exactly one VPN:", 1, infos.length);
             UnderlyingNetworkInfo info = infos[0];
-            assertEquals("Unexpected VPN owner:", (int) vpnUid, info.ownerUid);
-            assertEquals("Unexpected VPN interface:", vpnIfname, info.iface);
+            assertEquals("Unexpected VPN owner:", (int) vpnUid, info.getOwnerUid());
+            assertEquals("Unexpected VPN interface:", vpnIfname, info.getIface());
             assertSameElementsNoDuplicates(underlyingIfaces,
-                    info.underlyingIfaces.toArray(new String[0]));
+                    info.getUnderlyingIfaces().toArray(new String[0]));
         } else {
             assertEquals(0, infos.length);
             return;
@@ -5965,8 +5965,8 @@ public class ConnectivityServiceTest {
         // network for the VPN...
         verify(mStatsManager, never()).notifyNetworkStatus(any(List.class),
                 any(List.class), any() /* anyString() doesn't match null */,
-                argThat(infos -> infos.get(0).underlyingIfaces.size() == 1
-                        && WIFI_IFNAME.equals(infos.get(0).underlyingIfaces.get(0))));
+                argThat(infos -> infos.get(0).getUnderlyingIfaces().size() == 1
+                        && WIFI_IFNAME.equals(infos.get(0).getUnderlyingIfaces().get(0))));
         verifyNoMoreInteractions(mStatsManager);
         reset(mStatsManager);
 
@@ -5979,8 +5979,8 @@ public class ConnectivityServiceTest {
         waitForIdle();
         verify(mStatsManager).notifyNetworkStatus(any(List.class),
                 any(List.class), any() /* anyString() doesn't match null */,
-                argThat(vpnInfos -> vpnInfos.get(0).underlyingIfaces.size() == 1
-                        && WIFI_IFNAME.equals(vpnInfos.get(0).underlyingIfaces.get(0))));
+                argThat(vpnInfos -> vpnInfos.get(0).getUnderlyingIfaces().size() == 1
+                        && WIFI_IFNAME.equals(vpnInfos.get(0).getUnderlyingIfaces().get(0))));
         mEthernetNetworkAgent.disconnect();
         waitForIdle();
         reset(mStatsManager);