0

[Android] Create FeatureOverrides to replace FeatureList

FeatureList is almost completely test code; create a separate class to
extract all of this code with a clearer API.

Creating the API in this CL to migrate upstream and downstream
separately, as well as moving the implementation.

Migrate one usage as example (though it doesn't exercise most APIs).

Low-Coverage-Reason: Adding API for large scale migration.
Bug: 345483590,386813115
Change-Id: If9ff847a68b8946138e38a83c980f214d4b8a64c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6112344
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Owners-Override: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1401906}
This commit is contained in:
Henrique Nakashima
2025-01-03 11:22:19 -08:00
committed by Chromium LUCI CQ
parent de915cbc3d
commit 076a4736c0
4 changed files with 191 additions and 16 deletions
base
chrome/browser/tab_group_sync/android/java/src/org/chromium/chrome/browser/tab_group_sync

@ -4430,6 +4430,7 @@ if (is_android) {
"android/java/src/org/chromium/base/EventLog.java",
"android/java/src/org/chromium/base/FeatureList.java",
"android/java/src/org/chromium/base/FeatureMap.java",
"android/java/src/org/chromium/base/FeatureOverrides.java",
"android/java/src/org/chromium/base/FeatureParam.java",
"android/java/src/org/chromium/base/Features.java",
"android/java/src/org/chromium/base/FieldTrialList.java",

@ -18,7 +18,11 @@ import org.chromium.build.annotations.Nullable;
import java.util.HashMap;
import java.util.Map;
/** Provides shared capabilities for feature flag support. */
/**
* Provides shared capabilities for feature flag support.
*
* <p>TODO(crbug.com/345483590): Move all override logic and TestValues to FeatureOverrides.
*/
@NullMarked
@JNINamespace("base::android")
public class FeatureList {
@ -208,9 +212,10 @@ public class FeatureList {
/**
* Adds overrides to feature flags and field trial parameters in addition to existing ones.
*
* <p>An alias for #mergeTestValues(testValues, replace=true).
* @deprecated use {@link FeatureOverrides#apply()}
*/
@VisibleForTesting
@Deprecated
public static void setTestValues(TestValues testValues) {
assert testValues != null;
mergeTestValues(testValues, /* replace= */ true);
@ -224,8 +229,11 @@ public class FeatureList {
*
* <p>@Features annotations and @CommandLineFlags --enable/disable-features are affected by
* this.
*
* @deprecated use {@link FeatureOverrides#removeAllIncludingAnnotations()}
*/
@VisibleForTesting
@Deprecated
public static void removeAllTestOverrides() {
overwriteTestValues(null);
}
@ -244,12 +252,16 @@ public class FeatureList {
/**
* Adds overrides to feature flags and field trial parameters in addition to existing ones.
*
* <p>TODO(crbug.com/386813115): Migrate test class usages to #setTestValues to make this
* private.
* <p>TODO(crbug.com/386813115): Migrate test class usages to {@link
* FeatureOverrides.Builder#apply()} or {@link FeatureOverrides.Builder#applyWithoutOverwrite()}
* and make this private.
*
* @deprecated use {@link FeatureOverrides.Builder#apply()} or {@link
* FeatureOverrides.Builder#applyWithoutOverwrite()}
* @param testValuesToMerge the TestValues to merge into existing ones
* @param replace if true, replaces existing values (e.g. from @EnableFeatures annotations)
* @param replace if true, replaces existing overrides; otherwise preserve them
*/
@Deprecated
public static void mergeTestValues(TestValues testValuesToMerge, boolean replace) {
TestValues newTestValues = new TestValues();
if (sTestFeatures != null) {
@ -259,14 +271,26 @@ public class FeatureList {
overwriteTestValues(newTestValues);
}
/** Override a feature flag with a test value. */
/**
* Override a feature flag with a test value.
*
* @deprecated use {@link FeatureOverrides#enable(String)}, {@link
* FeatureOverrides#disable(String)}, or {@link FeatureOverrides#overrideFlag(String,
* boolean)}
*/
@Deprecated
public static void setTestFeature(String featureName, boolean testValue) {
TestValues testValues = new TestValues();
testValues.addFeatureFlagOverride(featureName, testValue);
mergeTestValues(testValues, /* replace= */ true);
}
/** Override a feature param with a test value. */
/**
* Override a feature param with a test value.
*
* @deprecated use {@link FeatureOverrides#overrideParam(String, String, String)}
*/
@Deprecated
public static void setTestFeatureParam(String featureName, String paramName, String testValue) {
TestValues testValues = new TestValues();
testValues.addFieldTrialParamOverride(featureName, paramName, testValue);

@ -0,0 +1,151 @@
// Copyright 2025 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.base;
import androidx.annotation.Nullable;
import org.chromium.build.annotations.NullMarked;
/**
* Allows overriding feature flags and parameters for tests. Prefer @Features annotations.
*
* <p>This is useful when the values set are dynamic, for example in parameterized tests or in tests
* that needs to change the flag/param values in the middle of the test rather than from the
* beginning.
*
* <p>The @Features.EnableFeatures and @Features.DisableFeatures annotations use FeatureOverrides,
* but also apply the changes more broadly; they include applying the override in native and they
* batch according to flag configuration.
*/
@NullMarked
public class FeatureOverrides {
public static class Builder extends FeatureList.TestValues {
@Nullable private String mLastFeatureName;
private Builder() {}
/**
* Apply overrides to feature flags and field trial parameters in addition to existing ones.
*
* <p>On conflict, overwrites the previous override.
*/
public void apply() {
FeatureList.mergeTestValues(this, /* replace= */ true);
}
/**
* Apply overrides to feature flags and field trial parameters in addition to existing ones.
*
* <p>On conflict, the previous override is preserved.
*/
public void applyWithoutOverwrite() {
FeatureList.mergeTestValues(this, /* replace= */ false);
}
/** Enable a feature flag. */
public Builder enable(String featureName) {
addFeatureFlagOverride(featureName, true);
mLastFeatureName = featureName;
return this;
}
/** Disable a feature flag. */
public Builder disable(String featureName) {
addFeatureFlagOverride(featureName, false);
mLastFeatureName = null;
return this;
}
/** Override a boolean param for the last feature flag enabled. */
public Builder param(String paramName, boolean value) {
return param(getLastFeatureName(), paramName, String.valueOf(value));
}
/** Override an int param for the last feature flag enabled. */
public Builder param(String paramName, int value) {
return param(getLastFeatureName(), paramName, String.valueOf(value));
}
/** Override a double param for the last feature flag enabled. */
public Builder param(String paramName, double value) {
return param(getLastFeatureName(), paramName, String.valueOf(value));
}
/** Override a String param for the last feature flag enabled. */
public Builder param(String paramName, String value) {
return param(getLastFeatureName(), paramName, value);
}
private String getLastFeatureName() {
if (mLastFeatureName == null) {
throw new IllegalArgumentException(
"param(paramName, value) should be used after enable()");
}
return mLastFeatureName;
}
/** Override a boolean param. */
public Builder param(String featureName, String paramName, boolean value) {
return param(featureName, paramName, String.valueOf(value));
}
/** Override an int param. */
public Builder param(String featureName, String paramName, int value) {
return param(featureName, paramName, String.valueOf(value));
}
/** Override a double param. */
public Builder param(String featureName, String paramName, double value) {
return param(featureName, paramName, String.valueOf(value));
}
/** Override a String param. */
public Builder param(String featureName, String paramName, String value) {
addFieldTrialParamOverride(featureName, paramName, value);
return this;
}
}
/** Use {@link #newBuilder()}. */
private FeatureOverrides() {}
/** Create a Builder for overriding feature flags and field trial parameters. */
public static FeatureOverrides.Builder newBuilder() {
return new FeatureOverrides.Builder();
}
/** Enable a feature flag for testing. */
public static void enable(String featureName) {
FeatureList.setTestFeature(featureName, true);
}
/** Disable a feature flag for testing. */
public static void disable(String featureName) {
FeatureList.setTestFeature(featureName, false);
}
/** Override a feature flag for testing. */
public static void overrideFlag(String featureName, boolean testValue) {
FeatureList.setTestFeature(featureName, testValue);
}
/** Override a feature param for testing. */
public static void overrideParam(String featureName, String paramName, String testValue) {
FeatureList.setTestFeatureParam(featureName, paramName, testValue);
}
/**
* Rarely necessary. Remove all Java overrides to feature flags and field trial parameters.
*
* <p>You don't need to call this on tearDown() or at the end of a test. ResettersForTesting
* already resets test values.
*
* <p>@Features annotations and @CommandLineFlags --enable/disable-features are affected by
* this.
*/
public static void removeAllIncludingAnnotations() {
FeatureList.removeAllTestOverrides();
}
}

@ -30,7 +30,7 @@ import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.robolectric.annotation.Config;
import org.chromium.base.FeatureList;
import org.chromium.base.FeatureOverrides;
import org.chromium.base.Token;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
@ -87,14 +87,13 @@ public class TabGroupSyncUtilsUnitTest {
@Test
public void testStaleGroupsNotAddedToSync() {
// Override the finch param to 90 days.
FeatureList.TestValues testValues = new FeatureList.TestValues();
testValues.addFeatureFlagOverride(ChromeFeatureList.TAB_GROUP_SYNC_ANDROID, true);
testValues.addFieldTrialParamOverride(
ChromeFeatureList.TAB_GROUP_SYNC_ANDROID,
TabGroupSyncUtils
.PARAM_MAX_DAYS_OF_STALENESS_ACCEPTED_FOR_ADDING_TAB_GROUP_TO_SYNC_ON_STARTUP,
String.valueOf(90));
FeatureList.setTestValues(testValues);
FeatureOverrides.newBuilder()
.enable(ChromeFeatureList.TAB_GROUP_SYNC_ANDROID)
.param(
TabGroupSyncUtils
.PARAM_MAX_DAYS_OF_STALENESS_ACCEPTED_FOR_ADDING_TAB_GROUP_TO_SYNC_ON_STARTUP,
90)
.apply();
long now = System.currentTimeMillis();