Store field trial parameters in shared memory

base/metrics/field_trial.cc

 // Copyright (c) 2012 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/field_trial.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/base_switches.h"
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
+#include "base/metrics/field_trial_param_associator.h"
 #include "base/pickle.h"
 #include "base/process/memory.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 
 namespace base {
 
@@ skipping 38 matching lines @@
   // Expected size for 32/64-bit check.
   static constexpr size_t kExpectedInstanceSize = 8;
 
   // Whether or not this field trial is activated. This is really just a boolean
   // but marked as a uint32_t for portability reasons.
   uint32_t activated;
 
   // Size of the pickled structure, NOT the total size of this entry.
   uint32_t size;
 
+  // Returns an iterator over the data containing names and params.
+  PickleIterator GetPickleIterator() const {
+    char* src = reinterpret_cast<char*>(const_cast<FieldTrialEntry*>(this)) +
+                sizeof(FieldTrialEntry);
+
+    Pickle pickle(src, size);
+    return PickleIterator(pickle);
+  }
+
+  // Takes the iterator and writes out the first two items into |trial_name| and
+  // |group_name|.
+  bool ReadStringPair(PickleIterator* iter,
+                      StringPiece* trial_name,

[bcwhite, 2016/11/23 21:03:52]

The name of this method is generic.  If you use string1_out and string2_out for
parameter names then you can reuse this method.

Might as well create a WriteStringPair and use that, too.  Or do away with this
completely.  It doesn't make much sense to have one but not the other.

[lawrencewu, 2016/11/23 21:49:20]

I don't think we can totally reuse this method, since we'll miss out on the case
where we can't read anymore, and we'll return false there.

I'm not opposed to the WriteStringPair method though -- Changed to add that.

+                      StringPiece* group_name) const {
+    if (!iter->ReadStringPiece(trial_name))
+      return false;
+    if (!iter->ReadStringPiece(group_name))
+      return false;
+    return true;
+  }
+
   // Calling this is only valid when the entry is initialized. That is, it
   // resides in shared memory and has a pickle containing the trial name and
   // group name following it.
   bool GetTrialAndGroupName(StringPiece* trial_name,
                             StringPiece* group_name) const {
-    char* src = reinterpret_cast<char*>(const_cast<FieldTrialEntry*>(this)) +
-                sizeof(FieldTrialEntry);
+    PickleIterator iter = GetPickleIterator();
+    return ReadStringPair(&iter, trial_name, group_name);
+  }
 
-    Pickle pickle(src, size);
-    PickleIterator pickle_iter(pickle);
+  // Calling this is only valid when the entry is initialized as well. Reads the
+  // parameters following the trial and group name and stores them as key-value
+  // mappings in |params|.
+  bool GetParams(std::map<std::string, std::string>* params) const {
+    PickleIterator iter = GetPickleIterator();
+    StringPiece tmp;
+    if (!ReadStringPair(&iter, &tmp, &tmp))
+      return false;
 
-    if (!pickle_iter.ReadStringPiece(trial_name))
-      return false;
-    if (!pickle_iter.ReadStringPiece(group_name))
-      return false;
-    return true;
+    while (true) {
+      StringPiece key;
+      StringPiece value;
+      if (!iter.ReadStringPiece(&key))

[bcwhite, 2016/11/23 21:03:52]

Can you just reuse ReadStringPair here?  iter must have some kind of "has more"
flag you can test, right?

[lawrencewu, 2016/11/23 21:49:20]

Unfortunately, no, there doesn't seem to be a "has more" method:
https://cs.chromium.org/chromium/src/base/pickle.h?q=PickleIterator&sq=packag...

[bcwhite, 2016/11/24 18:29:47]

You can reuse the existing method with a little creativity.

  if (!ReadStringPair(&key, &value))
    return key.empty();  // Non-empty is bad: got one of a pair.

[lawrencewu, 2016/11/24 21:50:35]

That's very clever -- done.

+        return true;  // No more params to read.
+      if (!iter.ReadStringPiece(&value))
+        return false;
+      (*params)[key.as_string()] = value.as_string();
+    }
   }
 };
 
+// Writes out the field trial's contents (via trial_state) to the pickle. The
+// format of the pickle looks like:
+// TrialName, GroupName, ParamKey1, ParamValue1, ParamKey2, ParamValue2, ...
+// If there are no parameters, then it just ends at GroupName.
+bool PickleFieldTrial(const FieldTrial::State& trial_state, Pickle* pickle) {
+  if (!pickle->WriteString(trial_state.trial_name))
+    return false;
+  if (!pickle->WriteString(trial_state.group_name))
+    return false;
+
+  // Get field trial params.
+  std::map<std::string, std::string> params;
+  FieldTrialParamAssociator::GetInstance()->GetFieldTrialParamsWithoutFallback(
+      trial_state.trial_name.as_string(), trial_state.group_name.as_string(),
+      &params);
+
+  // Write params to pickle.
+  for (const auto& param : params) {
+    pickle->WriteString(StringPiece(param.first));

[bcwhite, 2016/11/23 21:03:52]

You're testing the return value of WriteString above but not here.

[lawrencewu, 2016/11/23 21:49:20]

Fixed.

+    pickle->WriteString(StringPiece(param.second));
+  }
+  return true;
+}
+
 // Created a time value based on |year|, |month| and |day_of_month| parameters.
 Time CreateTimeFromParams(int year, int month, int day_of_month) {
   DCHECK_GT(year, 1970);
   DCHECK_GT(month, 0);
   DCHECK_LT(month, 13);
   DCHECK_GT(day_of_month, 0);
   DCHECK_LT(day_of_month, 32);
 
   Time::Exploded exploded;
   exploded.year = year;
@@ skipping 783 matching lines @@
 }
 
 // static
 size_t FieldTrialList::GetFieldTrialCount() {
   if (!global_)
     return 0;
   AutoLock auto_lock(global_->lock_);
   return global_->registered_.size();
 }
 
+// static
+bool FieldTrialList::GetParamsFromSharedMemory(
+    FieldTrial* field_trial,
+    std::map<std::string, std::string>* params) {
+  DCHECK(global_);
+  // If the field trial allocator is not set up yet, then there are several
+  // cases:
+  //   - We are in the browser process and the allocator has not been set up
+  //   yet. If we got here, then we couldn't find the params in
+  //   FieldTrialParamAssociator, so it's definitely not here. Return false.
+  //   - Using shared memory for field trials is not enabled. If we got here,
+  //   then there's nothing in shared memory. Return false.
+  //   - We are in the child process and the allocator has not been set up yet.
+  //   If this is the case, then you are calling this too early. The field trial
+  //   allocator should get set up very early in the lifecycle. Try to see if
+  //   you can call it after it's been set up.
+  if (!global_->field_trial_allocator_)
+    return false;
+
+  // If ref_ isn't set, then the field trial data can't be in shared memory.
+  if (!field_trial->ref_)
+    return false;
+
+  const FieldTrialEntry* entry =
+      global_->field_trial_allocator_->GetAsObject<const FieldTrialEntry>(
+          field_trial->ref_, kFieldTrialType);

[bcwhite, 2016/11/23 21:03:52]

Verify: GetAllocSize() >= sizeof(entry) + entry->size

Since you're checking all the read/write results, you're concerned that the data
might be bad so you check this in code as well and not in a DCHECK.

[lawrencewu, 2016/11/23 21:49:20]

Added.

+  return entry->GetParams(params);
+}
+
+// static
+void FieldTrialList::ClearParamsFromSharedMemoryForTesting() {
+  if (!global_ || !global_->field_trial_allocator_)
+    return;
+  AutoLock auto_lock(global_->lock_);

[bcwhite, 2016/11/23 21:03:52]

The comment in the .h file says lock_ protects access to registered_ but you're
not accessing that in this method.

[lawrencewu, 2016/11/23 21:49:20]

Changed comment to reflect that lock_ also protects access to
field_trial_allocator_.

+
+  // To clear the params, we iterate through every item in the allocator, copy
+  // just the trial and group name into a newly-allocated segment and then clear
+  // the existing item.
+  FieldTrialAllocator* allocator = global_->field_trial_allocator_.get();
+  FieldTrialAllocator::Iterator mem_iter(allocator);
+
+  FieldTrial::FieldTrialRef prev_ref;
+  while ((prev_ref = mem_iter.GetNextOfType(kFieldTrialType)) !=
+         FieldTrialAllocator::kReferenceNull) {
+    // Get the existing field trial entry in shared memory.
+    const FieldTrialEntry* prev_entry =
+        allocator->GetAsObject<const FieldTrialEntry>(prev_ref,
+                                                      kFieldTrialType);
+    StringPiece trial_name;
+    StringPiece group_name;
+    if (!prev_entry->GetTrialAndGroupName(&trial_name, &group_name))
+      continue;
+
+    // Write a new entry, minus the params.
+    Pickle pickle;
+    pickle.WriteString(trial_name);
+    pickle.WriteString(group_name);
+    size_t total_size = sizeof(FieldTrialEntry) + pickle.size();
+    FieldTrial::FieldTrialRef new_ref =
+        allocator->Allocate(total_size, kFieldTrialType);
+    FieldTrialEntry* new_entry =
+        allocator->GetAsObject<FieldTrialEntry>(new_ref, kFieldTrialType);
+    new_entry->activated = prev_entry->activated;
+    new_entry->size = pickle.size();

[bcwhite, 2016/11/23 21:03:52]

Don't you need to copy the pickle data?

[lawrencewu, 2016/11/23 21:49:20]

Whoops, good catch -- yes, fixed.

[bcwhite, 2016/11/24 18:29:47]

If your tests didn't catch that then your tests are incomplete.  Make sure there
is a test that stores and restores a complete set of data.

[lawrencewu, 2016/11/24 21:50:35]

Done. I found another bug while writing the test, too.

+
+    // Update the ref on the field trial.
+    FieldTrial* trial = Find(trial_name.as_string());
+    trial->ref_ = new_ref;
+
+    // Mark the existing entry as unused.
+    uint32_t old_type = allocator->GetType(prev_ref);

[bcwhite, 2016/11/23 21:03:52]

You already know from line 995 that the type is kFieldTrialType.

[lawrencewu, 2016/11/23 21:49:20]

Fixed.

+    allocator->ChangeType(prev_ref, 0, old_type);
+  }
+}
+
 #if defined(OS_WIN)
 // static
 bool FieldTrialList::CreateTrialsFromWindowsHandle(HANDLE handle) {
   SharedMemoryHandle shm_handle(handle, GetCurrentProcId());
 
   // shm gets deleted when it gets out of scope, but that's OK because we need
   // it only for the duration of this method.
   std::unique_ptr<SharedMemory> shm(new SharedMemory(shm_handle, true));
   if (!shm.get()->Map(kFieldTrialAllocationSize))
     TerminateBecauseOutOfMemory(kFieldTrialAllocationSize);
@@ skipping 123 matching lines @@
   FieldTrial::State trial_state;
   if (!field_trial->GetStateWhileLocked(&trial_state))
     return;
 
   // Or if we've already added it. We must check after GetState since it can
   // also add to the allocator.
   if (field_trial->ref_)
     return;
 
   Pickle pickle;
-  pickle.WriteString(trial_state.trial_name);
-  pickle.WriteString(trial_state.group_name);
+  if (!PickleFieldTrial(trial_state, &pickle))
+    return;
 
   size_t total_size = sizeof(FieldTrialEntry) + pickle.size();
   FieldTrial::FieldTrialRef ref =
       allocator->Allocate(total_size, kFieldTrialType);
   if (ref == FieldTrialAllocator::kReferenceNull)
     return;
 
   FieldTrialEntry* entry =
       allocator->GetAsObject<FieldTrialEntry>(ref, kFieldTrialType);
   entry->activated = trial_state.activated;
@@ skipping 57 matching lines @@
     return;
   }
   AutoLock auto_lock(global_->lock_);
   CHECK(!global_->PreLockedFind(trial->trial_name())) << trial->trial_name();
   trial->AddRef();
   trial->SetTrialRegistered();
   global_->registered_[trial->trial_name()] = trial;
 }
 
 }  // namespace base