Add options to write version number to registry.

chrome/installer/setup/setup_main.cc

Index: chrome/installer/setup/setup_main.cc
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 29cdf06f0c723e32b61f32c47e91c349d2c39730..50d89ca51ed887d5de5dea24a2d6596a592cb6ae 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -82,6 +82,15 @@ using installer::Products;
 
 const wchar_t kGoogleUpdatePipeName[] = L"\\\\.\\pipe\\GoogleCrashServices\\";

[grt (UTC plus 2), 2015/09/11 18:14:26]

please move all of these constants that are only used in this file into the
unnamed namespace below.

[bcwhite, 2015/09/11 19:24:44]

Done.

 const wchar_t kSystemPrincipalSid[] = L"S-1-5-18";
+const wchar_t kDisplayVersion[] = L"DisplayVersion";
+const wchar_t kMsiInstallRegistryPrefix[] =
+    L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Installer\\UserData\\"
+    L"S-1-5-18\\Products\\";
+const wchar_t kMsiInstallRegistryPostfix[] = L"\\InstallProperties";
+const wchar_t kMsiUninstallRegistryPrefix[] =
+    L"SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\";
+const wchar_t kMsiUninstallRegistryPostfix[] = L"";
+const wchar_t kMsiDisplayVersionOverwriteDelay[] = L"10";  // seconds as string
 
 const MINIDUMP_TYPE kLargerDumpType = static_cast<MINIDUMP_TYPE>(
     MiniDumpWithProcessThreadData |  // Get PEB and TEB.
@@ -90,6 +99,81 @@ const MINIDUMP_TYPE kLargerDumpType = static_cast<MINIDUMP_TYPE>(
 
 namespace {
 
+// Overwrite an existing DisplayVersion as written by the MSI installer
+// with the real version number of Chrome.
+LONG OverwriteDisplayVersion(const base::string16& path,
+                             const base::string16& value) {
+  base::win::RegKey key;
+  LONG result = 0;
+  base::string16 existing;
+  if ((result = key.Open(HKEY_LOCAL_MACHINE, path.c_str(),
+                         KEY_QUERY_VALUE | KEY_SET_VALUE | KEY_WOW64_64KEY))
+      != ERROR_SUCCESS) {
+    LOG(ERROR) << "Failed to set DisplayVersion: " << path << " not found";
+    return result;
+  }
+  if ((result = key.ReadValue(kDisplayVersion, &existing)) != ERROR_SUCCESS) {
+    LOG(ERROR) << "Failed to set DisplayVersion: " << kDisplayVersion
+               << " not found under " << path;
+    return result;
+  }
+  if ((result = key.WriteValue(kDisplayVersion, value.c_str()))
+      != ERROR_SUCCESS) {
+    LOG(ERROR) << "Failed to set DisplayVersion: " << kDisplayVersion
+               << " could not be written under " << path;
+    return result;
+  }
+  VLOG(1) << "Set DisplayVersion at " << path << " to " << value
+          << " from " << existing;
+  return ERROR_SUCCESS;
+}
+
+LONG OverwriteDisplayVersions(const base::string16& product,
+                              const base::string16& value) {
+  // The version is held in two places.  Frist change it in the MSI Installer
+  // registry entry.  It is held under a "squashed guid" key.
+  base::string16 reg_path(kMsiInstallRegistryPrefix);
+  reg_path += base::ASCIIToUTF16(

[grt (UTC plus 2), 2015/09/11 18:14:26]

can GuidToSquid operate on base::string16 so we don't need to convert to ascii
and back like this?

[bcwhite, 2015/09/11 19:24:44]

Done.

+      installer::GuidToSquid(base::UTF16ToASCII(product)));
+  reg_path += kMsiInstallRegistryPostfix;
+  LONG result1 = OverwriteDisplayVersion(reg_path, value);
+
+  // The display version also exists under the Unininstall registry key with
+  // the original guid.
+  reg_path = kMsiUninstallRegistryPrefix;

[grt (UTC plus 2), 2015/09/11 18:14:26]

i think it's more clear to use base::StringPrintf with a format string rather
than hand-roll like this. may as well do the same for the path above, too.

[bcwhite, 2015/09/11 19:24:45]

Done.

+  reg_path += L"{";
+  reg_path += product;
+  reg_path += L"}";
+  reg_path += kMsiUninstallRegistryPostfix;
+  LONG result2 = OverwriteDisplayVersion(reg_path, value);
+
+  return result1 != ERROR_SUCCESS ? result1 : result2;
+}
+
+void DelayedOverwriteDisplayVersions(const base::FilePath& setup_exe,
+                                     const base::string16& id,
+                                     const Version& version) {
+  // This process has to be able to exit so we launch ourselves with
+  // instructions on what to do and then return.
+  base::CommandLine command_line(setup_exe);
+  command_line.AppendSwitchNative(
+      installer::switches::kSetDisplayVersionProduct, id);
+  command_line.AppendSwitchNative(installer::switches::kSetDisplayVersionValue,

[grt (UTC plus 2), 2015/09/11 18:14:26]

nit: use AppendSwitchASCII and remove the ASCIIToUFT16 conversion here

[bcwhite, 2015/09/11 19:24:44]

Done.

+                                  base::ASCIIToUTF16(version.GetString()));
+  command_line.AppendSwitchNative(installer::switches::kDelay,
+                                  kMsiDisplayVersionOverwriteDelay);
+
+  base::LaunchOptions launch_options;
+  launch_options.force_breakaway_from_job_ = true;
+  base::Process writer = base::LaunchProcess(command_line, launch_options);
+  if (!writer.IsValid()) {
+    LOG(ERROR) << "Failed to set DisplayVersion: "

[grt (UTC plus 2), 2015/09/11 18:14:26]

tip: PLOG(ERROR) and remove the GetLastError() tidbit. PLOG will magically
format the last-error-code onto the end of the message. nice, eh?

[bcwhite, 2015/09/11 19:24:45]

Done.

+               << "could not launch subprocess to make desired changes."
+               << " (error=" << ::GetLastError() << ")\n"

[grt (UTC plus 2), 2015/09/11 18:14:26]

i think it's best not to put newlines in log messages. i like when each line
begins with the log header.

[bcwhite, 2015/09/11 19:24:44]

Done.

+               << command_line.GetCommandLineString();
+  }
+}
+
 // Returns NULL if no compressed archive is available for processing, otherwise
 // returns a patch helper configured to uncompress and patch.
 scoped_ptr<installer::ArchivePatchHelper> CreateChromeArchiveHelper(
@@ -920,6 +1004,18 @@ bool HandleNonInstallCmdLineOptions(const InstallationState& original_state,
                                     const base::CommandLine& cmd_line,
                                     InstallerState* installer_state,
                                     int* exit_code) {
+  // This option is independent of all others so doesn't belong in the if/else
+  // block below.
+  if (cmd_line.HasSwitch(installer::switches::kDelay)) {
+    const std::string delay_seconds_string(
+        cmd_line.GetSwitchValueASCII(installer::switches::kDelay));
+    int delay_seconds;
+    if (base::StringToInt(delay_seconds_string, &delay_seconds) &&
+        delay_seconds > 0) {
+      base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(delay_seconds));
+    }
+  }
+
   // TODO(gab): Add a local |status| variable which each block below sets;
   // only determine the |exit_code| from |status| at the end (this will allow
   // this method to validate that
@@ -1163,6 +1259,15 @@ bool HandleNonInstallCmdLineOptions(const InstallationState& original_state,
     bool updates_enabled = GoogleUpdateSettings::ReenableAutoupdates();
     *exit_code = updates_enabled ? installer::REENABLE_UPDATES_SUCCEEDED :
                                    installer::REENABLE_UPDATES_FAILED;
+  } else if (cmd_line.HasSwitch(
+                 installer::switches::kSetDisplayVersionProduct)) {
+    const base::string16 registry_product(
+        cmd_line.GetSwitchValueNative(
+            installer::switches::kSetDisplayVersionProduct));
+    const base::string16 registry_value(
+        cmd_line.GetSwitchValueNative(
+            installer::switches::kSetDisplayVersionValue));
+    *exit_code = OverwriteDisplayVersions(registry_product, registry_value);
   } else {
     handled = false;
   }
@@ -1553,13 +1658,24 @@ InstallStatus InstallProductsHelper(const InstallationState& original_state,
     }
   }
 
-  // If installation completed successfully, return the path to the directory
-  // containing the newly installed setup.exe and uncompressed archive if the
-  // caller requested it.
-  if (installer_directory &&
-      InstallUtil::GetInstallReturnCode(install_status) == 0) {
-    *installer_directory =
-        installer_state.GetInstallerDirectory(*installer_version);
+  // If the installation completed successfully...
+  if (InstallUtil::GetInstallReturnCode(install_status) == 0) {
+    // Update the DisplayVersion created by an MSI-based install.
+    std::string install_id;
+    if (prefs.GetString(installer::master_preferences::kMsiProductId,
+                        &install_id)) {
+      base::FilePath new_setup =
+          installer_state.GetInstallerDirectory(*installer_version)
+          .Append(kSetupExe);
+      DelayedOverwriteDisplayVersions(
+          new_setup, base::ASCIIToUTF16(install_id), *installer_version);

[grt (UTC plus 2), 2015/09/11 18:14:26]

nit: pass install_id directly and use AppendSwitchASCII in
DelayedOverwriteDisplayVersions.

[bcwhite, 2015/09/11 19:24:44]

Done.

[grt (UTC plus 2), 2015/09/11 20:07:01]

Not done?

[bcwhite, 2015/09/12 03:49:23]

Weird.  I absolutely did change that; no idea how it got reverted.  Done again.

+    }
+    // Return the path to the directory containing the newly installed
+    // setup.exe and uncompressed archive if the caller requested it.
+    if (installer_directory) {
+      *installer_directory =
+          installer_state.GetInstallerDirectory(*installer_version);
+    }
   }
 
   // temp_path's dtor will take care of deleting or scheduling itself for