[Chromecast] Use base::FeatureList to control features.

chromecast/base/cast_features.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/base/cast_features.h"
+
+#include "base/command_line.h"
+#include "base/feature_list.h"
+#include "base/lazy_instance.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_param_associator.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
+
+namespace chromecast {
+namespace {
+
+// A constant used to always activate a FieldTrial.
+const base::FieldTrial::Probability k100PercentProbability = 100;
+
+// The name of the default group to use for Cast DCS features.
+const char kDefaultDCSFeaturesGroup[] = "default_dcs_features_group";
+
+base::LazyInstance<std::unordered_set<int32_t>>::DestructorAtExit

[halliwell, 2017/04/20 02:49:30]

don't we usually just do leaky unless there's a strong reason not to?

[slan, 2017/04/20 16:26:02]

Chromium seems to be fairly split. There are 17 uses of DestructorAtExit to wrap
static std containers: 

https://cs.chromium.org/search/?q=base::LazyInstance<std::(.*)>::DestructorAt...

... and 28 uses of Leaky:

https://cs.chromium.org/search/?q=base::LazyInstance<std::(.*)>::Leaky


Switched to Leaky, as it doesn't really matter if the dtor is called here.

+    g_experiment_ids = LAZY_INSTANCE_INITIALIZER;
+bool g_experiment_ids_initialized = false;
+
+void SetExperimentIds(const base::ListValue& list) {
+  DCHECK(!g_experiment_ids_initialized);
+  int32_t id;
+  auto ids = base::MakeUnique<std::unordered_set<int32_t>>();
+  for (size_t i = 0; i < list.GetSize(); ++i) {
+    if (list.GetInteger(i, &id)) {
+      ids->insert(id);
+    } else {
+      LOG(ERROR) << "Non-integer value found in experiment id list!";
+    }
+  }
+  g_experiment_ids.Get().swap(*ids);
+  g_experiment_ids_initialized = true;
+}
+
+}  // namespace
+
+void InitializeFeatureList(const base::DictionaryValue& dcs_features,
+                           const base::ListValue& dcs_experiment_ids,
+                           const std::string& cmd_line_enable_features,
+                           const std::string& cmd_line_disable_features) {
+  DCHECK(!base::FeatureList::GetInstance());
+
+  // Set the experiments.
+  SetExperimentIds(dcs_experiment_ids);
+
+  // Initialize the FeatureList from the command line.
+  auto feature_list = base::MakeUnique<base::FeatureList>();
+  feature_list->InitializeFromCommandLine(cmd_line_enable_features,
+                                          cmd_line_disable_features);
+
+  // Override defaults from the DCS config.
+  base::DictionaryValue::Iterator iterator(dcs_features);

[halliwell, 2017/04/20 02:49:30]

nit, can init iterator inside for (...)?

[slan, 2017/04/20 16:26:02]

Done, also updated the type and variable name to be more brief.

+  for (; !iterator.IsAtEnd(); iterator.Advance()) {
+    // Each feature must have its own FieldTrial object. Since experiments are
+    // controlled server-side for Chromecast, and this class is designed with a
+    // client-side experimentation framework in mind, these parameters are
+    // carefully chosen:
+    //   - The field trial name is unused for our purposes, we simple need it to
+    //     be unique among features.
+    //   - The probability is hard-coded to 100% so that the FeatureList always
+    //     respects the value from DCS.
+    //   - The default group is unused; it will be the same for every feature.
+    //   - Expiration year, month, and day use a special value such that the
+    //     feature will never expire.
+    //   - SESSION_RANDOMIZED is used to prevent the need for an
+    //     entropy_provider. However, this value doesn't matter.
+    //   - We don't care about the group_id.
+    //
+    const std::string& field_trial_name = iterator.key();
+    auto* field_trial = base::FieldTrialList::FactoryGetFieldTrial(
+        field_trial_name, k100PercentProbability, kDefaultDCSFeaturesGroup,
+        base::FieldTrialList::kNoExpirationYear, 1 /* month */, 1 /* day */,
+        base::FieldTrial::SESSION_RANDOMIZED, nullptr);
+
+    bool enabled;
+    if (iterator.value().GetAsBoolean(&enabled)) {
+      // A boolean entry simply either enables or disables a feature.
+      feature_list->RegisterFieldTrialOverride(
+          iterator.key(),
+          enabled ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
+                  : base::FeatureList::OVERRIDE_DISABLE_FEATURE,
+          field_trial);
+      continue;
+    }
+
+    const base::DictionaryValue* params_dict;
+    if (iterator.value().GetAsDictionary(&params_dict)) {
+      // A dictionary entry implies that the feature is enabled.
+      feature_list->RegisterFieldTrialOverride(
+          iterator.key(), base::FeatureList::OVERRIDE_ENABLE_FEATURE,
+          field_trial);
+
+      // If the feature has not been overriden from the command line, set its
+      // parameters accordingly.
+      if (!feature_list->IsFeatureOverriddenFromCommandLine(
+              iterator.key(), base::FeatureList::OVERRIDE_DISABLE_FEATURE)) {
+        // Build a map of the FieldTrial parameters and associate it to the
+        // FieldTrial.
+        base::FieldTrialParamAssociator::FieldTrialParams params;
+        base::DictionaryValue::Iterator params_iterator(*params_dict);

[halliwell, 2017/04/20 02:49:30]

ditto init in for(...)

[slan, 2017/04/20 16:26:02]

Done.

+        for (; !params_iterator.IsAtEnd(); params_iterator.Advance()) {
+          std::string val;
+          if (params_iterator.value().GetAsString(&val)) {
+            params[params_iterator.key()] = val;
+          } else {
+            LOG(ERROR) << "Entry in params dict for \"" << iterator.key()
+                       << "\""
+                       << " feature is not a string. Skipping.";
+          }
+        }
+
+        // Register the params, so that they can be queried by client code.
+        bool success = base::AssociateFieldTrialParams(
+            field_trial_name, kDefaultDCSFeaturesGroup, params);
+        DCHECK(success);
+      }
+      continue;
+    }
+
+    // Other base::Value types are not supported.
+    LOG(ERROR) << "A DCS feature mapped to an unsupported value. key: "
+               << iterator.key() << " type: " << iterator.value().type();
+  }
+
+  base::FeatureList::SetInstance(std::move(feature_list));
+}
+
+std::unique_ptr<base::DictionaryValue> GetOverriddenFeaturesForStorage(
+    const base::DictionaryValue& features) {
+  auto persistent_dict = base::MakeUnique<base::DictionaryValue>();
+
+  // |features| maps feature names to either a boolean or a dict of params.
+  base::DictionaryValue::Iterator iterator(features);

[halliwell, 2017/04/20 02:49:30]

ditto

[slan, 2017/04/20 16:26:02]

Done.

+  for (; !iterator.IsAtEnd(); iterator.Advance()) {
+    bool enabled;
+    if (iterator.value().GetAsBoolean(&enabled)) {
+      persistent_dict->SetBoolean(iterator.key(), enabled);
+      continue;
+    }
+
+    const base::DictionaryValue* params_dict;
+    if (iterator.value().GetAsDictionary(&params_dict)) {
+      auto params = base::MakeUnique<base::DictionaryValue>();
+      base::DictionaryValue::Iterator params_iterator(*params_dict);
+      bool bval;
+      int ival;
+      double dval;
+      std::string sval;
+      for (; !params_iterator.IsAtEnd(); params_iterator.Advance()) {
+        const auto& param_key = params_iterator.key();

[halliwell, 2017/04/20 02:49:31]

ditto

[slan, 2017/04/20 16:26:02]

Done.

+        const auto& param_val = params_iterator.value();
+        if (param_val.GetAsBoolean(&bval)) {
+          params->SetString(param_key, bval ? "true" : "false");
+        } else if (param_val.GetAsInteger(&ival)) {
+          params->SetString(param_key, base::IntToString(ival));
+        } else if (param_val.GetAsDouble(&dval)) {
+          params->SetString(param_key, base::DoubleToString(dval));
+        } else if (param_val.GetAsString(&sval)) {
+          params->SetString(param_key, sval);
+        } else {
+          LOG(ERROR) << "Entry in params dict for \"" << iterator.key() << "\""
+                     << " is not of a supported type (key: "
+                     << params_iterator.key() << ", type: " << param_val.type();
+        }
+      }
+      persistent_dict->Set(iterator.key(), std::move(params));
+      continue;
+    }
+
+    // Other base::Value types are not supported.
+    LOG(ERROR) << "A DCS feature mapped to an unsupported value. key: "
+               << iterator.key() << " type: " << iterator.value().type();
+  }
+
+  return persistent_dict;
+}
+
+std::unordered_set<int32_t> GetDCSExperimentIds() {
+  DCHECK(g_experiment_ids_initialized);
+  return g_experiment_ids.Get();
+}
+
+void ResetCastFeaturesForTest() {
+  g_experiment_ids_initialized = false;
+  base::FeatureList::ClearInstanceForTesting();
+}
+
+}  // namespace chromecast