Add debug info for field trial allocation crash

base/metrics/field_trial.cc

Index: base/metrics/field_trial.cc
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc
index eb90b4dabe4d719f301cf19bb9e8a63896a09964..eb882da14f18c03b45682c29f6aa5501c100a0bf 100644
--- a/base/metrics/field_trial.cc
+++ b/base/metrics/field_trial.cc
@@ -184,6 +184,48 @@ void AddForceFieldTrialsFlag(CommandLine* cmd_line) {
   }
 }
 
+// In order to debug the crash where two field trials with the same name but
+// different groups get added to the allocator, let's try to get the stack
+// frame where the second duplicate gets added here as well as the trial/group
+// names by going through the allocator and checking if a trial with the same
+// name already exists.
+void CheckAllocatorForTrial(FieldTrialList::FieldTrialAllocator* allocator,
+                            const std::string& trial_name,
+                            const std::string& group_name) {
+  FieldTrialList::FieldTrialAllocator::Iterator iter(allocator);
+  FieldTrial::FieldTrialRef ref;
+  while ((ref = iter.GetNextOfType(kFieldTrialType)) !=
+         FieldTrialList::FieldTrialAllocator::kReferenceNull) {
+    const FieldTrialEntry* entry =
+        allocator->GetAsObject<const FieldTrialEntry>(ref, kFieldTrialType);
+
+    StringPiece trial_name_to_check;
+    StringPiece group_name_to_check;
+    if (!entry->GetTrialAndGroupName(&trial_name_to_check,
+                                     &group_name_to_check)) {
+      CHECK(false);
+    }
+
+    if (trial_name_to_check.as_string() == trial_name) {
+      constexpr size_t buf_size = 100;
+
+      char trial_name_c_str[buf_size];
+      char group_name_c_str[buf_size];
+      char existing_group_name_c_str[buf_size];
+
+      strlcpy(trial_name_c_str, trial_name.c_str(), buf_size);
+      strlcpy(group_name_c_str, group_name.c_str(), buf_size);
+      strlcpy(existing_group_name_c_str,
+              group_name_to_check.as_string().c_str(), buf_size);

[brucedawson, 2016/11/22 17:55:24]

Unfortunately, calling as_string() is exactly as expensive as copying the
StringPiece to a std::string object. Since it's done twice it ends up being more
expensive.

If this code will be deleted soon then I guess it doesn't matter, but any of
this code which sticks around should be audited to ensure that we aren't
creating std::string objects purely for avoidable convenience.

[Alexei Svitkine (slow), 2016/11/22 18:49:30]

+1

You should be able to strlcpy() from the StringPiece without as_string().

However, I agree that we may be able to remove this debug code if we confirm the
crashes have all been fixed.

+
+      debug::Alias(trial_name_c_str);
+      debug::Alias(group_name_c_str);
+      debug::Alias(existing_group_name_c_str);
+      CHECK_EQ(group_name_to_check.as_string(), group_name);
+    }
+  }
+}
+
 #if defined(OS_WIN)
 HANDLE CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) {
   HANDLE src = allocator->shared_memory()->handle().GetHandle();
@@ -860,14 +902,30 @@ bool FieldTrialList::CreateTrialsFromSharedMemory(
     // If we failed to create the field trial, crash with debug info.
     // TODO(665129): Remove this when the crash is resolved.
     if (!trial) {
+      constexpr size_t buf_size = 100;
+      char trial_name_c_str[buf_size] = {0};
+      char group_name_c_str[buf_size] = {0};
+      char existing_group_name_c_str[buf_size] = {0};
+
+      // Copy the names over to the stack.
       std::string trial_name_string = trial_name.as_string();
+      strlcpy(trial_name_c_str, trial_name_string.c_str(), buf_size);
       std::string group_name_string = group_name.as_string();
+      strlcpy(group_name_c_str, group_name_string.c_str(), buf_size);
+
+      // Alias the trial and group name.
+      debug::Alias(trial_name_c_str);
+      debug::Alias(group_name_c_str);
+
+      // Copy and alias the existing field trial name, if there is one.
       FieldTrial* existing_field_trial =
           FieldTrialList::Find(trial_name_string);
-      if (existing_field_trial)
-        debug::Alias(existing_field_trial->group_name_internal().c_str());
-      debug::Alias(trial_name_string.c_str());
-      debug::Alias(group_name_string.c_str());
+      if (existing_field_trial) {
+        std::string existing_group_name_string =
+            existing_field_trial->group_name_internal();
+        strlcpy(existing_group_name_c_str, group_name_string.c_str(), buf_size);
+        debug::Alias(existing_group_name_c_str);
+      }
       CHECK(!trial_name_string.empty());
       CHECK(!group_name_string.empty());
       CHECK_EQ(existing_field_trial->group_name_internal(),
@@ -931,6 +989,10 @@ void FieldTrialList::AddToAllocatorWhileLocked(FieldTrial* field_trial) {
   if (allocator->IsReadonly())
     return;
 
+  // TODO(665129): Remove this code once the bug has been fixed.
+  CheckAllocatorForTrial(allocator, field_trial->trial_name(),
+                         field_trial->group_name_internal());
+
   FieldTrial::State trial_state;
   if (!field_trial->GetStateWhileLocked(&trial_state))
     return;