Merge "Remove unnecessary arguments from isTetheringFeatureEnabled" into main

Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java

@@ -22,9 +22,7 @@ import static android.net.ConnectivityManager.TYPE_MOBILE;
 import static android.net.ConnectivityManager.TYPE_MOBILE_DUN;
 import static android.net.ConnectivityManager.TYPE_MOBILE_HIPRI;
 import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
-import static android.provider.DeviceConfig.NAMESPACE_TETHERING;
 
-import static com.android.net.module.util.DeviceConfigUtils.TETHERING_MODULE_NAME;
 import static com.android.networkstack.apishim.ConstantsShim.KEY_CARRIER_SUPPORTS_TETHERING_BOOL;
 
 import android.content.ContentResolver;
@@ -179,16 +177,30 @@ public class TetheringConfiguration {
      */
     @VisibleForTesting
     public static class Dependencies {
-        boolean isFeatureEnabled(@NonNull Context context, @NonNull String namespace,
+        boolean isFeatureEnabled(@NonNull Context context, @NonNull String name) {
-                @NonNull String name, @NonNull String moduleName, boolean defaultEnabled) {
+            return DeviceConfigUtils.isTetheringFeatureEnabled(context, name);
-            return DeviceConfigUtils.isTetheringFeatureEnabled(context, namespace, name,
-                    moduleName, defaultEnabled);
         }
 
         boolean getDeviceConfigBoolean(@NonNull String namespace, @NonNull String name,
                 boolean defaultValue) {
             return DeviceConfig.getBoolean(namespace, name, defaultValue);
         }
+
+        /**
+         * TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION is used to force enable the feature on specific
+         * R devices. Just checking the flag value is enough since the flag has been pushed to
+         * enable the feature on the old version and any new binary will always have a version
+         * number newer than the flag.
+         * This flag is wrongly configured in the connectivity namespace so this method reads the
+         * flag value from the connectivity namespace. But the tethering module should use the
+         * tethering namespace. This method can be removed after R EOL.
+         */
+        boolean isTetherForceUpstreamAutomaticFeatureEnabled() {
+            final int flagValue = DeviceConfigUtils.getDeviceConfigPropertyInt(
+                    NAMESPACE_CONNECTIVITY, TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION,
+                    0 /* defaultValue */);
+            return flagValue > 0;
+        }
     }
 
     public TetheringConfiguration(@NonNull Context ctx, @NonNull SharedLog log, int id) {
@@ -237,7 +249,7 @@ public class TetheringConfiguration {
         // - S, T: can be enabled/disabled by resource config_tether_upstream_automatic.
         // - U+  : automatic mode only.
         final boolean forceAutomaticUpstream = SdkLevel.isAtLeastU() || (!SdkLevel.isAtLeastS()
-                && isConnectivityFeatureEnabled(ctx, TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION));
+                && mDeps.isTetherForceUpstreamAutomaticFeatureEnabled());
         chooseUpstreamAutomatically = forceAutomaticUpstream || getResourceBoolean(
                 res, R.bool.config_tether_upstream_automatic, false /** defaultValue */);
         preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, isDunRequired);
@@ -607,32 +619,13 @@ public class TetheringConfiguration {
 
     private boolean shouldEnableWearTethering(Context context) {
         return SdkLevel.isAtLeastT()
-            && isTetheringFeatureEnabled(context, TETHER_ENABLE_WEAR_TETHERING);
+            && mDeps.isFeatureEnabled(context, TETHER_ENABLE_WEAR_TETHERING);
     }
 
     private boolean getDeviceConfigBoolean(final String name, final boolean defaultValue) {
         return mDeps.getDeviceConfigBoolean(NAMESPACE_CONNECTIVITY, name, defaultValue);
     }
 
-    /**
-     * This is deprecated because connectivity namespace already be used for NetworkStack mainline
-     * module. Tethering should use its own namespace to roll out the feature flag.
-     * @deprecated new caller should use isTetheringFeatureEnabled instead.
-     */
-    @Deprecated
-    private boolean isConnectivityFeatureEnabled(Context ctx, String featureVersionFlag) {
-        return isFeatureEnabled(ctx, NAMESPACE_CONNECTIVITY, featureVersionFlag);
-    }
-
-    private boolean isTetheringFeatureEnabled(Context ctx, String featureVersionFlag) {
-        return isFeatureEnabled(ctx, NAMESPACE_TETHERING, featureVersionFlag);
-    }
-
-    private boolean isFeatureEnabled(Context ctx, String namespace, String featureVersionFlag) {
-        return mDeps.isFeatureEnabled(ctx, namespace, featureVersionFlag, TETHERING_MODULE_NAME,
-                false /* defaultEnabled */);
-    }
-
     private Resources getResources(Context ctx, int subId) {
         if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) {
             return getResourcesForSubIdWrapper(ctx, subId);

Tethering/tests/unit/src/com/android/networkstack/tethering/FakeTetheringConfiguration.java

@@ -28,9 +28,8 @@ public class FakeTetheringConfiguration extends TetheringConfiguration {
     FakeTetheringConfiguration(Context ctx, SharedLog log, int id) {
         super(ctx, log, id, new Dependencies() {
             @Override
-            boolean isFeatureEnabled(@NonNull Context context, @NonNull String namespace,
+            boolean isFeatureEnabled(@NonNull Context context, @NonNull String name) {
-                    @NonNull String name, @NonNull String moduleName, boolean defaultEnabled) {
+                return false;
-                return defaultEnabled;
             }
 
             @Override
@@ -38,6 +37,11 @@ public class FakeTetheringConfiguration extends TetheringConfiguration {
                     boolean defaultValue) {
                 return defaultValue;
             }
+
+            @Override
+            boolean isTetherForceUpstreamAutomaticFeatureEnabled() {
+                return false;
+            }
         });
     }
 

Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java

@@ -155,9 +155,8 @@ public class TetheringConfigurationTest {
         private ArrayMap<String, Boolean> mMockFlags = new ArrayMap<>();
 
         @Override
-        boolean isFeatureEnabled(@NonNull Context context, @NonNull String namespace,
+        boolean isFeatureEnabled(@NonNull Context context, @NonNull String name) {
-                @NonNull String name, @NonNull String moduleName, boolean defaultEnabled) {
+            return isMockFlagEnabled(name, false /* defaultEnabled */);
-            return isMockFlagEnabled(name, defaultEnabled);
         }
 
         @Override
@@ -172,6 +171,12 @@ public class TetheringConfigurationTest {
             return isMockFlagEnabled(name, defaultValue);
         }
 
+        @Override
+        boolean isTetherForceUpstreamAutomaticFeatureEnabled() {
+            return isMockFlagEnabled(TetheringConfiguration.TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION,
+                    false /* defaultEnabled */);
+        }
+
         private boolean isMockFlagEnabled(@NonNull String name, boolean defaultEnabled) {
             final Boolean flag = mMockFlags.getOrDefault(name, defaultEnabled);
             // Value in the map can also be null

service-t/src/com/android/server/NsdService.java

@@ -1713,8 +1713,7 @@ public class NsdService extends INsdManager.Stub {
          */
         public boolean isMdnsDiscoveryManagerEnabled(Context context) {
             return isAtLeastU() || DeviceConfigUtils.isTetheringFeatureEnabled(context,
-                    NAMESPACE_TETHERING, MDNS_DISCOVERY_MANAGER_VERSION,
+                    MDNS_DISCOVERY_MANAGER_VERSION);
-                    DeviceConfigUtils.TETHERING_MODULE_NAME, false /* defaultEnabled */);
         }
 
         /**
@@ -1725,8 +1724,7 @@ public class NsdService extends INsdManager.Stub {
          */
         public boolean isMdnsAdvertiserEnabled(Context context) {
             return isAtLeastU() || DeviceConfigUtils.isTetheringFeatureEnabled(context,
-                    NAMESPACE_TETHERING, MDNS_ADVERTISER_VERSION,
+                    MDNS_ADVERTISER_VERSION);
-                    DeviceConfigUtils.TETHERING_MODULE_NAME, false /* defaultEnabled */);
         }
 
         /**
@@ -1743,8 +1741,7 @@ public class NsdService extends INsdManager.Stub {
          * @see DeviceConfigUtils#isTetheringFeatureEnabled
          */
         public boolean isFeatureEnabled(Context context, String feature) {
-            return DeviceConfigUtils.isTetheringFeatureEnabled(context, NAMESPACE_TETHERING,
+            return DeviceConfigUtils.isTetheringFeatureEnabled(context, feature);
-                    feature, DeviceConfigUtils.TETHERING_MODULE_NAME, false /* defaultEnabled */);
         }
 
         /**

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

@@ -93,12 +93,10 @@ import static android.net.OemNetworkPreferences.OEM_NETWORK_PREFERENCE_TEST;
 import static android.net.OemNetworkPreferences.OEM_NETWORK_PREFERENCE_TEST_ONLY;
 import static android.os.Process.INVALID_UID;
 import static android.os.Process.VPN_UID;
-import static android.provider.DeviceConfig.NAMESPACE_TETHERING;
 import static android.system.OsConstants.ETH_P_ALL;
 import static android.system.OsConstants.IPPROTO_TCP;
 import static android.system.OsConstants.IPPROTO_UDP;
 
-import static com.android.net.module.util.DeviceConfigUtils.TETHERING_MODULE_NAME;
 import static com.android.net.module.util.NetworkMonitorUtils.isPrivateDnsValidationRequired;
 import static com.android.net.module.util.PermissionUtils.checkAnyPermissionOf;
 import static com.android.net.module.util.PermissionUtils.enforceAnyPermissionOf;
@@ -1424,8 +1422,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
          * @see DeviceConfigUtils#isTetheringFeatureEnabled
          */
         public boolean isFeatureEnabled(Context context, String name) {
-            return DeviceConfigUtils.isTetheringFeatureEnabled(context, NAMESPACE_TETHERING, name,
+            return DeviceConfigUtils.isTetheringFeatureEnabled(context, name);
-                    TETHERING_MODULE_NAME, false /* defaultValue */);
         }
 
         /**