Commit 394c2f09 authored by Robbie McElrath's avatar Robbie McElrath Committed by Commit Bot

Create base.FeatureList to configure test feature values.

This moves ChromeFeatureList.setTestFeatures to a new base.FeatureList
class so it can be shared by other *FeatureList implementations.
Eventually this class will the the primary method of querying Feature
state, but for now it only handles test configuration.

See crrev.com/c/2095445 and the bug for more information.

Bug: 1060097
Change-Id: Idd0c8ebc447f9f936192f81fc1465600f28ad7d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099470Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750241}
parent d5dc1a26
...@@ -3275,6 +3275,7 @@ if (is_android) { ...@@ -3275,6 +3275,7 @@ if (is_android) {
"android/java/src/org/chromium/base/DiscardableReferencePool.java", "android/java/src/org/chromium/base/DiscardableReferencePool.java",
"android/java/src/org/chromium/base/EarlyTraceEvent.java", "android/java/src/org/chromium/base/EarlyTraceEvent.java",
"android/java/src/org/chromium/base/EventLog.java", "android/java/src/org/chromium/base/EventLog.java",
"android/java/src/org/chromium/base/FeatureList.java",
"android/java/src/org/chromium/base/FieldTrialList.java", "android/java/src/org/chromium/base/FieldTrialList.java",
"android/java/src/org/chromium/base/FileUtils.java", "android/java/src/org/chromium/base/FileUtils.java",
"android/java/src/org/chromium/base/Function.java", "android/java/src/org/chromium/base/Function.java",
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// 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 androidx.annotation.VisibleForTesting;
import java.util.Map;
/**
* Provides shared capabilities for feature flag support.
*/
public class FeatureList {
/** Map that stores substitution feature flags for tests. */
private static @Nullable Map<String, Boolean> sTestFeatures;
/** Access to default values of the native feature flag. */
private static boolean sTestCanUseDefaults;
private FeatureList() {}
/**
* This is called explicitly for instrumentation tests via Features#applyForInstrumentation().
* Unit tests and Robolectric tests must not invoke this and should rely on the {@link Features}
* annotations to enable or disable any feature flags.
*/
@VisibleForTesting
public static void setTestCanUseDefaultsForTesting() {
sTestCanUseDefaults = true;
}
/**
* We reset the value to false after the instrumentation test to avoid any unwanted
* persistence of the state. This is invoked by Features#reset().
*/
@VisibleForTesting
public static void resetTestCanUseDefaultsForTesting() {
sTestCanUseDefaults = false;
}
/**
* Sets the feature flags to use in JUnit tests, since native calls are not available there.
*/
@VisibleForTesting
public static void setTestFeatures(Map<String, Boolean> testFeatures) {
sTestFeatures = testFeatures;
}
/**
* @return Whether test feature values have been configured.
*/
@VisibleForTesting
public static boolean hasTestFeatures() {
return sTestFeatures != null;
}
/**
* Returns the test value of the feature with the given name.
*
* @param featureName The name of the feature to query.
* @return The test value set for the feature, or null if no test value has been set.
* @throws IllegalArgumentException if no test value was set and default values aren't allowed.
*/
@VisibleForTesting
public static Boolean getTestValueForFeature(String featureName) {
if (hasTestFeatures()) {
if (sTestFeatures.containsKey(featureName)) {
return sTestFeatures.get(featureName);
}
if (!sTestCanUseDefaults) {
throw new IllegalArgumentException("No test value configured for " + featureName);
}
}
return null;
}
}
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.flags; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.flags;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.FeatureList;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.MainDex; import org.chromium.base.annotations.MainDex;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
...@@ -26,44 +27,34 @@ import java.util.Map; ...@@ -26,44 +27,34 @@ import java.util.Map;
@JNINamespace("chrome::android") @JNINamespace("chrome::android")
@MainDex @MainDex
public abstract class ChromeFeatureList { public abstract class ChromeFeatureList {
/** Map that stores substitution feature flags for tests. */
private static Map<String, Boolean> sTestFeatures;
/** Prevent instantiation. */ /** Prevent instantiation. */
private ChromeFeatureList() {} private ChromeFeatureList() {}
/** Access to default values of the native chrome feature flag. */
private static boolean sTestCanUseDefaults;
/** /**
* This is called explicitly for instrumentation tests via Features#applyForInstrumentation(). * @see FeatureList#setTestCanUseDefaultsForTesting
* Unit tests and Robolectric tests must not invoke this and should rely on the {@link Features}
* annotations to enable or disable any feature flags.
*/ */
// TODO(crbug.com/1060097): Migrate callers to the FeatureList equivalent function.
@VisibleForTesting @VisibleForTesting
public static void setTestCanUseDefaultsForTesting() { public static void setTestCanUseDefaultsForTesting() {
sTestCanUseDefaults = true; FeatureList.setTestCanUseDefaultsForTesting();
} }
/** /**
* We reset the value to false after the instrumentation test to avoid any unwanted * @see FeatureList#resetTestCanUseDefaultsForTesting
* persistence of the state. This is invoked by Features#reset().
*/ */
// TODO(crbug.com/1060097): Migrate callers to the FeatureList equivalent function.
@VisibleForTesting @VisibleForTesting
public static void resetTestCanUseDefaultsForTesting() { public static void resetTestCanUseDefaultsForTesting() {
sTestCanUseDefaults = false; FeatureList.resetTestCanUseDefaultsForTesting();
} }
/** /**
* Sets the feature flags to use in JUnit tests, since native calls are not available there. * @see FeatureList#setTestFeatures
* Do not use directly, prefer using the {@link Features} annotation.
*
* @see Features
* @see Features.Processor
*/ */
// TODO(crbug.com/1060097): Migrate callers to the FeatureList equivalent function.
@VisibleForTesting @VisibleForTesting
public static void setTestFeatures(Map<String, Boolean> features) { public static void setTestFeatures(Map<String, Boolean> features) {
sTestFeatures = features; FeatureList.setTestFeatures(features);
} }
/** /**
...@@ -72,7 +63,7 @@ public abstract class ChromeFeatureList { ...@@ -72,7 +63,7 @@ public abstract class ChromeFeatureList {
* in tests if test features have been set). * in tests if test features have been set).
*/ */
public static boolean isInitialized() { public static boolean isInitialized() {
if (sTestFeatures != null) return true; if (FeatureList.hasTestFeatures()) return true;
return isNativeInitialized(); return isNativeInitialized();
} }
...@@ -120,13 +111,9 @@ public abstract class ChromeFeatureList { ...@@ -120,13 +111,9 @@ public abstract class ChromeFeatureList {
* @return Whether the feature is enabled or not. * @return Whether the feature is enabled or not.
*/ */
public static boolean isEnabled(String featureName) { public static boolean isEnabled(String featureName) {
/** FeatureFlags set for testing override the native default value. */ // FeatureFlags set for testing override the native default value.
if (sTestFeatures != null) { Boolean testValue = FeatureList.getTestValueForFeature(featureName);
Boolean enabled = sTestFeatures.get(featureName); if (testValue != null) return testValue;
if (enabled != null) return enabled;
if (!sTestCanUseDefaults) throw new IllegalArgumentException(featureName);
}
return isEnabledInNative(featureName); return isEnabledInNative(featureName);
} }
...@@ -142,7 +129,7 @@ public abstract class ChromeFeatureList { ...@@ -142,7 +129,7 @@ public abstract class ChromeFeatureList {
* the specified parameter does not exist. * the specified parameter does not exist.
*/ */
public static String getFieldTrialParamByFeature(String featureName, String paramName) { public static String getFieldTrialParamByFeature(String featureName, String paramName) {
if (sTestFeatures != null) return ""; if (FeatureList.hasTestFeatures()) return "";
assert isInitialized(); assert isInitialized();
return ChromeFeatureListJni.get().getFieldTrialParamByFeature(featureName, paramName); return ChromeFeatureListJni.get().getFieldTrialParamByFeature(featureName, paramName);
} }
...@@ -161,7 +148,7 @@ public abstract class ChromeFeatureList { ...@@ -161,7 +148,7 @@ public abstract class ChromeFeatureList {
*/ */
public static int getFieldTrialParamByFeatureAsInt( public static int getFieldTrialParamByFeatureAsInt(
String featureName, String paramName, int defaultValue) { String featureName, String paramName, int defaultValue) {
if (sTestFeatures != null) return defaultValue; if (FeatureList.hasTestFeatures()) return defaultValue;
assert isInitialized(); assert isInitialized();
return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsInt( return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsInt(
featureName, paramName, defaultValue); featureName, paramName, defaultValue);
...@@ -181,7 +168,7 @@ public abstract class ChromeFeatureList { ...@@ -181,7 +168,7 @@ public abstract class ChromeFeatureList {
*/ */
public static double getFieldTrialParamByFeatureAsDouble( public static double getFieldTrialParamByFeatureAsDouble(
String featureName, String paramName, double defaultValue) { String featureName, String paramName, double defaultValue) {
if (sTestFeatures != null) return defaultValue; if (FeatureList.hasTestFeatures()) return defaultValue;
assert isInitialized(); assert isInitialized();
return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsDouble( return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsDouble(
featureName, paramName, defaultValue); featureName, paramName, defaultValue);
...@@ -201,7 +188,7 @@ public abstract class ChromeFeatureList { ...@@ -201,7 +188,7 @@ public abstract class ChromeFeatureList {
*/ */
public static boolean getFieldTrialParamByFeatureAsBoolean( public static boolean getFieldTrialParamByFeatureAsBoolean(
String featureName, String paramName, boolean defaultValue) { String featureName, String paramName, boolean defaultValue) {
if (sTestFeatures != null) return defaultValue; if (FeatureList.hasTestFeatures()) return defaultValue;
assert isInitialized(); assert isInitialized();
return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsBoolean( return ChromeFeatureListJni.get().getFieldTrialParamByFeatureAsBoolean(
featureName, paramName, defaultValue); featureName, paramName, defaultValue);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment