Updating FeatureList default initialization pattern

base/feature_list.cc

 // 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/feature_list.h"
 
 #include <stddef.h>
 
 #include <utility>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.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;
 
+// Tracks whether the FeatureList instance was initialized via an accessor.
+bool g_initialized_from_accessor = false;
+
 // Some characters are not allowed to appear in feature names or the associated
 // field trial names, as they are used as special characters for command-line
 // serialization. This function checks that the strings are ASCII (since they
 // are used in command-line API functions that require ASCII) and whether there
 // are any reserved characters present, returning true if the string is valid.
 // Only called in DCHECKs.
 bool IsValidFeatureOrFieldTrialName(const std::string& name) {
   return IsStringASCII(name) && name.find_first_of(",<*") == std::string::npos;
 }
 
 }  // namespace
 
-FeatureList::FeatureList()
-  : initialized_(false),
-    initialized_from_command_line_(false) {
-}
+FeatureList::FeatureList() {}
 
 FeatureList::~FeatureList() {}
 
 void FeatureList::InitializeFromCommandLine(
     const std::string& enable_features,
     const std::string& disable_features) {
   DCHECK(!initialized_);
 
   // Process disabled features first, so that disabled ones take precedence over
   // enabled ones (since RegisterOverride() uses insert()).
@@ skipping 74 matching lines @@
     target_list->append(entry.first);
     if (entry.second.field_trial) {
       target_list->push_back('<');
       target_list->append(entry.second.field_trial->trial_name());
     }
   }
 }
 
 // static
 bool FeatureList::IsEnabled(const Feature& feature) {
-  return GetInstance()->IsFeatureEnabled(feature);
+  if (!g_instance) {
+    g_initialized_from_accessor = true;
+    return feature.default_state == FEATURE_ENABLED_BY_DEFAULT;
+  }
+  return g_instance->IsFeatureEnabled(feature);
 }
 
 // static
 FieldTrial* FeatureList::GetFieldTrial(const Feature& feature) {
   return GetInstance()->GetAssociatedFieldTrial(feature);
 }
 
 // static
 std::vector<std::string> FeatureList::SplitFeatureListString(
     const std::string& input) {
   return SplitString(input, ",", TRIM_WHITESPACE, SPLIT_WANT_NONEMPTY);
 }
 
 // static
 bool FeatureList::InitializeInstance(const std::string& enable_features,
                                      const std::string& disable_features) {
   // We want to initialize a new instance here to support command-line features
   // in testing better. For example, we initialize a dummy instance in
   // base/test/test_suite.cc, and override it in content/browser/
   // browser_main_loop.cc.
   // On the other hand, we want to avoid re-initialization from command line.
   // For example, we initialize an instance in chrome/browser/
   // chrome_browser_main.cc and do not override it in content/browser/
   // browser_main_loop.cc.
+  // If the singleton was previously initialized from within an accessor, we
+  // want to prevent callers from reinitializing the singleton and masking the
+  // accessor call(s) which likely returned incorrect information.
+  CHECK(!g_initialized_from_accessor);
   bool instance_existed_before = false;
   if (g_instance) {
     if (g_instance->initialized_from_command_line_)
       return false;
 
     delete g_instance;
     g_instance = nullptr;
     instance_existed_before = true;
   }
 
@@ skipping 14 matching lines @@
   instance->FinalizeInitialization();
 
   // Note: Intentional leak of global singleton.
   g_instance = instance.release();
 }
 
 // static
 void FeatureList::ClearInstanceForTesting() {
   delete g_instance;
   g_instance = nullptr;
+  g_initialized_from_accessor = false;
 }
 
 void FeatureList::FinalizeInitialization() {
   DCHECK(!initialized_);
   initialized_ = true;
 }
 
 bool FeatureList::IsFeatureEnabled(const Feature& feature) {
   DCHECK(initialized_);
   DCHECK(IsValidFeatureOrFieldTrialName(feature.name)) << feature.name;
@@ skipping 83 matching lines @@
   return it->second == &feature;
 }
 
 FeatureList::OverrideEntry::OverrideEntry(OverrideState overridden_state,
                                           FieldTrial* field_trial)
     : overridden_state(overridden_state),
       field_trial(field_trial),
       overridden_by_field_trial(field_trial != nullptr) {}
 
 }  // namespace base