Merge "Add flag to force choosing upstreams automatically"
diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
index 799637c..413b0cb 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
@@ -33,6 +33,8 @@
 import android.text.TextUtils;
 
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.modules.utils.build.SdkLevel;
+import com.android.net.module.util.DeviceConfigUtils;
 
 import java.io.PrintWriter;
 import java.util.ArrayList;
@@ -94,6 +96,20 @@
             "tether_enable_select_all_prefix_ranges";
 
     /**
+     * Experiment flag to force choosing upstreams automatically.
+     *
+     * This setting is intended to help force-enable the feature on OEM devices that disabled it
+     * via resource overlays, and later noticed issues. To that end, it overrides
+     * config_tether_upstream_automatic when set to true.
+     *
+     * This flag is enabled if !=0 and less than the module APK version: see
+     * {@link DeviceConfigUtils#isFeatureEnabled}. It is also ignored after R, as later devices
+     * should just set config_tether_upstream_automatic to true instead.
+     */
+    public static final String TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION =
+            "tether_force_upstream_automatic_version";
+
+    /**
      * Default value that used to periodic polls tether offload stats from tethering offload HAL
      * to make the data warnings work.
      */
@@ -146,7 +162,9 @@
 
         isDunRequired = checkDunRequired(ctx);
 
-        chooseUpstreamAutomatically = getResourceBoolean(
+        final boolean forceAutomaticUpstream = !SdkLevel.isAtLeastS()
+                && isFeatureEnabled(ctx, TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION);
+        chooseUpstreamAutomatically = forceAutomaticUpstream || getResourceBoolean(
                 res, R.bool.config_tether_upstream_automatic, false /** defaultValue */);
         preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, isDunRequired);
 
@@ -453,6 +471,11 @@
         return DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY, name);
     }
 
+    @VisibleForTesting
+    protected boolean isFeatureEnabled(Context ctx, String featureVersionFlag) {
+        return DeviceConfigUtils.isFeatureEnabled(ctx, NAMESPACE_CONNECTIVITY, featureVersionFlag);
+    }
+
     private Resources getResources(Context ctx, int subId) {
         if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) {
             return getResourcesForSubIdWrapper(ctx, subId);
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
index 354e753..8cfa7d0 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
@@ -53,6 +53,8 @@
 
 import android.content.Context;
 import android.content.Intent;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
 import android.content.res.Resources;
 import android.net.util.SharedLog;
 import android.os.Bundle;
@@ -87,11 +89,13 @@
     private static final String[] PROVISIONING_APP_NAME = {"some", "app"};
     private static final String PROVISIONING_NO_UI_APP_NAME = "no_ui_app";
     private static final String PROVISIONING_APP_RESPONSE = "app_response";
+    private static final String TEST_PACKAGE_NAME = "com.android.tethering.test";
 
     @Mock private CarrierConfigManager mCarrierConfigManager;
     @Mock private Context mContext;
     @Mock private Resources mResources;
     @Mock private SharedLog mLog;
+    @Mock private PackageManager mPm;
     @Mock private EntitlementManager.OnUiEntitlementFailedListener mEntitlementFailedListener;
 
     // Like so many Android system APIs, these cannot be mocked because it is marked final.
@@ -182,7 +186,7 @@
     }
 
     @Before
-    public void setUp() {
+    public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
         mMockingSession = mockitoSession()
                 .initMocks(this)
@@ -196,6 +200,9 @@
                 eq(EntitlementManager.DISABLE_PROVISIONING_SYSPROP_KEY), anyBoolean()));
         doReturn(null).when(
                 () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY), anyString()));
+        doReturn(mPm).when(mContext).getPackageManager();
+        doReturn(TEST_PACKAGE_NAME).when(mContext).getPackageName();
+        doReturn(new PackageInfo()).when(mPm).getPackageInfo(anyString(), anyInt());
 
         when(mResources.getStringArray(R.array.config_tether_dhcp_range))
                 .thenReturn(new String[0]);
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
index 237e2c2..1f4e371 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
@@ -30,12 +30,16 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
 import android.content.res.Resources;
 import android.net.util.SharedLog;
+import android.os.Build;
 import android.provider.DeviceConfig;
 import android.telephony.TelephonyManager;
 
@@ -43,9 +47,14 @@
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.internal.util.test.BroadcastInterceptingContext;
+import com.android.net.module.util.DeviceConfigUtils;
+import com.android.testutils.DevSdkIgnoreRule;
+import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter;
+import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
 
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
@@ -60,13 +69,18 @@
 public class TetheringConfigurationTest {
     private final SharedLog mLog = new SharedLog("TetheringConfigurationTest");
 
+    @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule();
+
     private static final String[] PROVISIONING_APP_NAME = {"some", "app"};
     private static final String PROVISIONING_NO_UI_APP_NAME = "no_ui_app";
     private static final String PROVISIONING_APP_RESPONSE = "app_response";
+    private static final String TEST_PACKAGE_NAME = "com.android.tethering.test";
+    private static final long TEST_PACKAGE_VERSION = 1234L;
     @Mock private Context mContext;
     @Mock private TelephonyManager mTelephonyManager;
     @Mock private Resources mResources;
     @Mock private Resources mResourcesForSubId;
+    @Mock private PackageManager mPackageManager;
     private Context mMockContext;
     private boolean mHasTelephonyManager;
     private boolean mEnableLegacyDhcpServer;
@@ -100,6 +114,16 @@
             }
             return super.getSystemService(name);
         }
+
+        @Override
+        public PackageManager getPackageManager() {
+            return mPackageManager;
+        }
+
+        @Override
+        public String getPackageName() {
+            return TEST_PACKAGE_NAME;
+        }
     }
 
     @Before
@@ -110,9 +134,15 @@
                 .mockStatic(DeviceConfig.class)
                 .strictness(Strictness.WARN)
                 .startMocking();
+        DeviceConfigUtils.resetPackageVersionCacheForTest();
         doReturn(null).when(
                 () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY),
                 eq(TetheringConfiguration.TETHER_ENABLE_LEGACY_DHCP_SERVER)));
+        setTetherForceUpstreamAutomaticFlagVersion(null);
+
+        final PackageInfo pi = new PackageInfo();
+        pi.setLongVersionCode(TEST_PACKAGE_VERSION);
+        doReturn(pi).when(mPackageManager).getPackageInfo(eq(TEST_PACKAGE_NAME), anyInt());
 
         when(mResources.getStringArray(R.array.config_tether_dhcp_range)).thenReturn(
                 new String[0]);
@@ -141,6 +171,7 @@
     @After
     public void tearDown() throws Exception {
         mMockingSession.finishMocking();
+        DeviceConfigUtils.resetPackageVersionCacheForTest();
     }
 
     private TetheringConfiguration getTetheringConfiguration(int... legacyTetherUpstreamTypes) {
@@ -455,4 +486,52 @@
                 mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
         assertTrue(testEnable.isSelectAllPrefixRangeEnabled());
     }
+
+    @Test
+    public void testChooseUpstreamAutomatically() throws Exception {
+        when(mResources.getBoolean(R.bool.config_tether_upstream_automatic))
+                .thenReturn(true);
+        assertChooseUpstreamAutomaticallyIs(true);
+
+        when(mResources.getBoolean(R.bool.config_tether_upstream_automatic))
+                .thenReturn(false);
+        assertChooseUpstreamAutomaticallyIs(false);
+    }
+
+    // The flag override only works on R-
+    @Test @IgnoreAfter(Build.VERSION_CODES.R)
+    public void testChooseUpstreamAutomatically_FlagOverride() throws Exception {
+        when(mResources.getBoolean(R.bool.config_tether_upstream_automatic))
+                .thenReturn(false);
+        setTetherForceUpstreamAutomaticFlagVersion(TEST_PACKAGE_VERSION - 1);
+        assertTrue(DeviceConfigUtils.isFeatureEnabled(mMockContext, NAMESPACE_CONNECTIVITY,
+                TetheringConfiguration.TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION));
+
+        assertChooseUpstreamAutomaticallyIs(true);
+
+        setTetherForceUpstreamAutomaticFlagVersion(0L);
+        assertChooseUpstreamAutomaticallyIs(false);
+
+        setTetherForceUpstreamAutomaticFlagVersion(Long.MAX_VALUE);
+        assertChooseUpstreamAutomaticallyIs(false);
+    }
+
+    @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testChooseUpstreamAutomatically_FlagOverrideAfterR() throws Exception {
+        when(mResources.getBoolean(R.bool.config_tether_upstream_automatic))
+                .thenReturn(false);
+        setTetherForceUpstreamAutomaticFlagVersion(TEST_PACKAGE_VERSION - 1);
+        assertChooseUpstreamAutomaticallyIs(false);
+    }
+
+    private void setTetherForceUpstreamAutomaticFlagVersion(Long version) {
+        doReturn(version == null ? null : Long.toString(version)).when(
+                () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY),
+                        eq(TetheringConfiguration.TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION)));
+    }
+
+    private void assertChooseUpstreamAutomaticallyIs(boolean value) {
+        assertEquals(value, new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID)
+                .chooseUpstreamAutomatically);
+    }
 }
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index 60fddb5..c5a43b7 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -358,6 +358,11 @@
         }
 
         @Override
+        protected boolean isFeatureEnabled(Context ctx, String featureVersionFlag) {
+            return false;
+        }
+
+        @Override
         protected Resources getResourcesForSubIdWrapper(Context ctx, int subId) {
             return mResources;
         }