Commit 79fe0a2a authored by Joon Ahn's avatar Joon Ahn Committed by Commit Bot

Reland "flags: Add support for FEATURE_WITH_PARAMS_VALUE_TYPE feature in switches"

Add escapes for special characters in param names and values.

This is a reland of efbdaef4

Original change's description:
> flags: Add support for FEATURE_WITH_PARAMS_VALUE_TYPE feature in switches
>
> Fixed:805766
> Test:autoninja -C out/mychrome components_tests && ./out/mychrome
> components_tests
> TESTED=autoninja -C out_hatch/Release chrome && deploy_chrome
> --build-dir=out_hatch/Release --device=$DUT
>
> Verified by setting a feature with param in chrome://flags on hatch
>
> Change-Id: I83f8c81237c3db50a4f23bf5b17e73363bf10504
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401336
> Commit-Queue: Weilun Shi <sweilun@chromium.org>
> Reviewed-by: Weilun Shi <sweilun@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#806278}

Change-Id: If059b946bd8596aebec440e14e5f6a8a66b222da
Fixed: 1130035
Bug: 805766
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419636
Auto-Submit: Joon Ahn <joonbug@chromium.org>
Reviewed-by: default avatarWeilun Shi <sweilun@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808975}
parent 6b84d337
......@@ -27,6 +27,7 @@ static_library("flags_ui") {
"//components/prefs",
"//components/strings",
"//components/variations",
"//components/variations/field_trial_config",
"//ui/base",
"//url",
]
......
......@@ -23,6 +23,7 @@
#include "components/flags_ui/feature_entry.h"
#include "components/flags_ui/flags_storage.h"
#include "components/flags_ui/flags_ui_switches.h"
#include "components/variations/field_trial_config/field_trial_util.h"
#include "components/variations/variations_associated_data.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h"
......@@ -927,10 +928,27 @@ void FlagsState::GenerateFlagsToSwitchesMapping(
AddFeatureMapping(entry.NameForOption(j), std::string(), false,
name_to_switch_map);
} else {
AddFeatureMapping(entry.NameForOption(j),
entry.feature.feature->name,
state == FeatureEntry::FeatureState::ENABLED,
name_to_switch_map);
const FeatureEntry::FeatureVariation* variation =
entry.VariationForOption(j);
std::string feature_name(entry.feature.feature->name);
std::vector<std::string> params_value;
if (variation) {
feature_name.append(":");
for (int i = 0; i < variation->num_params; ++i) {
std::string param_name =
variations::EscapeValue(variation->params[i].param_name);
std::string param_value =
variations::EscapeValue(variation->params[i].param_value);
params_value.push_back(
param_name.append("/").append(param_value));
}
}
AddFeatureMapping(
entry.NameForOption(j),
feature_name.append(base::JoinString(params_value, "/")),
state == FeatureEntry::FeatureState::ENABLED,
name_to_switch_map);
}
}
break;
......
......@@ -47,6 +47,7 @@ const char kFlags8[] = "flag8";
const char kFlags9[] = "flag9";
const char kFlags10[] = "flag10";
const char kFlags11[] = "flag11";
const char kFlags12[] = "flag12";
const char kSwitch1[] = "switch";
const char kSwitch2[] = "switch2";
......@@ -73,22 +74,36 @@ const char kDummySentinelEndSwitch[] = "dummy-end";
const char kTestTrial[] = "TestTrial";
const char kTestParam1[] = "param1";
const char kTestParam2[] = "param2";
const char kTestParam3[] = "param:/3";
const char kTestParamValue[] = "value";
const base::Feature kTestFeature1{"FeatureName1",
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kTestFeature2{"FeatureName2",
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kTestFeature3{"FeatureName3",
base::FEATURE_DISABLED_BY_DEFAULT};
const FeatureEntry::FeatureParam kTestVariationOther1[] = {
{kTestParam1, kTestParamValue}};
const FeatureEntry::FeatureParam kTestVariationOther2[] = {
{kTestParam2, kTestParamValue}};
const FeatureEntry::FeatureParam kTestVariationOther3[] = {
{kTestParam1, kTestParamValue},
{kTestParam3, kTestParamValue},
};
const FeatureEntry::FeatureVariation kTestVariations1[] = {
{"dummy description 1", kTestVariationOther1, 1, nullptr}};
const FeatureEntry::FeatureVariation kTestVariations2[] = {
{"dummy description 2", kTestVariationOther2, 1, nullptr}};
const FeatureEntry::FeatureVariation kTestVariations3[] = {
{"dummy description 1", kTestVariationOther1, 1, nullptr},
{"dummy description 2", kTestVariationOther2, 1, nullptr},
{"dummy description 3", kTestVariationOther3, 2, nullptr}};
const char kTestVariation3Cmdline[] =
"FeatureName3:param1/value/param%3A%2F3/value";
const char kDummyName[] = "";
const char kDummyDescription[] = "";
......@@ -148,7 +163,12 @@ static FeatureEntry kEntries[] = {
kTestTrial)},
{kFlags11, kDummyName, kDummyDescription,
0, // Ends up being mapped to the current platform.
ORIGIN_LIST_VALUE_TYPE(kStringSwitch, kValueForStringSwitch)}};
ORIGIN_LIST_VALUE_TYPE(kStringSwitch, kValueForStringSwitch)},
{kFlags12, kDummyName, kDummyDescription,
0, // Ends up being mapped to the current platform.
FEATURE_WITH_PARAMS_VALUE_TYPE(kTestFeature3,
kTestVariations3,
kTestTrial)}};
class FlagsStateTest : public ::testing::Test,
public flags_ui::FlagsState::Delegate {
......@@ -293,6 +313,19 @@ TEST_F(FlagsStateTest, ConvertFlagsToSwitches) {
EXPECT_TRUE(command_line2.HasSwitch(kSwitch1));
EXPECT_FALSE(command_line2.HasSwitch(switches::kFlagSwitchesBegin));
EXPECT_FALSE(command_line2.HasSwitch(switches::kFlagSwitchesEnd));
base::CommandLine command_line3(base::CommandLine::NO_PROGRAM);
// Enable 3rd variation (@4 since 0 is enable).
flags_state_->SetFeatureEntryEnabled(
&flags_storage_, std::string(kFlags12).append("@4"), true);
flags_state_->ConvertFlagsToSwitches(&flags_storage_, &command_line3,
kNoSentinels, kEnableFeatures,
kDisableFeatures);
EXPECT_TRUE(command_line3.HasSwitch(kEnableFeatures));
EXPECT_EQ(command_line3.GetSwitchValueASCII(kEnableFeatures),
kTestVariation3Cmdline);
}
TEST_F(FlagsStateTest, RegisterAllFeatureVariationParameters) {
......@@ -913,7 +946,7 @@ TEST_F(FlagsStateTest, GetFlagFeatureEntries) {
&supported_entries, &unsupported_entries,
base::BindRepeating(&SkipFeatureEntry));
// All |kEntries| except for |kFlags3| should be supported.
EXPECT_EQ(10u, supported_entries.GetSize());
EXPECT_EQ(11u, supported_entries.GetSize());
EXPECT_EQ(1u, unsupported_entries.GetSize());
EXPECT_EQ(base::size(kEntries),
supported_entries.GetSize() + unsupported_entries.GetSize());
......
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