Return Windows error code when create-process fails.

chrome/installer/mini_installer/mini_installer.cc

Index: chrome/installer/mini_installer/mini_installer.cc
diff --git a/chrome/installer/mini_installer/mini_installer.cc b/chrome/installer/mini_installer/mini_installer.cc
index acbdc260ac6c8ed2c744bb29a7e25897dc6d3d1f..9d3d8bc2eb500a5a1b13380e70f6e6aa9145a97c 100644
--- a/chrome/installer/mini_installer/mini_installer.cc
+++ b/chrome/installer/mini_installer/mini_installer.cc
@@ -39,6 +39,8 @@ typedef DWORD ProcessExitCode;
 typedef StackString<MAX_PATH> PathString;
 typedef StackString<MAX_PATH * 4> CommandString;
 
+DWORD LastWindowsError = 0;
+
 // This structure passes data back and forth for the processing
 // of resource callbacks.
 struct Context {
@@ -68,13 +70,14 @@ class RegKey {
   // (of |value_size| wchar_t units).  Returns ERROR_SUCCESS,
   // ERROR_FILE_NOT_FOUND, ERROR_MORE_DATA, or some other error.  |value| is
   // guaranteed to be null-terminated on success.
-  LONG ReadValue(const wchar_t* value_name,
-                 wchar_t* value,
-                 size_t value_size) const;
+  LONG ReadSZValue(const wchar_t* value_name,
+                   wchar_t* value,
+                   size_t value_size) const;
 
-  // Write a REG_SZ value to the registry.  |value| must be null-terminated.
+  // Write a value to the registry.  SZ |value| must be null-terminated.
   // Returns ERROR_SUCCESS or an error code.
-  LONG WriteValue(const wchar_t* value_name, const wchar_t* value);
+  LONG WriteSZValue(const wchar_t* value_name, const wchar_t* value);
+  LONG WriteDWValue(const wchar_t* value_name, DWORD value);
 
   // Closes the key if it was open.
   void Close();
@@ -91,7 +94,7 @@ LONG RegKey::Open(HKEY key, const wchar_t* sub_key, REGSAM access) {
   return ::RegOpenKeyEx(key, sub_key, NULL, access, &key_);
 }
 
-LONG RegKey::ReadValue(const wchar_t* value_name,
+LONG RegKey::ReadSZValue(const wchar_t* value_name,
                        wchar_t* value,
                        size_t value_size) const {
   DWORD type;
@@ -114,12 +117,18 @@ LONG RegKey::ReadValue(const wchar_t* value_name,
   return result;
 }
 
-LONG RegKey::WriteValue(const wchar_t* value_name, const wchar_t* value) {
+LONG RegKey::WriteSZValue(const wchar_t* value_name, const wchar_t* value) {
   return ::RegSetValueEx(key_, value_name, 0, REG_SZ,
                          reinterpret_cast<const BYTE*>(value),
                          (lstrlen(value) + 1) * sizeof(wchar_t));
 }
 
+LONG RegKey::WriteDWValue(const wchar_t* value_name, DWORD value) {
+  return ::RegSetValueEx(key_, value_name, 0, REG_DWORD,
+                         reinterpret_cast<const BYTE*>(&value),
+                         sizeof(value));
+}
+
 void RegKey::Close() {
   if (key_ != NULL) {
     ::RegCloseKey(key_);
@@ -130,13 +139,13 @@ void RegKey::Close() {
 // Helper function to read a value from registry. Returns true if value
 // is read successfully and stored in parameter value. Returns false otherwise.
 // |size| is measured in wchar_t units.
-bool ReadValueFromRegistry(HKEY root_key, const wchar_t *sub_key,
-                           const wchar_t *value_name, wchar_t *value,
-                           size_t size) {
+bool ReadSZValueFromRegistry(HKEY root_key, const wchar_t *sub_key,
+                             const wchar_t *value_name, wchar_t *value,
+                             size_t size) {
   RegKey key;
 
   if (key.Open(root_key, sub_key, KEY_QUERY_VALUE) == ERROR_SUCCESS &&
-      key.ReadValue(value_name, value, size) == ERROR_SUCCESS) {
+      key.ReadSZValue(value_name, value, size) == ERROR_SUCCESS) {
     return true;
   }
   return false;
@@ -155,29 +164,26 @@ bool OpenClientStateKey(HKEY root_key, const wchar_t* app_guid, REGSAM access,
                     access | KEY_WOW64_32KEY) == ERROR_SUCCESS);
 }
 
-// This function sets the flag in registry to indicate that Google Update
-// should try full installer next time. If the current installer works, this
-// flag is cleared by setup.exe at the end of install. The flag will by default
-// be written to HKCU, but if --system-level is included in the command line,
-// it will be written to HKLM instead.
-// TODO(grt): Write a unit test for this that uses registry virtualization.
-void SetInstallerFlags(const Configuration& configuration) {
-  RegKey key;
-  const REGSAM key_access = KEY_QUERY_VALUE | KEY_SET_VALUE;
+// Opens the Google Update ClientState key for the current install
+// configuration.  This includes locating the correct key in the face of
+// multi-install.  The flag will by default be written to HKCU, but if
+// --system-level is included in the command line, it will be written to
+// HKLM instead.
+// TODO(bcwhite): Write a unit test for this that uses registry virtualization.
+bool OpenInstallStateKey(const Configuration& configuration, RegKey* key) {
   const HKEY root_key =
       configuration.is_system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
-  // This is ignored if multi-install is true.
   const wchar_t* app_guid =
       configuration.has_chrome_frame() ?
           google_update::kChromeFrameAppGuid :
           configuration.chrome_app_guid();
-  StackString<128> value;
+  const REGSAM key_access = KEY_QUERY_VALUE | KEY_SET_VALUE;
   LONG ret = ERROR_SUCCESS;
 
   // When multi_install is true, we are potentially:
   // 1. Performing a multi-install of some product(s) on a clean machine.
   //    Neither the product(s) nor the multi-installer will have a ClientState
-  //    key in the registry, so there is nothing to be done.
+  //    key in the registry, so there is no key to be modified.
   // 2. Upgrading an existing multi-install.  The multi-installer will have a
   //    ClientState key in the registry.  Only it need be modified.
   // 3. Migrating a single-install into a multi-install.  The product will have
@@ -187,30 +193,53 @@ void SetInstallerFlags(const Configuration& configuration) {
   // modify the product's ClientState.  Otherwise, we check the
   // multi-installer's ClientState and modify it if it exists.
   if (configuration.is_multi_install()) {
-    if (OpenClientStateKey(root_key, app_guid, key_access, &key)) {
+    if (OpenClientStateKey(root_key, app_guid, key_access, key)) {
       // The product has a client state key.  See if it's a single-install.
-      ret = key.ReadValue(kApRegistryValue, value.get(), value.capacity());
-      if (ret != ERROR_FILE_NOT_FOUND &&
-          (ret != ERROR_SUCCESS ||
-           FindTagInStr(value.get(), kMultiInstallTag, NULL))) {
-        // Error or case 2: modify the multi-installer's value.
-        key.Close();
-        app_guid = google_update::kMultiInstallAppGuid;
-      }  // else case 3: modify this value.
-    } else {
-      // case 1 or 2: modify the multi-installer's value.
-      key.Close();
-      app_guid = google_update::kMultiInstallAppGuid;
+      StackString<128> value;
+      ret = key->ReadSZValue(kApRegistryValue, value.get(), value.capacity());
+      if (ret == ERROR_FILE_NOT_FOUND ||
+          (ret == ERROR_SUCCESS &&
+           !FindTagInStr(value.get(), kMultiInstallTag, NULL))) {
+        // yes -- case 3: modify this key.
+        return true;
+      }
     }
+    // error, case 1, or case 2: modify the multi-installer's key.
+    key->Close();
+    app_guid = google_update::kMultiInstallAppGuid;
   }
 
-  if (!key.is_valid()) {
-    if (!OpenClientStateKey(root_key, app_guid, key_access, &key))
-      return;
+  return OpenClientStateKey(root_key, app_guid, key_access, key);
+}
 
-    value.clear();
-    ret = key.ReadValue(kApRegistryValue, value.get(), value.capacity());
+// Writes install results into registry where it is read by Google Update.
+void WriteInstallResults(const Configuration& configuration,
+                         DWORD exit_code, DWORD last_error) {
+#if defined(GOOGLE_CHROME_BUILD)
+  RegKey key;
+  if (OpenInstallStateKey(configuration, &key)) {
+    key.WriteDWValue(kInstallerResultRegistryValue,
+                     exit_code ? 1 /* FAILED_CUSTOM_ERROR */
+                               : 0 /* SUCCESS */);
+    key.WriteDWValue(kInstallerErrorRegistryValue, exit_code);
+    key.WriteDWValue(kInstallerExtraCode1RegistryValue, last_error);
+    key.Close();
   }
+#endif
+}
+
+// This function sets the flag in registry to indicate that Google Update
+// should try full installer next time. If the current installer works, this
+// flag is cleared by setup.exe at the end of install.
+void SetInstallerFlags(const Configuration& configuration) {
+  RegKey key;
+  StackString<128> value;
+  LONG ret = ERROR_SUCCESS;
+
+  if (!OpenInstallStateKey(configuration, &key))
+    return;
+
+  ret = key.ReadSZValue(kApRegistryValue, value.get(), value.capacity());
 
   // The conditions below are handling two cases:
   // 1. When ap value is present, we want to add the required tag only if it is
@@ -223,7 +252,7 @@ void SetInstallerFlags(const Configuration& configuration) {
 
     if (!StrEndsWith(value.get(), kFullInstallerSuffix) &&
         value.append(kFullInstallerSuffix)) {
-      key.WriteValue(kApRegistryValue, value.get());
+      key.WriteSZValue(kApRegistryValue, value.get());
     }
   }
 }
@@ -238,7 +267,7 @@ ProcessExitCode GetSetupExePathForAppGuid(bool system_level,
   const HKEY root_key = system_level ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   RegKey key;
   if (!OpenClientStateKey(root_key, app_guid, KEY_QUERY_VALUE, &key) ||
-      (key.ReadValue(kUninstallRegistryValue, path, size) != ERROR_SUCCESS)) {
+      (key.ReadSZValue(kUninstallRegistryValue, path, size) != ERROR_SUCCESS)) {
     return UNABLE_TO_FIND_REGISTRY_KEY;
   }
 
@@ -294,6 +323,7 @@ ProcessExitCode RunProcessAndWait(const wchar_t* exe_path, wchar_t* cmdline) {
   PROCESS_INFORMATION pi = {0};
   if (!::CreateProcess(exe_path, cmdline, NULL, NULL, FALSE, CREATE_NO_WINDOW,
                        NULL, NULL, &si, &pi)) {
+    LastWindowsError = ::GetLastError();
     return COULD_NOT_CREATE_PROCESS;
   }
 
@@ -301,8 +331,10 @@ ProcessExitCode RunProcessAndWait(const wchar_t* exe_path, wchar_t* cmdline) {
 
   ProcessExitCode exit_code = SUCCESS_EXIT_CODE;
   DWORD wr = ::WaitForSingleObject(pi.hProcess, INFINITE);
-  if (WAIT_OBJECT_0 != wr || !::GetExitCodeProcess(pi.hProcess, &exit_code))
+  if (WAIT_OBJECT_0 != wr || !::GetExitCodeProcess(pi.hProcess, &exit_code)) {
+    LastWindowsError = ::GetLastError();
     exit_code = WAIT_FOR_PROCESS_FAILED;
+  }
 
   ::CloseHandle(pi.hProcess);
 
@@ -451,8 +483,10 @@ ProcessExitCode UnpackBinaryResources(const Configuration& configuration,
   // is a problem in fetching B7 resource, just return an error.
   if (!::EnumResourceNames(module, kLZMAResourceType, OnResourceFound,
                            reinterpret_cast<LONG_PTR>(&context)) ||
-      archive_path->length() == 0)
+      archive_path->length() == 0) {
+    LastWindowsError = ::GetLastError();
     return UNABLE_TO_EXTRACT_CHROME_ARCHIVE;
+  }
 
   ProcessExitCode exit_code = SUCCESS_EXIT_CODE;
 
@@ -501,8 +535,10 @@ ProcessExitCode UnpackBinaryResources(const Configuration& configuration,
   // (compressed setup).
   if (!::EnumResourceNames(module, kLZCResourceType, OnResourceFound,
                            reinterpret_cast<LONG_PTR>(&context)) &&
-      ::GetLastError() != ERROR_RESOURCE_TYPE_NOT_FOUND)
+      ::GetLastError() != ERROR_RESOURCE_TYPE_NOT_FOUND) {
+    LastWindowsError = ::GetLastError();
     return UNABLE_TO_EXTRACT_SETUP_B7;
+  }
 
   if (setup_path->length() > 0) {
     // Uncompress LZ compressed resource. Setup is packed with 'MSCF'
@@ -522,8 +558,10 @@ ProcessExitCode UnpackBinaryResources(const Configuration& configuration,
 #if defined(COMPONENT_BUILD)
     // Extract the (uncompressed) modules required by setup.exe.
     if (!::EnumResourceNames(module, kBinResourceType, WriteResourceToDirectory,
-                             reinterpret_cast<LONG_PTR>(base_path)))
+                             reinterpret_cast<LONG_PTR>(base_path))) {
+      LastWindowsError = ::GetLastError();
       return UNABLE_TO_EXTRACT_SETUP;
+    }
 #endif
 
     return exit_code;
@@ -535,8 +573,10 @@ ProcessExitCode UnpackBinaryResources(const Configuration& configuration,
   // it from create_installer_archive.py).
   if (!::EnumResourceNames(module, kBinResourceType, OnResourceFound,
                            reinterpret_cast<LONG_PTR>(&context)) &&
-      ::GetLastError() != ERROR_RESOURCE_TYPE_NOT_FOUND)
+      ::GetLastError() != ERROR_RESOURCE_TYPE_NOT_FOUND) {
+    LastWindowsError = ::GetLastError();
     return UNABLE_TO_EXTRACT_SETUP_BN;
+  }
 
   if (setup_path->length() > 0) {
     if (setup_path->comparei(setup_dest_path.get()) != 0) {
@@ -827,8 +867,8 @@ bool ProcessNonInstallOperations(const Configuration& configuration,
 // we continue to support it.
 bool ShouldDeleteExtractedFiles() {
   wchar_t value[2] = {0};
-  if (ReadValueFromRegistry(HKEY_CURRENT_USER, kCleanupRegistryKey,
-                            kCleanupRegistryValue, value, _countof(value)) &&
+  if (ReadSZValueFromRegistry(HKEY_CURRENT_USER, kCleanupRegistryKey,
+                              kCleanupRegistryValue, value, _countof(value)) &&
       value[0] == L'0') {
     return false;
   }
@@ -886,6 +926,7 @@ ProcessExitCode WMain(HMODULE module) {
   if (ShouldDeleteExtractedFiles())
     DeleteExtractedFiles(base_path.get(), archive_path.get(), setup_path.get());
 
+  WriteInstallResults(configuration, exit_code, LastWindowsError);

[grt (UTC plus 2), 2015/07/27 15:21:37]

i had a thought over the weekend: mini_installer should not write results when
RunSetup succeeds in launching setup.exe. setup.exe is responsible for writing
its own results to the registry. we wouldn't want mini_installer to overwrite
it.

   return exit_code;
 }
 
@@ -894,6 +935,7 @@ ProcessExitCode WMain(HMODULE module) {
 int MainEntryPoint() {
   mini_installer::ProcessExitCode result =
       mini_installer::WMain(::GetModuleHandle(NULL));
+
   ::ExitProcess(result);
 }