Change FieldTrial::State to not use StringPiece members.

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"
@@ skipping 65 matching lines @@
   if (!pickle->WriteString(string2))
     return false;
   return true;
 }
 
 // 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 (!WriteStringPair(pickle, trial_state.trial_name, trial_state.group_name))
+  if (!WriteStringPair(pickle, *trial_state.trial_name,
+                       *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);
+      *trial_state.trial_name, *trial_state.group_name, &params);
 
   // Write params to pickle.
   for (const auto& param : params) {
     if (!WriteStringPair(pickle, param.first, param.second))
       return false;
   }
   return true;
 }
 
 // Created a time value based on |year|, |month| and |day_of_month| parameters.
@@ skipping 35 matching lines @@
   //   static_cast<FieldTrial::Probability>(100 * 0.57) == 56
   //   static_cast<FieldTrial::Probability>(100 * 0.58) == 57
   //   static_cast<FieldTrial::Probability>(100 * 0.59) == 59
   const double kEpsilon = 1e-8;
   const FieldTrial::Probability result =
       static_cast<FieldTrial::Probability>(divisor * entropy_value + kEpsilon);
   // Ensure that adding the epsilon still results in a value < |divisor|.
   return std::min(result, divisor - 1);
 }
 
+// Separate type from FieldTrial::State so that it can use StringPieces.
+struct FieldTrialStringEntry {
+  StringPiece trial_name;
+  StringPiece group_name;
+  bool activated;
+};
+
 // Parses the --force-fieldtrials string |trials_string| into |entries|.
 // Returns true if the string was parsed correctly. On failure, the |entries|
 // array may end up being partially filled.
 bool ParseFieldTrialsString(const std::string& trials_string,
-                            std::vector<FieldTrial::State>* entries) {
+                            std::vector<FieldTrialStringEntry>* entries) {
   const StringPiece trials_string_piece(trials_string);
 
   size_t next_item = 0;
   while (next_item < trials_string.length()) {
     size_t name_end = trials_string.find(kPersistentStringSeparator, next_item);
     if (name_end == trials_string.npos || next_item == name_end)
       return false;
     size_t group_name_end =
         trials_string.find(kPersistentStringSeparator, name_end + 1);
     if (name_end + 1 == group_name_end)
       return false;
     if (group_name_end == trials_string.npos)
       group_name_end = trials_string.length();
 
-    FieldTrial::State entry;
+    FieldTrialStringEntry entry;
     // Verify if the trial should be activated or not.
     if (trials_string[next_item] == kActivationMarker) {
       // Name cannot be only the indicator.
       if (name_end - next_item == 1)
         return false;
       next_item++;
       entry.activated = true;
     }
     entry.trial_name =
         trials_string_piece.substr(next_item, name_end - next_item);
@@ skipping 56 matching lines @@
 bool FieldTrial::enable_benchmarking_ = false;
 
 int FieldTrialList::kNoExpirationYear = 0;
 
 //------------------------------------------------------------------------------
 // FieldTrial methods and members.
 
 FieldTrial::EntropyProvider::~EntropyProvider() {
 }
 
-FieldTrial::State::State() : activated(false) {}
+FieldTrial::State::State()
+    : trial_name(nullptr), group_name(nullptr), activated(false) {}

[brucedawson, 2017/01/24 00:44:14]

See comment in header file and consider removing the initialization list here.

[Alexei Svitkine (slow), 2017/01/24 16:40:19]

Done.

 
 FieldTrial::State::State(const State& other) = default;
 
 FieldTrial::State::~State() {}
 
 bool FieldTrial::FieldTrialEntry::GetTrialAndGroupName(
     StringPiece* trial_name,
     StringPiece* group_name) const {
   PickleIterator iter = GetPickleIterator();
   return ReadStringPair(&iter, trial_name, group_name);
@@ skipping 193 matching lines @@
   DCHECK_NE(group_, kNotFinalized);
   active_group->trial_name = trial_name_;
   active_group->group_name = group_name_;
   return true;
 }
 
 bool FieldTrial::GetState(State* field_trial_state) {
   if (!enable_field_trial_)
     return false;
   FinalizeGroupChoice();
-  field_trial_state->trial_name = trial_name_;
-  field_trial_state->group_name = group_name_;
+  field_trial_state->trial_name = &trial_name_;
+  field_trial_state->group_name = &group_name_;
   field_trial_state->activated = group_reported_;
   return true;
 }
 
 bool FieldTrial::GetStateWhileLocked(State* field_trial_state) {
   if (!enable_field_trial_)
     return false;
   FinalizeGroupChoiceImpl(true);
-  field_trial_state->trial_name = trial_name_;
-  field_trial_state->group_name = group_name_;
+  field_trial_state->trial_name = &trial_name_;
+  field_trial_state->group_name = &group_name_;
   field_trial_state->activated = group_reported_;
   return true;
 }
 
 //------------------------------------------------------------------------------
 // FieldTrialList methods and members.
 
 // static
 FieldTrialList* FieldTrialList::global_ = NULL;
 
@@ skipping 167 matching lines @@
 void FieldTrialList::AllStatesToString(std::string* output) {
   if (!global_)
     return;
   AutoLock auto_lock(global_->lock_);
 
   for (const auto& registered : global_->registered_) {
     FieldTrial::State trial;
     if (!registered.second->GetStateWhileLocked(&trial))
       continue;
     DCHECK_EQ(std::string::npos,
-              trial.trial_name.find(kPersistentStringSeparator));
+              trial.trial_name->find(kPersistentStringSeparator));
     DCHECK_EQ(std::string::npos,
-              trial.group_name.find(kPersistentStringSeparator));
+              trial.group_name->find(kPersistentStringSeparator));
     if (trial.activated)
       output->append(1, kActivationMarker);
-    trial.trial_name.AppendToString(output);
+    output->append(*trial.trial_name);

[brucedawson, 2017/01/24 00:44:14]

This reads much cleaner, as a minor side effect.

[Alexei Svitkine (slow), 2017/01/24 16:40:19]

Acknowledged.

     output->append(1, kPersistentStringSeparator);
-    trial.group_name.AppendToString(output);
+    output->append(*trial.group_name);
     output->append(1, kPersistentStringSeparator);
   }
 }
 
 // static
 void FieldTrialList::GetActiveFieldTrialGroups(
     FieldTrial::ActiveGroups* active_groups) {
   DCHECK(active_groups->empty());
   if (!global_)
     return;
   AutoLock auto_lock(global_->lock_);
 
   for (RegistrationMap::iterator it = global_->registered_.begin();
        it != global_->registered_.end(); ++it) {
     FieldTrial::ActiveGroup active_group;
     if (it->second->GetActiveGroup(&active_group))
       active_groups->push_back(active_group);
   }
 }
 
 // static
 void FieldTrialList::GetActiveFieldTrialGroupsFromString(
     const std::string& trials_string,
     FieldTrial::ActiveGroups* active_groups) {
-  std::vector<FieldTrial::State> entries;
+  std::vector<FieldTrialStringEntry> entries;
   if (!ParseFieldTrialsString(trials_string, &entries))
     return;
 
   for (const auto& entry : entries) {
     if (entry.activated) {
       FieldTrial::ActiveGroup group;
       group.trial_name = entry.trial_name.as_string();
       group.group_name = entry.group_name.as_string();
       active_groups->push_back(group);
     }
@@ skipping 31 matching lines @@
 }
 
 // static
 bool FieldTrialList::CreateTrialsFromString(
     const std::string& trials_string,
     const std::set<std::string>& ignored_trial_names) {
   DCHECK(global_);
   if (trials_string.empty() || !global_)
     return true;
 
-  std::vector<FieldTrial::State> entries;
+  std::vector<FieldTrialStringEntry> entries;
   if (!ParseFieldTrialsString(trials_string, &entries))
     return false;
 
   for (const auto& entry : entries) {
     const std::string trial_name = entry.trial_name.as_string();
     const std::string group_name = entry.group_name.as_string();
 
     if (ContainsKey(ignored_trial_names, trial_name))
       continue;
 
@@ skipping 590 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