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

chrome/installer/gcapi/gcapi_omaha_experiment.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/installer/gcapi/gcapi_omaha_experiment.h"
 
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "chrome/installer/gcapi/gcapi.h"
+#include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_experiment_util.h"
 #include "chrome/installer/util/google_update_settings.h"
 
 using base::Time;
 using base::TimeDelta;
 
 namespace {
 
 // Returns the number of weeks since 2/3/2003.
 int GetCurrentRlzWeek() {
   Time::Exploded february_third_2003_exploded = {2003, 2, 1, 3, 0, 0, 0, 0};
   Time f = Time::FromUTCExploded(february_third_2003_exploded);
   TimeDelta delta = Time::Now() - f;
   return delta.InDays() / 7;
 }
 
-bool SetLabel(const wchar_t* brand_code, const wchar_t* label, int shell_mode) {
+bool SetExperimentLabel(const wchar_t* brand_code,
+                        const string16& label,
+                        int shell_mode) {
   if (!brand_code) {
     return false;
   }
 
+  const bool system_level = shell_mode == GCAPI_INVOKED_UAC_ELEVATION;
+
+  string16 experiment_labels;
+  if (!GoogleUpdateSettings::ReadExperimentLabels(system_level,
+                                                  &experiment_labels)) {
+    return false;
+  }
+
+  // First erase this label from the existing experiment labels if it exists.
+  size_t existing_label_begin = experiment_labels.find(label + L"=");
+  if (existing_label_begin != string16::npos) {

[Alexei Svitkine (slow), 2013/08/30 14:17:19]

I think it would be safer to also check that either existing_label_begin == 0 or
experiment_labels[existing_label_begin - 1] == ','.

So you don't end up finding this in the middle of some other labels.

Alternatively, just do what variations_util.cc does and use
SplitStringUsingSubstr() and iterate over each label and re-add them if they
don't match |label|. This way, even if there multiple existing duplicates,
they'll all be removed.

[gab, 2013/08/30 17:53:11]

This is mitigated by searching for (label + "="); or is even the '=' not
considered a special character?
I thought about that, but it's too complex for this purpose imo and existing
duplicates for |label| are not possible as this has always previously clobbered.

[Alexei Svitkine (slow), 2013/08/30 18:16:11]

I guess it depends how they're parsed. If each entry gets split on the first
'=', then presumably the value can still have a "=" in it. That's why I was
suggesting the safer approach. (But perhaps like you say it may not be an issue
in practice.)

I'll leave it up to you.

[gab, 2013/08/30 22:57:14]

Added extra safety for this.

+    size_t existing_label_end =
+        experiment_labels.find(google_update::kExperimentLabelSep,
+                               existing_label_begin);
+    // Note: if |existing_label_end == string16::npos| this will work anyways
+    // as npos is defined as the biggest possible unsigned integer and erase
+    // will thus, by definition, erase to the end of the string.
+    experiment_labels.erase(existing_label_begin,
+                            existing_label_end - existing_label_begin);
+  }
+
   int week_number = GetCurrentRlzWeek();
   if (week_number < 0 || week_number > 999)
     week_number = 999;
 
-  string16 experiment_labels;
-  base::SStringPrintf(&experiment_labels,
+  string16 gcapi_experiment_label;
+  base::SStringPrintf(&gcapi_experiment_label,
                       L"%ls=%ls_%d|%ls",
-                      label,
+                      label.c_str(),
                       brand_code,
                       week_number,
                       installer::BuildExperimentDateString().c_str());
 
-  return GoogleUpdateSettings::SetExperimentLabels(
-      shell_mode == GCAPI_INVOKED_UAC_ELEVATION,
-      experiment_labels);
+  if (!experiment_labels.empty())
+    experiment_labels.append(google_update::kExperimentLabelSep);
+  experiment_labels.append(gcapi_experiment_label);
+
+  return GoogleUpdateSettings::SetExperimentLabels(system_level,
+                                                   experiment_labels);
 }
 
 }  // namespace
 
 bool SetReactivationExperimentLabels(const wchar_t* brand_code,
                                      int shell_mode) {
-  return SetLabel(brand_code, L"reacbrand", shell_mode);
+  return SetExperimentLabel(brand_code, L"reacbrand", shell_mode);
 }
 
 bool SetRelaunchExperimentLabels(const wchar_t* brand_code, int shell_mode) {
-  return SetLabel(brand_code, L"relaunchbrand", shell_mode);
+  return SetExperimentLabel(brand_code, L"relaunchbrand", shell_mode);
 }