Initial implementation of FeatureList in base/.

base/metrics/feature_list.cc

Index: base/metrics/feature_list.cc
diff --git a/base/metrics/feature_list.cc b/base/metrics/feature_list.cc
new file mode 100644
index 0000000000000000000000000000000000000000..57c199290586057813d76a4070f7b50c0238b908
--- /dev/null
+++ b/base/metrics/feature_list.cc
@@ -0,0 +1,115 @@
+// Copyright 2015 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 "base/metrics/feature_list.h"
+
+#include <vector>
+
+#include "base/logging.h"
+#include "base/strings/string_split.h"
+
+namespace base {
+
+namespace {
+
+// Pointer to the FeatureList instance singleton that was set via
+// FeatureList::SetInstance(). Does not use base/memory/singleton.h in order to
+// have more control over initialization timing. Leaky.
+FeatureList* g_instance = nullptr;
+
+// Splits a comma-separated string containing feature names into a vector.
+std::vector<std::string> SplitFeatureListString(const std::string& input) {
+  return SplitString(input, ",", TRIM_WHITESPACE, SPLIT_WANT_NONEMPTY);
+}
+
+}  // namespace
+
+FeatureList::FeatureList() : initialized_(false) {}
+
+FeatureList::~FeatureList() {}
+
+void FeatureList::InitializeFromCommandLine(
+    const std::string& enable_features,
+    const std::string& disable_features) {

[Ilya Sherman, 2015/09/01 03:55:25]

Currently, there is no guard preventing this method from being called twice
during initialization.  Should there be?

[Alexei Svitkine (slow), 2015/09/01 15:53:43]

I don't think it's really worth guarding against - since that will only be
called during initialization anyway in early start up (and it's already checked
that it's done during initialization). So I don't think there's any opportunity
for abuse, etc.

But if you prefer we check that, it can be done by switching the |initialized_|
bool to an enum with different states. Let me know what you think - I've left it
as-is for now.

+  DCHECK(!initialized_);
+
+  // Process disabled features first, so that disabled ones take precedence over
+  // enabled ones (since RegisterOverride() uses insert()).
+  for (const auto& feature_name : SplitFeatureListString(disable_features)) {
+    RegisterOverride(feature_name, false);
+  }
+  for (const auto& feature_name : SplitFeatureListString(enable_features)) {
+    RegisterOverride(feature_name, true);
+  }
+}
+
+void FeatureList::FinalizeInitialization() {
+  DCHECK(!initialized_);
+  initialized_ = true;
+}
+
+bool FeatureList::IsFeatureEnabled(const Feature& feature) {
+  DCHECK(initialized_);
+  DCHECK(CheckFeatureIdentity(feature)) << feature.name;
+
+  auto it = overrides_.find(feature.name);
+  if (it != overrides_.end()) {
+    auto entry = &*it->second;

[Ilya Sherman, 2015/09/01 03:55:25]

nit: Boy, this line makes me do a double-take.  I think something like "const
OverrideEntry& entry = *it->second;" would be a little clearer.

[Alexei Svitkine (slow), 2015/09/01 15:53:43]

Done. In the future it won't be able to be a const (i.e. when there's a lock for
killable features), but we can keep it like you suggest for now.

+    // TODO(asvitkine) Expand this section as more support is added.
+    return entry->overriden_state;
+  }
+  // Otherwise, return the default state.
+  return feature.default_state;
+}
+
+// static
+bool FeatureList::IsEnabled(const Feature& feature) {
+  DCHECK(GetInstance());

[Ilya Sherman, 2015/09/01 03:55:25]

nit: This line seems a little silly, since the following line would crash if the
instance were null.

[Alexei Svitkine (slow), 2015/09/01 15:53:43]

Done.

+  return GetInstance()->IsFeatureEnabled(feature);
+}
+
+// static
+FeatureList* FeatureList::GetInstance() {
+  return g_instance;
+}
+
+// static
+void FeatureList::SetInstance(scoped_ptr<FeatureList> instance) {
+  DCHECK(!g_instance);
+  instance->FinalizeInitialization();
+
+  // Note: Intentional leak of global singleton.
+  g_instance = instance.release();
+}
+
+// static
+void FeatureList::ClearInstanceForTesting() {
+  delete g_instance;
+  g_instance = nullptr;
+}
+
+void FeatureList::RegisterOverride(const std::string& feature_name,
+                                   bool overriden_state) {
+  DCHECK(!initialized_);
+  overrides_.insert(feature_name,
+                    make_scoped_ptr(new OverrideEntry(overriden_state)));
+}
+
+bool FeatureList::CheckFeatureIdentity(const Feature& feature) {
+  AutoLock auto_lock(feature_identity_tracker_lock_);
+
+  auto it = feature_identity_tracker_.find(feature.name);
+  if (it == feature_identity_tracker_.end()) {
+    // If it's not tracked yet, register it.
+    feature_identity_tracker_[feature.name] = &feature;
+    return true;
+  }
+  // Compare address of |feature| to the existing tracked entry.
+  return it->second == &feature;
+}
+
+FeatureList::OverrideEntry::OverrideEntry(bool overriden_state)
+    : overriden_state(overriden_state) {}
+
+}  // namespace base