Merge "Update the logic of isFeatureEnabled and isFeatureNotChickenedOut" into main
diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java
index 468d7bd..93ccb85 100644
--- a/service-t/src/com/android/server/NsdService.java
+++ b/service-t/src/com/android/server/NsdService.java
@@ -26,6 +26,7 @@
import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT;
import static android.net.nsd.NsdManager.RESOLVE_SERVICE_SUCCEEDED;
import static android.provider.DeviceConfig.NAMESPACE_TETHERING;
+
import static com.android.modules.utils.build.SdkLevel.isAtLeastU;
import static com.android.networkstack.apishim.ConstantsShim.REGISTER_NSD_OFFLOAD_ENGINE;
import static com.android.server.connectivity.mdns.MdnsAdvertiser.AdvertiserMetrics;
@@ -1709,7 +1710,7 @@
mMdnsSocketClient, LOGGER.forSubComponent("MdnsDiscoveryManager"));
handler.post(() -> mMdnsSocketClient.setCallback(mMdnsDiscoveryManager));
MdnsFeatureFlags flags = new MdnsFeatureFlags.Builder().setIsMdnsOffloadFeatureEnabled(
- mDeps.isTetheringFeatureNotChickenedOut(
+ mDeps.isTetheringFeatureNotChickenedOut(mContext,
MdnsFeatureFlags.NSD_FORCE_DISABLE_MDNS_OFFLOAD)).build();
mAdvertiser = deps.makeMdnsAdvertiser(handler.getLooper(), mMdnsSocketProvider,
new AdvertiserCallback(), LOGGER.forSubComponent("MdnsAdvertiser"), flags);
@@ -1763,8 +1764,8 @@
/**
* @see DeviceConfigUtils#isTetheringFeatureNotChickenedOut
*/
- public boolean isTetheringFeatureNotChickenedOut(String feature) {
- return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(feature);
+ public boolean isTetheringFeatureNotChickenedOut(Context context, String feature) {
+ return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context, feature);
}
/**
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index 4b24aaf..6a34a24 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -254,7 +254,7 @@
if (sInitialized) return;
if (sEnableJavaBpfMap == null) {
sEnableJavaBpfMap = SdkLevel.isAtLeastU() ||
- DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context,
BPF_NET_MAPS_FORCE_DISABLE_JAVA_BPF_MAP);
}
Log.d(TAG, "BpfNetMaps is initialized with sEnableJavaBpfMap=" + sEnableJavaBpfMap);
diff --git a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
index 3befcfa..11345d3 100644
--- a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
+++ b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
@@ -974,7 +974,7 @@
* @return whether the feature is enabled
*/
public boolean isTetheringFeatureNotChickenedOut(@NonNull final String name) {
- return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(name);
+ return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(mContext, name);
}
/**
diff --git a/service/src/com/android/server/connectivity/KeepaliveTracker.java b/service/src/com/android/server/connectivity/KeepaliveTracker.java
index feba821..a51f09f 100644
--- a/service/src/com/android/server/connectivity/KeepaliveTracker.java
+++ b/service/src/com/android/server/connectivity/KeepaliveTracker.java
@@ -993,7 +993,7 @@
*/
public boolean isAddressTranslationEnabled(@NonNull Context context) {
return DeviceConfigUtils.isFeatureSupported(context, FEATURE_CLAT_ADDRESS_TRANSLATE)
- && DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ && DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context,
CONFIG_DISABLE_CLAT_ADDRESS_TRANSLATE);
}
}
diff --git a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
index e646f37..5b75699 100644
--- a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
+++ b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
@@ -40,6 +40,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.function.Supplier;
/**
* Utilities for modules to query {@link DeviceConfig} and flags.
@@ -70,6 +71,9 @@
sNetworkStackModuleVersion = -1;
}
+ private static final int FORCE_ENABLE_FEATURE_FLAG_VALUE = 1;
+ private static final int FORCE_DISABLE_FEATURE_FLAG_VALUE = -1;
+
private static volatile long sPackageVersion = -1;
private static long getPackageVersion(@NonNull final Context context) {
// sPackageVersion may be set by another thread just after this check, but querying the
@@ -186,9 +190,8 @@
*/
public static boolean isNetworkStackFeatureEnabled(@NonNull Context context,
@NonNull String name, boolean defaultEnabled) {
- final long packageVersion = getPackageVersion(context);
- return isFeatureEnabled(context, packageVersion, NAMESPACE_CONNECTIVITY, name,
- defaultEnabled);
+ return isFeatureEnabled(NAMESPACE_CONNECTIVITY, name, defaultEnabled,
+ () -> getPackageVersion(context));
}
/**
@@ -202,7 +205,7 @@
*
* If the feature is disabled by default and enabled by flag push, this method should be used.
* If the feature is enabled by default and disabled by flag push (kill switch),
- * {@link #isTetheringFeatureNotChickenedOut(String)} should be used.
+ * {@link #isTetheringFeatureNotChickenedOut(Context, String)} should be used.
*
* @param context The global context information about an app environment.
* @param name The name of the property to look up.
@@ -210,17 +213,24 @@
*/
public static boolean isTetheringFeatureEnabled(@NonNull Context context,
@NonNull String name) {
- final long packageVersion = getTetheringModuleVersion(context);
- return isFeatureEnabled(context, packageVersion, NAMESPACE_TETHERING, name,
- false /* defaultEnabled */);
+ return isFeatureEnabled(NAMESPACE_TETHERING, name, false /* defaultEnabled */,
+ () -> getTetheringModuleVersion(context));
}
- private static boolean isFeatureEnabled(@NonNull Context context, long packageVersion,
- @NonNull String namespace, String name, boolean defaultEnabled) {
- final int propertyVersion = getDeviceConfigPropertyInt(namespace, name,
- 0 /* default value */);
- return (propertyVersion == 0 && defaultEnabled)
- || (propertyVersion != 0 && packageVersion >= (long) propertyVersion);
+ private static boolean isFeatureEnabled(@NonNull String namespace,
+ String name, boolean defaultEnabled, Supplier<Long> packageVersionSupplier) {
+ final int flagValue = getDeviceConfigPropertyInt(namespace, name, 0 /* default value */);
+ switch (flagValue) {
+ case 0:
+ return defaultEnabled;
+ case FORCE_DISABLE_FEATURE_FLAG_VALUE:
+ return false;
+ case FORCE_ENABLE_FEATURE_FLAG_VALUE:
+ return true;
+ default:
+ final long packageVersion = packageVersionSupplier.get();
+ return packageVersion >= (long) flagValue;
+ }
}
// Guess the tethering module name based on the package prefix of the connectivity resources
@@ -331,42 +341,38 @@
}
/**
- * Check whether one specific experimental feature in specific namespace from
- * {@link DeviceConfig} is not disabled. Feature can be disabled by setting a non-zero
- * value in the property. If the feature is enabled by default and disabled by flag push
- * (kill switch), this method should be used. If the feature is disabled by default and
- * enabled by flag push, {@link #isFeatureEnabled} should be used.
- *
- * @param namespace The namespace containing the property to look up.
- * @param name The name of the property to look up.
- * @return true if this feature is enabled, or false if disabled.
- */
- private static boolean isFeatureNotChickenedOut(String namespace, String name) {
- final int propertyVersion = getDeviceConfigPropertyInt(namespace, name,
- 0 /* default value */);
- return propertyVersion == 0;
- }
-
- /**
* Check whether one specific experimental feature in Tethering module from {@link DeviceConfig}
* is not disabled.
+ * If the feature is enabled by default and disabled by flag push (kill switch), this method
+ * should be used.
+ * If the feature is disabled by default and enabled by flag push,
+ * {@link #isTetheringFeatureEnabled(Context, String)} should be used.
*
+ * @param context The global context information about an app environment.
* @param name The name of the property in tethering module to look up.
* @return true if this feature is enabled, or false if disabled.
*/
- public static boolean isTetheringFeatureNotChickenedOut(String name) {
- return isFeatureNotChickenedOut(NAMESPACE_TETHERING, name);
+ public static boolean isTetheringFeatureNotChickenedOut(@NonNull Context context, String name) {
+ return isFeatureEnabled(NAMESPACE_TETHERING, name, true /* defaultEnabled */,
+ () -> getTetheringModuleVersion(context));
}
/**
* Check whether one specific experimental feature in NetworkStack module from
* {@link DeviceConfig} is not disabled.
+ * If the feature is enabled by default and disabled by flag push (kill switch), this method
+ * should be used.
+ * If the feature is disabled by default and enabled by flag push,
+ * {@link #isNetworkStackFeatureEnabled(Context, String)} should be used.
*
+ * @param context The global context information about an app environment.
* @param name The name of the property in NetworkStack module to look up.
* @return true if this feature is enabled, or false if disabled.
*/
- public static boolean isNetworkStackFeatureNotChickenedOut(String name) {
- return isFeatureNotChickenedOut(NAMESPACE_CONNECTIVITY, name);
+ public static boolean isNetworkStackFeatureNotChickenedOut(
+ @NonNull Context context, String name) {
+ return isFeatureEnabled(NAMESPACE_CONNECTIVITY, name, true /* defaultEnabled */,
+ () -> getPackageVersion(context));
}
/**
diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
index 5a96bcb..9e04676 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
@@ -228,27 +228,57 @@
}
@Test
- public void testIsNetworkStackFeatureEnabled() {
+ public void testIsFeatureEnabled() {
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
- assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
- }
-
- @Test
- public void testIsTetheringFeatureEnabled() {
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
}
-
@Test
- public void testFeatureDefaultEnabled() {
+ public void testIsFeatureEnabledFeatureDefaultDisabled() throws Exception {
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+
+ // If the flag is unset, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
+ }
+
+ @Test
+ public void testIsFeatureEnabledFeatureForceEnabled() throws Exception {
+ doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+
+ // If the feature is force enabled, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
+ }
+
+ @Test
+ public void testIsFeatureEnabledFeatureForceDisabled() throws Exception {
+ doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertFalse(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+ assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+
+ // If the feature is force disabled, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
@@ -415,25 +445,65 @@
}
@Test
- public void testIsTetheringFeatureNotChickenedOut() throws Exception {
- doReturn("0").when(() -> DeviceConfig.getProperty(
- eq(NAMESPACE_TETHERING), eq(TEST_EXPERIMENT_FLAG)));
- assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
-
- doReturn(TEST_FLAG_VALUE_STRING).when(
- () -> DeviceConfig.getProperty(eq(NAMESPACE_TETHERING), eq(TEST_EXPERIMENT_FLAG)));
- assertFalse(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
+ public void testIsFeatureNotChickenedOut() {
+ doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
}
@Test
- public void testIsNetworkStackFeatureNotChickenedOut() throws Exception {
- doReturn("0").when(() -> DeviceConfig.getProperty(
- eq(NAMESPACE_CONNECTIVITY), eq(TEST_EXPERIMENT_FLAG)));
- assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
+ public void testIsFeatureNotChickenedOutFeatureDefaultEnabled() throws Exception {
+ doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
- doReturn(TEST_FLAG_VALUE_STRING).when(
- () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY),
- eq(TEST_EXPERIMENT_FLAG)));
- assertFalse(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
+ // If the flag is unset, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
+ }
+
+ @Test
+ public void testIsFeatureNotChickenedOutFeatureForceEnabled() throws Exception {
+ doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+ assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+
+ // If the feature is force enabled, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
+ }
+
+ @Test
+ public void testIsFeatureNotChickenedOutFeatureForceDisabled() throws Exception {
+ doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
+ TEST_EXPERIMENT_FLAG));
+ doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+ TEST_EXPERIMENT_FLAG));
+ assertFalse(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+ assertFalse(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
+ mContext, TEST_EXPERIMENT_FLAG));
+
+ // If the feature is force disabled, package info is not queried
+ verify(mContext, never()).getPackageManager();
+ verify(mContext, never()).getPackageName();
+ verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
}