From 258ea3c3c33b21b0a3e23170e3edf0a1e49c3005 Mon Sep 17 00:00:00 2001
From: James Mattis <jmattis@google.com>
Date: Sun, 15 Nov 2020 15:04:40 -0800
Subject: [PATCH] Update requestsSortedById() to sort by collection

Update requestsSortedById() to sort NetworkRequestInfo by their
nested collection of NetworkRequest objects vs a single request.
Before the NetworkRequestInfo with the request with the lowest
requestId would be sorted to the top. Now the NetworkRequestInfo
which contains the request with the lowest requestId will be
sorted to the top.

Bug: 173292541
Bug: 171991028
Test: atest FrameworksNetTests
Change-Id: I51e3c00d59443e37ddbf168c423d13df8d14fa64
---
 .../android/server/ConnectivityService.java   | 13 +++++--
 .../server/ConnectivityServiceTest.java       | 38 +++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index ab11ff86e0..4480479979 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -2697,10 +2697,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
     /**
      * Return an array of all current NetworkRequest sorted by request id.
      */
-    private NetworkRequestInfo[] requestsSortedById() {
+    @VisibleForTesting
+    protected NetworkRequestInfo[] requestsSortedById() {
         NetworkRequestInfo[] requests = new NetworkRequestInfo[0];
         requests = mNetworkRequests.values().toArray(requests);
-        Arrays.sort(requests, Comparator.comparingInt(nri -> nri.request.requestId));
+        // Sort the array based off the NRI containing the min requestId in its requests.
+        Arrays.sort(requests,
+                Comparator.comparingInt(nri -> Collections.min(nri.mRequests,
+                        Comparator.comparingInt(req -> req.requestId)).requestId
+                )
+        );
         return requests;
     }
 
@@ -5367,7 +5373,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
      * Tracks info about the requester.
      * Also used to notice when the calling process dies so we can self-expire
      */
-    private class NetworkRequestInfo implements IBinder.DeathRecipient {
+    @VisibleForTesting
+    protected class NetworkRequestInfo implements IBinder.DeathRecipient {
         final List<NetworkRequest> mRequests;
         final NetworkRequest request;
 
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 6293befb2c..408dd0a835 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -259,7 +259,10 @@ import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
 import org.mockito.stubbing.Answer;
 
+import java.io.FileDescriptor;
 import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.net.DatagramSocket;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
@@ -7585,4 +7588,39 @@ public class ConnectivityServiceTest {
 
         mCm.unregisterNetworkCallback(networkCallback);
     }
+
+    @Test
+    public void testDumpDoesNotCrash() {
+        StringWriter stringWriter = new StringWriter();
+
+        mService.dump(new FileDescriptor(), new PrintWriter(stringWriter), new String[0]);
+
+        assertFalse(stringWriter.toString().isEmpty());
+    }
+
+    @Test
+    public void testRequestsSortedByIdSortsCorrectly() {
+        final TestNetworkCallback genericNetworkCallback = new TestNetworkCallback();
+        final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback();
+        final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback();
+        final NetworkRequest genericRequest = new NetworkRequest.Builder()
+                .clearCapabilities().build();
+        final NetworkRequest wifiRequest = new NetworkRequest.Builder()
+                .addTransportType(TRANSPORT_WIFI).build();
+        final NetworkRequest cellRequest = new NetworkRequest.Builder()
+                .addTransportType(TRANSPORT_CELLULAR).build();
+        mCm.registerNetworkCallback(genericRequest, genericNetworkCallback);
+        mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback);
+        mCm.registerNetworkCallback(cellRequest, cellNetworkCallback);
+
+        ConnectivityService.NetworkRequestInfo[] nriOutput = mService.requestsSortedById();
+
+        assertTrue(nriOutput.length > 1);
+        for (int i = 0; i < nriOutput.length - 1; i++) {
+            boolean isRequestIdInOrder =
+                    nriOutput[i].mRequests.get(0).requestId
+                            < nriOutput[i + 1].mRequests.get(0).requestId;
+            assertTrue(isRequestIdInOrder);
+        }
+    }
 }