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

Unified Diff: base/metrics/feature_list.h

Issue 1278403003: Initial implementation of FeatureList in base/. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 4 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/metrics/BUILD.gn ('k') | base/metrics/feature_list.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/metrics/feature_list.h
diff --git a/base/metrics/feature_list.h b/base/metrics/feature_list.h
new file mode 100644
index 0000000000000000000000000000000000000000..554a8ffbf75fc7ce0fe09fd4fb534bfe79c7a32c
--- /dev/null
+++ b/base/metrics/feature_list.h
@@ -0,0 +1,137 @@
+// 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.
+
+#ifndef BASE_METRICS_FEATURE_LIST_H_
+#define BASE_METRICS_FEATURE_LIST_H_
+
+#include <map>
+#include <string>
+
+#include "base/base_export.h"
+#include "base/basictypes.h"
+#include "base/containers/scoped_ptr_map.h"
+#include "base/gtest_prod_util.h"
+#include "base/synchronization/lock.h"
+
+namespace base {
+
+// The Feature struct is used to define the default state for a feature. See
+// comment below for more details. There must only ever be one struct instance
+// for a given feature name - generally defined as a global variable.
+struct BASE_EXPORT Feature {
+ // The name of the feagture.
rkaplow 2015/08/30 19:36:22 feature
Alexei Svitkine (slow) 2015/08/31 16:21:01 Done.
+ const char* const name;
+
+ // The default state (i.e. enabled or disabled) for this feature.
+ const bool default_state;
Ilya Sherman 2015/09/01 03:55:25 Let's use an enum rather than a bool. "default_st
+};
+
+// The FeatureList class is used to determine whether a given feature is on or
+// off. It provides an authoritative answer, taking into account command-line
+// overrides and experimental control.
+//
+// The basic use case is for any feature that can be toggled (e.g. through
+// command-line or an experiment) to have a defined Feature struct, e.g.:
+//
+// struct base::Feature kMyGreatFeature {
+// "MyGreatFeature", true
+// };
+//
+// Then, client code that wishes to query the state of the feature would check:
+//
+// if (base::FeatureList::IsEnabled(kMyGreatFeature)) {
rkaplow 2015/08/30 19:36:22 super minor nit but for even for sample syntax I w
Alexei Svitkine (slow) 2015/08/31 16:21:01 Done.
+//
+// Behind the scenes, the above call would take into account any command-line
+// flags to enable or disable the feature, any experiments that may control it
rkaplow 2015/08/30 19:36:22 I would make it clear here the ordering of priorit
Alexei Svitkine (slow) 2015/08/31 16:21:01 Done.
+// and finally its default state, to determine whether the feature is on.
+//
+// After initial initialization (which should be done single-threaded), the
+// FeatureList API is thread safe.
+//
+// Note: This class is a singleton, but does not use base/memory/singleton.h in
+// order to have control over its initialization sequence. Specifically, the
+// intended use is to create an instance of this class and fully initialize it,
+// before setting it as the singleton for a process, via SetInstance().
+class BASE_EXPORT FeatureList {
+ public:
+ FeatureList();
+ ~FeatureList();
+
+ // Initializes feature overrides via command-line flags |enable_features| and
+ // |disable_features|, each of which is a comma-separated list of features to
+ // enable or disable, respectively. If a feature appears on both lists, then
+ // it will be disabled. Must only be invoked during initialization phase
+ // (before FinalizeInitialization() has been called).
+ void InitializeFromCommandLine(const std::string& enable_features,
+ const std::string& disable_features);
+
+ // Finalizes the initialization state of the FeatureList instance, so that no
+ // further overrides can be registered. For the singleton feature list, this
+ // gets called when it is registered via SetInstance().
+ void FinalizeInitialization();
+
+ // Returns whether the given |feature| is enabled. This is member function is
+ // normally called through the static IsEnabled() function, which corresponds
+ // to GeInstance()->IsFeatureEnabled(). Must only be called on a FeatureList
+ // that has been fully initialized (FinalizeInitialization() called).
+ bool IsFeatureEnabled(const Feature& feature);
rkaplow 2015/08/30 19:36:22 can this be private?
Alexei Svitkine (slow) 2015/08/31 16:21:01 I don't think there's a problem with it being publ
+
+ // Returns whether the given |feature| is enabled. Must only be called after
+ // the singleton instance has been registered via SetInstance(). Additionally,
+ // a feature with a given name must only have a single corresponding Feature
+ // struct, which is checked for in debug builds.
+ static bool IsEnabled(const Feature& feature);
+
+ // Returns the singleton instance of FeatureList. Will return null until an
+ // instance is registered via SetInstance().
+ static FeatureList* GetInstance();
+
+ // Registers the given |instance| to be the singleton feature list for this
+ // process. This should only be called once and |instance| must not be null.
+ static void SetInstance(scoped_ptr<FeatureList> instance);
+
+ // Clears the previously-registered singleton instance for tests.
+ static void ClearInstanceForTesting();
+
+ private:
+ FRIEND_TEST_ALL_PREFIXES(FeatureListTest, CheckFeatureIdentity);
Ilya Sherman 2015/09/01 03:55:25 Please use a mechanism that exposes a more narrow
+
+ // Registers an override for feature |feature_name|. The override specifies
+ // whether the feature should be on or off (via |overridden_state|), which
+ // will take precedence over the feature's default state.
+ void RegisterOverride(const std::string& feature_name, bool overriden_state);
+
+ // Checks whether |feature| has the same address as any previously seen
+ // Feature structs with the same name. Used only from DCHECKs and tests.
+ bool CheckFeatureIdentity(const Feature& feature);
+
+ struct OverrideEntry {
+ // The overridden enable (on/off) state of the feature.
+ bool overriden_state;
+
+ // TODO(asvitkine): Expand this as more support is added.
+
+ explicit OverrideEntry(bool overriden_state);
+ };
+ // Map from feature name to an OverrideEntry struct for the feature, if it
+ // exists. A ScopedPtrMap<> is used because OverrideEntry will later expand
+ // to hold additional non-copyable fields.
+ ScopedPtrMap<std::string, scoped_ptr<OverrideEntry>> overrides_;
+
+ // Locked map that keeps track of seen features, to ensure a single feature is
+ // only defined once. This verification is only done in builds with DCHECKs
+ // enabled.
+ Lock feature_identity_tracker_lock_;
+ std::map<std::string, const Feature*> feature_identity_tracker_;
+
+ // Whether this object has been fully initialized. This gets set to true as a
+ // result of FinalizeInitialization().
+ bool initialized_;
+
+ DISALLOW_COPY_AND_ASSIGN(FeatureList);
+};
+
+} // namespace base
+
+#endif // BASE_METRICS_FEATURE_LIST_H_
« no previous file with comments | « base/metrics/BUILD.gn ('k') | base/metrics/feature_list.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698