Skip to content

Commit

Permalink
Remove support for expired/disabled trials.
Browse files Browse the repository at this point in the history 
Bug: b/242336301

Change-Id: I3a2586ea88d0b9f5bc6fc03af5b1d38217ccdc7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3860053
Auto-Submit: Steven Holte <holte@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1044420}
  • Loading branch information
Steven Holte authored and Chromium LUCI CQ committed Sep 8, 2022
1 parent e7e03ca commit 20903dd
Show file tree
Hide file tree
Showing 21 changed files with 188 additions and 649 deletions.
51 changes: 10 additions & 41 deletions base/metrics/field_trial.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void AddFeatureAndFieldTrialFlags(CommandLine* cmd_line) {
cmd_line->AppendSwitchASCII(switches::kDisableFeatures, disabled_features);

std::string field_trial_states;
FieldTrialList::AllStatesToString(&field_trial_states, false);
FieldTrialList::AllStatesToString(&field_trial_states);
if (!field_trial_states.empty()) {
cmd_line->AppendSwitchASCII(switches::kForceFieldTrials,
field_trial_states);
Expand Down Expand Up @@ -280,23 +280,6 @@ bool FieldTrial::FieldTrialEntry::ReadStringPair(
return true;
}

void FieldTrial::Disable() {
// Group choice has already been reported to observers so we can't disable
// the study.
DCHECK(!group_reported_);
enable_field_trial_ = false;

// In case we are disabled after initialization, we need to switch
// the trial to the default group.
if (group_ != kNotFinalized) {
// Only reset when not already the default group, because in case we were
// forced to the default group, the group number may not be
// kDefaultGroupNumber, so we should keep it as is.
if (group_name_ != default_group_name_)
SetGroupChoice(default_group_name_, kDefaultGroupNumber);
}
}

void FieldTrial::AppendGroup(const std::string& name,
Probability group_probability) {
// When the group choice was previously forced, we only need to return the
Expand All @@ -320,7 +303,7 @@ void FieldTrial::AppendGroup(const std::string& name,
DCHECK_LE(group_probability, divisor_);
DCHECK_GE(group_probability, 0);

if (enable_benchmarking_ || !enable_field_trial_)
if (enable_benchmarking_)
group_probability = 0;

accumulated_group_probability_ += group_probability;
Expand Down Expand Up @@ -393,7 +376,6 @@ FieldTrial::FieldTrial(StringPiece trial_name,
accumulated_group_probability_(0),
next_group_number_(kDefaultGroupNumber + 1),
group_(kNotFinalized),
enable_field_trial_(true),
forced_(false),
group_reported_(false),
trial_registered_(false),
Expand Down Expand Up @@ -440,23 +422,19 @@ void FieldTrial::FinalizeGroupChoiceImpl(bool is_locked) {
}

bool FieldTrial::GetActiveGroup(ActiveGroup* active_group) const {
if (!group_reported_ || !enable_field_trial_)
if (!group_reported_)
return false;
DCHECK_NE(group_, kNotFinalized);
active_group->trial_name = trial_name_;
active_group->group_name = group_name_;
return true;
}

bool FieldTrial::GetStateWhileLocked(PickleState* field_trial_state,
bool include_disabled) {
if (!include_disabled && !enable_field_trial_)
return false;
void FieldTrial::GetStateWhileLocked(PickleState* field_trial_state) {
FinalizeGroupChoiceImpl(true);
field_trial_state->trial_name = &trial_name_;
field_trial_state->group_name = &group_name_;
field_trial_state->activated = group_reported_;
return true;
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -570,8 +548,7 @@ std::vector<FieldTrial::State> FieldTrialList::GetAllFieldTrialStates(
AutoLock auto_lock(global_->lock_);
for (const auto& registered : global_->registered_) {
FieldTrial::PickleState trial;
if (!registered.second->GetStateWhileLocked(&trial, true))
continue;
registered.second->GetStateWhileLocked(&trial);
DCHECK_EQ(std::string::npos,
trial.trial_name->find(kPersistentStringSeparator));
DCHECK_EQ(std::string::npos,
Expand All @@ -586,16 +563,14 @@ std::vector<FieldTrial::State> FieldTrialList::GetAllFieldTrialStates(
}

// static
void FieldTrialList::AllStatesToString(std::string* output,
bool include_disabled) {
void FieldTrialList::AllStatesToString(std::string* output) {
if (!global_)
return;
AutoLock auto_lock(global_->lock_);

for (const auto& registered : global_->registered_) {
FieldTrial::PickleState trial;
if (!registered.second->GetStateWhileLocked(&trial, include_disabled))
continue;
registered.second->GetStateWhileLocked(&trial);
DCHECK_EQ(std::string::npos,
trial.trial_name->find(kPersistentStringSeparator));
DCHECK_EQ(std::string::npos,
Expand All @@ -610,15 +585,13 @@ void FieldTrialList::AllStatesToString(std::string* output,
}

// static
std::string FieldTrialList::AllParamsToString(bool include_disabled,
EscapeDataFunc encode_data_func) {
std::string FieldTrialList::AllParamsToString(EscapeDataFunc encode_data_func) {
FieldTrialParamAssociator* params_associator =
FieldTrialParamAssociator::GetInstance();
std::string output;
for (const auto& registered : GetRegisteredTrials()) {
FieldTrial::PickleState trial;
if (!registered.second->GetStateWhileLocked(&trial, include_disabled))
continue;
registered.second->GetStateWhileLocked(&trial);
DCHECK_EQ(std::string::npos,
trial.trial_name->find(kPersistentStringSeparator));
DCHECK_EQ(std::string::npos,
Expand Down Expand Up @@ -919,9 +892,6 @@ void FieldTrialList::NotifyFieldTrialGroupSelection(FieldTrial* field_trial) {
return;
field_trial->group_reported_ = true;

if (!field_trial->enable_field_trial_)
return;

++global_->num_ongoing_notify_field_trial_group_selection_calls_;

ActivateFieldTrialEntryWhileLocked(field_trial);
Expand Down Expand Up @@ -1369,8 +1339,7 @@ void FieldTrialList::AddToAllocatorWhileLocked(
return;

FieldTrial::PickleState trial_state;
if (!field_trial->GetStateWhileLocked(&trial_state, false))
return;
field_trial->GetStateWhileLocked(&trial_state);

// Or if we've already added it. We must check after GetState since it can
// also add to the allocator.
Expand Down
36 changes: 10 additions & 26 deletions base/metrics/field_trial.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
FieldTrial(const FieldTrial&) = delete;
FieldTrial& operator=(const FieldTrial&) = delete;

// Disables this trial, meaning the default group is always selected. May be
// called immediately after construction or at any time after initialization;
// however, it cannot be called after group(). Once disabled, there is no way
// to re-enable a trial.
void Disable();

// Establishes the name and probability of the next group in this trial.
// Sometimes, based on construction randomization, this call may cause the
// provided group to be *THE* group selected for use in this instance.
Expand Down Expand Up @@ -324,13 +318,8 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
bool GetActiveGroup(ActiveGroup* active_group) const;

// Returns the trial name and selected group name for this field trial via
// the output parameter |field_trial_state| for all the studies when
// |include_disabled| is true. In case when |include_disabled| is false, if
// the trial has not been disabled true is returned and |field_trial_state|
// is filled in; otherwise, the result is false and |field_trial_state| is
// left untouched.
bool GetStateWhileLocked(PickleState* field_trial_state,
bool include_disabled);
// the output parameter |field_trial_state| for all the studies.
void GetStateWhileLocked(PickleState* field_trial_state);

// Returns the group_name. A winner need not have been chosen.
const std::string& group_name_internal() const { return group_name_; }
Expand Down Expand Up @@ -364,10 +353,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> {
// has been called.
std::string group_name_;

// When enable_field_trial_ is false, field trial reverts to the 'default'
// group.
bool enable_field_trial_;

// When forced_ is true, we return the chosen group from AppendGroup when
// appropriate.
bool forced_;
Expand Down Expand Up @@ -488,21 +473,20 @@ class BASE_EXPORT FieldTrialList {
// resurrection in another process. This allows randomization to be done in
// one process, and secondary processes can be synchronized on the result.
// The resulting string contains the name and group name pairs of all
// registered FieldTrials including disabled based on |include_disabled|,
// registered FieldTrials,
// with "/" used to separate all names and to terminate the string. All
// activated trials have their name prefixed with "*". This string is parsed
// by |CreateTrialsFromString()|.
static void AllStatesToString(std::string* output, bool include_disabled);
static void AllStatesToString(std::string* output);

// Creates a persistent representation of all FieldTrial params for
// resurrection in another process. The returned string contains the trial
// name and group name pairs of all registered FieldTrials including disabled
// based on |include_disabled| separated by '.'. The pair is followed by ':'
// separator and list of param name and values separated by '/'. It also takes
// |encode_data_func| function pointer for encodeing special charactors.
// This string is parsed by |AssociateParamsFromString()|.
static std::string AllParamsToString(bool include_disabled,
EscapeDataFunc encode_data_func);
// name and group name pairs of all registered FieldTrials. The pair is
// followed by ':' separator and list of param name and values separated by
// '/'. It also takes |encode_data_func| function pointer for encodeing
// special charactors. This string is parsed by
// |AssociateParamsFromString()|.
static std::string AllParamsToString(EscapeDataFunc encode_data_func);

// Fills in the supplied vector |active_groups| (which must be empty when
// called) with a snapshot of all registered FieldTrials for which the group
Expand Down
Loading

0 comments on commit 20903dd

Please sign in to comment.