Deduplicate items after clear interface in NetworkStats

Follow-up from commit Ie60829a65d0d9b5b63ad353695a820c0586e3665,
the interface field was cleared before returning the result to
the caller. However, this can cause problems in the
NetworkStats#subtract method. If the interface field is cleared,
the findIndexHinted method can match to a wrong entry.
This is because the keys of multiple entries will now be the
same. This can cause the subtract result to be unexpectedly
large and the return value of getUidStatsForTransport to be
mismatched with the values retrieved from other APIs.

Test: atest FrameworksNetTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest \
      FrameworksNetTests:android.net.connectivity.android.net.NetworkStatsTest
Bug: 290728278
Change-Id: I891ab29b8a2902663febc7c32b04417caf510926

framework-t/src/android/net/NetworkStats.java

@@ -1147,7 +1147,8 @@ public final class NetworkStats implements Parcelable, Iterable<NetworkStats.Ent
             entry.txPackets = left.txPackets[i];
             entry.operations = left.operations[i];
 
-            // find remote row that matches, and subtract
+            // Find the remote row that matches and subtract.
+            // The returned row must be uniquely matched.
             final int j = right.findIndexHinted(entry.iface, entry.uid, entry.set, entry.tag,
                     entry.metered, entry.roaming, entry.defaultNetwork, i);
             if (j != -1) {
@@ -1304,13 +1305,14 @@ public final class NetworkStats implements Parcelable, Iterable<NetworkStats.Ent
 
     /**
      * Removes the interface name from all entries.
-     * This mutates the original structure in place.
+     * This returns a newly constructed object instead of mutating the original structure.
      * @hide
      */
-    public void clearInterfaces() {
+    @NonNull
-        for (int i = 0; i < size; i++) {
+    public NetworkStats clearInterfaces() {
-            iface[i] = null;
+        final Entry temp = new Entry();
-        }
+        return map(entry -> temp.setKeys(IFACE_ALL, entry.getUid(), entry.getSet(),
+                entry.getTag(), entry.getMetered(), entry.getRoaming(), entry.getDefaultNetwork()));
     }
 
     /**

service-t/src/com/android/server/net/NetworkStatsService.java

@@ -1744,8 +1744,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
             // information. This is because no caller needs this information for now, and it
             // makes it easier to change the implementation later by using the histories in the
             // recorder.
-            stats.clearInterfaces();
+            return stats.clearInterfaces();
-            return stats;
         } catch (RemoteException e) {
             Log.wtf(TAG, "Error compiling UID stats", e);
             return new NetworkStats(0L, 0);

tests/unit/java/android/net/NetworkStatsTest.java

@@ -1073,30 +1073,35 @@ public class NetworkStatsTest {
         final NetworkStats.Entry entry1 = new NetworkStats.Entry(
                 "test1", 10100, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
                 DEFAULT_NETWORK_NO, 1024L, 50L, 100L, 20L, 0L);
-
         final NetworkStats.Entry entry2 = new NetworkStats.Entry(
                 "test2", 10101, SET_DEFAULT, 0xF0DD, METERED_NO, ROAMING_NO,
                 DEFAULT_NETWORK_NO, 51200, 25L, 101010L, 50L, 0L);
+        final NetworkStats.Entry entry3 = new NetworkStats.Entry(
+                "test3", 10101, SET_DEFAULT, 0xF0DD, METERED_NO, ROAMING_NO,
+                DEFAULT_NETWORK_NO, 1, 2L, 3L, 4L, 5L);
 
         stats.insertEntry(entry1);
         stats.insertEntry(entry2);
+        stats.insertEntry(entry3);
 
         // Verify that the interfaces have indeed been recorded.
-        assertEquals(2, stats.size());
+        assertEquals(3, stats.size());
         assertValues(stats, 0, "test1", 10100, SET_DEFAULT, TAG_NONE, METERED_NO,
                 ROAMING_NO, DEFAULT_NETWORK_NO, 1024L, 50L, 100L, 20L, 0L);
         assertValues(stats, 1, "test2", 10101, SET_DEFAULT, 0xF0DD, METERED_NO,
                 ROAMING_NO, DEFAULT_NETWORK_NO, 51200, 25L, 101010L, 50L, 0L);
+        assertValues(stats, 2, "test3", 10101, SET_DEFAULT, 0xF0DD, METERED_NO,
+                ROAMING_NO, DEFAULT_NETWORK_NO, 1, 2L, 3L, 4L, 5L);
 
         // Clear interfaces.
-        stats.clearInterfaces();
+        final NetworkStats ifaceClearedStats = stats.clearInterfaces();
 
-        // Verify that the interfaces are cleared.
+        // Verify that the interfaces are cleared, and key-duplicated items are merged.
-        assertEquals(2, stats.size());
+        assertEquals(2, ifaceClearedStats.size());
-        assertValues(stats, 0, null /* iface */, 10100, SET_DEFAULT, TAG_NONE, METERED_NO,
+        assertValues(ifaceClearedStats, 0, null /* iface */, 10100, SET_DEFAULT, TAG_NONE,
-                ROAMING_NO, DEFAULT_NETWORK_NO, 1024L, 50L, 100L, 20L, 0L);
+                METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 1024L, 50L, 100L, 20L, 0L);
-        assertValues(stats, 1, null /* iface */, 10101, SET_DEFAULT, 0xF0DD, METERED_NO,
+        assertValues(ifaceClearedStats, 1, null /* iface */, 10101, SET_DEFAULT, 0xF0DD,
-                ROAMING_NO, DEFAULT_NETWORK_NO, 51200, 25L, 101010L, 50L, 0L);
+                METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, 51201, 27L, 101013L, 54L, 5L);
     }
 
     private static void assertContains(NetworkStats stats,  String iface, int uid, int set,

tests/unit/java/com/android/server/net/NetworkStatsBaseTest.java

@@ -41,6 +41,7 @@ import java.util.Arrays;
 abstract class NetworkStatsBaseTest {
     static final String TEST_IFACE = "test0";
     static final String TEST_IFACE2 = "test1";
+    static final String TEST_IFACE3 = "test2";
     static final String TUN_IFACE = "test_nss_tun0";
     static final String TUN_IFACE2 = "test_nss_tun1";
 

tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java

@@ -1250,8 +1250,9 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
                 TEST_IFACE2, IMSI_1, null /* wifiNetworkKey */,
                 false /* isTemporarilyNotMetered */, false /* isRoaming */);
 
-        final NetworkStateSnapshot[] states = new NetworkStateSnapshot[] {
+        final NetworkStateSnapshot[] states = new NetworkStateSnapshot[]{
-                mobileState, buildWifiState()};
+                mobileState, buildWifiState(false, TEST_IFACE, null),
+                buildWifiState(false, TEST_IFACE3, null)};
         mService.notifyNetworkStatus(NETWORKS_MOBILE, states, getActiveIface(states),
                 new UnderlyingNetworkInfo[0]);
         setMobileRatTypeAndWaitForIdle(TelephonyManager.NETWORK_TYPE_LTE);
@@ -1266,16 +1267,22 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
         final NetworkStats.Entry entry3 = new NetworkStats.Entry(
                 TEST_IFACE, UID_BLUE, SET_DEFAULT, 0xBEEF, METERED_NO, ROAMING_NO,
                 DEFAULT_NETWORK_NO, 1024L, 8L, 512L, 4L, 2L);
+        // Add an entry that with different wifi interface, but expected to be merged into entry3
+        // after clearing interface information.
+        final NetworkStats.Entry entry4 = new NetworkStats.Entry(
+                TEST_IFACE3, UID_BLUE, SET_DEFAULT, 0xBEEF, METERED_NO, ROAMING_NO,
+                DEFAULT_NETWORK_NO, 1L, 2L, 3L, 4L, 5L);
 
         final TetherStatsParcel[] emptyTetherStats = {};
         // The interfaces that expect to be used to query the stats.
-        final String[] wifiIfaces = {TEST_IFACE};
+        final String[] wifiIfaces = {TEST_IFACE, TEST_IFACE3};
         incrementCurrentTime(HOUR_IN_MILLIS);
         mockDefaultSettings();
-        mockNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3)
+        mockNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 4)
                 .insertEntry(entry1)
                 .insertEntry(entry2)
-                .insertEntry(entry3), emptyTetherStats, wifiIfaces);
+                .insertEntry(entry3)
+                .insertEntry(entry4), emptyTetherStats, wifiIfaces);
 
         // getUidStatsForTransport (through getNetworkStatsUidDetail) adds all operation counts
         // with active interface, and the interface here is mobile interface, so this test makes
@@ -1293,7 +1300,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
         assertValues(wifiStats, null /* iface */, UID_RED, SET_DEFAULT, 0xF00D,
                 METERED_NO, ROAMING_NO, METERED_NO, 50L, 5L, 50L, 5L, 1L);
         assertValues(wifiStats, null /* iface */, UID_BLUE, SET_DEFAULT, 0xBEEF,
-                METERED_NO, ROAMING_NO, METERED_NO, 1024L, 8L, 512L, 4L, 2L);
+                METERED_NO, ROAMING_NO, METERED_NO, 1025L, 10L, 515L, 8L, 7L);
 
         final String[] mobileIfaces = {TEST_IFACE2};
         mockNetworkStatsUidDetail(buildEmptyStats(), emptyTetherStats, mobileIfaces);