[Android] Break deps from Features and FieldTrials to CachedFlag/Param

This is a preparation to move most of the code in base.cached_flags to
components/cached_flags. The dependencies that need to be broken for
this to be possible are all for testing and are removed in this CL.

Bug: 370797986
Change-Id: Ib0de691436bd9dfb526921a464916519d10777de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5902519
Commit-Queue: Henrique Nakashima <[email protected]>
Reviewed-by: Andrew Grieve <[email protected]>
Reviewed-by: Calder Kitagawa <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1363291}


CrOS-Libchrome-Original-Commit: e33fc53d7cbb742fd457c29713cf99e993534403

Author: Henrique Nakashima

base/android/java/src/org/chromium/base/cached_flags/AllCachedFieldTrialParameters.java

@@ -8,36 +8,12 @@
 
 import androidx.annotation.AnyThread;
 
-import org.json.JSONException;
-import org.json.JSONObject;
-
 import org.chromium.base.FeatureMap;
 
-import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Map;
 
 /** AllCachedFieldTrialParameters caches all the parameters for a feature. */
 public class AllCachedFieldTrialParameters extends CachedFieldTrialParameter {
-    /** Decodes a previously encoded map. Returns empty map on parse error. */
-    private static Map<String, String> decodeJsonEncodedMap(String value) {
-        Map<String, String> resultingMap = new HashMap<String, String>();
-        try {
-            final JSONObject json = new JSONObject(value);
-            Iterator<String> keys = json.keys();
-            while (keys.hasNext()) {
-                final String key = keys.next();
-                resultingMap.put(key, json.getString(key));
-            }
-            return resultingMap;
-        } catch (JSONException e) {
-        }
-        return new HashMap<>();
-    }
-
-    private static String encodeParams(Map<String, String> params) {
-        return new JSONObject(params).toString();
-    }
 
     public AllCachedFieldTrialParameters(FeatureMap featureMap, String featureName) {
         // As this includes all parameters, the parameterName is empty.
@@ -47,7 +23,7 @@ public AllCachedFieldTrialParameters(FeatureMap featureMap, String featureName)
     /** Returns a map of field trial parameter to value. */
     @AnyThread
     public Map<String, String> getParams() {
-        return decodeJsonEncodedMap(getConsistentStringValue());
+        return CachedFlagsSharedPreferences.decodeJsonEncodedMap(getConsistentStringValue());
     }
 
     @AnyThread
@@ -81,13 +57,15 @@ private String getConsistentStringValue() {
     void writeCacheValueToEditor(final SharedPreferences.Editor editor) {
         final Map<String, String> params =
                 mFeatureMap.getFieldTrialParamsForFeature(getFeatureName());
-        editor.putString(getSharedPreferenceKey(), encodeParams(params));
+        editor.putString(
+                getSharedPreferenceKey(), CachedFlagsSharedPreferences.encodeParams(params));
     }
 
     /** Sets the parameters for the specified feature when used in tests. */
     public static void setForTesting(String featureName, Map<String, String> params) {
-        String preferenceKey = generateSharedPreferenceKey(featureName, "");
-        String overrideValue = encodeParams(params);
+        String preferenceKey =
+                CachedFlagsSharedPreferences.generateParamSharedPreferenceKey(featureName, "");
+        String overrideValue = CachedFlagsSharedPreferences.encodeParams(params);
         ValuesOverridden.setOverrideForTesting(preferenceKey, overrideValue);
     }
 }

base/android/java/src/org/chromium/base/cached_flags/CachedFieldTrialParameter.java

@@ -110,23 +110,12 @@ public String getParameterName() {
         return mType;
     }
 
-    /**
-     * @return A human-readable string uniquely identifying the field trial parameter.
-     */
-    private static String generateFullName(String featureName, String parameterName) {
-        return featureName + ":" + parameterName;
-    }
-
-    static String generateSharedPreferenceKey(String featureName, String parameterName) {
-        return CachedFlagsSharedPreferences.FLAGS_FIELD_TRIAL_PARAM_CACHED.createKey(
-                generateFullName(featureName, parameterName));
-    }
-
     /**
      * @return The SharedPreferences key to cache the field trial parameter.
      */
     String getSharedPreferenceKey() {
-        return generateSharedPreferenceKey(getFeatureName(), getParameterName());
+        return CachedFlagsSharedPreferences.generateParamSharedPreferenceKey(
+                getFeatureName(), getParameterName());
     }
 
     /**
@@ -136,14 +125,4 @@ String getSharedPreferenceKey() {
      * loaded yet.
      */
     abstract void writeCacheValueToEditor(SharedPreferences.Editor editor);
-
-    /**
-     * Forces a field trial parameter value for testing. This is only for the annotation processor
-     * to use. Tests should use "PARAMETER.setForTesting()" instead.
-     */
-    public static void setForTesting(
-            String featureName, String variationName, String stringVariationValue) {
-        String preferenceKey = generateSharedPreferenceKey(featureName, variationName);
-        ValuesOverridden.setOverrideForTesting(preferenceKey, stringVariationValue);
-    }
 }

base/android/java/src/org/chromium/base/cached_flags/CachedFlag.java

@@ -150,25 +150,6 @@ String getSharedPreferenceKey() {
         return mPreferenceKey;
     }
 
-    /**
-     * Sets the feature flags to use in JUnit and instrumentation tests.
-     *
-     * @deprecated Do not call this from tests; use @EnableFeatures/@DisableFeatures annotations
-     *     instead.
-     */
-    @Deprecated
-    public static void setFeaturesForTesting(Map<String, Boolean> features) {
-        synchronized (ValuesReturned.sBoolValues) {
-            for (Map.Entry<String, Boolean> entry : features.entrySet()) {
-                String featureName = entry.getKey();
-                Boolean flagValue = entry.getValue();
-                String sharedPreferencesKey =
-                        CachedFlagsSharedPreferences.FLAGS_CACHED.createKey(featureName);
-                ValuesReturned.sBoolValues.put(sharedPreferencesKey, flagValue);
-            }
-        }
-    }
-
     /** Create a Map of feature names -> {@link CachedFlag} from multiple lists of CachedFlags. */
     public static Map<String, CachedFlag> createCachedFlagMap(
             List<List<CachedFlag>> allCachedFlagsLists) {

base/android/java/src/org/chromium/base/cached_flags/CachedFlagUtils.java

@@ -42,16 +42,4 @@ public static void cacheFieldTrialParameters(
         }
         editor.apply();
     }
-
-    /**
-     * Do not call this from tests.
-     *
-     * <p>Test runners already reset this state.
-     *
-     * <p>Exceptions are tests that test the flags infrastructure.
-     */
-    public static void resetFlagsForTesting() {
-        ValuesReturned.clearForTesting();
-        ValuesOverridden.removeOverrides();
-    }
 }

base/android/java/src/org/chromium/base/cached_flags/CachedFlagsSharedPreferences.java

@@ -4,16 +4,25 @@
 
 package org.chromium.base.cached_flags;
 
+import org.json.JSONException;
+import org.json.JSONObject;
+
+import org.chromium.base.Log;
 import org.chromium.base.shared_preferences.KeyPrefix;
 import org.chromium.base.shared_preferences.PreferenceKeyRegistry;
 import org.chromium.base.shared_preferences.SharedPreferencesManager;
 import org.chromium.build.BuildConfig;
 
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 
 /** Shared preferences used by org.chromium.base.cached_flags */
 public class CachedFlagsSharedPreferences {
+    private static final String TAG = "CachedFlags";
+
     /** CachedFlags store flag values for the next run with this prefix. */
     public static final KeyPrefix FLAGS_CACHED = new KeyPrefix("Chrome.Flags.CachedFlag.*");
 
@@ -46,4 +55,41 @@ public class CachedFlagsSharedPreferences {
     public static SharedPreferencesManager getInstance() {
         return SharedPreferencesManager.getInstanceForRegistry(REGISTRY);
     }
+
+    /**
+     * @return A human-readable string uniquely identifying the field trial parameter.
+     */
+    private static String generateParamFullName(String featureName, String parameterName) {
+        return featureName + ":" + parameterName;
+    }
+
+    public static String generateParamSharedPreferenceKey(
+            String featureName, String parameterName) {
+        return FLAGS_FIELD_TRIAL_PARAM_CACHED.createKey(
+                generateParamFullName(featureName, parameterName));
+    }
+
+    public static String encodeParams(Map<String, String> params) {
+        return new JSONObject(params).toString();
+    }
+
+    /** Decodes a previously encoded map. Returns empty map on parse error. */
+    static Map<String, String> decodeJsonEncodedMap(String value) {
+        Map<String, String> resultingMap = new HashMap<>();
+        if (value.isEmpty()) {
+            return resultingMap;
+        }
+        try {
+            final JSONObject json = new JSONObject(value);
+            Iterator<String> keys = json.keys();
+            while (keys.hasNext()) {
+                final String key = keys.next();
+                resultingMap.put(key, json.getString(key));
+            }
+            return resultingMap;
+        } catch (JSONException e) {
+            Log.e(TAG, "Error parsing JSON", e);
+            return new HashMap<>();
+        }
+    }
 }

base/android/java/src/org/chromium/base/cached_flags/ValuesOverridden.java

@@ -10,13 +10,18 @@
 import java.util.Map;
 
 /** Keeps track of values overridden for testing for cached flags and field trial parameters. */
-class ValuesOverridden {
+public abstract class ValuesOverridden {
     @CheckDiscard(
             "Should only exist in tests and in debug builds, should be optimized out in "
                     + "Release.")
     private static Map<String, String> sOverridesTestFeatures;
 
-    static void setOverrideForTesting(String preferenceKey, String overrideValue) {
+    /**
+     * Set an |overrideValue| to be used in place of the disk value of a |preferenceKey| in tests.
+     *
+     * <p>Don't use directly from tests.
+     */
+    public static void setOverrideForTesting(String preferenceKey, String overrideValue) {
         if (sOverridesTestFeatures == null) {
             sOverridesTestFeatures = new HashMap<>();
         }
@@ -46,7 +51,12 @@ static Double getDouble(String preferenceName) {
         return stringValue != null ? Double.valueOf(stringValue) : null;
     }
 
-    static void removeOverrides() {
+    /**
+     * Remove all override values set.
+     *
+     * <p>Don't use directly from tests.
+     */
+    public static void removeOverrides() {
         sOverridesTestFeatures = null;
     }
 }

base/android/java/src/org/chromium/base/cached_flags/ValuesReturned.java

@@ -10,7 +10,7 @@
 import java.util.Map;
 
 /** Keeps track of values returned for cached flags and field trial parameters. */
-abstract class ValuesReturned {
+public abstract class ValuesReturned {
     @GuardedBy("sBoolValues")
     static final Map<String, Boolean> sBoolValues = new HashMap<>();
 
@@ -23,7 +23,12 @@ abstract class ValuesReturned {
     @GuardedBy("sDoubleValues")
     static final Map<String, Double> sDoubleValues = new HashMap<>();
 
-    static void clearForTesting() {
+    /**
+     * Forget the values returned this run for tests. New values will be calculated when needed.
+     *
+     * <p>Do not call this directly from tests.
+     */
+    public static void clearForTesting() {
         synchronized (sBoolValues) {
             sBoolValues.clear();
         }
@@ -37,4 +42,21 @@ static void clearForTesting() {
             sDoubleValues.clear();
         }
     }
+
+    /**
+     * Sets the feature flags to use in JUnit and instrumentation tests.
+     *
+     * <p>Do not call this from tests; use @EnableFeatures/@DisableFeatures annotations instead.
+     */
+    public static void setFeaturesForTesting(Map<String, Boolean> features) {
+        synchronized (sBoolValues) {
+            for (Map.Entry<String, Boolean> entry : features.entrySet()) {
+                String featureName = entry.getKey();
+                Boolean flagValue = entry.getValue();
+                String sharedPreferencesKey =
+                        CachedFlagsSharedPreferences.FLAGS_CACHED.createKey(featureName);
+                sBoolValues.put(sharedPreferencesKey, flagValue);
+            }
+        }
+    }
 }

base/android/junit/src/org/chromium/base/cached_flags/CachedFeatureFlagsSafeModeUnitTest.java

@@ -8,7 +8,6 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import org.chromium.base.test.util.BaseFlagTestRule;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -18,9 +17,10 @@
 import org.chromium.base.FeatureList;
 import org.chromium.base.FeatureList.TestValues;
 import org.chromium.base.FeatureMap;
+import org.chromium.base.cached_flags.CachedFlagsSafeMode.Behavior;
 import org.chromium.base.task.test.PausedExecutorTestRule;
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.base.cached_flags.CachedFlagsSafeMode.Behavior;
+import org.chromium.base.test.util.BaseFlagTestRule;
 
 import java.util.Arrays;
 
@@ -712,7 +712,8 @@ private void assertCachedParamsEqualNative2() {
     }
 
     private static void clearMemory() {
-        CachedFlagUtils.resetFlagsForTesting();
+        ValuesReturned.clearForTesting();
+        ValuesOverridden.removeOverrides();
         CachedFlagsSafeMode.getInstance().clearMemoryForTesting();
     }
 }

base/android/junit/src/org/chromium/base/cached_flags/CachedFieldTrialParameterUnitTest.java

@@ -145,7 +145,8 @@ public void testNativeNotInitializedPrefsCached_getsFromPrefs() {
         CachedFlagUtils.cacheFieldTrialParameters(PARAMS_TO_CACHE);
 
         // Simulate a second run
-        CachedFlagUtils.resetFlagsForTesting();
+        ValuesReturned.clearForTesting();
+        ValuesOverridden.removeOverrides();
 
         // Set different values in native which shouldn't be used
         TestValues testValues = new TestValues();
@@ -175,7 +176,10 @@ public void testSetForTesting() {
         INT_PARAM.setForTesting(INT_PARAM_TEST_OVERRIDE);
         DOUBLE_PARAM.setForTesting(DOUBLE_PARAM_TEST_OVERRIDE);
         STRING2_PARAM.setForTesting(STRING2_PARAM_TEST_OVERRIDE);
-        AllCachedFieldTrialParameters.setForTesting(FEATURE_B, ALL_PARAM_TEST_OVERRIDE);
+        String preferenceKey =
+                CachedFlagsSharedPreferences.generateParamSharedPreferenceKey(FEATURE_B, "");
+        String overrideValue = CachedFlagsSharedPreferences.encodeParams(ALL_PARAM_TEST_OVERRIDE);
+        ValuesOverridden.setOverrideForTesting(preferenceKey, overrideValue);
 
         // Should not take priority over the overrides
         CachedFlagUtils.cacheFieldTrialParameters(PARAMS_TO_CACHE);

base/android/junit/src/org/chromium/base/cached_flags/CachedFlagUnitTest.java

@@ -30,7 +30,8 @@ public class CachedFlagUnitTest {
 
     @After
     public void tearDown() {
-        CachedFlagUtils.resetFlagsForTesting();
+        ValuesReturned.clearForTesting();
+        ValuesOverridden.removeOverrides();
     }
 
     @Test(expected = AssertionError.class)
@@ -81,7 +82,8 @@ public void testNativeNotInitializedPrefsCached_getsFromPrefs() {
         assertIsEnabledMatches(A_OFF_B_ON, featureA, featureB);
 
         // Pretend the app was restarted. The SharedPrefs should remain.
-        CachedFlagUtils.resetFlagsForTesting();
+        ValuesReturned.clearForTesting();
+        ValuesOverridden.removeOverrides();
 
         // Simulate ChromeFeatureList retrieving new, different values for the flags.
         FeatureList.setTestFeatures(A_ON_B_ON);
@@ -98,7 +100,8 @@ public void testNativeNotInitializedPrefsCached_getsFromPrefs() {
         assertIsEnabledMatches(A_OFF_B_ON, featureA, featureB);
 
         // Pretend the app was restarted again.
-        CachedFlagUtils.resetFlagsForTesting();
+        ValuesReturned.clearForTesting();
+        ValuesOverridden.removeOverrides();
 
         // The SharedPrefs should retain the latest values.
         assertIsEnabledMatches(A_ON_B_ON, featureA, featureB);