Refactor SetOmahaExperimentLabel out of gcpai and into install_util.

chrome/installer/util/install_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.
 //
 // See the corresponding header file for description of the functions in this
 // file.
 
 #include "chrome/installer/util/install_util.h"
 
 #include <shellapi.h>
@@ skipping 21 matching lines @@
 #include "chrome/installer/util/installation_state.h"
 #include "chrome/installer/util/l10n_string_util.h"
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item_list.h"
 
 using base::win::RegKey;
 using installer::ProductState;
 
 namespace {
 
+const wchar_t kExperimentLabels[] = L"experiment_labels";

[grt (UTC plus 2), 2012/11/24 02:58:37]

Since this is a value defined by Omaha, please move it into
google_update_constants.{cc,h}.

[SteveT, 2012/11/26 20:45:06]

Done.

+
 const wchar_t kStageBinaryPatching[] = L"binary_patching";
 const wchar_t kStageBuilding[] = L"building";
 const wchar_t kStageConfiguringAutoLaunch[] = L"configuring_auto_launch";
 const wchar_t kStageCopyingPreferencesFile[] = L"copying_prefs";
 const wchar_t kStageCreatingShortcuts[] = L"creating_shortcuts";
 const wchar_t kStageEnsemblePatching[] = L"ensemble_patching";
 const wchar_t kStageExecuting[] = L"executing";
 const wchar_t kStageFinishing[] = L"finishing";
 const wchar_t kStagePreconditions[] = L"preconditions";
 const wchar_t kStageRefreshingPolicy[] = L"refreshing_policy";
@@ skipping 460 matching lines @@
                            date_str, arraysize(date_str));
   if (len) {
     --len;  // Subtract terminating \0.
   } else {
     PLOG(DFATAL) << "GetDateFormat";
   }
 
   return string16(date_str, len);
 }
 
+// static
+bool InstallUtil::SetOmahaExperimentLabel(const string16& experiment_label,

[grt (UTC plus 2), 2012/11/24 02:58:37]

Upon further consideration, I think this belongs in
google_update_settings.{cc,h} since that's where most of our Omaha-interop stuff
lives. In that case, the name can be simplified to just SetExperimentLabels
(plural for consistency with the name of the registry value).

[grt (UTC plus 2), 2012/11/24 03:03:44]

Also, please add a test case to google_update_settings_unittest.cc.

[SteveT, 2012/11/26 20:45:06]

Moved it.

[SteveT, 2012/11/26 20:45:06]

Currently trying to understand the tests in google_update_settings_unittest.cc
(I'm guessing you mean the one under chrome/installer/util, as there is also one
under chrome/browser/google as well.

I'll let you have a look at the questions in my response (which my change a few
things) before responding with a test.

+                                          bool system_install) {
+  HKEY registry_hive = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;

[grt (UTC plus 2), 2012/11/24 02:58:37]

nit: other code in the installer uses either the name reg_root or just root for
this.

[SteveT, 2012/11/26 20:45:06]

Done.

+
+  // Use the browser distribution and install level to write to the correct
+  // client state/app guid key.
+  bool success = false;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  if (dist->ShouldWriteExperimentLabels()) {
+    string16 client_state_path(
+        system_install ? dist->GetStateMediumKey() : dist->GetStateKey());
+    RegKey client_state(
+        registry_hive, client_state_path.c_str(), KEY_SET_VALUE);
+    success = client_state.WriteValue(kExperimentLabels,

[grt (UTC plus 2), 2012/11/24 03:03:44]

It looks like Omaha deletes the value in the registry when its setter is called
with an empty string. For consistency, I imagine you should do the same here.

[SteveT, 2012/11/26 20:45:06]

Okay, done. The return value will also represent the success or failure of the
erase (I'll assume the caller knows whether or not they are passing an empty
string into this).

+        experiment_label.c_str()) == ERROR_SUCCESS;
+  }
+
+  return success;
+}
+
 // Open |path| with minimal access to obtain information about it, returning
 // true and populating |handle| on success.
 // static
 bool InstallUtil::ProgramCompare::OpenForInfo(const FilePath& path,
                                               base::win::ScopedHandle* handle) {
   DCHECK(handle);
   handle->Set(base::CreatePlatformFile(path, base::PLATFORM_FILE_OPEN, NULL,
                                        NULL));
   return handle->IsValid();
 }
@@ skipping 49 matching lines @@
   // Open the program and see if it references the expected file.
   base::win::ScopedHandle handle;
   BY_HANDLE_FILE_INFORMATION info = {};
 
   return (OpenForInfo(program, &handle) &&
           GetInfo(handle, &info) &&
           info.dwVolumeSerialNumber == file_info_.dwVolumeSerialNumber &&
           info.nFileIndexHigh == file_info_.nFileIndexHigh &&
           info.nFileIndexLow == file_info_.nFileIndexLow);
 }