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

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

Issue 2146903002: Clean-up the code for default variations in chrome://flags. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: iOS test crash fix? #2 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
« no previous file with comments | « components/flags_ui/feature_entry_macros.h ('k') | components/flags_ui/flags_state_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 181 matching lines...) Expand 10 before | Expand all | Expand 10 after
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 void 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 if (feature_variation) {
205 params[feature_variation.params[i].param_name] = 205 // Copy the parameters for non-null variations.
206 feature_variation.params[i].param_value; 206 for (int i = 0; i < feature_variation->num_params; ++i) {
207 params[feature_variation->params[i].param_name] =
208 feature_variation->params[i].param_value;
209 }
207 } 210 }
208 211
209 bool success = variations::AssociateVariationParams( 212 bool success = variations::AssociateVariationParams(
210 feature_trial_name, internal::kTrialGroupAboutFlags, params); 213 feature_trial_name, internal::kTrialGroupAboutFlags, params);
211 if (success) { 214 if (success) {
212 // Successful association also means that no group is created and selected 215 // Successful association also means that no group is created and selected
213 // for the trial, yet. Thus, create the trial to select the group. This way, 216 // for the trial, yet. Thus, create the trial to select the group. This way,
214 // the parameters cannot get overwritten in later phases (such as from the 217 // the parameters cannot get overwritten in later phases (such as from the
215 // server). 218 // server).
216 base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial( 219 base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
(...skipping 219 matching lines...) Expand 10 before | Expand all | Expand 10 after
436 FlagsStorage* flags_storage) { 439 FlagsStorage* flags_storage) {
437 std::set<std::string> enabled_entries; 440 std::set<std::string> enabled_entries;
438 GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries); 441 GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries);
439 442
440 for (size_t i = 0; i < num_feature_entries_; ++i) { 443 for (size_t i = 0; i < num_feature_entries_; ++i) {
441 const FeatureEntry& e = feature_entries_[i]; 444 const FeatureEntry& e = feature_entries_[i];
442 if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) { 445 if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
443 for (int j = 0; j < e.num_options; ++j) { 446 for (int j = 0; j < e.num_options; ++j) {
444 const FeatureEntry::FeatureVariation* variation = 447 const FeatureEntry::FeatureVariation* variation =
445 e.VariationForOption(j); 448 e.VariationForOption(j);
446 if (variation != nullptr && enabled_entries.count(e.NameForOption(j))) { 449 if (e.StateForOption(j) == FeatureEntry::FeatureState::ENABLED &&
447 // If the option is selected by the user & has variation, register it. 450 enabled_entries.count(e.NameForOption(j))) {
448 RegisterFeatureVariationParameters(e.feature_trial_name, *variation); 451 // If the option is enabled by the user, register it.
452 RegisterFeatureVariationParameters(e.feature_trial_name, variation);
449 // TODO(jkrcal) The code does not associate the feature with the field 453 // TODO(jkrcal) The code does not associate the feature with the field
450 // trial |e.feature_trial_name|. The reason is that features 454 // trial |e.feature_trial_name|. The reason is that features
451 // overridden in chrome://flags are translated to command-line flags 455 // overridden in chrome://flags are translated to command-line flags
452 // and thus treated earlier in the initialization. The fix requires 456 // and thus treated earlier in the initialization. The fix requires
453 // larger changes. As a result: 457 // larger changes. As a result:
454 // - the API calls to variations::GetVariationParamValueByFeature and 458 // - the API calls to variations::GetVariationParamValueByFeature and
455 // to variations::GetVariationParamsByFeature do not work; and 459 // to variations::GetVariationParamsByFeature do not work; and
456 // - the API call to base::FeatureList::IsEnabled does not mark the 460 // - the API call to base::FeatureList::IsEnabled does not mark the
457 // field trial as active (and the trial does not appear in UMA). 461 // field trial as active (and the trial does not appear in UMA).
458 // If the code calls variations::GetVariationParamValue or 462 // If the code calls variations::GetVariationParamValue or
(...skipping 260 matching lines...) Expand 10 before | Expand all | Expand 10 after
719 } 723 }
720 724
721 std::set<std::string> new_enabled_entries = 725 std::set<std::string> new_enabled_entries =
722 base::STLSetIntersection<std::set<std::string>>(platform_entries, 726 base::STLSetIntersection<std::set<std::string>>(platform_entries,
723 *result); 727 *result);
724 728
725 result->swap(new_enabled_entries); 729 result->swap(new_enabled_entries);
726 } 730 }
727 731
728 } // namespace flags_ui 732 } // namespace flags_ui
OLDNEW
« no previous file with comments | « components/flags_ui/feature_entry_macros.h ('k') | components/flags_ui/flags_state_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698