AW: add helper class for applying batches of flags

No change to production logic.

This adds FlagOverrideHelper to help apply batches of commandline flags
and base::Features to the CommandLine singleton. It also is responsible
for mapping flag names to Flag instances, resolving a TODO. This class
is currently not used in production, but will be used in a future CL to
apply flag overrides indicated from the developer UI.

This adds robolectric unit tests to cover several corner cases.

Bug: 981143
Test: run_android_webview_junit_tests -f *FlagOverrideHelperTest#*
Change-Id: Ie982fc66aa62bf3d51ffe74c04c708b435157261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1952032
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722627}

android_webview/BUILD.gn

@@ -424,6 +424,7 @@ android_library("common_java") {
   java_files = [
     "java/src/org/chromium/android_webview/common/AwResource.java",
     "java/src/org/chromium/android_webview/common/Flag.java",
+    "java/src/org/chromium/android_webview/common/FlagOverrideHelper.java",
     "java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java",
     "java/src/org/chromium/android_webview/common/services/ServiceNames.java",
   ]

android_webview/java/src/org/chromium/android_webview/common/FlagOverrideHelper.java

+// Copyright 2019 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.android_webview.common;
+
+import android.text.TextUtils;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
+
+import org.chromium.base.CommandLine;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Helper class to apply multiple features/flags to the global CommandLine singleton. This class and
+ * its static methods assume the CommandLine has already been initialized.
+ */
+public class FlagOverrideHelper {
+    private Map<String, Flag> mFlagMap = new HashMap<>();
+
+    public FlagOverrideHelper(Flag[] flagList) {
+        for (Flag flag : flagList) {
+            mFlagMap.put(flag.getName(), flag);
+        }
+    }
+
+    @VisibleForTesting
+    public static List<String> getCommaDelimitedSwitchValue(String name) {
+        return CommandLine.getInstance().hasSwitch(name)
+                ? Arrays.asList(CommandLine.getInstance().getSwitchValue(name).split(","))
+                : new ArrayList<>();
+    }
+
+    @VisibleForTesting
+    public static void setCommaDelimitedSwitchValue(String name, @NonNull List<String> value) {
+        if (value.isEmpty()) {
+            CommandLine.getInstance().removeSwitch(name);
+        } else {
+            CommandLine.getInstance().appendSwitchWithValue(name, TextUtils.join(",", value));
+        }
+    }
+
+    /**
+     * Apply the flag overrides specified in {@code overrides} to the {@link CommandLine} singleton.
+     * The flag names in {@code overrides} must have been already declared in the {@code flagList}
+     * passed to this instance's constructor.
+     *
+     * @param overrides a Map of flag names to override state (enabled or disabled).
+     */
+    public void applyFlagOverrides(Map<String, Boolean> overrides) {
+        Set<String> enabledFeatures = new HashSet<>();
+        Set<String> disabledFeatures = new HashSet<>();
+        enabledFeatures.addAll(getCommaDelimitedSwitchValue("enable-features"));
+        disabledFeatures.addAll(getCommaDelimitedSwitchValue("disable-features"));
+
+        for (Map.Entry<String, Boolean> entry : overrides.entrySet()) {
+            Flag flag = getFlagForName(entry.getKey());
+            boolean enabled = entry.getValue();
+            if (flag.isBaseFeature()) {
+                if (enabled) {
+                    enabledFeatures.add(flag.getName());
+                    disabledFeatures.remove(flag.getName());
+                } else {
+                    enabledFeatures.remove(flag.getName());
+                    disabledFeatures.add(flag.getName());
+                }
+            } else {
+                if (enabled) {
+                    CommandLine.getInstance().appendSwitch(flag.getName());
+                } else {
+                    CommandLine.getInstance().removeSwitch(flag.getName());
+                }
+            }
+        }
+
+        setCommaDelimitedSwitchValue("enable-features", new ArrayList<>(enabledFeatures));
+        setCommaDelimitedSwitchValue("disable-features", new ArrayList<>(disabledFeatures));
+    }
+
+    /**
+     * Fetches a {@link Flag} corresponding to {@code name}, based on the Flag array passed to the
+     * constructor.
+     *
+     * @param name the name of the {@link Flag} to look up.
+     * @return the desired {@link Flag}.
+     * @throws RuntimeException if this cannot find {@code name} in the list.
+     */
+    private Flag getFlagForName(@NonNull String name) {
+        if (mFlagMap.containsKey(name)) {
+            return mFlagMap.get(name);
+        }
+        // This should not be reached.
+        throw new RuntimeException("Unable to find flag '" + name + "' in the list.");
+    }
+}

android_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java

@@ -29,7 +29,4 @@ public final class ProductionSupportedFlagList {
             Flag.commandLine("webview-log-js-console-messages",
                     "Mirrors JavaScript console messages to system logs."),
     };
-
-    // TODO(ntfschr): add a method to map String to Flag instance, when we support retrieving flags
-    // from the service.
 }

android_webview/junit/src/org/chromium/android_webview/robolectric/common/FlagOverrideHelperTest.java

+// Copyright 2019 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.android_webview.robolectric.common;
+
+import android.support.test.filters.SmallTest;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import org.chromium.android_webview.common.Flag;
+import org.chromium.android_webview.common.FlagOverrideHelper;
+import org.chromium.base.CommandLine;
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Unit tests for FlagOverrideHelper.
+ */
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class FlagOverrideHelperTest {
+    @Before
+    public void setUp() {
+        // Create a fresh CommandLine instance for each test.
+        CommandLine.init(null);
+    }
+
+    private static final Flag[] sMockFlagList = {
+            Flag.commandLine("flag-1", "This is flag 1"),
+            Flag.commandLine("flag-2", "This is flag 2"),
+            Flag.baseFeature("feature-1", "This is feature 1"),
+            Flag.baseFeature("feature-2", "This is feature 2"),
+    };
+
+    private <T> Set<T> arrayToSet(T... array) {
+        return new HashSet<>(Arrays.asList(array));
+    }
+
+    private void assertFeaturesHelper(Set<String> expectedFeaturesSet, boolean enabled) {
+        Set<String> actualFeaturesSet =
+                new HashSet<>(FlagOverrideHelper.getCommaDelimitedSwitchValue(
+                        enabled ? "enable-features" : "disable-features"));
+        Assert.assertEquals(
+                String.format("Wrong set of %s features", enabled ? "enabled" : "disabled"),
+                expectedFeaturesSet, actualFeaturesSet);
+    }
+
+    private void assertEnabledFeatures(Set<String> expectedFeaturesSet) {
+        assertFeaturesHelper(expectedFeaturesSet, true);
+    }
+
+    private void assertDisabledFeatures(Set<String> expectedFeaturesSet) {
+        assertFeaturesHelper(expectedFeaturesSet, false);
+    }
+
+    @Test
+    @SmallTest
+    public void testGetCommaDelimitedSwitchValue() {
+        CommandLine.getInstance().appendSwitchWithValue("foo", "val1,val2");
+        List<String> values = FlagOverrideHelper.getCommaDelimitedSwitchValue("foo");
+        Assert.assertEquals(Arrays.asList("val1", "val2"), values);
+    }
+
+    @Test
+    @SmallTest
+    public void testSetCommaDelimitedSwitchValue() {
+        FlagOverrideHelper.setCommaDelimitedSwitchValue("foo", Arrays.asList("val1", "val2"));
+        Assert.assertEquals("val1,val2", CommandLine.getInstance().getSwitchValue("foo"));
+    }
+
+    @Test
+    @SmallTest
+    public void testUnknownFlag() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("unknown-flag", true);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        try {
+            helper.applyFlagOverrides(map); // should throw an exception
+
+            Assert.fail("Expected a RuntimeException for 'unknown-flag'");
+        } catch (RuntimeException e) {
+            // This is expected.
+        }
+    }
+
+    @Test
+    @SmallTest
+    public void testAddFlag() {
+        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"));
+    }
+
+    @Test
+    @SmallTest
+    public void testRemoveFlag_notYetAdded() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("flag-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+        Assert.assertFalse("The 'flag-2' commandline flag should not be applied",
+                CommandLine.getInstance().hasSwitch("flag-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testRemoveFlag_alreadyAdded() {
+        CommandLine.getInstance().appendSwitch("flag-2");
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("flag-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+        Assert.assertFalse("The 'flag-2' commandline flag should not be applied",
+                CommandLine.getInstance().hasSwitch("flag-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testEnableBaseFeature_notYetEnabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-1", true);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet("feature-1"));
+        assertDisabledFeatures(arrayToSet());
+    }
+
+    @Test
+    @SmallTest
+    public void testEnableBaseFeature_multiple() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-1", true);
+        map.put("feature-2", true);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet("feature-1", "feature-2"));
+        assertDisabledFeatures(arrayToSet());
+    }
+
+    @Test
+    @SmallTest
+    public void testEnableBaseFeature_alreadyEnabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-1", true);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        CommandLine.getInstance().appendSwitchWithValue("enable-features", "feature-1");
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet("feature-1"));
+        assertDisabledFeatures(arrayToSet());
+    }
+
+    @Test
+    @SmallTest
+    public void testEnableBaseFeature_alreadyDisabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-1", true);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        CommandLine.getInstance().appendSwitchWithValue("disable-features", "feature-1");
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet("feature-1"));
+        assertDisabledFeatures(arrayToSet());
+    }
+
+    @Test
+    @SmallTest
+    public void testDisableBaseFeature_notYetEnabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet());
+        assertDisabledFeatures(arrayToSet("feature-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testDisableBaseFeature_multiple() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-1", false);
+        map.put("feature-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet());
+        assertDisabledFeatures(arrayToSet("feature-1", "feature-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testDisableBaseFeature_alreadyEnabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        CommandLine.getInstance().appendSwitchWithValue("enable-features", "feature-2");
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet());
+        assertDisabledFeatures(arrayToSet("feature-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testDisableBaseFeature_alreadyDisabled() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("feature-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        CommandLine.getInstance().appendSwitchWithValue("disable-features", "feature-2");
+        helper.applyFlagOverrides(map);
+
+        assertEnabledFeatures(arrayToSet());
+        assertDisabledFeatures(arrayToSet("feature-2"));
+    }
+
+    @Test
+    @SmallTest
+    public void testLotsOfFlagsAndFeatures() {
+        Map<String, Boolean> map = new HashMap<>();
+        map.put("flag-1", true);
+        map.put("flag-2", false);
+        map.put("feature-1", true);
+        map.put("feature-2", false);
+        FlagOverrideHelper helper = new FlagOverrideHelper(sMockFlagList);
+        helper.applyFlagOverrides(map);
+
+        Assert.assertTrue("The 'flag-1' commandline flag should be applied",
+                CommandLine.getInstance().hasSwitch("flag-1"));
+        Assert.assertFalse("The 'flag-2' commandline flag should not be applied",
+                CommandLine.getInstance().hasSwitch("flag-2"));
+        assertEnabledFeatures(arrayToSet("feature-1"));
+        assertDisabledFeatures(arrayToSet("feature-2"));
+    }
+}

android_webview/test/BUILD.gn

@@ -452,6 +452,7 @@ junit_binary("android_webview_junit_tests") {
     "../junit/src/org/chromium/android_webview/robolectric/AwLayoutSizerTest.java",
     "../junit/src/org/chromium/android_webview/robolectric/FindAddressTest.java",
     "../junit/src/org/chromium/android_webview/robolectric/AwScrollOffsetManagerTest.java",
+    "../junit/src/org/chromium/android_webview/robolectric/common/FlagOverrideHelperTest.java",
     "../junit/src/org/chromium/android_webview/robolectric/common/services/ServiceNamesTest.java",
     "../junit/src/org/chromium/android_webview/robolectric/services/AwVariationsSeedFetcherTest.java",
   ]