Refactor SetOmahaExperimentLabel out of gcpai and into install_util.

chrome/installer/util/google_update_settings_unittest.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 <windows.h>
 #include <shlwapi.h>  // For SHDeleteKey.
 
 #include "base/memory/scoped_ptr.h"
 #include "base/test/test_reg_util_win.h"
 #include "base/utf_string_conversions.h"
@@ skipping 18 matching lines @@
 const wchar_t kGoogleUpdateUpdateDefault[] = L"UpdateDefault";
 const wchar_t kGoogleUpdateUpdatePrefix[] = L"Update";
 const GoogleUpdateSettings::UpdatePolicy kDefaultUpdatePolicy =
 #if defined(GOOGLE_CHROME_BUILD)
     GoogleUpdateSettings::AUTOMATIC_UPDATES;
 #else
     GoogleUpdateSettings::UPDATES_DISABLED;
 #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");
     registry_overrides_.OverrideRegistry(HKEY_CURRENT_USER, L"HKCU_pit");
   }
 
   enum SystemUserInstall {
     SYSTEM_INSTALL,
     USER_INSTALL,
   };
@@ skipping 51 matching lines @@
           EXPECT_TRUE(GoogleUpdateSettings::GetChromeChannelAndModifiers(
               is_system, &ret_channel));
           EXPECT_STREQ(channel, ret_channel.c_str())
               << "Expecting channel \"" << channel
               << "\" for ap=\"" << ap << "\"";
         }
       }
     }
   }
 
+  // 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) {
     HKEY reg_root = HKEY_CURRENT_USER;
     std::wstring reg_key = GetApKeyPath();
     work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
     work_item_list->AddSetRegValueWorkItem(reg_root, reg_key,
         google_update::kRegApField, value.c_str(), true);
     if (!work_item_list->Do()) {
       work_item_list->Rollback();
@@ skipping 423 matching lines @@
             RegKey(HKEY_LOCAL_MACHINE, kGoogleUpdatePoliciesKey,
                    KEY_SET_VALUE).WriteValue(app_policy_value.c_str(),
                                              static_cast<DWORD>(3)));
   is_overridden = true;
   EXPECT_EQ(GoogleUpdateSettings::UPDATES_DISABLED,
             GoogleUpdateSettings::GetAppUpdatePolicy(kTestProductGuid,
                                                      &is_overridden));
   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,
 // according to the param.
 class GetUninstallCommandLine : public GoogleUpdateSettingsTest,
                                 public testing::WithParamInterface<bool> {
  protected:
   static const wchar_t kDummyCommand[];
 
   virtual void SetUp() OVERRIDE {
@@ skipping 363 matching lines @@
         StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
                    StatsState::FALSE_SETTING, StatsState::FALSE_SETTING),
         StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
                    StatsState::FALSE_SETTING, StatsState::TRUE_SETTING),
         StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
                    StatsState::TRUE_SETTING, StatsState::NO_SETTING),
         StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
                    StatsState::TRUE_SETTING, StatsState::FALSE_SETTING),
         StatsState(StatsState::kSystemLevel, StatsState::MULTI_INSTALL,
                    StatsState::TRUE_SETTING, StatsState::TRUE_SETTING)));