Add options to write version number to registry.

chrome/installer/setup/setup_main.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 "chrome/installer/setup/setup_main.h"
 
 #include <windows.h>
 #include <msi.h>
 #include <shellapi.h>
 #include <shlobj.h>
@@ skipping 62 matching lines @@
 using installer::InstallerState;
 using installer::InstallationState;
 using installer::InstallationValidator;
 using installer::MasterPreferences;
 using installer::Product;
 using installer::ProductState;
 using installer::Products;
 
 const wchar_t kGoogleUpdatePipeName[] = L"\\\\.\\pipe\\GoogleCrashServices\\";
 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 kMsiDisplayVersionOverwriteDelay[] = L"30";  // seconds as string
 
 const MINIDUMP_TYPE kLargerDumpType = static_cast<MINIDUMP_TYPE>(
     MiniDumpWithProcessThreadData |  // Get PEB and TEB.
     MiniDumpWithUnloadedModules |  // Get unloaded modules when available.
     MiniDumpWithIndirectlyReferencedMemory);  // Get memory referenced by stack.
 
 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;
+}
+
+void DelayedOverwriteDisplayVersion(const base::FilePath& setup_exe,
+                                    const std::string& id,
+                                    const Version& version) {
+  base::string16 reg_path(kMsiInstallRegistryPrefix);
+  reg_path += base::UTF8ToUTF16(id);
+  reg_path += kMsiInstallRegistryPostfix;
+
+  // This process has to be able to exit so we launch ourselves with
+  // instructions on what to do and then return.
+  base::string16 command_line;

[robertshield, 2015/08/18 17:27:26]

base::CommandLine provides convenience methods to do this much more simply.
AppendSwitchNative() will help.

[bcwhite, 2015/09/02 15:21:45]

Done.

+  command_line += L"\"";
+  command_line += setup_exe.AsUTF16Unsafe();
+  command_line += L"\" --";
+  command_line += base::UTF8ToUTF16(
+      installer::switches::kSetDisplayVersionPath);
+  command_line += L"=";
+  command_line += reg_path;
+  command_line += L" --";
+  command_line += base::UTF8ToUTF16(
+      installer::switches::kSetDisplayVersionValue);
+  command_line += L"=";
+  command_line += base::UTF8ToUTF16(version.GetString());
+  command_line += L" --";
+  command_line += base::UTF8ToUTF16(installer::switches::kDelay);
+  command_line += L"=";
+  command_line += kMsiDisplayVersionOverwriteDelay;
+
+  STARTUPINFOW si = {sizeof(si)};
+  PROCESS_INFORMATION pi = {0};
+  if (!::CreateProcess(setup_exe.AsUTF16Unsafe().c_str(),

[robertshield, 2015/08/18 17:27:26]

use base::LaunchProcess here
(https://code.google.com/p/chromium/codesearch#chromium/src/base/process/launc...)

[grt (UTC plus 2), 2015/08/19 17:43:30]

i wonder if this should also use force_breakaway_from_job_? i don't know whether
or not msiexec runs procs in a job, but i think it should be safe to try to
breakaway in any case. what do you think, robert? the reason to do so is that it
would be bad if msiexec was using a job object and did something like wait for
all procs in the job to complete.

[robertshield, 2015/08/19 17:57:07]

I don't think it would hurt. MSI could also conceivably be using the job object
to perform install cancellation, which we'd then escape from. Afaict we no-op
above in OverwriteDisplayVersion if the expected values aren't present so this
should be okay. 

[bcwhite, 2015/09/02 15:21:45]

Why here but not elsewhere in this file?

[grt (UTC plus 2), 2015/09/02 20:43:55]

base::LaunchProcess should always be used unless something special it doesn't
support is needed. If there are other direct uses of CreateProcess here, they
should likely be switched. It's very easy to mis-use CreateProcess in
interesting ways.

[bcwhite, 2015/09/09 18:20:22]

Done.

+                       (LPWSTR)command_line.c_str(),  // discards const

[robertshield, 2015/08/18 17:27:26]

can you const_cast instead?

[bcwhite, 2015/09/02 15:21:45]

Already gone with switch to base::CommandLine.

+                       NULL, NULL, FALSE, CREATE_NO_WINDOW, NULL, NULL,
+                       &si, &pi)) {
+    LOG(ERROR) << "Failed to set DisplayVersion: "
+               << "could not launch subprocess to make desired changes."
+               << " (error=" << ::GetLastError() << ")\n"
+               << command_line;
+    return;
+  }
+  ::CloseHandle(pi.hThread);
+}
+
 // 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(
     const base::FilePath& setup_exe,
     const base::CommandLine& command_line,
     const installer::InstallerState& installer_state,
     const base::FilePath& working_directory) {
   // A compressed archive is ordinarily given on the command line by the mini
   // installer. If one was not given, look for chrome.packed.7z next to the
   // running program.
@@ skipping 810 matching lines @@
 
 // This method processes any command line options that make setup.exe do
 // various tasks other than installation (renaming chrome.exe, showing eula
 // among others). This function returns true if any such command line option
 // has been found and processed (so setup.exe should exit at that point).
 bool HandleNonInstallCmdLineOptions(const InstallationState& original_state,
                                     const base::FilePath& setup_exe,
                                     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) {
+      ::Sleep(delay_seconds * 1000);
+    }
+  }
+
   // 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
   // (!handled || status != installer::UNKNOWN_STATUS)).
   bool handled = true;
   // TODO(tommi): Split these checks up into functions and use a data driven
   // map of switch->function.
   if (cmd_line.HasSwitch(installer::switches::kUpdateSetupExe)) {
     installer::InstallStatus status = installer::SETUP_PATCH_FAILED;
     // If --update-setup-exe command line option is given, we apply the given
@@ skipping 214 matching lines @@
                                                patch_file,
                                                output_file);
     } else {
       *exit_code = installer::PATCH_INVALID_ARGUMENTS;
     }
   } else if (cmd_line.HasSwitch(installer::switches::kReenableAutoupdates)) {
     // setup.exe has been asked to attempt to reenable updates for Chrome.
     bool updates_enabled = GoogleUpdateSettings::ReenableAutoupdates();
     *exit_code = updates_enabled ? installer::REENABLE_UPDATES_SUCCEEDED :
                                    installer::REENABLE_UPDATES_FAILED;
+  } else if (cmd_line.HasSwitch(installer::switches::kSetDisplayVersionPath)) {
+    const base::string16 registry_path(
+        cmd_line.GetSwitchValueNative(
+            installer::switches::kSetDisplayVersionPath));
+    const base::string16 registry_value(

[grt (UTC plus 2), 2015/08/19 17:43:30]

i think it's better to extract the version from the current binary rather than
pass it on the command line (see chrome/app/client_util.cc's
GetCurrentModuleVersion for code to copy/paste).

[bcwhite, 2015/09/02 15:21:45]

If the original process has already gone through the trouble to get it, is it
worth doing this?  Also, this leaves the flexibility of the caller being able to
choose something different if so desired.  Lastly, it is (IMO) more symmetric to
pass both key & value rather than just one.

+        cmd_line.GetSwitchValueNative(
+            installer::switches::kSetDisplayVersionValue));
+    *exit_code = OverwriteDisplayVersion(registry_path, registry_value);
   } else {
     handled = false;
   }
 
   return handled;
 }
 
 // Returns the Custom information for the client identified by the exe path
 // passed in. This information is used for crash reporting.
 google_breakpad::CustomClientInfo* GetCustomInfo(const wchar_t* exe_path) {
@@ skipping 364 matching lines @@
           .Append(setup_path.BaseName());
     }
     const Products& products = installer_state.products();
     for (Products::const_iterator it = products.begin(); it < products.end();
          ++it) {
       const Product& product = **it;
       product.LaunchUserExperiment(setup_path, install_status, system_install);
     }
   }
 
-  // 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::kMsiInstallId,
+                        &install_id)) {
+      DelayedOverwriteDisplayVersion(setup_exe, install_id, *installer_version);

[grt (UTC plus 2), 2015/08/19 17:43:30]

if i'm reading the code correctly, |setup_exe| is the path to the currently
running setup.exe, which will be deleted once this execution of setup.exe exits.
this code should instead use the path to the setup.exe that was just installed.
the call to GetInstallerDirectory on line 1648 gives you the directory
containing the installed setup.exe.

[bcwhite, 2015/09/02 15:21:45]

Done.

+    }
+    // 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
   // deletion at reboot when this scope closes.
   VLOG(1) << "Deleting temporary directory " << temp_path.path().value();
 
   return install_status;
 }
 
 }  // namespace installer
@@ skipping 144 matching lines @@
     // Note that we allow the status installer::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is
     // never invoked directly by Google Update.
     return_code = InstallUtil::GetInstallReturnCode(install_status);
   }
 
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;
 }