From 348bbb02312f214649f6b4676801b031f0d60eb5 Mon Sep 17 00:00:00 2001
From: Remi NGUYEN VAN <reminv@google.com>
Date: Tue, 19 Jul 2022 16:33:04 +0900
Subject: [PATCH] Add back compat config for NSD

Add back compat config for RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS, which was
lost when moving NsdManager to framework-connectivity-t.

This causes NsdManager to start mdnsresponder again when used by apps
with target SDK < 31.

The change also changes the compat ID used, to make sure it does not
conflict with the ID already in use in S and below, when the module is
installed on such a platform. This is the only ChangeId used by
framework-t.

Also add a CtsNetTestCasesMaxTargetSdk30 test to verify that behavior.

Bug: 235355681
Test: atest CtsNetTestCasesMaxTargetSdk30
Change-Id: I7ca6051d0a4ba5aff3e44bece2cbac22eb1be32d
---
 TEST_MAPPING                                  | 22 ++++++++++++
 Tethering/apex/Android.bp                     |  5 ++-
 framework-t/Android.bp                        |  5 +++
 .../src/android/net/nsd/NsdManager.java       | 16 +++++----
 tests/cts/net/Android.bp                      | 15 ++++++++
 .../net/src/android/net/cts/NsdManagerTest.kt | 36 +++++++++++++++++++
 .../java/android/net/nsd/NsdManagerTest.java  | 20 +++++------
 .../com/android/server/NsdServiceTest.java    |  8 ++---
 8 files changed, 106 insertions(+), 21 deletions(-)

diff --git a/TEST_MAPPING b/TEST_MAPPING
index 4774866712..1e8babf8a6 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -35,6 +35,17 @@
       ]
     },
     // CTS tests that target older SDKs.
+    {
+      "name": "CtsNetTestCasesMaxTargetSdk30",
+      "options": [
+        {
+          "exclude-annotation": "com.android.testutils.SkipPresubmit"
+        },
+        {
+          "exclude-annotation": "androidx.test.filters.RequiresDevice"
+        }
+      ]
+    },
     {
       "name": "CtsNetTestCasesMaxTargetSdk31",
       "options": [
@@ -102,6 +113,17 @@
         }
       ]
     },
+    {
+      "name": "CtsNetTestCasesMaxTargetSdk30[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]",
+      "options": [
+        {
+          "exclude-annotation": "com.android.testutils.SkipPresubmit"
+        },
+        {
+          "exclude-annotation": "androidx.test.filters.RequiresDevice"
+        }
+      ]
+    },
     {
       "name": "CtsNetTestCasesMaxTargetSdk31[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]",
       "options": [
diff --git a/Tethering/apex/Android.bp b/Tethering/apex/Android.bp
index a7028b7c02..60b4e91de8 100644
--- a/Tethering/apex/Android.bp
+++ b/Tethering/apex/Android.bp
@@ -108,7 +108,10 @@ apex {
 
     androidManifest: "AndroidManifest.xml",
 
-    compat_configs: ["connectivity-platform-compat-config"],
+    compat_configs: [
+        "connectivity-platform-compat-config",
+        "connectivity-t-platform-compat-config",
+    ],
 }
 
 apex_key {
diff --git a/framework-t/Android.bp b/framework-t/Android.bp
index 80477f105a..c76416f21c 100644
--- a/framework-t/Android.bp
+++ b/framework-t/Android.bp
@@ -142,3 +142,8 @@ java_sdk_library {
         "//packages/modules/Wifi/service/tests/wifitests",
     ],
 }
+
+platform_compat_config {
+    name: "connectivity-t-platform-compat-config",
+    src: ":framework-connectivity-t",
+}
diff --git a/framework-t/src/android/net/nsd/NsdManager.java b/framework-t/src/android/net/nsd/NsdManager.java
index 3fcc11b6bc..fb3b1d67b1 100644
--- a/framework-t/src/android/net/nsd/NsdManager.java
+++ b/framework-t/src/android/net/nsd/NsdManager.java
@@ -139,17 +139,21 @@ public final class NsdManager {
      * The platform will only keep the daemon running as long as there are
      * any legacy apps connected.
      *
-     * After Android 12, directly communicate with native daemon might not
-     * work since the native damon won't always stay alive.
-     * Use the NSD APIs from NsdManager as the replacement is recommended.
-     * An another alternative could be bundling your own mdns solutions instead of
+     * After Android 12, direct communication with the native daemon might not work since the native
+     * daemon won't always stay alive. Using the NSD APIs from NsdManager as the replacement is
+     * recommended.
+     * Another alternative could be bundling your own mdns solutions instead of
      * depending on the system mdns native daemon.
      *
+     * This compatibility change applies to Android 13 and later only. To toggle behavior on
+     * Android 12 and Android 12L, use RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS.
+     *
      * @hide
      */
     @ChangeId
     @EnabledSince(targetSdkVersion = android.os.Build.VERSION_CODES.S)
-    public static final long RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS = 191844585L;
+    // This was a platform change ID with value 191844585L before T
+    public static final long RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER = 235355681L;
 
     /**
      * Broadcast intent action to indicate whether network service discovery is
@@ -500,7 +504,7 @@ public final class NsdManager {
 
         // Only proactively start the daemon if the target SDK < S, otherwise the internal service
         // would automatically start/stop the native daemon as needed.
-        if (!CompatChanges.isChangeEnabled(RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)) {
+        if (!CompatChanges.isChangeEnabled(RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)) {
             try {
                 mService.startDaemon();
             } catch (RemoteException e) {
diff --git a/tests/cts/net/Android.bp b/tests/cts/net/Android.bp
index 62f37bba2c..a6179fc60a 100644
--- a/tests/cts/net/Android.bp
+++ b/tests/cts/net/Android.bp
@@ -128,3 +128,18 @@ android_test {
     ],
 }
 
+android_test {
+    name: "CtsNetTestCasesMaxTargetSdk30",  // Must match CtsNetTestCasesMaxTargetSdk30 annotation.
+    defaults: [
+        "CtsNetTestCasesDefaults",
+        "CtsNetTestCasesApiStableDefaults",
+    ],
+    target_sdk_version: "30",
+    package_name: "android.net.cts.maxtargetsdk30",  // CTS package names must be unique.
+    instrumentation_target_package: "android.net.cts.maxtargetsdk30",
+    test_suites: [
+        "cts",
+        "general-tests",
+        "mts-networking",
+    ],
+}
diff --git a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
index 64cc97d265..a02be858ad 100644
--- a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
@@ -16,6 +16,7 @@
 package android.net.cts
 
 import android.Manifest.permission.MANAGE_TEST_NETWORKS
+import android.app.compat.CompatChanges
 import android.net.ConnectivityManager
 import android.net.ConnectivityManager.NetworkCallback
 import android.net.LinkProperties
@@ -46,6 +47,7 @@ import android.net.nsd.NsdManager.DiscoveryListener
 import android.net.nsd.NsdManager.RegistrationListener
 import android.net.nsd.NsdManager.ResolveListener
 import android.net.nsd.NsdServiceInfo
+import android.os.Build
 import android.os.Handler
 import android.os.HandlerThread
 import android.os.Process.myTid
@@ -56,17 +58,23 @@ import androidx.test.runner.AndroidJUnit4
 import com.android.net.module.util.ArrayTrackRecord
 import com.android.net.module.util.TrackRecord
 import com.android.networkstack.apishim.NsdShimImpl
+import com.android.testutils.ConnectivityModuleTest
+import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.TestableNetworkAgent
 import com.android.testutils.TestableNetworkCallback
+import com.android.testutils.filters.CtsNetTestCasesMaxTargetSdk30
 import com.android.testutils.runAsShell
 import com.android.testutils.tryTest
 import org.junit.After
 import org.junit.Assert.assertArrayEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Assume.assumeTrue
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import java.io.File
 import java.net.ServerSocket
 import java.nio.charset.StandardCharsets
 import java.util.Random
@@ -89,6 +97,10 @@ private val nsdShim = NsdShimImpl.newInstance()
 @AppModeFull(reason = "Socket cannot bind in instant app mode")
 @RunWith(AndroidJUnit4::class)
 class NsdManagerTest {
+    // Rule used to filter CtsNetTestCasesMaxTargetSdkXX
+    @get:Rule
+    val ignoreRule = DevSdkIgnoreRule()
+
     private val context by lazy { InstrumentationRegistry.getInstrumentation().context }
     private val nsdManager by lazy { context.getSystemService(NsdManager::class.java) }
 
@@ -692,6 +704,30 @@ class NsdManagerTest {
         }
     }
 
+    @Test @CtsNetTestCasesMaxTargetSdk30("Socket is started with the service up to target SDK 30")
+    fun testManagerCreatesLegacySocket() {
+        nsdManager // Ensure the lazy-init member is initialized, so NsdManager is created
+        val socket = File("/dev/socket/mdnsd")
+        val timeout = System.currentTimeMillis() + TIMEOUT_MS
+        while (!socket.exists() && System.currentTimeMillis() < timeout) {
+            Thread.sleep(10)
+        }
+        assertTrue("$socket was not found after $TIMEOUT_MS ms", socket.exists())
+    }
+
+    // The compat change is part of a connectivity module update that applies to T+
+    @ConnectivityModuleTest @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
+    @Test @CtsNetTestCasesMaxTargetSdk30("Socket is started with the service up to target SDK 30")
+    fun testManagerCreatesLegacySocket_CompatChange() {
+        // The socket may have been already created by some other app, or some other test, in which
+        // case this test cannot verify creation. At least verify that the compat change is
+        // disabled in a process with max SDK 30; unit tests already verify that start is requested
+        // when the compat change is disabled.
+        // Note that before T the compat constant had a different int value.
+        assertFalse(CompatChanges.isChangeEnabled(
+                NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER))
+    }
+
     /**
      * Register a service and return its registration record.
      */
diff --git a/tests/unit/java/android/net/nsd/NsdManagerTest.java b/tests/unit/java/android/net/nsd/NsdManagerTest.java
index 32274bc25e..e3dbb14432 100644
--- a/tests/unit/java/android/net/nsd/NsdManagerTest.java
+++ b/tests/unit/java/android/net/nsd/NsdManagerTest.java
@@ -81,70 +81,70 @@ public class NsdManagerTest {
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testResolveServiceS() throws Exception {
         verify(mServiceConn, never()).startDaemon();
         doTestResolveService();
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testResolveServicePreS() throws Exception {
         verify(mServiceConn).startDaemon();
         doTestResolveService();
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testDiscoverServiceS() throws Exception {
         verify(mServiceConn, never()).startDaemon();
         doTestDiscoverService();
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testDiscoverServicePreS() throws Exception {
         verify(mServiceConn).startDaemon();
         doTestDiscoverService();
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testParallelResolveServiceS() throws Exception {
         verify(mServiceConn, never()).startDaemon();
         doTestParallelResolveService();
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testParallelResolveServicePreS() throws Exception {
         verify(mServiceConn).startDaemon();
         doTestParallelResolveService();
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testInvalidCallsS() throws Exception {
         verify(mServiceConn, never()).startDaemon();
         doTestInvalidCalls();
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testInvalidCallsPreS() throws Exception {
         verify(mServiceConn).startDaemon();
         doTestInvalidCalls();
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testRegisterServiceS() throws Exception {
         verify(mServiceConn, never()).startDaemon();
         doTestRegisterService();
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testRegisterServicePreS() throws Exception {
         verify(mServiceConn).startDaemon();
         doTestRegisterService();
diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java
index 07884cfc56..181339305e 100644
--- a/tests/unit/java/com/android/server/NsdServiceTest.java
+++ b/tests/unit/java/com/android/server/NsdServiceTest.java
@@ -159,7 +159,7 @@ public class NsdServiceTest {
     }
 
     @Test
-    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @DisableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testPreSClients() throws Exception {
         // Pre S client connected, the daemon should be started.
         connectClient(mService);
@@ -186,7 +186,7 @@ public class NsdServiceTest {
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testNoDaemonStartedWhenClientsConnect() throws Exception {
         // Creating an NsdManager will not cause daemon startup.
         connectClient(mService);
@@ -220,7 +220,7 @@ public class NsdServiceTest {
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testClientRequestsAreGCedAtDisconnection() throws Exception {
         final NsdManager client = connectClient(mService);
         final INsdManagerCallback cb1 = getCallback();
@@ -263,7 +263,7 @@ public class NsdServiceTest {
     }
 
     @Test
-    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS)
+    @EnableCompatChanges(NsdManager.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER)
     public void testCleanupDelayNoRequestActive() throws Exception {
         final NsdManager client = connectClient(mService);
 
-- 
GitLab