Merge "Fix network callback with the same PendingIntent does not release"

service/src/com/android/server/ConnectivityService.java

@@ -6280,7 +6280,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 callingAttributionTag);
         if (VDBG) log("pendingListenForNetwork for " + nri);
 
-        mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_LISTENER, nri));
+        mHandler.sendMessage(mHandler.obtainMessage(
+                    EVENT_REGISTER_NETWORK_LISTENER_WITH_INTENT, nri));
     }
 
     /** Returns the next Network provider ID. */

tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java

@@ -72,6 +72,7 @@ import static android.system.OsConstants.AF_UNSPEC;
 import static com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity;
 import static com.android.compatibility.common.util.SystemUtil.runShellCommand;
 import static com.android.compatibility.common.util.SystemUtil.runWithShellPermissionIdentity;
+import static com.android.modules.utils.build.SdkLevel.isAtLeastS;
 import static com.android.networkstack.apishim.ConstantsShim.BLOCKED_REASON_LOCKDOWN_VPN;
 import static com.android.networkstack.apishim.ConstantsShim.BLOCKED_REASON_NONE;
 import static com.android.testutils.MiscAsserts.assertThrows;
@@ -936,8 +937,8 @@ public class ConnectivityManagerTest {
             // noticeably flaky.
             Thread.sleep(NO_CALLBACK_TIMEOUT_MS);
 
-            // TODO: BUG (b/189868426): this should also apply to listens
+            // For R- frameworks, listens will receive duplicated callbacks. See b/189868426.
-            if (!useListen) {
+            if (isAtLeastS() || !useListen) {
                 assertEquals("PendingIntent should only be received once", 1, receivedCount.get());
             }
         } finally {
@@ -952,8 +953,8 @@ public class ConnectivityManagerTest {
             boolean useListen) {
         assertArrayEquals(filed.networkCapabilities.getCapabilities(),
                 broadcasted.networkCapabilities.getCapabilities());
-        // TODO: BUG (b/189868426): this should also apply to listens
+        // For R- frameworks, listens will receive duplicated callbacks. See b/189868426.
-        if (useListen) return;
+        if (!isAtLeastS() && useListen) return;
         assertArrayEquals(filed.networkCapabilities.getTransportTypes(),
                 broadcasted.networkCapabilities.getTransportTypes());
     }

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -5747,38 +5747,60 @@ public class ConnectivityServiceTest {
     @Test
     public void testNetworkCallbackMaximum() throws Exception {
         final int MAX_REQUESTS = 100;
-        final int CALLBACKS = 89;
+        final int CALLBACKS = 87;
-        final int INTENTS = 11;
+        final int DIFF_INTENTS = 10;
+        final int SAME_INTENTS = 10;
         final int SYSTEM_ONLY_MAX_REQUESTS = 250;
-        assertEquals(MAX_REQUESTS, CALLBACKS + INTENTS);
+        // Assert 1 (Default request filed before testing) + CALLBACKS + DIFF_INTENTS +
+        // 1 (same intent) = MAX_REQUESTS - 1, since the capacity is MAX_REQUEST - 1.
+        assertEquals(MAX_REQUESTS - 1, 1 + CALLBACKS + DIFF_INTENTS + 1);
 
         NetworkRequest networkRequest = new NetworkRequest.Builder().build();
         ArrayList<Object> registered = new ArrayList<>();
 
-        int j = 0;
+        for (int j = 0; j < CALLBACKS; j++) {
-        while (j++ < CALLBACKS / 2) {
+            final NetworkCallback cb = new NetworkCallback();
-            NetworkCallback cb = new NetworkCallback();
+            if (j < CALLBACKS / 2) {
                 mCm.requestNetwork(networkRequest, cb);
-            registered.add(cb);
+            } else {
-        }
-        while (j++ < CALLBACKS) {
-            NetworkCallback cb = new NetworkCallback();
                 mCm.registerNetworkCallback(networkRequest, cb);
+            }
             registered.add(cb);
         }
-        j = 0;
+
-        while (j++ < INTENTS / 2) {
+        // Since ConnectivityService will de-duplicate the request with the same intent,
+        // register multiple times does not really increase multiple requests.
+        final PendingIntent same_pi = PendingIntent.getBroadcast(mContext, 0 /* requestCode */,
+                new Intent("same"), FLAG_IMMUTABLE);
+        for (int j = 0; j < SAME_INTENTS; j++) {
+            mCm.registerNetworkCallback(networkRequest, same_pi);
+            // Wait for the requests with the same intent to be de-duplicated. Because
+            // ConnectivityService side incrementCountOrThrow in binder, decrementCount in handler
+            // thread, waitForIdle is needed to ensure decrementCount being invoked for same intent
+            // requests before doing further tests.
+            waitForIdle();
+        }
+        for (int j = 0; j < SAME_INTENTS; j++) {
+            mCm.requestNetwork(networkRequest, same_pi);
+            // Wait for the requests with the same intent to be de-duplicated.
+            // Refer to the reason above.
+            waitForIdle();
+        }
+        registered.add(same_pi);
+
+        for (int j = 0; j < DIFF_INTENTS; j++) {
+            if (j < DIFF_INTENTS / 2) {
                 final PendingIntent pi = PendingIntent.getBroadcast(mContext, 0 /* requestCode */,
                         new Intent("a" + j), FLAG_IMMUTABLE);
                 mCm.requestNetwork(networkRequest, pi);
                 registered.add(pi);
-        }
+            } else {
-        while (j++ < INTENTS) {
                 final PendingIntent pi = PendingIntent.getBroadcast(mContext, 0 /* requestCode */,
                         new Intent("b" + j), FLAG_IMMUTABLE);
                 mCm.registerNetworkCallback(networkRequest, pi);
                 registered.add(pi);
             }
+        }
 
         // Test that the limit is enforced when MAX_REQUESTS simultaneous requests are added.
         assertThrows(TooManyRequestsException.class, () ->