Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(175)

Side by Side Diff: components/flags_ui/flags_state.cc

Issue 2129543002: Registering field trial for a feature overridden in chrome://flags. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: After code review #1 Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/flags_ui/flags_state.h" 5 #include "components/flags_ui/flags_state.h"
6 6
7 #include <memory> 7 #include <memory>
8 #include <utility> 8 #include <utility>
9 9
10 #include "base/callback.h" 10 #include "base/callback.h"
(...skipping 179 matching lines...) Expand 10 before | Expand all | Expand 10 after
190 result->Append(std::move(value)); 190 result->Append(std::move(value));
191 } 191 }
192 return result; 192 return result;
193 } 193 }
194 194
195 // Registers variation parameters specified by |feature_variation| for the field 195 // Registers variation parameters specified by |feature_variation| for the field
196 // trial named |feature_trial_name|, unless a group for this trial has already 196 // trial named |feature_trial_name|, unless a group for this trial has already
197 // been created (e.g. via command-line switches that take precedence over 197 // been created (e.g. via command-line switches that take precedence over
198 // about:flags). In the trial, the function creates a new constant group called 198 // about:flags). In the trial, the function creates a new constant group called
199 // |kTrialGroupAboutFlags|. 199 // |kTrialGroupAboutFlags|.
200 void RegisterFeatureVariationParameters( 200 base::FieldTrial* RegisterFeatureVariationParameters(
201 const std::string& feature_trial_name, 201 const std::string& feature_trial_name,
202 const FeatureEntry::FeatureVariation& feature_variation) { 202 const FeatureEntry::FeatureVariation& feature_variation) {
203 std::map<std::string, std::string> params; 203 std::map<std::string, std::string> params;
204 for (int i = 0; i < feature_variation.num_params; ++i) { 204 for (int i = 0; i < feature_variation.num_params; ++i) {
205 params[feature_variation.params[i].param_name] = 205 params[feature_variation.params[i].param_name] =
206 feature_variation.params[i].param_value; 206 feature_variation.params[i].param_value;
207 } 207 }
208 208
209 bool success = variations::AssociateVariationParams( 209 bool success = variations::AssociateVariationParams(
210 feature_trial_name, internal::kTrialGroupAboutFlags, params); 210 feature_trial_name, internal::kTrialGroupAboutFlags, params);
211 if (success) { 211 if (!success)
212 // Successful association also means that no group is created and selected 212 return nullptr;
213 // for the trial, yet. Thus, create the trial to select the group. This way, 213 // Successful association also means that no group is created and selected
214 // the parameters cannot get overwritten in later phases (such as from the 214 // for the trial, yet. Thus, create the trial to select the group. This way,
215 // server). 215 // the parameters cannot get overwritten in later phases (such as from the
216 base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial( 216 // server).
217 feature_trial_name, internal::kTrialGroupAboutFlags); 217 base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
218 if (!trial) { 218 feature_trial_name, internal::kTrialGroupAboutFlags);
219 DLOG(WARNING) << "Could not create the trial " << feature_trial_name 219 if (!trial) {
220 << " with group " << internal::kTrialGroupAboutFlags; 220 DLOG(WARNING) << "Could not create the trial " << feature_trial_name
221 } 221 << " with group " << internal::kTrialGroupAboutFlags;
222 } 222 }
223 return trial;
223 } 224 }
224 225
225 } // namespace 226 } // namespace
226 227
227 // Keeps track of affected switches for each FeatureEntry, based on which 228 // Keeps track of affected switches for each FeatureEntry, based on which
228 // choice is selected for it. 229 // choice is selected for it.
229 struct SwitchEntry { 230 struct SwitchEntry {
230 // Corresponding base::Feature to toggle. 231 // Corresponding base::Feature to toggle.
231 std::string feature_name; 232 std::string feature_name;
232 233
(...skipping 193 matching lines...) Expand 10 before | Expand all | Expand 10 after
426 flags_storage->SetFlags(no_entries); 427 flags_storage->SetFlags(no_entries);
427 } 428 }
428 429
429 void FlagsState::Reset() { 430 void FlagsState::Reset() {
430 needs_restart_ = false; 431 needs_restart_ = false;
431 flags_switches_.clear(); 432 flags_switches_.clear();
432 appended_switches_.clear(); 433 appended_switches_.clear();
433 } 434 }
434 435
435 void FlagsState::RegisterAllFeatureVariationParameters( 436 void FlagsState::RegisterAllFeatureVariationParameters(
436 FlagsStorage* flags_storage) { 437 FlagsStorage* flags_storage,
438 base::FeatureList* feature_list) {
437 std::set<std::string> enabled_entries; 439 std::set<std::string> enabled_entries;
438 GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries); 440 GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries);
439 441
440 for (size_t i = 0; i < num_feature_entries_; ++i) { 442 for (size_t i = 0; i < num_feature_entries_; ++i) {
441 const FeatureEntry& e = feature_entries_[i]; 443 const FeatureEntry& e = feature_entries_[i];
442 if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) { 444 if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
443 for (int j = 0; j < e.num_options; ++j) { 445 for (int j = 0; j < e.num_options; ++j) {
444 const FeatureEntry::FeatureVariation* variation = 446 const FeatureEntry::FeatureVariation* variation =
445 e.VariationForOption(j); 447 e.VariationForOption(j);
446 if (variation != nullptr && enabled_entries.count(e.NameForOption(j))) { 448 if (variation != nullptr && enabled_entries.count(e.NameForOption(j))) {
447 // If the option is selected by the user & has variation, register it. 449 // If the option is selected by the user & has variation, register it.
448 RegisterFeatureVariationParameters(e.feature_trial_name, *variation); 450 base::FieldTrial* field_trial = RegisterFeatureVariationParameters(
449 // TODO(jkrcal) The code does not associate the feature with the field 451 e.feature_trial_name, *variation);
450 // trial |e.feature_trial_name|. The reason is that features 452
451 // overridden in chrome://flags are translated to command-line flags 453 if (!field_trial)
452 // and thus treated earlier in the initialization. The fix requires 454 continue;
453 // larger changes. As a result: 455 feature_list->RegisterFieldTrialOverride(
454 // - the API calls to variations::GetVariationParamValueByFeature and 456 e.feature->name,
455 // to variations::GetVariationParamsByFeature do not work; and 457 base::FeatureList::OverrideState::OVERRIDE_ENABLE_FEATURE,
456 // - the API call to base::FeatureList::IsEnabled does not mark the 458 field_trial);
457 // field trial as active (and the trial does not appear in UMA).
458 // If the code calls variations::GetVariationParamValue or
459 // variations::GetVariationParams providing the trial name, everything
460 // should work fine.
461 } 459 }
462 } 460 }
463 } 461 }
464 } 462 }
465 } 463 }
466 464
467 void FlagsState::GetFlagFeatureEntries( 465 void FlagsState::GetFlagFeatureEntries(
468 FlagsStorage* flags_storage, 466 FlagsStorage* flags_storage,
469 FlagAccess access, 467 FlagAccess access,
470 base::ListValue* supported_entries, 468 base::ListValue* supported_entries,
(...skipping 248 matching lines...) Expand 10 before | Expand all | Expand 10 after
719 } 717 }
720 718
721 std::set<std::string> new_enabled_entries = 719 std::set<std::string> new_enabled_entries =
722 base::STLSetIntersection<std::set<std::string>>(platform_entries, 720 base::STLSetIntersection<std::set<std::string>>(platform_entries,
723 *result); 721 *result);
724 722
725 result->swap(new_enabled_entries); 723 result->swap(new_enabled_entries);
726 } 724 }
727 725
728 } // namespace flags_ui 726 } // namespace flags_ui
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698