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

As described on http://crbug.com/266955#c7

Also adding AtExitManager to gcapi_test.exe; this is required to support MasterPreferences's LazyInstance used by BrowserDistribution, used by google_update's ReadExperimentLabels().

Introducing GCAPITestRegistryOverrider as a class to be added as a member to GCAPI test fixtures that require registry overriding; extracted from the existing GCAPIReactivationTest fixture.

Move Windows-specific variations_util.cc code to experiment_labels_win.cc

BUG=266955
TEST=gcapi_test.exe

Review URL: https://codereview.chromium.org/23579003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233493 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/metrics/variations/variations_registry_syncer_win.cc

@@ -8,7 +8,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
-#include "chrome/common/metrics/variations/variations_util.h"
+#include "chrome/common/metrics/variations/experiment_labels_win.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/install_util.h"
 

chrome/chrome_common.gypi

@@ -336,6 +336,8 @@
         'common/metrics/metrics_log_manager.h',
         'common/metrics/metrics_service_base.cc',
         'common/metrics/metrics_service_base.h',
+        'common/metrics/variations/experiment_labels_win.cc',
+        'common/metrics/variations/experiment_labels_win.h',
         'common/metrics/variations/uniformity_field_trials.cc',
         'common/metrics/variations/uniformity_field_trials.h',
         'common/metrics/variations/variations_util.cc',

chrome/chrome_installer.gypi

@@ -65,7 +65,10 @@
           ],
           'sources': [
             'installer/gcapi/gcapi_last_run_test.cc',
+            'installer/gcapi/gcapi_omaha_experiment_test.cc',
             'installer/gcapi/gcapi_reactivation_test.cc',
+            'installer/gcapi/gcapi_test_registry_overrider.cc',
+            'installer/gcapi/gcapi_test_registry_overrider.h',
             'installer/gcapi/gcapi_test.cc',
             'installer/gcapi/gcapi_test.rc',
             'installer/gcapi/resource.h',

chrome/chrome_tests_unit.gypi

@@ -1851,6 +1851,7 @@
         'common/metrics/caching_permuted_entropy_provider_unittest.cc',
         'common/metrics/metrics_log_base_unittest.cc',
         'common/metrics/metrics_log_manager_unittest.cc',
+        'common/metrics/variations/experiment_labels_win_unittest.cc',
         'common/metrics/variations/variations_util_unittest.cc',
         'common/multi_process_lock_unittest.cc',
         'common/net/url_fixer_upper_unittest.cc',

chrome/common/metrics/variations/experiment_labels_win.cc

+// Copyright 2013 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/experiment_labels_win.h"
+
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/google_update_experiment_util.h"
+#include "components/variations/variations_associated_data.h"
+
+namespace chrome_variations {
+
+namespace {
+
+const wchar_t kVariationPrefix[] = L"CrVar";
+
+// This method builds a single experiment label for a Chrome Variation,
+// including a timestamp that is a year in the future from |current_time|. Since
+// multiple headers can be transmitted, |count| is a number that is appended
+// after the label key to differentiate the labels.
+string16 CreateSingleExperimentLabel(int count, VariationID id,
+                                     const base::Time& current_time) {
+  // Build the parts separately so they can be validated.
+  const string16 key = kVariationPrefix + base::IntToString16(count);
+  DCHECK_LE(key.size(), 8U);
+  const string16 value = base::IntToString16(id);
+  DCHECK_LE(value.size(), 8U);
+  string16 label(key);
+  label += L'=';
+  label += value;
+  label += L'|';
+  label += installer::BuildExperimentDateString(current_time);
+  return label;
+}
+
+}  // namespace
+
+string16 BuildGoogleUpdateExperimentLabel(
+    const base::FieldTrial::ActiveGroups& active_groups) {
+  string16 experiment_labels;
+  int counter = 0;
+
+  const base::Time current_time(base::Time::Now());
+
+  // 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 += google_update::kExperimentLabelSep;
+    experiment_labels += CreateSingleExperimentLabel(++counter, id,
+                                                     current_time);
+  }
+
+  return experiment_labels;
+}
+
+string16 ExtractNonVariationLabels(const string16& labels) {
+  string16 non_variation_labels;
+
+  // First, split everything by the label separator.
+  std::vector<string16> 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, kVariationPrefix, false))
+      continue;
+
+    // Dump the whole thing, including the timestamp.
+    if (!non_variation_labels.empty())
+      non_variation_labels += google_update::kExperimentLabelSep;
+    non_variation_labels += *it;
+  }
+
+  return non_variation_labels;
+}
+
+string16 CombineExperimentLabels(const string16& variation_labels,
+                                 const string16& other_labels) {
+  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 += google_update::kExperimentLabelSep;
+  combined_labels += variation_labels;
+  return combined_labels;
+}
+
+}  // namespace chrome_variations

chrome/common/metrics/variations/experiment_labels_win.h

+// Copyright 2013 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.
+
+#ifndef CHROME_COMMON_METRICS_VARIATIONS_EXPERIMENT_LABELS_WIN_H_
+#define CHROME_COMMON_METRICS_VARIATIONS_EXPERIMENT_LABELS_WIN_H_
+
+#include "base/metrics/field_trial.h"
+#include "base/strings/string16.h"
+
+namespace chrome_variations {
+
+// Takes the list of active groups and builds the label for the ones that have
+// Google Update VariationID associated with them. This will return an empty
+// string if there are no such groups.
+string16 BuildGoogleUpdateExperimentLabel(
+    const base::FieldTrial::ActiveGroups& active_groups);
+
+// Creates a final combined experiment labels string with |variation_labels|
+// and |other_labels|, appropriately appending a separator based on their
+// contents. It is assumed that |variation_labels| and |other_labels| do not
+// have leading or trailing separators.
+string16 CombineExperimentLabels(const string16& variation_labels,
+                                 const string16& other_labels);
+
+// Takes the value of experiment_labels from the registry and returns a valid
+// experiment_labels string value containing only the labels that are not
+// associated with Chrome Variations.
+string16 ExtractNonVariationLabels(const string16& labels);
+
+}  // namespace chrome_variations
+
+#endif  // CHROME_COMMON_METRICS_VARIATIONS_EXPERIMENT_LABELS_WIN_H_

chrome/common/metrics/variations/experiment_labels_win_unittest.cc

+// Copyright 2013 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/experiment_labels_win.h"
+
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/metrics/field_trial.h"
+#include "base/strings/string_split.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/variations/variations_associated_data.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chrome_variations {
+
+TEST(ExperimentLabelsTest, BuildGoogleUpdateExperimentLabel) {
+  const VariationID TEST_VALUE_A = 3300200;
+  const VariationID TEST_VALUE_B = 3300201;
+  const VariationID TEST_VALUE_C = 3300202;
+  const VariationID TEST_VALUE_D = 3300203;
+
+  struct {
+    const char* active_group_pairs;
+    const char* expected_ids;
+  } test_cases[] = {
+    // Empty group.
+    {"", ""},
+    // Group of 1.
+    {"FieldTrialA#Default", "3300200"},
+    // Group of 1, doesn't have an associated ID.
+    {"FieldTrialA#DoesNotExist", ""},
+    // Group of 3.
+    {"FieldTrialA#Default#FieldTrialB#Group1#FieldTrialC#Default",
+     "3300200#3300201#3300202"},
+    // Group of 3, one doesn't have an associated ID.
+    {"FieldTrialA#Default#FieldTrialB#DoesNotExist#FieldTrialC#Default",
+     "3300200#3300202"},
+    // Group of 3, all three don't have an associated ID.
+    {"FieldTrialX#Default#FieldTrialB#DoesNotExist#FieldTrialC#Default",
+     "3300202"},
+  };
+
+  // Register a few VariationIDs.
+  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialA", "Default",
+                             TEST_VALUE_A);
+  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialB", "Group1",
+                             TEST_VALUE_B);
+  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialC", "Default",
+                             TEST_VALUE_C);
+  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialD", "Default",
+                             TEST_VALUE_D);  // Not actually used.
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    // Parse the input groups.
+    base::FieldTrial::ActiveGroups groups;
+    std::vector<std::string> group_data;
+    base::SplitString(test_cases[i].active_group_pairs, '#', &group_data);
+    ASSERT_EQ(0U, group_data.size() % 2);
+    for (size_t j = 0; j < group_data.size(); j += 2) {
+      base::FieldTrial::ActiveGroup group;
+      group.trial_name = group_data[j];
+      group.group_name = group_data[j + 1];
+      groups.push_back(group);
+    }
+
+    // Parse the expected output.
+    std::vector<std::string> expected_ids_list;
+    base::SplitString(test_cases[i].expected_ids, '#', &expected_ids_list);
+
+    std::string experiment_labels_string = UTF16ToUTF8(
+        BuildGoogleUpdateExperimentLabel(groups));
+
+    // Split the VariationIDs from the labels for verification below.
+    std::vector<std::string> split_labels;
+    std::set<std::string> parsed_ids;
+    base::SplitString(experiment_labels_string, ';', &split_labels);
+    for (std::vector<std::string>::const_iterator it = split_labels.begin();
+         it != split_labels.end(); ++it) {
+      // The ID is precisely between the '=' and '|' characters in each label.
+      size_t index_of_equals = it->find('=');
+      size_t index_of_pipe = it->find('|');
+      ASSERT_NE(std::string::npos, index_of_equals);
+      ASSERT_NE(std::string::npos, index_of_pipe);
+      ASSERT_GT(index_of_pipe, index_of_equals);
+      parsed_ids.insert(it->substr(index_of_equals + 1,
+                                   index_of_pipe - index_of_equals - 1));
+    }
+
+    // Verify that the resulting string contains each of the expected labels,
+    // and nothing more. Note that the date is stripped out and ignored.
+    for (std::vector<std::string>::const_iterator it =
+             expected_ids_list.begin(); it != expected_ids_list.end(); ++it) {
+      std::set<std::string>::iterator it2 = parsed_ids.find(*it);
+      EXPECT_TRUE(parsed_ids.end() != it2);
+      parsed_ids.erase(it2);
+    }
+    EXPECT_TRUE(parsed_ids.empty());
+  }  // for
+}
+
+TEST(ExperimentLabelsTest, CombineExperimentLabels) {
+  struct {
+    const char* variations_labels;
+    const char* other_labels;
+    const char* expected_label;
+  } test_cases[] = {
+    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT",
+     "C=D|Tue, 21 Jan 2014 15:30:21 GMT",
+     "C=D|Tue, 21 Jan 2014 15:30:21 GMT;A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
+    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT",
+     "",
+     "A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
+    {"",
+     "A=B|Tue, 21 Jan 2014 15:30:21 GMT",
+     "A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
+    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT;C=D|Tue, 21 Jan 2014 15:30:21 GMT",
+     "P=Q|Tue, 21 Jan 2014 15:30:21 GMT;X=Y|Tue, 21 Jan 2014 15:30:21 GMT",
+     "P=Q|Tue, 21 Jan 2014 15:30:21 GMT;X=Y|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "A=B|Tue, 21 Jan 2014 15:30:21 GMT;C=D|Tue, 21 Jan 2014 15:30:21 GMT"},
+    {"",
+     "",
+     ""},
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    std::string result = UTF16ToUTF8(CombineExperimentLabels(
+        ASCIIToUTF16(test_cases[i].variations_labels),
+        ASCIIToUTF16(test_cases[i].other_labels)));
+    EXPECT_EQ(test_cases[i].expected_label, result);
+  }
+}
+
+TEST(ExperimentLabelsTest, ExtractNonVariationLabels) {
+  struct {
+    const char* input_label;
+    const char* expected_output;
+  } test_cases[] = {
+    // Empty
+    {"", ""},
+    // One
+    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT",
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // Three
+    {"CrVar1=123|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "experiment1=456|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "experiment2=789|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar1=123|Tue, 21 Jan 2014 15:30:21 GMT",
+     "experiment1=456|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "experiment2=789|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // One and one Variation
+    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT",
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // One and one Variation, flipped
+    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT",
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // Sandwiched
+    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar2=3310003|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar3=3310004|Tue, 21 Jan 2014 15:30:21 GMT",
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // Only Variations
+    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar2=3310003|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar3=3310004|Tue, 21 Jan 2014 15:30:21 GMT",
+     ""},
+    // Empty values
+    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT",
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // Trailing semicolon
+    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
+     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;",  // Note the semi here.
+     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
+    // Semis
+    {";;;;", ""},
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    std::string non_variation_labels = UTF16ToUTF8(
+        ExtractNonVariationLabels(ASCIIToUTF16(test_cases[i].input_label)));
+    EXPECT_EQ(test_cases[i].expected_output, non_variation_labels);
+  }
+}
+
+}  // namespace chrome_variations

chrome/common/metrics/variations/variations_util.cc

@@ -6,23 +6,15 @@
 
 #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/common/crash_keys.h"
-#include "chrome/installer/util/google_update_experiment_util.h"
 
 namespace chrome_variations {
 
 namespace {
 
-const char kVariationPrefix[] = "CrVar";
-const char kExperimentLabelSep[] = ";";
-
 // Populates |name_group_ids| based on |active_groups|.
 void GetFieldTrialActiveGroupIdsForActiveGroups(
     const base::FieldTrial::ActiveGroups& active_groups,
@@ -35,25 +27,6 @@ void GetFieldTrialActiveGroupIdsForActiveGroups(
   }
 }
 
-// This method builds a single experiment label for a Chrome Variation,
-// including a timestamp that is a year in the future from now. Since multiple
-// headers can be transmitted, |count| is a number that is appended after the
-// label key to differentiate the labels.
-string16 CreateSingleExperimentLabel(int count, VariationID id) {
-  // Build the parts separately so they can be validated.
-  const string16 key =
-      ASCIIToUTF16(kVariationPrefix) + base::IntToString16(count);
-  DCHECK_LE(key.size(), 8U);
-  const string16 value = base::IntToString16(id);
-  DCHECK_LE(value.size(), 8U);
-  string16 label(key);
-  label += ASCIIToUTF16("=");
-  label += value;
-  label += ASCIIToUTF16("|");
-  label += installer::BuildExperimentDateString();
-  return label;
-}
-
 }  // namespace
 
 void GetFieldTrialActiveGroupIds(
@@ -85,66 +58,6 @@ void SetChildProcessLoggingVariationList() {
   crash_keys::SetVariationsList(experiment_strings);
 }
 
-string16 BuildGoogleUpdateExperimentLabel(
-    const base::FieldTrial::ActiveGroups& active_groups) {
-  string16 experiment_labels;
-  int counter = 0;
-
-  // 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 += 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);
-
-  // 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 += *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));
-  // 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 += 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 {

chrome/common/metrics/variations/variations_util.h

@@ -31,24 +31,6 @@ void GetFieldTrialActiveGroupIdsAsStrings(std::vector<std::string>* output);
 // them to the child process logging module so it can save it for crash dumps.
 void SetChildProcessLoggingVariationList();
 
-// Takes the list of active groups and builds the label for the ones that have
-// Google Update VariationID associated with them. This will return an empty
-// string if there are no such groups.
-string16 BuildGoogleUpdateExperimentLabel(
-    const base::FieldTrial::ActiveGroups& active_groups);
-
-// Creates a final combined experiment labels string with |variation_labels|
-// and |other_labels|, appropriately appending a separator based on their
-// contents. It is assumed that |variation_labels| and |other_labels| do not
-// have leading or trailing separators.
-string16 CombineExperimentLabels(const string16& variation_labels,
-                                 const string16& other_labels);
-
-// Takes the value of experiment_labels from the registry and returns a valid
-// experiment_labels string value containing only the labels that are not
-// associated with Chrome Variations.
-string16 ExtractNonVariationLabels(const string16& labels);
-
 // Expose some functions for testing. These functions just wrap functionality
 // that is implemented above.
 namespace testing {

chrome/common/metrics/variations/variations_util_unittest.cc

@@ -6,48 +6,14 @@
 
 #include <set>
 #include <string>
+#include <vector>
 
 #include "base/metrics/field_trial.h"
-#include "base/strings/string_split.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
 #include "components/variations/metrics_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chrome_variations {
 
-namespace {
-
-const VariationID TEST_VALUE_A = 3300200;
-const VariationID TEST_VALUE_B = 3300201;
-const VariationID TEST_VALUE_C = 3300202;
-const VariationID TEST_VALUE_D = 3300203;
-
-// Tests whether a field trial is active (i.e. group() has been called on it).
-bool IsFieldTrialActive(const std::string& trial_name) {
-  base::FieldTrial::ActiveGroups active_groups;
-  base::FieldTrialList::GetActiveFieldTrialGroups(&active_groups);
-  for (size_t i = 0; i < active_groups.size(); ++i) {
-    if (active_groups[i].trial_name == trial_name)
-      return true;
-  }
-  return false;
-}
-
-// Call FieldTrialList::FactoryGetFieldTrial() with a future expiry date.
-scoped_refptr<base::FieldTrial> CreateFieldTrial(
-    const std::string& trial_name,
-    int total_probability,
-    const std::string& default_group_name,
-    int* default_group_number) {
-  return base::FieldTrialList::FactoryGetFieldTrial(
-      trial_name, total_probability, default_group_name,
-      base::FieldTrialList::kNoExpirationYear, 1, 1,
-      base::FieldTrial::SESSION_RANDOMIZED, default_group_number);
-}
-
-}  // namespace
-
 class VariationsUtilTest : public ::testing::Test {
  public:
   VariationsUtilTest() : field_trial_list_(NULL) {
@@ -104,171 +70,4 @@ TEST_F(VariationsUtilTest, GetFieldTrialActiveGroups) {
   EXPECT_EQ(0U, expected_groups.size());
 }
 
-TEST_F(VariationsUtilTest, BuildGoogleUpdateExperimentLabel) {
-  struct {
-    const char* active_group_pairs;
-    const char* expected_ids;
-  } test_cases[] = {
-    // Empty group.
-    {"", ""},
-    // Group of 1.
-    {"FieldTrialA#Default", "3300200"},
-    // Group of 1, doesn't have an associated ID.
-    {"FieldTrialA#DoesNotExist", ""},
-    // Group of 3.
-    {"FieldTrialA#Default#FieldTrialB#Group1#FieldTrialC#Default",
-     "3300200#3300201#3300202"},
-    // Group of 3, one doesn't have an associated ID.
-    {"FieldTrialA#Default#FieldTrialB#DoesNotExist#FieldTrialC#Default",
-     "3300200#3300202"},
-    // Group of 3, all three don't have an associated ID.
-    {"FieldTrialX#Default#FieldTrialB#DoesNotExist#FieldTrialC#Default",
-     "3300202"},
-  };
-
-  // Register a few VariationIDs.
-  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialA", "Default",
-                             TEST_VALUE_A);
-  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialB", "Group1",
-                             TEST_VALUE_B);
-  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialC", "Default",
-                             TEST_VALUE_C);
-  AssociateGoogleVariationID(GOOGLE_UPDATE_SERVICE, "FieldTrialD", "Default",
-                             TEST_VALUE_D);  // Not actually used.
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
-    // Parse the input groups.
-    base::FieldTrial::ActiveGroups groups;
-    std::vector<std::string> group_data;
-    base::SplitString(test_cases[i].active_group_pairs, '#', &group_data);
-    ASSERT_EQ(0U, group_data.size() % 2);
-    for (size_t j = 0; j < group_data.size(); j += 2) {
-      base::FieldTrial::ActiveGroup group;
-      group.trial_name = group_data[j];
-      group.group_name = group_data[j + 1];
-      groups.push_back(group);
-    }
-
-    // Parse the expected output.
-    std::vector<std::string> expected_ids_list;
-    base::SplitString(test_cases[i].expected_ids, '#', &expected_ids_list);
-
-    std::string experiment_labels_string = UTF16ToUTF8(
-        BuildGoogleUpdateExperimentLabel(groups));
-
-    // Split the VariationIDs from the labels for verification below.
-    std::vector<std::string> split_labels;
-    std::set<std::string> parsed_ids;
-    base::SplitString(experiment_labels_string, ';', &split_labels);
-    for (std::vector<std::string>::const_iterator it = split_labels.begin();
-         it != split_labels.end(); ++it) {
-      // The ID is precisely between the '=' and '|' characters in each label.
-      size_t index_of_equals = it->find('=');
-      size_t index_of_pipe = it->find('|');
-      ASSERT_NE(std::string::npos, index_of_equals);
-      ASSERT_NE(std::string::npos, index_of_pipe);
-      ASSERT_GT(index_of_pipe, index_of_equals);
-      parsed_ids.insert(it->substr(index_of_equals + 1,
-                                   index_of_pipe - index_of_equals - 1));
-    }
-
-    // Verify that the resulting string contains each of the expected labels,
-    // and nothing more. Note that the date is stripped out and ignored.
-    for (std::vector<std::string>::const_iterator it =
-             expected_ids_list.begin(); it != expected_ids_list.end(); ++it) {
-      std::set<std::string>::iterator it2 = parsed_ids.find(*it);
-      EXPECT_TRUE(parsed_ids.end() != it2);
-      parsed_ids.erase(it2);
-    }
-    EXPECT_TRUE(parsed_ids.empty());
-  }  // for
-}
-
-TEST_F(VariationsUtilTest, CombineExperimentLabels) {
-  struct {
-    const char* variations_labels;
-    const char* other_labels;
-    const char* expected_label;
-  } test_cases[] = {
-    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT",
-     "C=D|Tue, 21 Jan 2014 15:30:21 GMT",
-     "C=D|Tue, 21 Jan 2014 15:30:21 GMT;A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
-    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT",
-     "",
-     "A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
-    {"",
-     "A=B|Tue, 21 Jan 2014 15:30:21 GMT",
-     "A=B|Tue, 21 Jan 2014 15:30:21 GMT"},
-    {"A=B|Tue, 21 Jan 2014 15:30:21 GMT;C=D|Tue, 21 Jan 2014 15:30:21 GMT",
-     "P=Q|Tue, 21 Jan 2014 15:30:21 GMT;X=Y|Tue, 21 Jan 2014 15:30:21 GMT",
-     "P=Q|Tue, 21 Jan 2014 15:30:21 GMT;X=Y|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "A=B|Tue, 21 Jan 2014 15:30:21 GMT;C=D|Tue, 21 Jan 2014 15:30:21 GMT"},
-    {"",
-     "",
-     ""},
-  };
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
-    std::string result = UTF16ToUTF8(CombineExperimentLabels(
-        ASCIIToUTF16(test_cases[i].variations_labels),
-        ASCIIToUTF16(test_cases[i].other_labels)));
-    EXPECT_EQ(test_cases[i].expected_label, result);
-  }
-}
-
-TEST_F(VariationsUtilTest, ExtractNonVariationLabels) {
-  struct {
-    const char* input_label;
-    const char* expected_output;
-  } test_cases[] = {
-    // Empty
-    {"", ""},
-    // One
-    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT",
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // Three
-    {"CrVar1=123|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "experiment1=456|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "experiment2=789|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar1=123|Tue, 21 Jan 2014 15:30:21 GMT",
-     "experiment1=456|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "experiment2=789|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // One and one Variation
-    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT",
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // One and one Variation, flipped
-    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT",
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // Sandwiched
-    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar2=3310003|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar3=3310004|Tue, 21 Jan 2014 15:30:21 GMT",
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // Only Variations
-    {"CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar2=3310003|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar3=3310004|Tue, 21 Jan 2014 15:30:21 GMT",
-     ""},
-    // Empty values
-    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT",
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // Trailing semicolon
-    {"gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT;"
-     "CrVar1=3310002|Tue, 21 Jan 2014 15:30:21 GMT;",  // Note the semi here.
-     "gcapi_brand=123|Tue, 21 Jan 2014 15:30:21 GMT"},
-    // Semis
-    {";;;;", ""},
-  };
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
-    std::string non_variation_labels = UTF16ToUTF8(
-        ExtractNonVariationLabels(ASCIIToUTF16(test_cases[i].input_label)));
-    EXPECT_EQ(test_cases[i].expected_output, non_variation_labels);
-  }
-}
-
 }  // namespace chrome_variations

chrome/installer/gcapi/gcapi_omaha_experiment.cc

@@ -4,55 +4,102 @@
 
 #include "chrome/installer/gcapi/gcapi_omaha_experiment.h"
 
-#include "base/strings/string16.h"
+#include "base/basictypes.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.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;
+int GetCurrentRlzWeek(const base::Time& current_time) {
+  base::Time::Exploded february_third_2003_exploded =
+      {2003, 2, 1, 3, 0, 0, 0, 0};
+  base::Time f = base::Time::FromUTCExploded(february_third_2003_exploded);
+  base::TimeDelta delta = current_time - 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;
   }
 
-  int week_number = GetCurrentRlzWeek();
-  if (week_number < 0 || week_number > 999)
-    week_number = 999;
+  const bool system_level = shell_mode == GCAPI_INVOKED_UAC_ELEVATION;
 
-  string16 experiment_labels;
-  base::SStringPrintf(&experiment_labels,
-                      L"%ls=%ls_%d|%ls",
-                      label,
-                      brand_code,
-                      week_number,
-                      installer::BuildExperimentDateString().c_str());
+  string16 original_labels;
+  if (!GoogleUpdateSettings::ReadExperimentLabels(system_level,
+                                                  &original_labels)) {
+    return false;
+  }
+
+  // Split the original labels by the label separator.
+  std::vector<string16> entries;
+  base::SplitStringUsingSubstr(
+      original_labels, google_update::kExperimentLabelSep, &entries);
+
+  // Keep all labels, but the one we want to add/replace.
+  string16 new_labels;
+  for (std::vector<string16>::const_iterator it = entries.begin();
+       it != entries.end(); ++it) {
+    if (!it->empty() && !StartsWith(*it, label + L"=", true)) {
+      new_labels += *it;
+      new_labels += google_update::kExperimentLabelSep;
+    }
+  }
+
+  new_labels.append(
+      gcapi_internals::GetGCAPIExperimentLabel(brand_code, label));
 
-  return GoogleUpdateSettings::SetExperimentLabels(
-      shell_mode == GCAPI_INVOKED_UAC_ELEVATION,
-      experiment_labels);
+  return GoogleUpdateSettings::SetExperimentLabels(system_level,
+                                                   new_labels);
 }
 
 }  // namespace
 
+namespace gcapi_internals {
+
+const wchar_t kReactivationLabel[] = L"reacbrand";
+const wchar_t kRelaunchLabel[] = L"relaunchbrand";
+
+string16 GetGCAPIExperimentLabel(const wchar_t* brand_code,
+                                 const string16& label) {
+  // Keeps a fixed time state for this GCAPI instance; this makes tests reliable
+  // when crossing time boundaries on the system clock and doesn't otherwise
+  // affect results of this short lived binary.
+  static time_t instance_time_value = 0;
+  if (instance_time_value == 0)
+    instance_time_value = base::Time::Now().ToTimeT();
+
+  base::Time instance_time = base::Time::FromTimeT(instance_time_value);
+
+  string16 gcapi_experiment_label;
+  base::SStringPrintf(&gcapi_experiment_label,
+                      L"%ls=%ls_%d|%ls",
+                      label.c_str(),
+                      brand_code,
+                      GetCurrentRlzWeek(instance_time),
+                      installer::BuildExperimentDateString(
+                          instance_time).c_str());
+  return gcapi_experiment_label;
+}
+
+}  // namespace gcapi_internals
+
 bool SetReactivationExperimentLabels(const wchar_t* brand_code,
                                      int shell_mode) {
-  return SetLabel(brand_code, L"reacbrand", shell_mode);
+  return SetExperimentLabel(brand_code, gcapi_internals::kReactivationLabel,
+                            shell_mode);
 }
 
 bool SetRelaunchExperimentLabels(const wchar_t* brand_code, int shell_mode) {
-  return SetLabel(brand_code, L"relaunchbrand", shell_mode);
+  return SetExperimentLabel(brand_code, gcapi_internals::kRelaunchLabel,
+                            shell_mode);
 }

chrome/installer/gcapi/gcapi_omaha_experiment.h

@@ -5,6 +5,20 @@
 #ifndef CHROME_INSTALLER_GCAPI_GCAPI_OMAHA_EXPERIMENT_H_
 #define CHROME_INSTALLER_GCAPI_GCAPI_OMAHA_EXPERIMENT_H_
 
+#include "base/strings/string16.h"
+
+namespace gcapi_internals {
+
+extern const wchar_t kReactivationLabel[];
+extern const wchar_t kRelaunchLabel[];
+
+// Returns the full experiment label to be used by |label| (which is one of the
+// labels declared above) for |brand_code|.
+string16 GetGCAPIExperimentLabel(const wchar_t* brand_code,
+                                 const string16& label);
+
+}  // namespace gcapi_internals
+
 // Writes a reactivation brand code experiment label in the Chrome product and
 // binaries registry keys for |brand_code|. This experiment label will have a
 // expiration date of now plus one year. If |shell_mode| is set to

chrome/installer/gcapi/gcapi_omaha_experiment_test.cc

+// Copyright 2013 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 "chrome/installer/gcapi/gcapi.h"
+#include "chrome/installer/gcapi/gcapi_test_registry_overrider.h"
+#include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/google_update_settings.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const wchar_t kBrand[] = L"ABCD";
+const uint16 kUserLevel = GCAPI_INVOKED_STANDARD_SHELL;
+
+const wchar_t kSomeExperiments[] = L"myexp=1|Aug 2;yourexp=2|Sep 5";
+const wchar_t kSomeOtherExperiments[] = L"anotherexp=joe|Jun 7 2008";
+const wchar_t kSomeMoreExperiments[] = L"moreexp=foo|Jul 31 1999";
+
+class GCAPIOmahaExperimentTest : public ::testing::Test {
+ protected:
+  GCAPIOmahaExperimentTest()
+      : brand_(kBrand),
+        reactivation_label_(gcapi_internals::GetGCAPIExperimentLabel(
+            kBrand, gcapi_internals::kReactivationLabel)),
+        relaunch_label_(gcapi_internals::GetGCAPIExperimentLabel(
+            kBrand, gcapi_internals::kRelaunchLabel)) {
+  }
+
+  void VerifyExperimentLabels(const string16& expected_labels) {
+    string16 actual_labels;
+    EXPECT_TRUE(GoogleUpdateSettings::ReadExperimentLabels(false,
+                                                           &actual_labels));
+    EXPECT_EQ(expected_labels, actual_labels);
+  }
+
+  string16 brand_;
+  string16 reactivation_label_;
+  string16 relaunch_label_;
+
+  const GCAPITestRegistryOverrider gcapi_test_registry_overrider_;
+};
+
+TEST_F(GCAPIOmahaExperimentTest, SetReactivationLabelFromEmptyExperiments) {
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+  VerifyExperimentLabels(reactivation_label_);
+}
+
+// Test the relaunch label once; all other tests go more in depth, but since
+// both labels use the same logic underneath there is no need to test both in
+// depth.
+TEST_F(GCAPIOmahaExperimentTest, SetRelaunchLabelFromEmptyExperiments) {
+  ASSERT_TRUE(SetRelaunchExperimentLabels(kBrand, kUserLevel));
+  VerifyExperimentLabels(relaunch_label_);
+}
+
+TEST_F(GCAPIOmahaExperimentTest, SetReactivationLabelWithExistingExperiments) {
+  GoogleUpdateSettings::SetExperimentLabels(false, kSomeExperiments);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+TEST_F(GCAPIOmahaExperimentTest,
+       SetReactivationLabelWithExistingIdenticalExperiment) {
+  string16 previous_labels(kSomeExperiments);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(reactivation_label_);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(kSomeOtherExperiments);
+  GoogleUpdateSettings::SetExperimentLabels(false, previous_labels);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(kSomeOtherExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+TEST_F(GCAPIOmahaExperimentTest,
+       SetReactivationLabelWithExistingIdenticalAtBeginning) {
+  string16 previous_labels(reactivation_label_);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(kSomeExperiments);
+  GoogleUpdateSettings::SetExperimentLabels(false, previous_labels);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+TEST_F(GCAPIOmahaExperimentTest,
+       SetReactivationLabelWithFakeMatchInAnExperiment) {
+  string16 previous_labels(kSomeExperiments);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(L"blah_");
+  // Shouldn't match deletion criteria.
+  previous_labels.append(reactivation_label_);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(kSomeOtherExperiments);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  // Should match the deletion criteria.
+  previous_labels.append(reactivation_label_);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(kSomeMoreExperiments);
+  GoogleUpdateSettings::SetExperimentLabels(false, previous_labels);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(L"blah_");
+  expected_labels.append(reactivation_label_);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(kSomeOtherExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(kSomeMoreExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+TEST_F(GCAPIOmahaExperimentTest,
+       SetReactivationLabelWithFakeMatchInAnExperimentAndNoRealMatch) {
+  string16 previous_labels(kSomeExperiments);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(L"blah_");
+  // Shouldn't match deletion criteria.
+  previous_labels.append(reactivation_label_);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  previous_labels.append(kSomeOtherExperiments);
+  GoogleUpdateSettings::SetExperimentLabels(false, previous_labels);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(L"blah_");
+  expected_labels.append(reactivation_label_);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(kSomeOtherExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+TEST_F(GCAPIOmahaExperimentTest,
+       SetReactivationLabelWithExistingEntryWithLabelAsPrefix) {
+  string16 previous_labels(kSomeExperiments);
+  previous_labels.append(google_update::kExperimentLabelSep);
+  // Append prefix matching the label, but not followed by '='.
+  previous_labels.append(gcapi_internals::kReactivationLabel);
+  // Shouldn't match deletion criteria.
+  previous_labels.append(kSomeOtherExperiments);
+  GoogleUpdateSettings::SetExperimentLabels(false, previous_labels);
+
+  ASSERT_TRUE(SetReactivationExperimentLabels(kBrand, kUserLevel));
+
+  string16 expected_labels(kSomeExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(gcapi_internals::kReactivationLabel);
+  expected_labels.append(kSomeOtherExperiments);
+  expected_labels.append(google_update::kExperimentLabelSep);
+  expected_labels.append(reactivation_label_);
+  VerifyExperimentLabels(expected_labels);
+}
+
+}  // namespace

chrome/installer/gcapi/gcapi_reactivation_test.cc

@@ -5,16 +5,13 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/guid.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/strings/stringprintf.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/test/test_reg_util_win.h"
 #include "base/time/time.h"
 #include "base/win/registry.h"
 #include "chrome/installer/gcapi/gcapi.h"
 #include "chrome/installer/gcapi/gcapi_omaha_experiment.h"
 #include "chrome/installer/gcapi/gcapi_reactivation.h"
+#include "chrome/installer/gcapi/gcapi_test_registry_overrider.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -24,15 +21,7 @@ using base::win::RegKey;
 
 class GCAPIReactivationTest : public ::testing::Test {
  protected:
-  void SetUp() {
-    // Override keys - this is undone during destruction.
-    std::wstring hkcu_override = base::StringPrintf(
-        L"hkcu_override\\%ls", ASCIIToWide(base::GenerateGUID()));
-    override_manager_.OverrideRegistry(HKEY_CURRENT_USER, hkcu_override);
-    std::wstring hklm_override = base::StringPrintf(
-        L"hklm_override\\%ls", ASCIIToWide(base::GenerateGUID()));
-    override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE, hklm_override);
-  }
+  GCAPIReactivationTest() {}
 
   bool SetChromeInstallMarker(HKEY hive) {
     // Create the client state keys in the right places.
@@ -100,8 +89,7 @@ class GCAPIReactivationTest : public ::testing::Test {
     return L"ERROR";
   }
 
- private:
-  registry_util::RegistryOverrideManager override_manager_;
+  const GCAPITestRegistryOverrider gcapi_test_registry_overrider_;
 };
 
 TEST_F(GCAPIReactivationTest, CheckSetReactivationBrandCode) {
@@ -109,7 +97,6 @@ TEST_F(GCAPIReactivationTest, CheckSetReactivationBrandCode) {
   EXPECT_EQ(L"GAGA", GetReactivationString(HKEY_CURRENT_USER));
 
   EXPECT_TRUE(HasBeenReactivated());
-
 }
 
 TEST_F(GCAPIReactivationTest, CanOfferReactivation_Basic) {

chrome/installer/gcapi/gcapi_test.cc

@@ -4,6 +4,7 @@
 
 #include <stdio.h>
 
+#include "base/at_exit.h"
 #include "base/command_line.h"
 #include "chrome/installer/gcapi/gcapi.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -60,6 +61,7 @@ void call_dynamically() {
 const char kManualLaunchTests[] = "launch-chrome";
 
 int main(int argc, char* argv[]) {
+  base::AtExitManager exit_manager;
   CommandLine::Init(argc, argv);
 
   testing::InitGoogleTest(&argc, argv);

chrome/installer/gcapi/gcapi_test_registry_overrider.cc

+// Copyright 2013 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_test_registry_overrider.h"
+
+#include "base/guid.h"
+#include "base/strings/string16.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+
+GCAPITestRegistryOverrider::GCAPITestRegistryOverrider() {
+  // Override keys - this is undone during destruction.
+  string16 hkcu_override = base::StringPrintf(
+      L"hkcu_override\\%ls", ASCIIToWide(base::GenerateGUID()));
+  override_manager_.OverrideRegistry(HKEY_CURRENT_USER, hkcu_override);
+  string16 hklm_override = base::StringPrintf(
+      L"hklm_override\\%ls", ASCIIToWide(base::GenerateGUID()));
+  override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE, hklm_override);
+}
+
+GCAPITestRegistryOverrider::~GCAPITestRegistryOverrider() {
+}

chrome/installer/gcapi/gcapi_test_registry_overrider.h

+// Copyright 2013 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.
+
+#ifndef CHROME_INSTALLER_GCAPI_GCAPI_TEST_REGISTRY_OVERRIDER_H_
+#define CHROME_INSTALLER_GCAPI_GCAPI_TEST_REGISTRY_OVERRIDER_H_
+
+#include "base/test/test_reg_util_win.h"
+
+// Overrides the registry throughout its lifetime; used by GCAPI tests
+// overriding the registry.
+class GCAPITestRegistryOverrider {
+ public:
+  GCAPITestRegistryOverrider();
+  ~GCAPITestRegistryOverrider();
+
+ private:
+  registry_util::RegistryOverrideManager override_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(GCAPITestRegistryOverrider);
+};
+
+#endif  // CHROME_INSTALLER_GCAPI_GCAPI_TEST_REGISTRY_OVERRIDER_H_

chrome/installer/util/google_update_constants.cc

@@ -9,6 +9,7 @@ namespace google_update {
 const wchar_t kChromeUpgradeCode[] = L"{8A69D345-D564-463C-AFF1-A69D9E530F96}";
 
 const wchar_t kExperimentLabels[] = L"experiment_labels";
+const wchar_t kExperimentLabelSep[] = L";";
 
 const wchar_t kGoogleUpdateUpgradeCode[] =
     L"{430FD4D0-B729-4F61-AA34-91526481799D}";

chrome/installer/util/google_update_constants.h

@@ -16,6 +16,9 @@ extern const wchar_t kChromeUpgradeCode[];
 // itself and Chrome.
 extern const wchar_t kExperimentLabels[];
 
+// The separator used to separate items in kExperimentLabels.
+extern const wchar_t kExperimentLabelSep[];
+
 // The GUID Google Update uses to keep track of Google Update self-upgrades.
 extern const wchar_t kGoogleUpdateUpgradeCode[];
 

chrome/installer/util/google_update_experiment_util.cc

@@ -24,7 +24,7 @@ const char* const kMonths[] =
 
 }
 
-string16 BuildExperimentDateString() {
+string16 BuildExperimentDateString(const base::Time& current_time) {
   // The Google Update experiment_labels timestamp format is:
   // "DAY, DD0 MON YYYY HH0:MI0:SE0 TZ"
   //  DAY = 3 character day of week,
@@ -36,7 +36,7 @@ string16 BuildExperimentDateString() {
   //  SE0 = 2 digit second,
   //  TZ = 3 character timezone
   base::Time::Exploded then = {};
-  base::Time::Now().UTCExplode(&then);
+  current_time.UTCExplode(&then);
   then.year += 1;
   DCHECK(then.HasValidValues());
 

chrome/installer/util/google_update_experiment_util.h

@@ -7,11 +7,15 @@
 
 #include "base/strings/string16.h"
 
+namespace base {
+class Time;
+}
+
 namespace installer {
 
 // Constructs a date string in the format understood by Google Update for the
-// current time plus one year.
-string16 BuildExperimentDateString();
+// |current_time| plus one year.
+string16 BuildExperimentDateString(const base::Time& current_time);
 
 }  // namespace installer