Commit a8c241f3 authored by Nate Fischer's avatar Nate Fischer Committed by Commit Bot

AW: support hardcoded flag values in dev UI

This adds support for giving flags a predetermined value. This won't
have any noticeable effect yet because the only flags with values are
finch-related, which are queried either:

- early in startup, before dev UI applis flags
  (https://crbug.com/1059131), or
- in the :webview_service process, which the flag UI doesn't currently
  support (https://crbug.com/1058189)

This does not support applying parameters to base::Features, since this
isn't supported through the basic base::Feature API (it's Finch-specific
functionality).

Bug: 1047493
Test: run_android_webview_junit_tests -f *FlagOverrideHelperTest#*
Change-Id: I773014bea72b4b7156d9e094b431f4fc1e7d0257
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090882
Auto-Submit: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747902}
parent 31063c9e
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package org.chromium.android_webview.common; package org.chromium.android_webview.common;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
/** /**
* Class to represent a commandline flag. This is used by the developer UI to pass flags-to-toggle * Class to represent a commandline flag. This is used by the developer UI to pass flags-to-toggle
...@@ -13,29 +14,40 @@ import androidx.annotation.NonNull; ...@@ -13,29 +14,40 @@ import androidx.annotation.NonNull;
public class Flag { public class Flag {
private final String mName; private final String mName;
private final String mDescription; private final String mDescription;
private final String mEnabledStateValue;
private final boolean mIsBaseFeature; private final boolean mIsBaseFeature;
/** /**
* Creates a Flag which represents a {@code base::Feature}. * Creates a Flag which represents a {@code base::Feature}.
*/ */
public static Flag baseFeature(@NonNull String name, @NonNull String description) { public static Flag baseFeature(@NonNull String name, @NonNull String description) {
return new Flag(name, description, true); return new Flag(name, description, /*enabledStateValue=*/null, true);
} }
/** /**
* Creates a Flag which represents a commandline switch. * Creates a Flag which represents a commandline switch.
*/ */
public static Flag commandLine(@NonNull String name, @NonNull String description) { public static Flag commandLine(@NonNull String name, @NonNull String description) {
return new Flag(name, description, false); return new Flag(name, description, /*enabledStateValue=*/null, false);
}
/**
* Creates a Flag which represents a commandline switch with a value applied when enabled.
*/
public static Flag commandLine(
@NonNull String name, @NonNull String description, @NonNull String enabledStateValue) {
return new Flag(name, description, enabledStateValue, false);
} }
/** /**
* Calls should use {@link #baseFeature(String, String)} or {@link * Calls should use {@link #baseFeature(String, String)} or {@link
* #commandLine(String, String)} instead. * #commandLine(String, String)} instead.
*/ */
private Flag(@NonNull String name, @NonNull String description, boolean isBaseFeature) { private Flag(@NonNull String name, @NonNull String description,
@Nullable String enabledStateValue, boolean isBaseFeature) {
mName = name; mName = name;
mDescription = description; mDescription = description;
mEnabledStateValue = enabledStateValue;
mIsBaseFeature = isBaseFeature; mIsBaseFeature = isBaseFeature;
} }
...@@ -49,6 +61,15 @@ public class Flag { ...@@ -49,6 +61,15 @@ public class Flag {
return mDescription; return mDescription;
} }
/**
* Fetch the value to apply to the flag when enabled, or {@code null} if the flag doesn't take a
* value.
*/
@Nullable
public String getEnabledStateValue() {
return mEnabledStateValue;
}
/** /**
* Indicates whether this is a {@code base::Feature} or a commandline flag. * Indicates whether this is a {@code base::Feature} or a commandline flag.
* *
......
...@@ -73,7 +73,10 @@ public class FlagOverrideHelper { ...@@ -73,7 +73,10 @@ public class FlagOverrideHelper {
disabledFeatures.add(flag.getName()); disabledFeatures.add(flag.getName());
} }
} else { } else {
if (enabled) { if (enabled && flag.getEnabledStateValue() != null) {
CommandLine.getInstance().appendSwitchWithValue(
flag.getName(), flag.getEnabledStateValue());
} else if (enabled) {
CommandLine.getInstance().appendSwitch(flag.getName()); CommandLine.getInstance().appendSwitch(flag.getName());
} else { } else {
CommandLine.getInstance().removeSwitch(flag.getName()); CommandLine.getInstance().removeSwitch(flag.getName());
......
...@@ -30,15 +30,16 @@ public final class ProductionSupportedFlagList { ...@@ -30,15 +30,16 @@ public final class ProductionSupportedFlagList {
Flag.commandLine("show-composited-layer-borders", Flag.commandLine("show-composited-layer-borders",
"Renders a border around compositor layers to help debug and study layer " "Renders a border around compositor layers to help debug and study layer "
+ "compositing."), + "compositing."),
Flag.commandLine(AwSwitches.FINCH_SEED_EXPIRATION_AGE + "=0", Flag.commandLine(AwSwitches.FINCH_SEED_EXPIRATION_AGE,
"Forces all variations seeds to be considered stale."), "Forces all variations seeds to be considered stale.", "0"),
Flag.commandLine(AwSwitches.FINCH_SEED_IGNORE_PENDING_DOWNLOAD, Flag.commandLine(AwSwitches.FINCH_SEED_IGNORE_PENDING_DOWNLOAD,
"Forces the WebView service to reschedule a variations seed download job even " "Forces the WebView service to reschedule a variations seed download job even "
+ "if one is already pending."), + "if one is already pending."),
Flag.commandLine(AwSwitches.FINCH_SEED_MIN_DOWNLOAD_PERIOD + "=0", Flag.commandLine(AwSwitches.FINCH_SEED_MIN_DOWNLOAD_PERIOD,
"Disables throttling of variations seed download jobs."), "Disables throttling of variations seed download jobs.", "0"),
Flag.commandLine(AwSwitches.FINCH_SEED_MIN_UPDATE_PERIOD + "=0", Flag.commandLine(AwSwitches.FINCH_SEED_MIN_UPDATE_PERIOD,
"Disables throttling of new variations seed requests to the WebView service."), "Disables throttling of new variations seed requests to the WebView service.",
"0"),
Flag.commandLine("webview-log-js-console-messages", Flag.commandLine("webview-log-js-console-messages",
"Mirrors JavaScript console messages to system logs."), "Mirrors JavaScript console messages to system logs."),
Flag.commandLine(AwSwitches.CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH, Flag.commandLine(AwSwitches.CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH,
......
...@@ -39,6 +39,7 @@ public class FlagOverrideHelperTest { ...@@ -39,6 +39,7 @@ public class FlagOverrideHelperTest {
private static final Flag[] sMockFlagList = { private static final Flag[] sMockFlagList = {
Flag.commandLine("flag-1", "This is flag 1"), Flag.commandLine("flag-1", "This is flag 1"),
Flag.commandLine("flag-2", "This is flag 2"), Flag.commandLine("flag-2", "This is flag 2"),
Flag.commandLine("flag-3", "This is flag 3", "some-value"),
Flag.baseFeature("feature-1", "This is feature 1"), Flag.baseFeature("feature-1", "This is feature 1"),
Flag.baseFeature("feature-2", "This is feature 2"), Flag.baseFeature("feature-2", "This is feature 2"),
}; };
...@@ -105,6 +106,30 @@ public class FlagOverrideHelperTest { ...@@ -105,6 +106,30 @@ public class FlagOverrideHelperTest {
CommandLine.getInstance().hasSwitch("flag-1")); CommandLine.getInstance().hasSwitch("flag-1"));
} }
@Test
@SmallTest
public void testAddFlag_noValue() {
Map<String, Boolean> map = new HashMap<>();
map.put("flag-1", true);
FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
helper.applyFlagOverrides(map);
Assert.assertTrue("The 'flag-1' commandline flag should be applied",
CommandLine.getInstance().hasSwitch("flag-1"));
Assert.assertNull("The 'flag-1' commandline flag should not have a value",
CommandLine.getInstance().getSwitchValue("flag-1"));
}
@Test
@SmallTest
public void testAddFlag_withValue() {
Map<String, Boolean> map = new HashMap<>();
map.put("flag-3", true);
FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
helper.applyFlagOverrides(map);
Assert.assertEquals("The 'flag-3' commandline flag should have a value", "some-value",
CommandLine.getInstance().getSwitchValue("flag-3"));
}
@Test @Test
@SmallTest @SmallTest
public void testRemoveFlag_notYetAdded() { public void testRemoveFlag_notYetAdded() {
......
...@@ -162,7 +162,11 @@ public class FlagsActivity extends Activity { ...@@ -162,7 +162,11 @@ public class FlagsActivity extends Activity {
Spinner flagToggle = view.findViewById(R.id.flag_toggle); Spinner flagToggle = view.findViewById(R.id.flag_toggle);
Flag flag = getItem(position); Flag flag = getItem(position);
flagName.setText(flag.getName()); String label = flag.getName();
if (flag.getEnabledStateValue() != null) {
label += "=" + flag.getEnabledStateValue();
}
flagName.setText(label);
flagDescription.setText(flag.getDescription()); flagDescription.setText(flag.getDescription());
ArrayAdapter<String> adapter = ArrayAdapter<String> adapter =
new ArrayAdapter<>(FlagsActivity.this, R.layout.flag_states, sFlagStates); new ArrayAdapter<>(FlagsActivity.this, R.layout.flag_states, sFlagStates);
......
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