Commit 8423d172 authored by asvitkine's avatar asvitkine Committed by Commit bot

Expand FeatureList to support FieldTrial association.

This CL adds the following:
  - Two new APIs on FeatureList to be used during initialization. One to
associate a field trial for reporting purposes when the feature is forced
from the command line and the other to override the feature state via a
field trial.
  - Passing the FeatureList instance to VariationsService during browser
start up.
  - Extension of VariationsService (and associated proto changes) to
invoke the two above APIs, when processing variations with the new
proto fields.
  - A new API on FieldTrial to get the group name of a field trial without
activating it, used by VariationsService when association a field trial.

BUG=526169

Review URL: https://codereview.chromium.org/1306653004

Cr-Commit-Position: refs/heads/master@{#351199}
parent e570329a
...@@ -4,9 +4,11 @@ ...@@ -4,9 +4,11 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include <utility>
#include <vector> #include <vector>
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/field_trial.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
namespace base { namespace base {
...@@ -37,13 +39,55 @@ void FeatureList::InitializeFromCommandLine( ...@@ -37,13 +39,55 @@ void FeatureList::InitializeFromCommandLine(
// Process disabled features first, so that disabled ones take precedence over // Process disabled features first, so that disabled ones take precedence over
// enabled ones (since RegisterOverride() uses insert()). // enabled ones (since RegisterOverride() uses insert()).
for (const auto& feature_name : SplitFeatureListString(disable_features)) { for (const auto& feature_name : SplitFeatureListString(disable_features)) {
RegisterOverride(feature_name, OVERRIDE_DISABLE_FEATURE); RegisterOverride(feature_name, OVERRIDE_DISABLE_FEATURE, nullptr);
} }
for (const auto& feature_name : SplitFeatureListString(enable_features)) { for (const auto& feature_name : SplitFeatureListString(enable_features)) {
RegisterOverride(feature_name, OVERRIDE_ENABLE_FEATURE); RegisterOverride(feature_name, OVERRIDE_ENABLE_FEATURE, nullptr);
} }
} }
bool FeatureList::IsFeatureOverriddenFromCommandLine(
const std::string& feature_name,
OverrideState state) const {
auto it = overrides_.find(feature_name);
return it != overrides_.end() && it->second.overridden_state == state &&
!it->second.overridden_by_field_trial;
}
void FeatureList::RegisterFieldTrialOverride(const std::string& feature_name,
OverrideState override_state,
FieldTrial* field_trial) {
DCHECK(field_trial);
DCHECK(!ContainsKey(overrides_, feature_name) ||
!overrides_.find(feature_name)->second.field_trial)
<< "Feature " << feature_name
<< " has conflicting field trial overrides: "
<< overrides_.find(feature_name)->second.field_trial->trial_name()
<< " / " << field_trial->trial_name();
RegisterOverride(feature_name, override_state, field_trial);
}
void FeatureList::AssociateReportingFieldTrial(
const std::string& feature_name,
OverrideState for_overridden_state,
FieldTrial* field_trial) {
DCHECK(
IsFeatureOverriddenFromCommandLine(feature_name, for_overridden_state));
// Only one associated field trial is supported per feature. This is generally
// enforced server-side.
OverrideEntry* entry = &overrides_.find(feature_name)->second;
if (entry->field_trial) {
NOTREACHED() << "Feature " << feature_name
<< " already has trial: " << entry->field_trial->trial_name()
<< ", associating trial: " << field_trial->trial_name();
return;
}
entry->field_trial = field_trial;
}
// static // static
bool FeatureList::IsEnabled(const Feature& feature) { bool FeatureList::IsEnabled(const Feature& feature) {
return GetInstance()->IsFeatureEnabled(feature); return GetInstance()->IsFeatureEnabled(feature);
...@@ -81,6 +125,11 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) { ...@@ -81,6 +125,11 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) {
auto it = overrides_.find(feature.name); auto it = overrides_.find(feature.name);
if (it != overrides_.end()) { if (it != overrides_.end()) {
const OverrideEntry& entry = it->second; const OverrideEntry& entry = it->second;
// Activate the corresponding field trial, if necessary.
if (entry.field_trial)
entry.field_trial->group();
// TODO(asvitkine) Expand this section as more support is added. // TODO(asvitkine) Expand this section as more support is added.
return entry.overridden_state == OVERRIDE_ENABLE_FEATURE; return entry.overridden_state == OVERRIDE_ENABLE_FEATURE;
} }
...@@ -89,9 +138,14 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) { ...@@ -89,9 +138,14 @@ bool FeatureList::IsFeatureEnabled(const Feature& feature) {
} }
void FeatureList::RegisterOverride(const std::string& feature_name, void FeatureList::RegisterOverride(const std::string& feature_name,
OverrideState overridden_state) { OverrideState overridden_state,
FieldTrial* field_trial) {
DCHECK(!initialized_); DCHECK(!initialized_);
overrides_.insert(make_pair(feature_name, OverrideEntry(overridden_state))); // Note: The semantics of insert() is that it does not overwrite the entry if
// one already exists for the key. Thus, only the first override for a given
// feature name takes effect.
overrides_.insert(std::make_pair(
feature_name, OverrideEntry(overridden_state, field_trial)));
} }
bool FeatureList::CheckFeatureIdentity(const Feature& feature) { bool FeatureList::CheckFeatureIdentity(const Feature& feature) {
...@@ -107,7 +161,10 @@ bool FeatureList::CheckFeatureIdentity(const Feature& feature) { ...@@ -107,7 +161,10 @@ bool FeatureList::CheckFeatureIdentity(const Feature& feature) {
return it->second == &feature; return it->second == &feature;
} }
FeatureList::OverrideEntry::OverrideEntry(OverrideState overridden_state) FeatureList::OverrideEntry::OverrideEntry(OverrideState overridden_state,
: overridden_state(overridden_state) {} FieldTrial* field_trial)
: overridden_state(overridden_state),
field_trial(field_trial),
overridden_by_field_trial(field_trial != nullptr) {}
} // namespace base } // namespace base
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
namespace base { namespace base {
class FieldTrial;
// Specifies whether a given feature is enabled or disabled by default. // Specifies whether a given feature is enabled or disabled by default.
enum FeatureState { enum FeatureState {
FEATURE_DISABLED_BY_DEFAULT, FEATURE_DISABLED_BY_DEFAULT,
...@@ -83,6 +85,37 @@ class BASE_EXPORT FeatureList { ...@@ -83,6 +85,37 @@ class BASE_EXPORT FeatureList {
void InitializeFromCommandLine(const std::string& enable_features, void InitializeFromCommandLine(const std::string& enable_features,
const std::string& disable_features); const std::string& disable_features);
// Specifies whether a feature override enables or disables the feature.
enum OverrideState {
OVERRIDE_DISABLE_FEATURE,
OVERRIDE_ENABLE_FEATURE,
};
// Returns true if the state of |feature_name| has been overridden via
// |InitializeFromCommandLine()|.
bool IsFeatureOverriddenFromCommandLine(const std::string& feature_name,
OverrideState state) const;
// Associates a field trial for reporting purposes corresponding to the
// command-line setting the feature state to |for_overridden_state|. The trial
// will be activated when the state of the feature is first queried. This
// should be called during registration, after InitializeFromCommandLine() has
// been called but before the instance is registered via SetInstance().
void AssociateReportingFieldTrial(const std::string& feature_name,
OverrideState for_overridden_state,
FieldTrial* field_trial);
// Registers a field trial to override the enabled state of the specified
// feature to |override_state|. Command-line overrides still take precedence
// over field trials, so this will have no effect if the feature is being
// overridden from the command-line. The associated field trial will be
// activated when the feature state for this feature is queried. This should
// be called during registration, after InitializeFromCommandLine() has been
// called but before the instance is registered via SetInstance().
void RegisterFieldTrialOverride(const std::string& feature_name,
OverrideState override_state,
FieldTrial* field_trial);
// Returns whether the given |feature| is enabled. Must only be called after // Returns whether the given |feature| is enabled. Must only be called after
// the singleton instance has been registered via SetInstance(). Additionally, // the singleton instance has been registered via SetInstance(). Additionally,
// a feature with a given name must only have a single corresponding Feature // a feature with a given name must only have a single corresponding Feature
...@@ -103,10 +136,27 @@ class BASE_EXPORT FeatureList { ...@@ -103,10 +136,27 @@ class BASE_EXPORT FeatureList {
private: private:
FRIEND_TEST_ALL_PREFIXES(FeatureListTest, CheckFeatureIdentity); FRIEND_TEST_ALL_PREFIXES(FeatureListTest, CheckFeatureIdentity);
// Specifies whether a feature override enables or disables the feature. struct OverrideEntry {
enum OverrideState { // The overridden enable (on/off) state of the feature.
OVERRIDE_DISABLE_FEATURE, const OverrideState overridden_state;
OVERRIDE_ENABLE_FEATURE,
// An optional associated field trial, which will be activated when the
// state of the feature is queried for the first time. Weak pointer to the
// FieldTrial object that is owned by the FieldTrialList singleton.
base::FieldTrial* field_trial;
// Specifies whether the feature's state is overridden by |field_trial|.
// If it's not, and |field_trial| is not null, it means it is simply an
// associated field trial for reporting purposes (and |overridden_state|
// came from the command-line).
const bool overridden_by_field_trial;
// TODO(asvitkine): Expand this as more support is added.
// Constructs an OverrideEntry for the given |overridden_state|. If
// |field_trial| is not null, it implies that |overridden_state| comes from
// the trial, so |overridden_by_field_trial| will be set to true.
OverrideEntry(OverrideState overridden_state, FieldTrial* field_trial);
}; };
// Finalizes the initialization state of the FeatureList, so that no further // Finalizes the initialization state of the FeatureList, so that no further
...@@ -121,9 +171,14 @@ class BASE_EXPORT FeatureList { ...@@ -121,9 +171,14 @@ class BASE_EXPORT FeatureList {
// Registers an override for feature |feature_name|. The override specifies // Registers an override for feature |feature_name|. The override specifies
// whether the feature should be on or off (via |overridden_state|), which // whether the feature should be on or off (via |overridden_state|), which
// will take precedence over the feature's default state. // will take precedence over the feature's default state. If |field_trial| is
// not null, registers the specified field trial object to be associated with
// the feature, which will activate the field trial when the feature state is
// queried. If an override is already registered for the given feature, it
// will not be changed.
void RegisterOverride(const std::string& feature_name, void RegisterOverride(const std::string& feature_name,
OverrideState overridden_state); OverrideState overridden_state,
FieldTrial* field_trial);
// Verifies that there's only a single definition of a Feature struct for a // Verifies that there's only a single definition of a Feature struct for a
// given feature name. Keeps track of the first seen Feature struct for each // given feature name. Keeps track of the first seen Feature struct for each
...@@ -132,14 +187,6 @@ class BASE_EXPORT FeatureList { ...@@ -132,14 +187,6 @@ class BASE_EXPORT FeatureList {
// DCHECKs and tests. // DCHECKs and tests.
bool CheckFeatureIdentity(const Feature& feature); bool CheckFeatureIdentity(const Feature& feature);
struct OverrideEntry {
// The overridden enable (on/off) state of the feature.
const OverrideState overridden_state;
// TODO(asvitkine): Expand this as more support is added.
explicit OverrideEntry(OverrideState overridden_state);
};
// Map from feature name to an OverrideEntry struct for the feature, if it // Map from feature name to an OverrideEntry struct for the feature, if it
// exists. // exists.
std::map<std::string, OverrideEntry> overrides_; std::map<std::string, OverrideEntry> overrides_;
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/format_macros.h"
#include "base/metrics/field_trial.h"
#include "base/strings/stringprintf.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace base { namespace base {
...@@ -70,6 +73,9 @@ TEST_F(FeatureListTest, InitializeFromCommandLine) { ...@@ -70,6 +73,9 @@ TEST_F(FeatureListTest, InitializeFromCommandLine) {
for (size_t i = 0; i < arraysize(test_cases); ++i) { for (size_t i = 0; i < arraysize(test_cases); ++i) {
const auto& test_case = test_cases[i]; const auto& test_case = test_cases[i];
SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: [%s] [%s]", i,
test_case.enable_features,
test_case.disable_features));
ClearFeatureListInstance(); ClearFeatureListInstance();
scoped_ptr<FeatureList> feature_list(new FeatureList); scoped_ptr<FeatureList> feature_list(new FeatureList);
...@@ -105,4 +111,200 @@ TEST_F(FeatureListTest, CheckFeatureIdentity) { ...@@ -105,4 +111,200 @@ TEST_F(FeatureListTest, CheckFeatureIdentity) {
EXPECT_FALSE(feature_list()->CheckFeatureIdentity(kFeatureOnByDefault2)); EXPECT_FALSE(feature_list()->CheckFeatureIdentity(kFeatureOnByDefault2));
} }
TEST_F(FeatureListTest, FieldTrialOverrides) {
struct {
FeatureList::OverrideState trial1_state;
FeatureList::OverrideState trial2_state;
} test_cases[] = {
{FeatureList::OVERRIDE_DISABLE_FEATURE,
FeatureList::OVERRIDE_DISABLE_FEATURE},
{FeatureList::OVERRIDE_DISABLE_FEATURE,
FeatureList::OVERRIDE_ENABLE_FEATURE},
{FeatureList::OVERRIDE_ENABLE_FEATURE,
FeatureList::OVERRIDE_DISABLE_FEATURE},
{FeatureList::OVERRIDE_ENABLE_FEATURE,
FeatureList::OVERRIDE_ENABLE_FEATURE},
};
FieldTrial::ActiveGroup active_group;
for (size_t i = 0; i < arraysize(test_cases); ++i) {
const auto& test_case = test_cases[i];
SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]", i));
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
scoped_ptr<FeatureList> feature_list(new FeatureList);
FieldTrial* trial1 = FieldTrialList::CreateFieldTrial("TrialExample1", "A");
FieldTrial* trial2 = FieldTrialList::CreateFieldTrial("TrialExample2", "B");
feature_list->RegisterFieldTrialOverride(kFeatureOnByDefaultName,
test_case.trial1_state, trial1);
feature_list->RegisterFieldTrialOverride(kFeatureOffByDefaultName,
test_case.trial2_state, trial2);
RegisterFeatureListInstance(feature_list.Pass());
// Initially, neither trial should be active.
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial1->trial_name()));
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial2->trial_name()));
const bool expected_enabled_1 =
(test_case.trial1_state == FeatureList::OVERRIDE_ENABLE_FEATURE);
EXPECT_EQ(expected_enabled_1, FeatureList::IsEnabled(kFeatureOnByDefault));
// The above should have activated |trial1|.
EXPECT_TRUE(FieldTrialList::IsTrialActive(trial1->trial_name()));
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial2->trial_name()));
const bool expected_enabled_2 =
(test_case.trial2_state == FeatureList::OVERRIDE_ENABLE_FEATURE);
EXPECT_EQ(expected_enabled_2, FeatureList::IsEnabled(kFeatureOffByDefault));
// The above should have activated |trial2|.
EXPECT_TRUE(FieldTrialList::IsTrialActive(trial1->trial_name()));
EXPECT_TRUE(FieldTrialList::IsTrialActive(trial2->trial_name()));
}
}
TEST_F(FeatureListTest, CommandLineTakesPrecedenceOverFieldTrial) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
scoped_ptr<FeatureList> feature_list(new FeatureList);
// The feature is explicitly enabled on the command-line.
feature_list->InitializeFromCommandLine(kFeatureOffByDefaultName, "");
// But the FieldTrial would set the feature to disabled.
FieldTrial* trial = FieldTrialList::CreateFieldTrial("TrialExample2", "A");
feature_list->RegisterFieldTrialOverride(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
RegisterFeatureListInstance(feature_list.Pass());
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial->trial_name()));
// Command-line should take precedence.
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
// Since the feature is on due to the command-line, and not as a result of the
// field trial, the field trial should not be activated (since the Associate*
// API wasn't used.)
EXPECT_FALSE(FieldTrialList::IsTrialActive(trial->trial_name()));
}
TEST_F(FeatureListTest, IsFeatureOverriddenFromCommandLine) {
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
scoped_ptr<FeatureList> feature_list(new FeatureList);
// No features are overridden from the command line yet
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
// Now, enable |kFeatureOffByDefaultName| via the command-line.
feature_list->InitializeFromCommandLine(kFeatureOffByDefaultName, "");
// It should now be overridden for the enabled group.
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_TRUE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
// Register a field trial to associate with the feature and ensure that the
// results are still the same.
feature_list->AssociateReportingFieldTrial(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE,
FieldTrialList::CreateFieldTrial("Trial1", "A"));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_TRUE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
// Now, register a field trial to override |kFeatureOnByDefaultName| state
// and check that the function still returns false for that feature.
feature_list->RegisterFieldTrialOverride(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE,
FieldTrialList::CreateFieldTrial("Trial2", "A"));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
EXPECT_FALSE(feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOnByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
RegisterFeatureListInstance(feature_list.Pass());
// Check the expected feature states for good measure.
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOnByDefault));
}
TEST_F(FeatureListTest, AssociateReportingFieldTrial) {
struct {
const char* enable_features;
const char* disable_features;
bool expected_enable_trial_created;
bool expected_disable_trial_created;
} test_cases[] = {
// If no enable/disable flags are specified, no trials should be created.
{"", "", false, false},
// Enabling the feature should result in the enable trial created.
{kFeatureOffByDefaultName, "", true, false},
// Disabling the feature should result in the disable trial created.
{"", kFeatureOffByDefaultName, false, true},
};
const char kTrialName[] = "ForcingTrial";
const char kForcedOnGroupName[] = "ForcedOn";
const char kForcedOffGroupName[] = "ForcedOff";
for (size_t i = 0; i < arraysize(test_cases); ++i) {
const auto& test_case = test_cases[i];
SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: [%s] [%s]", i,
test_case.enable_features,
test_case.disable_features));
ClearFeatureListInstance();
FieldTrialList field_trial_list(nullptr);
scoped_ptr<FeatureList> feature_list(new FeatureList);
feature_list->InitializeFromCommandLine(test_case.enable_features,
test_case.disable_features);
FieldTrial* enable_trial = nullptr;
if (feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE)) {
enable_trial = base::FieldTrialList::CreateFieldTrial(kTrialName,
kForcedOnGroupName);
feature_list->AssociateReportingFieldTrial(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE,
enable_trial);
}
FieldTrial* disable_trial = nullptr;
if (feature_list->IsFeatureOverriddenFromCommandLine(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE)) {
disable_trial = base::FieldTrialList::CreateFieldTrial(
kTrialName, kForcedOffGroupName);
feature_list->AssociateReportingFieldTrial(
kFeatureOffByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE,
disable_trial);
}
EXPECT_EQ(test_case.expected_enable_trial_created, enable_trial != nullptr);
EXPECT_EQ(test_case.expected_disable_trial_created,
disable_trial != nullptr);
RegisterFeatureListInstance(feature_list.Pass());
EXPECT_FALSE(FieldTrialList::IsTrialActive(kTrialName));
if (disable_trial) {
EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOffByDefault));
EXPECT_TRUE(FieldTrialList::IsTrialActive(kTrialName));
EXPECT_EQ(kForcedOffGroupName, disable_trial->group_name());
} else if (enable_trial) {
EXPECT_TRUE(FeatureList::IsEnabled(kFeatureOffByDefault));
EXPECT_TRUE(FieldTrialList::IsTrialActive(kTrialName));
EXPECT_EQ(kForcedOnGroupName, enable_trial->group_name());
}
}
}
} // namespace base } // namespace base
...@@ -139,6 +139,11 @@ const std::string& FieldTrial::group_name() { ...@@ -139,6 +139,11 @@ const std::string& FieldTrial::group_name() {
return group_name_; return group_name_;
} }
const std::string& FieldTrial::GetGroupNameWithoutActivation() {
FinalizeGroupChoice();
return group_name_;
}
void FieldTrial::SetForced() { void FieldTrial::SetForced() {
// We might have been forced before (e.g., by CreateFieldTrial) and it's // We might have been forced before (e.g., by CreateFieldTrial) and it's
// first come first served, e.g., command line switch has precedence. // first come first served, e.g., command line switch has precedence.
......
...@@ -149,6 +149,11 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { ...@@ -149,6 +149,11 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
// is used as the group name. This causes a winner to be chosen if none was. // is used as the group name. This causes a winner to be chosen if none was.
const std::string& group_name(); const std::string& group_name();
// Finalizes the group choice and returns the chosen group, but does not mark
// the trial as active - so its state will not be reported until group_name()
// or similar is called.
const std::string& GetGroupNameWithoutActivation();
// Set the field trial as forced, meaning that it was setup earlier than // Set the field trial as forced, meaning that it was setup earlier than
// the hard coded registration of the field trial to override it. // the hard coded registration of the field trial to override it.
// This allows the code that was hard coded to register the field trial to // This allows the code that was hard coded to register the field trial to
......
...@@ -72,6 +72,8 @@ class FieldTrialTest : public testing::Test { ...@@ -72,6 +72,8 @@ class FieldTrialTest : public testing::Test {
private: private:
MessageLoop message_loop_; MessageLoop message_loop_;
FieldTrialList trial_list_; FieldTrialList trial_list_;
DISALLOW_COPY_AND_ASSIGN(FieldTrialTest);
}; };
// Test registration, and also check that destructors are called for trials // Test registration, and also check that destructors are called for trials
...@@ -376,6 +378,28 @@ TEST_F(FieldTrialTest, ActiveGroupsNotFinalized) { ...@@ -376,6 +378,28 @@ TEST_F(FieldTrialTest, ActiveGroupsNotFinalized) {
EXPECT_EQ(active_group.group_name, active_groups[0].group_name); EXPECT_EQ(active_group.group_name, active_groups[0].group_name);
} }
TEST_F(FieldTrialTest, GetGroupNameWithoutActivation) {
const char kTrialName[] = "TestTrial";
const char kSecondaryGroupName[] = "SecondaryGroup";
int default_group = -1;
scoped_refptr<FieldTrial> trial =
CreateFieldTrial(kTrialName, 100, kDefaultGroupName, &default_group);
trial->AppendGroup(kSecondaryGroupName, 50);
// The trial should start inactive.
EXPECT_FALSE(FieldTrialList::IsTrialActive(kTrialName));
// Calling |GetGroupNameWithoutActivation()| should not activate the trial.
std::string group_name = trial->GetGroupNameWithoutActivation();
EXPECT_FALSE(group_name.empty());
EXPECT_FALSE(FieldTrialList::IsTrialActive(kTrialName));
// Calling |group_name()| should activate it and return the same group name.
EXPECT_EQ(group_name, trial->group_name());
EXPECT_TRUE(FieldTrialList::IsTrialActive(kTrialName));
}
TEST_F(FieldTrialTest, Save) { TEST_F(FieldTrialTest, Save) {
std::string save_string; std::string save_string;
......
...@@ -678,9 +678,8 @@ void ChromeBrowserMainParts::SetupMetricsAndFieldTrials() { ...@@ -678,9 +678,8 @@ void ChromeBrowserMainParts::SetupMetricsAndFieldTrials() {
variations::VariationsService* variations_service = variations::VariationsService* variations_service =
browser_process_->variations_service(); browser_process_->variations_service();
if (variations_service) if (variations_service)
variations_service->CreateTrialsFromSeed(); variations_service->CreateTrialsFromSeed(feature_list.get());
// TODO(asvitkine): Pass |feature_list| to CreateTrialsFromSeed() above.
base::FeatureList::SetInstance(feature_list.Pass()); base::FeatureList::SetInstance(feature_list.Pass());
// This must be called after |local_state_| is initialized. // This must be called after |local_state_| is initialized.
......
...@@ -44,7 +44,7 @@ message Study { ...@@ -44,7 +44,7 @@ message Study {
// An experiment within the study. // An experiment within the study.
// //
// Next tag: 11 // Next tag: 12
message Experiment { message Experiment {
// A named parameter value for this experiment. // A named parameter value for this experiment.
// //
...@@ -79,10 +79,44 @@ message Study { ...@@ -79,10 +79,44 @@ message Study {
// Optional id used to uniquely identify this experiment for Chrome Sync. // Optional id used to uniquely identify this experiment for Chrome Sync.
optional uint64 chrome_sync_experiment_id = 10; optional uint64 chrome_sync_experiment_id = 10;
// Specifies the feature association parameters for this experiment group.
//
// Next tag: 5
message FeatureAssociation {
// Optional list of features to enable when this experiment is selected.
// Command-line overrides take precedence over this setting. No feature
// listed here should exist in the |disable_feature| list.
repeated string enable_feature = 1;
// Optional list of features to disable when this experiment is selected.
// Command-line overrides take precedence over this setting. No feature
// listed here should exist in the |enable_feature| list.
repeated string disable_feature = 2;
// Similar to |forcing_flag|, this is an optional name of a feature which
// will cause this experiment to be activated, if that feature is enabled
// from the command-line. Experiment with this set are not eligible for
// selection via a random dice roll.
// Mutually exclusive with |forcing_flag|, |forcing_feature_off| or
// having a non-zero |probability_weight|.
optional string forcing_feature_on = 3;
// Similar to |forcing_flag|, this is an optional name of a feature which
// will cause this experiment to be activated, if that feature is disabled
// from the command-line. Experiment with this set are not eligible for
// selection via a random dice roll.
// Mutually exclusive with |forcing_flag|, |forcing_feature_on| or having
// a non-zero |probability_weight|.
optional string forcing_feature_off = 4;
}
optional FeatureAssociation feature_association = 11;
// Optional name of a Chrome flag that, when present, causes this experiment // Optional name of a Chrome flag that, when present, causes this experiment
// to be forced. If the forcing_flag field is set, users will not be // to be forced. If the forcing_flag field is set, users will not be
// assigned to this experiment unless that flag is present in Chrome's // assigned to this experiment unless that flag is present in Chrome's
// command line. // command line.
// Mutually exclusive with |forcing_feature_on|, |forcing_feature_off| or
// having a non-zero |probability_weight|.
optional string forcing_flag = 5; optional string forcing_flag = 5;
// Parameter values for this experiment. // Parameter values for this experiment.
......
...@@ -223,7 +223,7 @@ VariationsService::VariationsService( ...@@ -223,7 +223,7 @@ VariationsService::VariationsService(
VariationsService::~VariationsService() { VariationsService::~VariationsService() {
} }
bool VariationsService::CreateTrialsFromSeed() { bool VariationsService::CreateTrialsFromSeed(base::FeatureList* feature_list) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
create_trials_from_seed_called_ = true; create_trials_from_seed_called_ = true;
...@@ -252,7 +252,8 @@ bool VariationsService::CreateTrialsFromSeed() { ...@@ -252,7 +252,8 @@ bool VariationsService::CreateTrialsFromSeed() {
GetCurrentFormFactor(), GetHardwareClass(), latest_country, GetCurrentFormFactor(), GetHardwareClass(), latest_country,
LoadPermanentConsistencyCountry(current_version, latest_country), LoadPermanentConsistencyCountry(current_version, latest_country),
base::Bind(&UIStringOverrider::OverrideUIString, base::Bind(&UIStringOverrider::OverrideUIString,
base::Unretained(&ui_string_overrider_))); base::Unretained(&ui_string_overrider_)),
feature_list);
const base::Time now = base::Time::Now(); const base::Time now = base::Time::Now();
......
...@@ -28,6 +28,7 @@ class PrefService; ...@@ -28,6 +28,7 @@ class PrefService;
class PrefRegistrySimple; class PrefRegistrySimple;
namespace base { namespace base {
class FeatureList;
class Version; class Version;
} }
...@@ -72,10 +73,12 @@ class VariationsService ...@@ -72,10 +73,12 @@ class VariationsService
~VariationsService() override; ~VariationsService() override;
// Creates field trials based on Variations Seed loaded from local prefs. If // Creates field trials based on the variations seed loaded from local state.
// there is a problem loading the seed data, all trials specified by the seed // If there is a problem loading the seed data, all trials specified by the
// may not be created. // seed may not be created. Some field trials are configured to override or
bool CreateTrialsFromSeed(); // associate with (for reporting) specific features. These associations are
// registered with |feature_list|.
bool CreateTrialsFromSeed(base::FeatureList* feature_list);
// Should be called before startup of the main message loop. // Should be called before startup of the main message loop.
void PerformPreMainMessageLoopStartup(); void PerformPreMainMessageLoopStartup();
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <vector> #include <vector>
#include "base/command_line.h" #include "base/command_line.h"
#include "base/feature_list.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "components/variations/processed_study.h" #include "components/variations/processed_study.h"
...@@ -79,6 +80,72 @@ void ApplyUIStringOverrides( ...@@ -79,6 +80,72 @@ void ApplyUIStringOverrides(
} }
} }
// Forces the specified |experiment| to be enabled in |study|.
void ForceExperimentState(
const Study& study,
const Study_Experiment& experiment,
const VariationsSeedProcessor::UIStringOverrideCallback& override_callback,
base::FieldTrial* trial) {
RegisterExperimentParams(study, experiment);
RegisterVariationIds(experiment, study.name());
if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
trial->group();
// UI Strings can only be overridden from ACTIVATION_AUTO experiments.
ApplyUIStringOverrides(experiment, override_callback);
}
}
// Registers feature overrides for the chosen experiment in the specified study.
void RegisterFeatureOverrides(const ProcessedStudy& processed_study,
base::FieldTrial* trial,
base::FeatureList* feature_list) {
const std::string& group_name = trial->GetGroupNameWithoutActivation();
int experiment_index = processed_study.GetExperimentIndexByName(group_name);
// The field trial was defined from |study|, so the active experiment's name
// must be in the |study|.
DCHECK_NE(-1, experiment_index);
const Study_Experiment& experiment =
processed_study.study()->experiment(experiment_index);
// Process all the features to enable.
int feature_count = experiment.feature_association().enable_feature_size();
for (int i = 0; i < feature_count; ++i) {
feature_list->RegisterFieldTrialOverride(
experiment.feature_association().enable_feature(i),
base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
}
// Process all the features to disable.
feature_count = experiment.feature_association().disable_feature_size();
for (int i = 0; i < feature_count; ++i) {
feature_list->RegisterFieldTrialOverride(
experiment.feature_association().disable_feature(i),
base::FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
}
}
// Checks if |experiment| is associated with a forcing flag or feature and if it
// is, returns whether it should be forced enabled based on the |command_line|
// or |feature_list| state.
bool ShouldForceExperiment(const Study_Experiment& experiment,
const base::CommandLine& command_line,
const base::FeatureList& feature_list) {
if (experiment.feature_association().has_forcing_feature_on()) {
return feature_list.IsFeatureOverriddenFromCommandLine(
experiment.feature_association().forcing_feature_on(),
base::FeatureList::OVERRIDE_ENABLE_FEATURE);
}
if (experiment.feature_association().has_forcing_feature_off()) {
return feature_list.IsFeatureOverriddenFromCommandLine(
experiment.feature_association().forcing_feature_off(),
base::FeatureList::OVERRIDE_DISABLE_FEATURE);
}
if (experiment.has_forcing_flag())
return command_line.HasSwitch(experiment.forcing_flag());
return false;
}
} // namespace } // namespace
VariationsSeedProcessor::VariationsSeedProcessor() { VariationsSeedProcessor::VariationsSeedProcessor() {
...@@ -97,7 +164,8 @@ void VariationsSeedProcessor::CreateTrialsFromSeed( ...@@ -97,7 +164,8 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
const std::string& hardware_class, const std::string& hardware_class,
const std::string& session_consistency_country, const std::string& session_consistency_country,
const std::string& permanent_consistency_country, const std::string& permanent_consistency_country,
const UIStringOverrideCallback& override_callback) { const UIStringOverrideCallback& override_callback,
base::FeatureList* feature_list) {
std::vector<ProcessedStudy> filtered_studies; std::vector<ProcessedStudy> filtered_studies;
FilterAndValidateStudies(seed, locale, reference_date, version, channel, FilterAndValidateStudies(seed, locale, reference_date, version, channel,
form_factor, hardware_class, form_factor, hardware_class,
...@@ -105,12 +173,13 @@ void VariationsSeedProcessor::CreateTrialsFromSeed( ...@@ -105,12 +173,13 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
permanent_consistency_country, &filtered_studies); permanent_consistency_country, &filtered_studies);
for (size_t i = 0; i < filtered_studies.size(); ++i) for (size_t i = 0; i < filtered_studies.size(); ++i)
CreateTrialFromStudy(filtered_studies[i], override_callback); CreateTrialFromStudy(filtered_studies[i], override_callback, feature_list);
} }
void VariationsSeedProcessor::CreateTrialFromStudy( void VariationsSeedProcessor::CreateTrialFromStudy(
const ProcessedStudy& processed_study, const ProcessedStudy& processed_study,
const UIStringOverrideCallback& override_callback) { const UIStringOverrideCallback& override_callback,
base::FeatureList* feature_list) {
const Study& study = *processed_study.study(); const Study& study = *processed_study.study();
// Check if any experiments need to be forced due to a command line // Check if any experiments need to be forced due to a command line
...@@ -118,28 +187,26 @@ void VariationsSeedProcessor::CreateTrialFromStudy( ...@@ -118,28 +187,26 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
for (int i = 0; i < study.experiment_size(); ++i) { for (int i = 0; i < study.experiment_size(); ++i) {
const Study_Experiment& experiment = study.experiment(i); const Study_Experiment& experiment = study.experiment(i);
if (experiment.has_forcing_flag() && if (ShouldForceExperiment(experiment, *command_line, *feature_list)) {
command_line->HasSwitch(experiment.forcing_flag())) { base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
scoped_refptr<base::FieldTrial> trial( study.name(), experiment.name());
base::FieldTrialList::CreateFieldTrial(study.name(), // If |trial| is null, then there might already be a trial forced to a
experiment.name()));
// If |trial| is NULL, then there might already be a trial forced to a
// different group (e.g. via --force-fieldtrials). Break out of the loop, // different group (e.g. via --force-fieldtrials). Break out of the loop,
// but don't return, so that variation ids and params for the selected // but don't return, so that variation ids and params for the selected
// group will still be picked up. // group will still be picked up.
if (!trial.get()) if (!trial)
break; break;
RegisterExperimentParams(study, experiment); if (experiment.feature_association().has_forcing_feature_on()) {
RegisterVariationIds(experiment, study.name()); feature_list->AssociateReportingFieldTrial(
if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) { experiment.feature_association().forcing_feature_on(),
trial->group(); base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
// UI Strings can only be overridden from ACTIVATION_AUTO experiments. } else if (experiment.feature_association().has_forcing_feature_off()) {
ApplyUIStringOverrides(experiment, override_callback); feature_list->AssociateReportingFieldTrial(
experiment.feature_association().forcing_feature_off(),
base::FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
} }
ForceExperimentState(study, experiment, override_callback, trial);
DVLOG(1) << "Trial " << study.name() << " forced by flag: "
<< experiment.forcing_flag();
return; return;
} }
} }
...@@ -169,14 +236,18 @@ void VariationsSeedProcessor::CreateTrialFromStudy( ...@@ -169,14 +236,18 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
randomization_seed, NULL)); randomization_seed, NULL));
bool has_overrides = false; bool has_overrides = false;
bool enables_or_disables_features = false;
for (int i = 0; i < study.experiment_size(); ++i) { for (int i = 0; i < study.experiment_size(); ++i) {
const Study_Experiment& experiment = study.experiment(i); const Study_Experiment& experiment = study.experiment(i);
RegisterExperimentParams(study, experiment); RegisterExperimentParams(study, experiment);
// Groups with forcing flags have probability 0 and will never be selected. // Groups with forcing flags have probability 0 and will never be selected.
// Therefore, there's no need to add them to the field trial. // Therefore, there's no need to add them to the field trial.
if (experiment.has_forcing_flag()) if (experiment.has_forcing_flag() ||
experiment.feature_association().has_forcing_feature_on() ||
experiment.feature_association().has_forcing_feature_off()) {
continue; continue;
}
if (experiment.name() != study.default_experiment_name()) if (experiment.name() != study.default_experiment_name())
trial->AppendGroup(experiment.name(), experiment.probability_weight()); trial->AppendGroup(experiment.name(), experiment.probability_weight());
...@@ -184,9 +255,17 @@ void VariationsSeedProcessor::CreateTrialFromStudy( ...@@ -184,9 +255,17 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
RegisterVariationIds(experiment, study.name()); RegisterVariationIds(experiment, study.name());
has_overrides = has_overrides || experiment.override_ui_string_size() > 0; has_overrides = has_overrides || experiment.override_ui_string_size() > 0;
if (experiment.feature_association().enable_feature_size() != 0 ||
experiment.feature_association().disable_feature_size() != 0) {
enables_or_disables_features = true;
}
} }
trial->SetForced(); trial->SetForced();
if (enables_or_disables_features)
RegisterFeatureOverrides(processed_study, trial.get(), feature_list);
if (processed_study.is_expired()) { if (processed_study.is_expired()) {
trial->Disable(); trial->Disable();
} else if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) { } else if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
......
...@@ -18,6 +18,10 @@ ...@@ -18,6 +18,10 @@
#include "components/variations/proto/study.pb.h" #include "components/variations/proto/study.pb.h"
#include "components/variations/proto/variations_seed.pb.h" #include "components/variations/proto/variations_seed.pb.h"
namespace base {
class FeatureList;
}
namespace variations { namespace variations {
class ProcessedStudy; class ProcessedStudy;
...@@ -42,7 +46,8 @@ class VariationsSeedProcessor { ...@@ -42,7 +46,8 @@ class VariationsSeedProcessor {
const std::string& hardware_class, const std::string& hardware_class,
const std::string& session_consistency_country, const std::string& session_consistency_country,
const std::string& permanent_consistency_country, const std::string& permanent_consistency_country,
const UIStringOverrideCallback& override_callback); const UIStringOverrideCallback& override_callback,
base::FeatureList* feature_list);
private: private:
friend class VariationsSeedProcessorTest; friend class VariationsSeedProcessorTest;
...@@ -71,7 +76,8 @@ class VariationsSeedProcessor { ...@@ -71,7 +76,8 @@ class VariationsSeedProcessor {
// Creates and registers a field trial from the |processed_study| data. // Creates and registers a field trial from the |processed_study| data.
// Disables the trial if |processed_study.is_expired| is true. // Disables the trial if |processed_study.is_expired| is true.
void CreateTrialFromStudy(const ProcessedStudy& processed_study, void CreateTrialFromStudy(const ProcessedStudy& processed_study,
const UIStringOverrideCallback& override_callback); const UIStringOverrideCallback& override_callback,
base::FeatureList* feature_list);
DISALLOW_COPY_AND_ASSIGN(VariationsSeedProcessor); DISALLOW_COPY_AND_ASSIGN(VariationsSeedProcessor);
}; };
......
...@@ -9,7 +9,10 @@ ...@@ -9,7 +9,10 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/feature_list.h"
#include "base/format_macros.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "components/variations/processed_study.h" #include "components/variations/processed_study.h"
#include "components/variations/variations_associated_data.h" #include "components/variations/variations_associated_data.h"
...@@ -105,19 +108,27 @@ class VariationsSeedProcessorTest : public ::testing::Test { ...@@ -105,19 +108,27 @@ class VariationsSeedProcessorTest : public ::testing::Test {
// process singletons. // process singletons.
testing::ClearAllVariationIDs(); testing::ClearAllVariationIDs();
testing::ClearAllVariationParams(); testing::ClearAllVariationParams();
base::FeatureList::ClearInstanceForTesting();
} }
bool CreateTrialFromStudy(const Study* study) { bool CreateTrialFromStudy(const Study* study) {
return CreateTrialFromStudyWithFeatureList(study, &feature_list_);
}
bool CreateTrialFromStudyWithFeatureList(const Study* study,
base::FeatureList* feature_list) {
ProcessedStudy processed_study; ProcessedStudy processed_study;
if (processed_study.Init(study, false)) { if (processed_study.Init(study, false)) {
VariationsSeedProcessor().CreateTrialFromStudy( VariationsSeedProcessor().CreateTrialFromStudy(
processed_study, override_callback_.callback()); processed_study, override_callback_.callback(), feature_list);
return true; return true;
} }
return false; return false;
} }
protected: protected:
base::FeatureList feature_list_;
TestOverrideStringCallback override_callback_; TestOverrideStringCallback override_callback_;
private: private:
...@@ -215,23 +226,27 @@ TEST_F(VariationsSeedProcessorTest, ...@@ -215,23 +226,27 @@ TEST_F(VariationsSeedProcessorTest,
// Check that adding [expired, non-expired] activates the non-expired one. // Check that adding [expired, non-expired] activates the non-expired one.
ASSERT_EQ(std::string(), base::FieldTrialList::FindFullName(kTrialName)); ASSERT_EQ(std::string(), base::FieldTrialList::FindFullName(kTrialName));
{ {
base::FeatureList feature_list;
base::FieldTrialList field_trial_list(NULL); base::FieldTrialList field_trial_list(NULL);
study1->set_expiry_date(TimeToProtoTime(year_ago)); study1->set_expiry_date(TimeToProtoTime(year_ago));
seed_processor.CreateTrialsFromSeed( seed_processor.CreateTrialsFromSeed(
seed, "en-CA", base::Time::Now(), version, Study_Channel_STABLE, seed, "en-CA", base::Time::Now(), version, Study_Channel_STABLE,
Study_FormFactor_DESKTOP, "", "", "", override_callback_.callback()); Study_FormFactor_DESKTOP, "", "", "", override_callback_.callback(),
&feature_list);
EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName)); EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
} }
// Check that adding [non-expired, expired] activates the non-expired one. // Check that adding [non-expired, expired] activates the non-expired one.
ASSERT_EQ(std::string(), base::FieldTrialList::FindFullName(kTrialName)); ASSERT_EQ(std::string(), base::FieldTrialList::FindFullName(kTrialName));
{ {
base::FeatureList feature_list;
base::FieldTrialList field_trial_list(NULL); base::FieldTrialList field_trial_list(NULL);
study1->clear_expiry_date(); study1->clear_expiry_date();
study2->set_expiry_date(TimeToProtoTime(year_ago)); study2->set_expiry_date(TimeToProtoTime(year_ago));
seed_processor.CreateTrialsFromSeed( seed_processor.CreateTrialsFromSeed(
seed, "en-CA", base::Time::Now(), version, Study_Channel_STABLE, seed, "en-CA", base::Time::Now(), version, Study_Channel_STABLE,
Study_FormFactor_DESKTOP, "", "", "", override_callback_.callback()); Study_FormFactor_DESKTOP, "", "", "", override_callback_.callback(),
&feature_list);
EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName)); EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
} }
} }
...@@ -436,7 +451,7 @@ TEST_F(VariationsSeedProcessorTest, StartsActive) { ...@@ -436,7 +451,7 @@ TEST_F(VariationsSeedProcessorTest, StartsActive) {
seed_processor.CreateTrialsFromSeed( seed_processor.CreateTrialsFromSeed(
seed, "en-CA", base::Time::Now(), base::Version("20.0.0.0"), seed, "en-CA", base::Time::Now(), base::Version("20.0.0.0"),
Study_Channel_STABLE, Study_FormFactor_DESKTOP, "", "", "", Study_Channel_STABLE, Study_FormFactor_DESKTOP, "", "", "",
override_callback_.callback()); override_callback_.callback(), &feature_list_);
// Non-specified and ACTIVATION_EXPLICIT should not start active, but // Non-specified and ACTIVATION_EXPLICIT should not start active, but
// ACTIVATION_AUTO should. // ACTIVATION_AUTO should.
...@@ -493,4 +508,200 @@ TEST_F(VariationsSeedProcessorTest, ForcingFlagAlreadyForced) { ...@@ -493,4 +508,200 @@ TEST_F(VariationsSeedProcessorTest, ForcingFlagAlreadyForced) {
EXPECT_EQ(kExperimentId, id); EXPECT_EQ(kExperimentId, id);
} }
TEST_F(VariationsSeedProcessorTest, FeatureEnabledOrDisableByTrial) {
struct base::Feature kFeatureOffByDefault {
"kOff", base::FEATURE_DISABLED_BY_DEFAULT
};
struct base::Feature kFeatureOnByDefault {
"kOn", base::FEATURE_ENABLED_BY_DEFAULT
};
struct base::Feature kUnrelatedFeature {
"kUnrelated", base::FEATURE_DISABLED_BY_DEFAULT
};
struct {
const char* enable_feature;
const char* disable_feature;
bool expected_feature_off_state;
bool expected_feature_on_state;
} test_cases[] = {
{nullptr, nullptr, false, true},
{kFeatureOnByDefault.name, nullptr, false, true},
{kFeatureOffByDefault.name, nullptr, true, true},
{nullptr, kFeatureOnByDefault.name, false, false},
{nullptr, kFeatureOffByDefault.name, false, true},
};
for (size_t i = 0; i < arraysize(test_cases); i++) {
const auto& test_case = test_cases[i];
SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]", i));
base::FieldTrialList field_trial_list(NULL);
base::FeatureList::ClearInstanceForTesting();
scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
Study study;
study.set_name("Study1");
study.set_default_experiment_name("B");
AddExperiment("B", 0, &study);
Study_Experiment* experiment = AddExperiment("A", 1, &study);
Study_Experiment_FeatureAssociation* association =
experiment->mutable_feature_association();
if (test_case.enable_feature)
association->add_enable_feature(test_case.enable_feature);
else if (test_case.disable_feature)
association->add_disable_feature(test_case.disable_feature);
EXPECT_TRUE(
CreateTrialFromStudyWithFeatureList(&study, feature_list.get()));
base::FeatureList::SetInstance(feature_list.Pass());
// |kUnrelatedFeature| should not be affected.
EXPECT_FALSE(base::FeatureList::IsEnabled(kUnrelatedFeature));
// Before the associated feature is queried, the trial shouldn't be active.
EXPECT_FALSE(base::FieldTrialList::IsTrialActive(study.name()));
EXPECT_EQ(test_case.expected_feature_off_state,
base::FeatureList::IsEnabled(kFeatureOffByDefault));
EXPECT_EQ(test_case.expected_feature_on_state,
base::FeatureList::IsEnabled(kFeatureOnByDefault));
// The field trial should get activated if it had a feature association.
const bool expected_field_trial_active =
test_case.enable_feature || test_case.disable_feature;
EXPECT_EQ(expected_field_trial_active,
base::FieldTrialList::IsTrialActive(study.name()));
}
}
TEST_F(VariationsSeedProcessorTest, FeatureAssociationAndForcing) {
struct base::Feature kFeatureOffByDefault {
"kFeatureOffByDefault", base::FEATURE_DISABLED_BY_DEFAULT
};
struct base::Feature kFeatureOnByDefault {
"kFeatureOnByDefault", base::FEATURE_ENABLED_BY_DEFAULT
};
enum OneHundredPercentGroup {
DEFAULT_GROUP,
ENABLE_GROUP,
DISABLE_GROUP,
};
const char kDefaultGroup[] = "Default";
const char kEnabledGroup[] = "Enabled";
const char kDisabledGroup[] = "Disabled";
const char kForcedOnGroup[] = "ForcedOn";
const char kForcedOffGroup[] = "ForcedOff";
struct {
const base::Feature& feature;
const char* enable_features_command_line;
const char* disable_features_command_line;
OneHundredPercentGroup one_hundred_percent_group;
const char* expected_group;
bool expected_feature_state;
bool expected_trial_activated;
} test_cases[] = {
// Check what happens without and command-line forcing flags - that the
// |one_hundred_percent_group| gets correctly selected and does the right
// thing w.r.t. to affecting the feature / activating the trial.
{kFeatureOffByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, false,
false},
{kFeatureOffByDefault, "", "", ENABLE_GROUP, kEnabledGroup, true, true},
{kFeatureOffByDefault, "", "", DISABLE_GROUP, kDisabledGroup, false,
true},
// Do the same as above, but for kFeatureOnByDefault feature.
{kFeatureOnByDefault, "", "", DEFAULT_GROUP, kDefaultGroup, true, false},
{kFeatureOnByDefault, "", "", ENABLE_GROUP, kEnabledGroup, true, true},
{kFeatureOnByDefault, "", "", DISABLE_GROUP, kDisabledGroup, false, true},
// Test forcing each feature on and off through the command-line and that
// the correct associated experiment gets chosen.
{kFeatureOffByDefault, kFeatureOffByDefault.name, "", DEFAULT_GROUP,
kForcedOnGroup, true, true},
{kFeatureOffByDefault, "", kFeatureOffByDefault.name, DEFAULT_GROUP,
kForcedOffGroup, false, true},
{kFeatureOnByDefault, kFeatureOnByDefault.name, "", DEFAULT_GROUP,
kForcedOnGroup, true, true},
{kFeatureOnByDefault, "", kFeatureOnByDefault.name, DEFAULT_GROUP,
kForcedOffGroup, false, true},
// Check that even if a feature should be enabled or disabled based on the
// the experiment probability weights, the forcing flag association still
// takes precedence. This is 4 cases as above, but with different values
// for |one_hundred_percent_group|.
{kFeatureOffByDefault, kFeatureOffByDefault.name, "", ENABLE_GROUP,
kForcedOnGroup, true, true},
{kFeatureOffByDefault, "", kFeatureOffByDefault.name, ENABLE_GROUP,
kForcedOffGroup, false, true},
{kFeatureOnByDefault, kFeatureOnByDefault.name, "", ENABLE_GROUP,
kForcedOnGroup, true, true},
{kFeatureOnByDefault, "", kFeatureOnByDefault.name, ENABLE_GROUP,
kForcedOffGroup, false, true},
{kFeatureOffByDefault, kFeatureOffByDefault.name, "", DISABLE_GROUP,
kForcedOnGroup, true, true},
{kFeatureOffByDefault, "", kFeatureOffByDefault.name, DISABLE_GROUP,
kForcedOffGroup, false, true},
{kFeatureOnByDefault, kFeatureOnByDefault.name, "", DISABLE_GROUP,
kForcedOnGroup, true, true},
{kFeatureOnByDefault, "", kFeatureOnByDefault.name, DISABLE_GROUP,
kForcedOffGroup, false, true},
};
for (size_t i = 0; i < arraysize(test_cases); i++) {
const auto& test_case = test_cases[i];
const int group = test_case.one_hundred_percent_group;
SCOPED_TRACE(base::StringPrintf(
"Test[%" PRIuS "]: %s [%s] [%s] %d", i, test_case.feature.name,
test_case.enable_features_command_line,
test_case.disable_features_command_line, static_cast<int>(group)));
base::FieldTrialList field_trial_list(NULL);
base::FeatureList::ClearInstanceForTesting();
scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
feature_list->InitializeFromCommandLine(
test_case.enable_features_command_line,
test_case.disable_features_command_line);
Study study;
study.set_name("Study1");
study.set_default_experiment_name("Default");
AddExperiment(kDefaultGroup, group == DEFAULT_GROUP ? 1 : 0, &study);
Study_Experiment* feature_enable =
AddExperiment(kEnabledGroup, group == ENABLE_GROUP ? 1 : 0, &study);
feature_enable->mutable_feature_association()->add_enable_feature(
test_case.feature.name);
Study_Experiment* feature_disable =
AddExperiment(kDisabledGroup, group == DISABLE_GROUP ? 1 : 0, &study);
feature_disable->mutable_feature_association()->add_disable_feature(
test_case.feature.name);
AddExperiment(kForcedOnGroup, 0, &study)
->mutable_feature_association()
->set_forcing_feature_on(test_case.feature.name);
AddExperiment(kForcedOffGroup, 0, &study)
->mutable_feature_association()
->set_forcing_feature_off(test_case.feature.name);
EXPECT_TRUE(
CreateTrialFromStudyWithFeatureList(&study, feature_list.get()));
base::FeatureList::SetInstance(feature_list.Pass());
// Trial should not be activated initially, but later might get activated
// depending on the expected values.
EXPECT_FALSE(base::FieldTrialList::IsTrialActive(study.name()));
EXPECT_EQ(test_case.expected_feature_state,
base::FeatureList::IsEnabled(test_case.feature));
EXPECT_EQ(test_case.expected_trial_activated,
base::FieldTrialList::IsTrialActive(study.name()));
}
}
} // namespace variations } // namespace variations
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