Store and retrieve features from shared memory

If using shared memory, stores base::Features in the field trial
allocator (under a different type defined in FeatureList) when
spawning the first new process. When a child process is created,
it will first try to get initialized from shared memory. If that
doesn't work, it'll fallback to the command line.

BUG=661224

Review-Url: https://codereview.chromium.org/2546653002
Cr-Commit-Position: refs/heads/master@{#436311}

base/feature_list.cc

@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
+#include "base/pickle.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 
@@ -27,6 +28,42 @@ FeatureList* g_instance = nullptr;
 // Tracks whether the FeatureList instance was initialized via an accessor.
 bool g_initialized_from_accessor = false;
 
+const uint32_t kFeatureType = 0x06567CA6 + 1;  // SHA1(FeatureEntry) v1
+
+// An allocator entry for a feature in shared memory. The FeatureEntry is
+// followed by a base::Pickle object that contains the feature and trial name.
+// Any changes to this structure requires a bump in kFeatureType defined above.
+struct FeatureEntry {
+  // Expected size for 32/64-bit check.
+  static constexpr size_t kExpectedInstanceSize = 8;
+
+  // Specifies whether a feature override enables or disables the feature. Same
+  // values as the OverrideState enum in feature_list.h
+  uint32_t override_state;
+
+  // Size of the pickled structure, NOT the total size of this entry.
+  uint32_t pickle_size;
+
+  // Reads the feature and trial name from the pickle. Calling this is only
+  // valid on an initialized entry that's in shared memory.
+  bool GetFeatureAndTrialName(StringPiece* feature_name,
+                              StringPiece* trial_name) const {
+    const char* src =
+        reinterpret_cast<const char*>(this) + sizeof(FeatureEntry);
+
+    Pickle pickle(src, pickle_size);
+    PickleIterator pickle_iter(pickle);
+
+    if (!pickle_iter.ReadStringPiece(feature_name))
+      return false;
+
+    // Return true because we are not guaranteed to have a trial name anyways.
+    auto sink = pickle_iter.ReadStringPiece(trial_name);
+    ALLOW_UNUSED_LOCAL(sink);
+    return true;
+  }
+};
+
 // 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
@@ -56,6 +93,31 @@ void FeatureList::InitializeFromCommandLine(
   initialized_from_command_line_ = true;
 }
 
+void FeatureList::InitializeFromSharedMemory(
+    PersistentMemoryAllocator* allocator) {
+  DCHECK(!initialized_);
+
+  PersistentMemoryAllocator::Iterator iter(allocator);
+
+  PersistentMemoryAllocator::Reference ref;
+  while ((ref = iter.GetNextOfType(kFeatureType)) !=
+         PersistentMemoryAllocator::kReferenceNull) {
+    const FeatureEntry* entry =
+        allocator->GetAsObject<const FeatureEntry>(ref, kFeatureType);
+
+    OverrideState override_state =
+        static_cast<OverrideState>(entry->override_state);
+
+    StringPiece feature_name;
+    StringPiece trial_name;
+    if (!entry->GetFeatureAndTrialName(&feature_name, &trial_name))
+      continue;
+
+    FieldTrial* trial = FieldTrialList::Find(trial_name.as_string());
+    RegisterOverride(feature_name, override_state, trial);
+  }
+}
+
 bool FeatureList::IsFeatureOverriddenFromCommandLine(
     const std::string& feature_name,
     OverrideState state) const {
@@ -98,6 +160,33 @@ void FeatureList::RegisterFieldTrialOverride(const std::string& feature_name,
   RegisterOverride(feature_name, override_state, field_trial);
 }
 
+void FeatureList::AddFeaturesToAllocator(PersistentMemoryAllocator* allocator) {
+  DCHECK(initialized_);
+
+  for (const auto& override : overrides_) {
+    Pickle pickle;
+    pickle.WriteString(override.first);
+    if (override.second.field_trial)
+      pickle.WriteString(override.second.field_trial->trial_name());
+
+    size_t total_size = sizeof(FeatureEntry) + pickle.size();
+    PersistentMemoryAllocator::Reference ref =
+        allocator->Allocate(total_size, kFeatureType);
+    if (!ref)
+      return;
+
+    FeatureEntry* entry =
+        allocator->GetAsObject<FeatureEntry>(ref, kFeatureType);
+    entry->override_state = override.second.overridden_state;
+    entry->pickle_size = pickle.size();
+
+    char* dst = reinterpret_cast<char*>(entry) + sizeof(FeatureEntry);
+    memcpy(dst, pickle.data(), pickle.size());
+
+    allocator->MakeIterable(ref);
+  }
+}
+
 void FeatureList::GetFeatureOverrides(std::string* enable_overrides,
                                       std::string* disable_overrides) {
   DCHECK(initialized_);

base/feature_list.h

@@ -13,6 +13,7 @@
 #include "base/base_export.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/metrics/persistent_memory_allocator.h"
 #include "base/strings/string_piece.h"
 #include "base/synchronization/lock.h"
 
@@ -92,6 +93,11 @@ class BASE_EXPORT FeatureList {
   void InitializeFromCommandLine(const std::string& enable_features,
                                  const std::string& disable_features);
 
+  // Initializes feature overrides through the field trial allocator, which
+  // we're using to store the feature names, their override state, and the name
+  // of the associated field trial.
+  void InitializeFromSharedMemory(PersistentMemoryAllocator* allocator);
+
   // Specifies whether a feature override enables or disables the feature.
   enum OverrideState {
     OVERRIDE_USE_DEFAULT,
@@ -124,6 +130,9 @@ class BASE_EXPORT FeatureList {
                                   OverrideState override_state,
                                   FieldTrial* field_trial);
 
+  // Loops through feature overrides and serializes them all into |allocator|.
+  void AddFeaturesToAllocator(PersistentMemoryAllocator* allocator);
+
   // Returns comma-separated lists of feature names (in the same format that is
   // accepted by InitializeFromCommandLine()) corresponding to features that
   // have been overridden - either through command-line or via FieldTrials. For
@@ -180,6 +189,10 @@ class BASE_EXPORT FeatureList {
 
  private:
   FRIEND_TEST_ALL_PREFIXES(FeatureListTest, CheckFeatureIdentity);
+  FRIEND_TEST_ALL_PREFIXES(FeatureListTest,
+                           StoreAndRetrieveFeaturesFromSharedMemory);
+  FRIEND_TEST_ALL_PREFIXES(FeatureListTest,
+                           StoreAndRetrieveAssociatedFeaturesFromSharedMemory);
 
   struct OverrideEntry {
     // The overridden enable (on/off) state of the feature.

base/feature_list_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/persistent_memory_allocator.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -468,4 +469,68 @@ TEST_F(FeatureListTest, UninitializedInstance_IsEnabledReturnsFalse) {
   EXPECT_FALSE(FeatureList::IsEnabled(kFeatureOffByDefault));
 }
 
+TEST_F(FeatureListTest, StoreAndRetrieveFeaturesFromSharedMemory) {
+  std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+
+  // Create some overrides.
+  feature_list->RegisterOverride(kFeatureOffByDefaultName,
+                                 FeatureList::OVERRIDE_ENABLE_FEATURE, nullptr);
+  feature_list->RegisterOverride(
+      kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE, nullptr);
+  feature_list->FinalizeInitialization();
+
+  // Create an allocator and store the overrides.
+  std::unique_ptr<SharedMemory> shm(new SharedMemory());
+  shm->CreateAndMapAnonymous(4 << 10);
+  SharedPersistentMemoryAllocator allocator(std::move(shm), 1, "", false);
+  feature_list->AddFeaturesToAllocator(&allocator);
+
+  std::unique_ptr<base::FeatureList> feature_list2(new base::FeatureList);
+
+  // Check that the new feature list is empty.
+  EXPECT_FALSE(feature_list2->IsFeatureOverriddenFromCommandLine(
+      kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
+  EXPECT_FALSE(feature_list2->IsFeatureOverriddenFromCommandLine(
+      kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
+
+  feature_list2->InitializeFromSharedMemory(&allocator);
+  // Check that the new feature list now has 2 overrides.
+  EXPECT_TRUE(feature_list2->IsFeatureOverriddenFromCommandLine(
+      kFeatureOffByDefaultName, FeatureList::OVERRIDE_ENABLE_FEATURE));
+  EXPECT_TRUE(feature_list2->IsFeatureOverriddenFromCommandLine(
+      kFeatureOnByDefaultName, FeatureList::OVERRIDE_DISABLE_FEATURE));
+}
+
+TEST_F(FeatureListTest, StoreAndRetrieveAssociatedFeaturesFromSharedMemory) {
+  FieldTrialList field_trial_list(nullptr);
+  std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+
+  // Create some overrides.
+  FieldTrial* trial1 = FieldTrialList::CreateFieldTrial("TrialExample1", "A");
+  FieldTrial* trial2 = FieldTrialList::CreateFieldTrial("TrialExample2", "B");
+  feature_list->RegisterFieldTrialOverride(
+      kFeatureOnByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial1);
+  feature_list->RegisterFieldTrialOverride(
+      kFeatureOffByDefaultName, FeatureList::OVERRIDE_USE_DEFAULT, trial2);
+  feature_list->FinalizeInitialization();
+
+  // Create an allocator and store the overrides.
+  std::unique_ptr<SharedMemory> shm(new SharedMemory());
+  shm->CreateAndMapAnonymous(4 << 10);
+  SharedPersistentMemoryAllocator allocator(std::move(shm), 1, "", false);
+  feature_list->AddFeaturesToAllocator(&allocator);
+
+  std::unique_ptr<base::FeatureList> feature_list2(new base::FeatureList);
+  feature_list2->InitializeFromSharedMemory(&allocator);
+  feature_list2->FinalizeInitialization();
+
+  // Check that the field trials are still associated.
+  FieldTrial* associated_trial1 =
+      feature_list2->GetAssociatedFieldTrial(kFeatureOnByDefault);
+  FieldTrial* associated_trial2 =
+      feature_list2->GetAssociatedFieldTrial(kFeatureOffByDefault);
+  EXPECT_EQ(associated_trial1, trial1);
+  EXPECT_EQ(associated_trial2, trial2);
+}
+
 }  // namespace base

base/metrics/field_trial.cc

@@ -10,7 +10,6 @@
 #include "base/base_switches.h"
 #include "base/build_time.h"
 #include "base/command_line.h"
-#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial_param_associator.h"
 #include "base/pickle.h"
@@ -49,6 +48,10 @@ const char kActivationMarker = '*';
 // for now while the implementation is fleshed out (e.g. data format, single
 // shared memory segment). See https://codereview.chromium.org/2365273004/ and
 // crbug.com/653874
+// The browser is the only process that has write access to the shared memory.
+// This is safe from race conditions because MakeIterable is a release operation
+// and GetNextOfType is an acquire operation, so memory writes before
+// MakeIterable happen before memory reads after GetNextOfType.
 const bool kUseSharedMemoryForFieldTrials = true;
 
 // Constants for the field trial allocator.
@@ -248,7 +251,19 @@ bool ParseFieldTrialsString(const std::string& trials_string,
   return true;
 }
 
-void AddForceFieldTrialsFlag(CommandLine* cmd_line) {
+void AddFeatureAndFieldTrialFlags(const char* enable_features_switch,
+                                  const char* disable_features_switch,
+                                  CommandLine* cmd_line) {
+  std::string enabled_features;
+  std::string disabled_features;
+  FeatureList::GetInstance()->GetFeatureOverrides(&enabled_features,
+                                                  &disabled_features);
+
+  if (!enabled_features.empty())
+    cmd_line->AppendSwitchASCII(enable_features_switch, enabled_features);
+  if (!disabled_features.empty())
+    cmd_line->AppendSwitchASCII(disable_features_switch, disabled_features);
+
   std::string field_trial_states;
   FieldTrialList::AllStatesToString(&field_trial_states);
   if (!field_trial_states.empty()) {
@@ -805,6 +820,24 @@ void FieldTrialList::CreateTrialsFromCommandLine(
   }
 }
 
+// static
+void FieldTrialList::CreateFeaturesFromCommandLine(
+    const base::CommandLine& command_line,
+    const char* enable_features_switch,
+    const char* disable_features_switch,
+    FeatureList* feature_list) {
+  // Fallback to command line if not using shared memory.
+  if (!kUseSharedMemoryForFieldTrials ||
+      !global_->field_trial_allocator_.get()) {
+    return feature_list->InitializeFromCommandLine(
+        command_line.GetSwitchValueASCII(enable_features_switch),
+        command_line.GetSwitchValueASCII(disable_features_switch));
+  }
+
+  feature_list->InitializeFromSharedMemory(
+      global_->field_trial_allocator_.get());
+}
+
 #if defined(POSIX_WITH_ZYGOTE)
 // static
 bool FieldTrialList::CreateTrialsFromDescriptor(int fd_key) {
@@ -854,12 +887,19 @@ int FieldTrialList::GetFieldTrialHandle() {
 // static
 void FieldTrialList::CopyFieldTrialStateToFlags(
     const char* field_trial_handle_switch,
+    const char* enable_features_switch,
+    const char* disable_features_switch,
     CommandLine* cmd_line) {
   // TODO(lawrencewu): Ideally, having the global would be guaranteed. However,
   // content browser tests currently don't create a FieldTrialList because they
   // don't run ChromeBrowserMainParts code where it's done for Chrome.
-  if (!global_)
+  // Some tests depend on the enable and disable features flag switch, though,
+  // so we can still add those even though AllStatesToString() will be a no-op.
+  if (!global_) {
+    AddFeatureAndFieldTrialFlags(enable_features_switch,
+                                 disable_features_switch, cmd_line);
     return;
+  }
 
 #if defined(OS_WIN) || defined(POSIX_WITH_ZYGOTE)
   // Use shared memory to pass the state if the feature is enabled, otherwise
@@ -869,7 +909,8 @@ void FieldTrialList::CopyFieldTrialStateToFlags(
     // If the readonly handle didn't get duplicated properly, then fallback to
     // original behavior.
     if (global_->readonly_allocator_handle_ == kInvalidPlatformFile) {
-      AddForceFieldTrialsFlag(cmd_line);
+      AddFeatureAndFieldTrialFlags(enable_features_switch,
+                                   disable_features_switch, cmd_line);
       return;
     }
 
@@ -895,7 +936,8 @@ void FieldTrialList::CopyFieldTrialStateToFlags(
   }
 #endif
 
-  AddForceFieldTrialsFlag(cmd_line);
+  AddFeatureAndFieldTrialFlags(enable_features_switch, disable_features_switch,
+                               cmd_line);
 }
 
 // static
@@ -1171,6 +1213,10 @@ void FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded() {
     AddToAllocatorWhileLocked(registered.second);
   }
 
+  // Add all existing features.
+  FeatureList::GetInstance()->AddFeaturesToAllocator(
+      global_->field_trial_allocator_.get());
+
 #if defined(OS_WIN) || defined(POSIX_WITH_ZYGOTE)
   // Set |readonly_allocator_handle_| so we can pass it to be inherited and
   // via the command line.

base/metrics/field_trial.h

@@ -65,6 +65,7 @@
 
 #include "base/base_export.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/files/file.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
@@ -508,6 +509,14 @@ class BASE_EXPORT FieldTrialList {
                                           const char* field_trial_handle_switch,
                                           int fd_key);
 
+  // Creates base::Feature overrides from the command line by first trying to
+  // use shared memory and then falling back to the command line if it fails.
+  static void CreateFeaturesFromCommandLine(
+      const base::CommandLine& command_line,
+      const char* enable_features_switch,
+      const char* disable_features_switch,
+      FeatureList* feature_list);
+
 #if defined(OS_WIN)
   // On Windows, we need to explicitly pass down any handles to be inherited.
   // This function adds the shared memory handle to field trial state to the
@@ -527,6 +536,8 @@ class BASE_EXPORT FieldTrialList {
   // Needs the |field_trial_handle_switch| argument to be passed in since base/
   // can't depend on content/.
   static void CopyFieldTrialStateToFlags(const char* field_trial_handle_switch,
+                                         const char* enable_features_switch,
+                                         const char* disable_features_switch,
                                          base::CommandLine* cmd_line);
 
   // Create a FieldTrial with the given |name| and using 100% probability for

base/metrics/field_trial_unittest.cc

@@ -19,6 +19,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/test/gtest_util.h"
 #include "base/test/mock_entropy_provider.h"
+#include "base/test/scoped_feature_list.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -1145,15 +1146,21 @@ TEST(FieldTrialListTest, TestCopyFieldTrialStateToFlags) {
   base::FilePath test_file_path = base::FilePath(FILE_PATH_LITERAL("Program"));
   base::CommandLine cmd_line = base::CommandLine(test_file_path);
   const char field_trial_handle[] = "test-field-trial-handle";
+  const char enable_features_switch[] = "test-enable-features";
+  const char disable_features_switch[] = "test-disable-features";
 
-  base::FieldTrialList::CopyFieldTrialStateToFlags(field_trial_handle,
-                                                   &cmd_line);
+  base::FieldTrialList::CopyFieldTrialStateToFlags(
+      field_trial_handle, enable_features_switch, disable_features_switch,
+      &cmd_line);
   EXPECT_TRUE(cmd_line.HasSwitch(field_trial_handle) ||
               cmd_line.HasSwitch(switches::kForceFieldTrials));
 }
 #endif
 
 TEST(FieldTrialListTest, InstantiateAllocator) {
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.Init();
+
   FieldTrialList field_trial_list(nullptr);
   FieldTrialList::CreateFieldTrial("Trial1", "Group1");
 
@@ -1176,6 +1183,9 @@ TEST(FieldTrialListTest, AddTrialsToAllocator) {
   // Scoping the first FieldTrialList, as we need another one to test that it
   // matches.
   {
+    test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.Init();
+
     FieldTrialList field_trial_list(nullptr);
     FieldTrialList::CreateFieldTrial("Trial1", "Group1");
     FieldTrialList::InstantiateFieldTrialAllocatorIfNeeded();
@@ -1198,6 +1208,9 @@ TEST(FieldTrialListTest, DoNotAddSimulatedFieldTrialsToAllocator) {
   constexpr char kTrialName[] = "trial";
   base::SharedMemoryHandle handle;
   {
+    test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.Init();
+
     // Create a simulated trial and a real trial and call group() on them, which
     // should only add the real trial to the field trial allocator.
     FieldTrialList field_trial_list(nullptr);
@@ -1230,6 +1243,9 @@ TEST(FieldTrialListTest, DoNotAddSimulatedFieldTrialsToAllocator) {
 }
 
 TEST(FieldTrialListTest, AssociateFieldTrialParams) {
+  test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.Init();
+
   std::string trial_name("Trial1");
   std::string group_name("Group1");
 
@@ -1266,6 +1282,9 @@ TEST(FieldTrialListTest, ClearParamsFromSharedMemory) {
 
   base::SharedMemoryHandle handle;
   {
+    test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.Init();
+
     // Create a field trial with some params.
     FieldTrialList field_trial_list(nullptr);
     FieldTrial* trial =

content/app/content_main_runner.cc

@@ -155,9 +155,9 @@ void InitializeFieldTrialAndFeatureList(
 #endif
 
   std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
-  feature_list->InitializeFromCommandLine(
-      command_line.GetSwitchValueASCII(switches::kEnableFeatures),
-      command_line.GetSwitchValueASCII(switches::kDisableFeatures));
+  base::FieldTrialList::CreateFeaturesFromCommandLine(
+      command_line, switches::kEnableFeatures, switches::kDisableFeatures,
+      feature_list.get());
   base::FeatureList::SetInstance(std::move(feature_list));
 }
 

content/browser/browser_child_process_host_impl.cc

@@ -219,19 +219,11 @@ void BrowserChildProcessHostImpl::TerminateAll() {
 // static
 void BrowserChildProcessHostImpl::CopyFeatureAndFieldTrialFlags(
     base::CommandLine* cmd_line) {
-  std::string enabled_features;
-  std::string disabled_features;
-  base::FeatureList::GetInstance()->GetFeatureOverrides(&enabled_features,
-                                                        &disabled_features);
-  if (!enabled_features.empty())
-    cmd_line->AppendSwitchASCII(switches::kEnableFeatures, enabled_features);
-  if (!disabled_features.empty())
-    cmd_line->AppendSwitchASCII(switches::kDisableFeatures, disabled_features);
-
   // If we run base::FieldTrials, we want to pass to their state to the
   // child process so that it can act in accordance with each state.
-  base::FieldTrialList::CopyFieldTrialStateToFlags(switches::kFieldTrialHandle,
-                                                   cmd_line);
+  base::FieldTrialList::CopyFieldTrialStateToFlags(
+      switches::kFieldTrialHandle, switches::kEnableFeatures,
+      switches::kDisableFeatures, cmd_line);
 }
 
 void BrowserChildProcessHostImpl::Launch(