GCAPI should append to the existing experiment_labels instead of clobbering them.

chrome/common/metrics/variations/variations_util.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 "chrome/common/metrics/variations/variations_util.h"
 
 #include <vector>
 
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/common/child_process_logging.h"
+#include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_experiment_util.h"
 
 namespace chrome_variations {
 
 namespace {
 
 const char kVariationPrefix[] = "CrVar";
-const char kExperimentLabelSep[] = ";";

[robertshield, 2013/08/30 01:13:53]

This has gone from a char to a wchar_t and doesn't seem to build on some
platforms.

[Alexei Svitkine (slow), 2013/08/30 02:16:47]

It should be a char16, since on some platforms wchar_t is 32 bits.

[gab, 2013/08/30 02:43:18]

Good catch; thanks for the hint; done.

 
 // Populates |name_group_ids| based on |active_groups|.
 void GetFieldTrialActiveGroupIdsForActiveGroups(
     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));
@@ skipping 77 matching lines @@
   // Find all currently active VariationIDs associated with Google Update.
   for (base::FieldTrial::ActiveGroups::const_iterator it =
        active_groups.begin(); it != active_groups.end(); ++it) {
     const VariationID id = GetGoogleVariationID(GOOGLE_UPDATE_SERVICE,
                                                 it->trial_name, it->group_name);
 
     if (id == EMPTY_ID)
       continue;
 
     if (!experiment_labels.empty())
-      experiment_labels += ASCIIToUTF16(kExperimentLabelSep);
+      experiment_labels += google_update::kExperimentLabelSep;
     experiment_labels += CreateSingleExperimentLabel(++counter, id);
   }
 
   return experiment_labels;
 }
 
 string16 ExtractNonVariationLabels(const string16& labels) {
-  const string16 separator = ASCIIToUTF16(kExperimentLabelSep);
   string16 non_variation_labels;
 
   // First, split everything by the label separator.
   std::vector<string16> entries;
-  base::SplitStringUsingSubstr(labels, separator, &entries);
+  base::SplitStringUsingSubstr(
+      labels, google_update::kExperimentLabelSep, &entries);
 
   // For each label, keep the ones that do not look like a Variations label.
   for (std::vector<string16>::const_iterator it = entries.begin();
        it != entries.end(); ++it) {
     if (it->empty() || StartsWith(*it, ASCIIToUTF16(kVariationPrefix), false))
       continue;
 
     // Dump the whole thing, including the timestamp.
     if (!non_variation_labels.empty())
-      non_variation_labels += separator;
+      non_variation_labels += google_update::kExperimentLabelSep;
     non_variation_labels += *it;
   }
 
   return non_variation_labels;
 }
 
 string16 CombineExperimentLabels(const string16& variation_labels,
                                  const string16& other_labels) {
-  const string16 separator = ASCIIToUTF16(kExperimentLabelSep);
-  DCHECK(!StartsWith(variation_labels, separator, false));
-  DCHECK(!EndsWith(variation_labels, separator, false));
-  DCHECK(!StartsWith(other_labels, separator, false));
-  DCHECK(!EndsWith(other_labels, separator, false));
+  DCHECK(!StartsWith(variation_labels, google_update::kExperimentLabelSep,
+                     false));
+  DCHECK(!EndsWith(variation_labels, google_update::kExperimentLabelSep,
+                   false));
+  DCHECK(!StartsWith(other_labels, google_update::kExperimentLabelSep, false));
+  DCHECK(!EndsWith(other_labels, google_update::kExperimentLabelSep, false));
   // Note that if either label is empty, a separator is not necessary.
   string16 combined_labels = other_labels;
   if (!other_labels.empty() && !variation_labels.empty())
-    combined_labels += separator;
+    combined_labels += google_update::kExperimentLabelSep;
   combined_labels += variation_labels;
   return combined_labels;
 }
 
 // Functions below are exposed for testing explicitly behind this namespace.
 // They simply wrap existing functions in this file.
 namespace testing {
 
 void TestGetFieldTrialActiveGroupIds(
     const base::FieldTrial::ActiveGroups& active_groups,
     std::vector<ActiveGroupId>* name_group_ids) {
   GetFieldTrialActiveGroupIdsForActiveGroups(active_groups,
                                              name_group_ids);
 }
 
 }  // namespace testing
 
 }  // namespace chrome_variations