Support suffixing for field trial names.

This is for services like UKM who want to report different field trial names.

Bug:746402
Change-Id: I0ca7cf7abb519583ecb35aa40a1ea672dfb80343

TBR=calamity@chromium.org,oysteine@chromium.org,nparker@chromium.org

Change-Id: I0ca7cf7abb519583ecb35aa40a1ea672dfb80343
Reviewed-on: https://chromium-review.googlesource.com/577634
Commit-Queue: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Reviewed-by: Alexei Svitkine (slow) <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487905}

chrome/browser/safe_browsing/permission_reporter.cc

@@ -8,6 +8,7 @@
 
 #include "base/hash.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_piece.h"
 #include "base/time/default_clock.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/common/safe_browsing/permission_report.pb.h"
@@ -242,7 +243,8 @@ bool PermissionReporter::BuildReport(const PermissionReportInfo& report_info,
 
   // Collect field trial data.
   std::vector<variations::ActiveGroupId> active_group_ids;
-  variations::GetFieldTrialActiveGroupIds(&active_group_ids);
+  variations::GetFieldTrialActiveGroupIds(base::StringPiece(),
+                                          &active_group_ids);
   for (auto active_group_id : active_group_ids) {
     PermissionReport::FieldTrial* field_trial = report.add_field_trials();
     field_trial->set_name_id(active_group_id.name);

chrome/browser/tracing/chrome_tracing_delegate.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
@@ -168,7 +169,8 @@ void ChromeTracingDelegate::GenerateMetadataDict(
     base::DictionaryValue* metadata_dict) {
   DCHECK(metadata_dict);
   std::vector<std::string> variations;
-  variations::GetFieldTrialActiveGroupIdsAsStrings(&variations);
+  variations::GetFieldTrialActiveGroupIdsAsStrings(base::StringPiece(),
+                                                   &variations);
 
   std::unique_ptr<base::ListValue> variations_list(new base::ListValue());
   for (const auto& it : variations)

components/metrics/metrics_log.cc

@@ -17,6 +17,7 @@
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/histogram_snapshot_manager.h"
 #include "base/metrics/metrics_hashes.h"
+#include "base/strings/string_piece.h"
 #include "base/sys_info.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ -248,7 +249,8 @@ void MetricsLog::RecordGeneralMetrics(
 
 void MetricsLog::GetFieldTrialIds(
     std::vector<ActiveGroupId>* field_trial_ids) const {
-  variations::GetFieldTrialActiveGroupIds(field_trial_ids);
+  // We use the default field trial suffixing (no suffix).
+  variations::GetFieldTrialActiveGroupIds(base::StringPiece(), field_trial_ids);
 }
 
 bool MetricsLog::HasEnvironment() const {

components/variations/active_field_trials.cc

@@ -17,15 +17,17 @@ namespace variations {
 
 namespace {
 
-// Populates |name_group_ids| based on |active_groups|.
+// Populates |name_group_ids| based on |active_groups|. Field trial names are
+// suffixed with |suffix| before hashing is executed.
 void GetFieldTrialActiveGroupIdsForActiveGroups(
+    base::StringPiece suffix,
     const base::FieldTrial::ActiveGroups& active_groups,
     std::vector<ActiveGroupId>* name_group_ids) {
   DCHECK(name_group_ids->empty());
-  for (base::FieldTrial::ActiveGroups::const_iterator it =
-       active_groups.begin(); it != active_groups.end(); ++it) {
-    name_group_ids->push_back(MakeActiveGroupId(it->trial_name,
-                                                it->group_name));
+  for (auto it = active_groups.begin(); it != active_groups.end(); ++it) {
+    name_group_ids->push_back(
+        MakeActiveGroupId(it->trial_name + suffix.as_string(),
+                          it->group_name + suffix.as_string()));
   }
 }
 
@@ -48,22 +50,23 @@ ActiveGroupId MakeActiveGroupId(base::StringPiece trial_name,
   return id;
 }
 
-void GetFieldTrialActiveGroupIds(
-    std::vector<ActiveGroupId>* name_group_ids) {
+void GetFieldTrialActiveGroupIds(base::StringPiece suffix,
+                                 std::vector<ActiveGroupId>* name_group_ids) {
   DCHECK(name_group_ids->empty());
   // A note on thread safety: Since GetActiveFieldTrialGroups() is thread
   // safe, and we operate on a separate list of that data, this function is
   // technically thread safe as well, with respect to the FieldTrialList data.
   base::FieldTrial::ActiveGroups active_groups;
   base::FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
-  GetFieldTrialActiveGroupIdsForActiveGroups(active_groups,
+  GetFieldTrialActiveGroupIdsForActiveGroups(suffix, active_groups,
                                              name_group_ids);
 }
 
-void GetFieldTrialActiveGroupIdsAsStrings(std::vector<std::string>* output) {
+void GetFieldTrialActiveGroupIdsAsStrings(base::StringPiece suffix,
+                                          std::vector<std::string>* output) {
   DCHECK(output->empty());
   std::vector<ActiveGroupId> name_group_ids;
-  GetFieldTrialActiveGroupIds(&name_group_ids);
+  GetFieldTrialActiveGroupIds(suffix, &name_group_ids);
   AppendActiveGroupIdsAsStrings(name_group_ids, output);
 }
 
@@ -77,9 +80,10 @@ void GetSyntheticTrialGroupIdsAsString(std::vector<std::string>* output) {
 namespace testing {
 
 void TestGetFieldTrialActiveGroupIds(
+    base::StringPiece suffix,
     const base::FieldTrial::ActiveGroups& active_groups,
     std::vector<ActiveGroupId>* name_group_ids) {
-  GetFieldTrialActiveGroupIdsForActiveGroups(active_groups,
+  GetFieldTrialActiveGroupIdsForActiveGroups(suffix, active_groups,
                                              name_group_ids);
 }
 

components/variations/active_field_trials.h

@@ -41,16 +41,21 @@ struct ActiveGroupIdCompare {
 // Fills the supplied vector |name_group_ids| (which must be empty when called)
 // with unique ActiveGroupIds for each Field Trial that has a chosen group.
 // Field Trials for which a group has not been chosen yet are NOT returned in
-// this list.
-void GetFieldTrialActiveGroupIds(std::vector<ActiveGroupId>* name_group_ids);
+// this list. Field trial names are suffixed with |suffix| before hashing is
+// executed.
+void GetFieldTrialActiveGroupIds(base::StringPiece suffix,
+                                 std::vector<ActiveGroupId>* name_group_ids);
 
 // Fills the supplied vector |output| (which must be empty when called) with
 // unique string representations of ActiveGroupIds for each Field Trial that
 // has a chosen group. The strings are formatted as "<TrialName>-<GroupName>",
 // with the names as hex strings. Field Trials for which a group has not been
-// chosen yet are NOT returned in this list.
-void GetFieldTrialActiveGroupIdsAsStrings(std::vector<std::string>* output);
+// chosen yet are NOT returned in this list. Field trial names are suffixed with
+// |suffix| before hashing is executed.
+void GetFieldTrialActiveGroupIdsAsStrings(base::StringPiece suffix,
+                                          std::vector<std::string>* output);
 
+// TODO(rkaplow): Support suffixing for synthetic trials.
 // Fills the supplied vector |output| (which must be empty when called) with
 // unique string representations of ActiveGroupIds for each Syntehtic Trial
 // group. The strings are formatted as "<TrialName>-<GroupName>",
@@ -63,6 +68,7 @@ void GetSyntheticTrialGroupIdsAsString(std::vector<std::string>* output);
 namespace testing {
 
 void TestGetFieldTrialActiveGroupIds(
+    base::StringPiece suffix,
     const base::FieldTrial::ActiveGroups& active_groups,
     std::vector<ActiveGroupId>* name_group_ids);
 

components/variations/active_field_trials_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <stddef.h>
 
+#include "base/strings/string_piece.h"
 #include "components/variations/metrics_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -39,7 +40,8 @@ TEST(ActiveFieldTrialsTest, GetFieldTrialActiveGroups) {
   expected_groups.insert(name_group_id);
 
   std::vector<ActiveGroupId> active_group_ids;
-  testing::TestGetFieldTrialActiveGroupIds(active_groups, &active_group_ids);
+  testing::TestGetFieldTrialActiveGroupIds(base::StringPiece(), active_groups,
+                                           &active_group_ids);
   EXPECT_EQ(2U, active_group_ids.size());
   for (size_t i = 0; i < active_group_ids.size(); ++i) {
     ActiveGroupIdSet::iterator expected_group =
@@ -50,4 +52,26 @@ TEST(ActiveFieldTrialsTest, GetFieldTrialActiveGroups) {
   EXPECT_EQ(0U, expected_groups.size());
 }
 
+TEST(ActiveFieldTrialsTest, GetFieldTrialActiveGroupsWithSuffix) {
+  std::string trial_one("trial one");
+  std::string group_one("group one");
+  std::string suffix("some_suffix");
+
+  base::FieldTrial::ActiveGroups active_groups;
+  base::FieldTrial::ActiveGroup active_group;
+  active_group.trial_name = trial_one;
+  active_group.group_name = group_one;
+  active_groups.push_back(active_group);
+
+  std::vector<ActiveGroupId> active_group_ids;
+  testing::TestGetFieldTrialActiveGroupIds(suffix, active_groups,
+                                           &active_group_ids);
+  EXPECT_EQ(1U, active_group_ids.size());
+
+  uint32_t expected_name = metrics::HashName("trial onesome_suffix");
+  uint32_t expected_group = metrics::HashName("group onesome_suffix");
+  EXPECT_EQ(expected_name, active_group_ids[0].name);
+  EXPECT_EQ(expected_group, active_group_ids[0].group);
+}
+
 }  // namespace variations

components/variations/metrics_util.h

@@ -9,6 +9,7 @@
 
 #include "base/strings/string_piece.h"
 
+// TODO(rkaplow): Move to variations namespace and rename file hashing.h.
 namespace metrics {
 
 // Computes a uint32_t hash of a given string based on its SHA1 hash. Suitable

components/variations/variations_util.cc

@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "base/strings/string_piece.h"
 #include "components/crash/core/common/crash_keys.h"
 #include "components/variations/active_field_trials.h"
 
@@ -13,7 +14,8 @@ namespace variations {
 
 void SetVariationListCrashKeys() {
   std::vector<std::string> experiment_strings;
-  GetFieldTrialActiveGroupIdsAsStrings(&experiment_strings);
+  GetFieldTrialActiveGroupIdsAsStrings(base::StringPiece(),
+                                       &experiment_strings);
   GetSyntheticTrialGroupIdsAsString(&experiment_strings);
   crash_keys::SetVariationsList(experiment_strings);
 }

components/version_ui/version_handler_helper.cc

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/metrics/field_trial.h"
+#include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "components/variations/active_field_trials.h"
@@ -30,7 +31,8 @@ std::unique_ptr<base::Value> GetVariationsList() {
   }
 #else
   // In release mode, display the hashes only.
-  variations::GetFieldTrialActiveGroupIdsAsStrings(&variations);
+  variations::GetFieldTrialActiveGroupIdsAsStrings(base::StringPiece(),
+                                                   &variations);
 #endif
 
   std::unique_ptr<base::ListValue> variations_list(new base::ListValue);