Refactor SetOmahaExperimentLabel out of gcpai and into install_util.

chrome/installer/util/google_update_settings_unittest.cc

Index: chrome/installer/util/google_update_settings_unittest.cc
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc
index de3f2e352d5c03797b6da0a4f1d6c07e9091e6fe..db0867e98883e075684986bb112d950467aa45ce 100644
--- a/chrome/installer/util/google_update_settings_unittest.cc
+++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -36,11 +36,12 @@ const GoogleUpdateSettings::UpdatePolicy kDefaultUpdatePolicy =
 #endif
 
 const wchar_t kTestProductGuid[] = L"{89F1B351-B15D-48D4-8F10-1298721CF13D}";
+const wchar_t kTestExperimentLabel[] = L"test_label_value";
 
 // This test fixture redirects the HKLM and HKCU registry hives for
 // the duration of the test to make it independent of the machine
 // and user settings.
-class GoogleUpdateSettingsTest: public testing::Test {
+class GoogleUpdateSettingsTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE {
     registry_overrides_.OverrideRegistry(HKEY_LOCAL_MACHINE, L"HKLM_pit");
@@ -112,6 +113,43 @@ class GoogleUpdateSettingsTest: public testing::Test {
     }
   }
 
+  // Test the writing and deleting functionality of the experiments label
+  // helper.
+  void TestExperimentsLabelHelper(SystemUserInstall install) {
+    EXPECT_TRUE(GoogleUpdateSettings::SetExperimentLabels(

[grt (UTC plus 2), 2013/01/08 04:34:06]

i think you need to make this test brand-aware.  do all of this work for a
GOOGLE_CHROME_BUILD (as well as test that the BrowserDistribution's
ShouldSetExperimentLabels method returns true).  for a non-GOOGLE_CHROME_BUILD,
only test that ShouldSet... returns false.

[SteveT, 2013/01/08 16:33:04]

Done.

It's a bit odd using ifdefs in a unit test - is this test actually run by our
bots on both build types?

Also, is it cool if I test the BrowserDistribution::ShouldSetExperimentLabels
behaviour here. Is there another test I should be doing this in? (like maybe
google_chrome_distribution_unittests.cc, though I don't see similar tests in
there...)

[grt (UTC plus 2), 2013/01/08 17:06:36]

The normal case on the trybots, CQ, etc is Chromium.
Yup, looks good to me.

+        install == SYSTEM_INSTALL, kTestExperimentLabel));
+
+    // Validate that something is written. Only worry about the label itself.
+    RegKey key;
+    std::wstring value;
+    BrowserDistribution* chrome =
+        BrowserDistribution::GetSpecificDistribution(
+            BrowserDistribution::CHROME_BROWSER);
+
+    HKEY root = install == SYSTEM_INSTALL ?
+        HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+    string16 state_key = install == SYSTEM_INSTALL ?
+        chrome->GetStateMediumKey() : chrome->GetStateKey();
+
+    EXPECT_EQ(ERROR_SUCCESS,
+              key.Open(root, state_key.c_str(), KEY_QUERY_VALUE));
+    EXPECT_EQ(ERROR_SUCCESS,
+        key.ReadValue(google_update::kExperimentLabels, &value));
+    EXPECT_EQ(kTestExperimentLabel, value);
+    key.Close();
+
+    // Now that the label is set, test the delete functionality. An empty label
+    // should result in deleting the value.
+    RegKey key2;

[grt (UTC plus 2), 2013/01/08 04:34:06]

you could re-use |key| on lines 147 and 149 rather than introducing a new
variable.

[SteveT, 2013/01/08 16:33:04]

Done.

Okay great. I thought you might have to close the original key to ensure the
value was delete-able.

[grt (UTC plus 2), 2013/01/08 17:06:36]

 2013/01/08 16:33:04, SteveT wrote:
I liked the explicit call to key.Close() on line 139 for the reason you state.
Please add that back in. The one line line 150 is superfluous, though, and could
be removed (or not if you like the symmetry).

[SteveT, 2013/01/08 17:53:15]

Done. Kept the second Close.

+    EXPECT_TRUE(GoogleUpdateSettings::SetExperimentLabels(
+        install == SYSTEM_INSTALL, string16()));
+    EXPECT_EQ(ERROR_SUCCESS,
+              key2.Open(root, state_key.c_str(), KEY_QUERY_VALUE));
+    EXPECT_EQ(ERROR_FILE_NOT_FOUND,
+        key2.ReadValue(google_update::kExperimentLabels, &value));
+    key2.Close();
+  }
+
   // Creates "ap" key with the value given as parameter. Also adds work
   // items to work_item_list given so that they can be rolled back later.
   bool CreateApKey(WorkItemList* work_item_list, const std::wstring& value) {
@@ -555,6 +593,14 @@ TEST_F(GoogleUpdateSettingsTest, GetAppUpdatePolicyAppOverride) {
   EXPECT_FALSE(is_overridden);
 }
 
+TEST_F(GoogleUpdateSettingsTest, ExperimentsLabelHelperSystem) {
+  TestExperimentsLabelHelper(SYSTEM_INSTALL);
+}
+
+TEST_F(GoogleUpdateSettingsTest, ExperimentsLabelHelperUser) {
+  TestExperimentsLabelHelper(USER_INSTALL);
+}
+
 #endif  // defined(GOOGLE_CHROME_BUILD)
 
 // Test GoogleUpdateSettings::GetUninstallCommandLine at system- or user-level,