Return Windows error code when create-process fails.

chrome/installer/mini_installer/mini_installer.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.
 
 // mini_installer.exe is the first exe that is run when chrome is being
 // installed or upgraded. It is designed to be extremely small (~5KB with no
 // extra resources linked) and it has two main jobs:
 //   1) unpack the resources (possibly decompressing some)
 //   2) run the real installer (setup.exe) with appropriate flags.
 //
@@ skipping 21 matching lines @@
 #include "chrome/installer/mini_installer/mini_installer_constants.h"
 #include "chrome/installer/mini_installer/mini_string.h"
 #include "chrome/installer/mini_installer/pe_resource.h"
 
 namespace mini_installer {
 
 typedef DWORD ProcessExitCode;
 typedef StackString<MAX_PATH> PathString;
 typedef StackString<MAX_PATH * 4> CommandString;
 
+DWORD LastWindowsError = 0;

[grt (UTC plus 2), 2015/07/28 02:30:07]

can you do away with this? i don't think it's necessary to have a global
variable.

[bcwhite, 2015/07/28 13:14:01]

Yes, though the global was the easiest.  I could change all the return codes to
be 64-bits and combine both exit-code and last-error DWORDs into the single
value.  Or return a pair<>.  Any preference?

[grt (UTC plus 2), 2015/07/28 14:00:03]

Pair or a POD struct w/ value semantics both sound fine. A struct may be more
clear since the reader won't need to remember who's on first.

+
 // This structure passes data back and forth for the processing
 // of resource callbacks.
 struct Context {
   // Input to the call back method. Specifies the dir to save resources.
   const wchar_t* base_path;
   // First output from call back method. Full path of Chrome archive.
   PathString* chrome_resource_path;
   // Second output from call back method. Full path of Setup archive/exe.
   PathString* setup_resource_path;
 };
 
 // A helper class used to manipulate the Windows registry.  Typically, members
 // return Windows last-error codes a la the Win32 registry API.
 class RegKey {
  public:
   RegKey() : key_(NULL) { }
   ~RegKey() { Close(); }
 
   // Opens the key named |sub_key| with given |access| rights.  Returns
   // ERROR_SUCCESS or some other error.
   LONG Open(HKEY key, const wchar_t* sub_key, REGSAM access);
 
   // Returns true if a key is open.
   bool is_valid() const { return key_ != NULL; }
 
-  // Read a REG_SZ value from the registry into the memory indicated by |value|
+  // Read a value from the registry into the memory indicated by |value|
   // (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;
+  LONG ReadDWValue(const wchar_t* value_name, DWORD* value) 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();
 
  private:
   RegKey(const RegKey&);
   RegKey& operator=(const RegKey&);
 
   HKEY key_;
 };  // class RegKey
 
 LONG RegKey::Open(HKEY key, const wchar_t* sub_key, REGSAM access) {
   Close();
   return ::RegOpenKeyEx(key, sub_key, NULL, access, &key_);
 }
 
-LONG RegKey::ReadValue(const wchar_t* value_name,
-                       wchar_t* value,
-                       size_t value_size) const {
+LONG RegKey::ReadSZValue(const wchar_t* value_name,
+                         wchar_t* value,
+                         size_t value_size) const {
   DWORD type;
   DWORD byte_length = static_cast<DWORD>(value_size * sizeof(wchar_t));
   LONG result = ::RegQueryValueEx(key_, value_name, NULL, &type,
                                   reinterpret_cast<BYTE*>(value),
                                   &byte_length);
   if (result == ERROR_SUCCESS) {
     if (type != REG_SZ) {
       result = ERROR_NOT_SUPPORTED;
     } else if (byte_length == 0) {
       *value = L'\0';
     } else if (value[byte_length/sizeof(wchar_t) - 1] != L'\0') {
       if ((byte_length / sizeof(wchar_t)) < value_size)
         value[byte_length / sizeof(wchar_t)] = L'\0';
       else
         result = ERROR_MORE_DATA;
     }
   }
   return result;
 }
 
-LONG RegKey::WriteValue(const wchar_t* value_name, const wchar_t* value) {
+LONG RegKey::ReadDWValue(const wchar_t* value_name, DWORD* value) const {
+  DWORD type;
+  DWORD byte_length = sizeof(*value);
+  LONG result = ::RegQueryValueEx(key_, value_name, NULL, &type,
+                                  reinterpret_cast<BYTE*>(value),
+                                  &byte_length);
+  if (result == ERROR_SUCCESS) {
+    if (type != REG_DWORD) {
+      result = ERROR_NOT_SUPPORTED;
+    } else if (byte_length != sizeof(*value)) {
+      result = ERROR_NO_DATA;
+    }
+  }
+  return result;
+}
+
+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_);
     key_ = NULL;
   }
 }
 
 // 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;
 }
 
 // Opens the Google Update ClientState key for a product.  This finds only
 // registry entries for Chrome; it does not support the Chromium registry
 // layout.
 bool OpenClientStateKey(HKEY root_key, const wchar_t* app_guid, REGSAM access,
                         RegKey* key) {
   PathString client_state_key;
   return client_state_key.assign(kClientStateKeyBase) &&
          client_state_key.append(app_guid) &&
          (key->Open(root_key,
                     client_state_key.get(),
                     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) {

[grt (UTC plus 2), 2015/07/28 02:30:06]

the logic in here that figures out which app_guid to use to communicate with
Google Update could be moved into Configuration and performed during its
initialization. this would make it testable in configuration_test.cc
(setup_unittests), which is finally being run on the trybots and waterfall
builders. the one hiccup that i see is that OpenClientStateKey (and maybe other
things) would need to be moved into some sort of utility file, which means
adding new source(s) to the build target. this requires an extra step, but i
think it's worth it due to the simplification here and the testability (that
TODO can become a DONE without too much effort). Configuration::app_guid() could
be the accessor that returns the guid by which the installer is to interact with
Google Update.

[bcwhite, 2015/07/28 13:14:01]

Okay.  Any objection to RegKey becoming more sophisticated and moving those
methods into it within a new file?

   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
   //    a ClientState key in the registry.  Only it need be modified.
   // To handle all cases, we inspect the product's ClientState to see if it
   // exists and its "ap" value does not contain "-multi".  This is case 3, so we
   // 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.
+// Don't write anything if there is already a result present, likely
+// written by setup.exe.
+void WriteInstallResults(const Configuration& configuration,
+                         DWORD exit_code, DWORD last_error) {
+#if defined(GOOGLE_CHROME_BUILD)
+  // Calls to setup.exe will write a "success" result if everything was good
+  // so we don't need to write anything from here.
+  if (exit_code == ERROR_SUCCESS)
+    return;
+
+  RegKey key;
+  DWORD value;
+  if (OpenInstallStateKey(configuration, &key)) {
+    if (key.ReadDWValue(kInstallerResultRegistryValue, &value)

[grt (UTC plus 2), 2015/07/28 02:30:06]

this adds a dependency on an implementation detail of Google Update, which seems
risky. can this be rejiggered so that it doesn't check for existing values?

[bcwhite, 2015/07/28 13:14:01]

I don't think so.  We have to check if there is a non-success value present
before writing our own.  These keys and values, as well as their behavior, are
documented.  We could instead make the decision based on the exit-code of
setup.exe but that would also be an implementation detail and an undocumented
one at that.

[grt (UTC plus 2), 2015/07/28 14:00:04]

Why? Does it not suffice to suppress writing when setup.exe is successfully
launched and exits?
Google Update is not under our control. Who knows when it will decide to move
the values to LastFoo?
setup.exe is part of Chrome, so we can establish any contract we want here.

[bcwhite, 2015/07/28 14:52:59]

If we set a constraint that the MI isn't allowed to have any kind of (reported)
failure after RunSetup completes, then sure.  It's complicated, though, by the
fact that setup runs twice and the "don't report" flag needs to be stored
somewhere that is either global or passed around.
Does it matter?  In the extremely unlikely event that Omaha reports in between
when setup completes and MI completes, we would report two results.  Omaha would
either show one additional successful install or two separate failures, both of
which could provide useful information.

Even ignoring the fact that most updates are started by Omaha which presumably
waits for completion before reporting, the number of split results would so
small as to be statistically insignificant, on the order of 0.01% (assuming 8s
of MI activity after setup completes and completely random update/omaha runs
throughout the day).

I don't think it warrants any additional complexity in order to try to mitigate
that.

+            != ERROR_SUCCESS || value == 0) {
+      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
   //    not present.
   // 2. When ap value is missing, we are going to create it with the required
   //    tag.
   if ((ret == ERROR_SUCCESS) || (ret == ERROR_FILE_NOT_FOUND)) {
     if (ret == ERROR_FILE_NOT_FOUND)
       value.clear();
 
     if (!StrEndsWith(value.get(), kFullInstallerSuffix) &&
         value.append(kFullInstallerSuffix)) {
-      key.WriteValue(kApRegistryValue, value.get());
+      key.WriteSZValue(kApRegistryValue, value.get());
     }
   }
 }
 
 // Gets the setup.exe path from Registry by looking at the value of Uninstall
 // string.  |size| is measured in wchar_t units.
 ProcessExitCode GetSetupExePathForAppGuid(bool system_level,
                                           const wchar_t* app_guid,
                                           const wchar_t* previous_version,
                                           wchar_t* path,
                                           size_t size) {
   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;
   }
 
   // Check that the path to the existing installer includes the expected
   // version number.  It's not necessary for accuracy to verify before/after
   // delimiters.
   if (!SearchStringI(path, previous_version))
     return PATCH_NOT_FOR_INSTALLED_VERSION;
 
   return SUCCESS_EXIT_CODE;
@@ skipping 35 matching lines @@
 }
 
 // Calls CreateProcess with good default parameters and waits for the process to
 // terminate returning the process exit code. |exit_code|, if non-NULL, is
 // populated with the process exit code.
 ProcessExitCode RunProcessAndWait(const wchar_t* exe_path, wchar_t* cmdline) {
   STARTUPINFOW si = {sizeof(si)};
   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;
   }
 
   ::CloseHandle(pi.hThread);
 
   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)) {

[grt (UTC plus 2), 2015/07/28 02:30:07]

please add a comment that this code assumes that wr == WAIT_FAILED if it !=
WAIT_OBJECT_0. i think that this is a safe assumption, but if it isn't we'll get
noise from GetLastError.

[bcwhite, 2015/07/28 13:14:01]

Done.

+    LastWindowsError = ::GetLastError();
     exit_code = WAIT_FOR_PROCESS_FAILED;
+  }
 
   ::CloseHandle(pi.hProcess);
 
   return exit_code;
 }
 
 // Appends any command line params passed to mini_installer to the given buffer
 // so that they can be passed on to setup.exe.
 // |buffer| is unchanged in case of error.
 void AppendCommandLineFlags(const Configuration& configuration,
@@ skipping 128 matching lines @@
     base_path,
     archive_path,
     setup_path,
   };
 
   // Get the resources of type 'B7' (7zip archive).
   // We need a chrome archive to do the installation. So if there
   // 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();

[grt (UTC plus 2), 2015/07/28 02:30:07]

the last error code is generally not reset to ERROR_SUCCESS (0) when an
arbitrary Win32 call succeeds, so GetLastError shouldn't be called for cases
where EnumResourceNames succeeds but the expected resource wasn't there. it'll
provide random noise which we won't be able to distinguish from the actual
signal.

[bcwhite, 2015/07/28 13:14:01]

Done.

     return UNABLE_TO_EXTRACT_CHROME_ARCHIVE;
+  }
 
   ProcessExitCode exit_code = SUCCESS_EXIT_CODE;
 
   // If we found setup 'B7' resource (used for differential updates), handle
   // it.  Note that this is only for Chrome; Chromium installs are always
   // "full" installs.
   if (setup_path->length() > 0) {
     CommandString cmd_line;
     PathString exe_path;
     // Get the path to setup.exe first.
@@ skipping 28 matching lines @@
     else if (!setup_path->assign(setup_dest_path.get()))
       exit_code = PATH_STRING_OVERFLOW;
 
     return exit_code;
   }
 
   // setup.exe wasn't sent as 'B7', lets see if it was sent as 'BL'
   // (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'
     // as opposed to old DOS way of 'SZDD'. Hence we don't use LZCopy.
     bool success = mini_installer::Expand(setup_path->get(),
                                           setup_dest_path.get());
     ::DeleteFile(setup_path->get());
     if (success) {
       if (!setup_path->assign(setup_dest_path.get())) {
         ::DeleteFile(setup_dest_path.get());
         exit_code = PATH_STRING_OVERFLOW;
       }
     } else {
       exit_code = UNABLE_TO_EXTRACT_SETUP_EXE;
     }
 
 #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;
   }
 
   // setup.exe still not found. So finally check if it was sent as 'BN'
   // (uncompressed setup).
   // TODO(tommi): We don't need BN anymore so let's remove it (and remove
   // 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) {
       if (!::MoveFileEx(setup_path->get(), setup_dest_path.get(),
                         MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING)) {
         ::DeleteFile(setup_path->get());
         setup_path->clear();
       } else if (!setup_path->assign(setup_dest_path.get())) {
         ::DeleteFile(setup_dest_path.get());
       }
@@ skipping 18 matching lines @@
   // Get the path to setup.exe first.
   if (::lstrlen(setup_path) > 0) {
     if (!cmd_line.assign(L"\"") ||
         !cmd_line.append(setup_path) ||
         !cmd_line.append(L"\""))
       return COMMAND_STRING_OVERFLOW;
   } else {
     ProcessExitCode exit_code = GetPreviousSetupExePath(
         configuration, cmd_line.get(), cmd_line.capacity());
     if (exit_code != SUCCESS_EXIT_CODE)
       return exit_code;

[grt (UTC plus 2), 2015/07/28 02:30:07]

if this is hit, the return value from the various registry functions (which are
"last error codes", just not using GetLastError) would be informative.

[bcwhite, 2015/07/28 13:14:01]

Okay.  This will be easier when the return-value includes both exit-code and
last-error.

   }
 
   // Append the command line param for chrome archive file.
   if (!cmd_line.append(L" --") ||
 #if defined(COMPONENT_BUILD)
       // For faster developer turnaround, the component build generates
       // uncompressed archives.
       !cmd_line.append(kCmdUncompressedArchive) ||
 #else
       !cmd_line.append(kCmdInstallArchive) ||
@@ skipping 231 matching lines @@
 
 // Returns true if we should delete the temp files we create (default).
 // Returns false iff the user has manually created a ChromeInstallerCleanup
 // string value in the registry under HKCU\\Software\\[Google|Chromium]
 // and set its value to "0".  That explicitly forbids the mini installer from
 // deleting these files.
 // Support for this has been publicly mentioned in troubleshooting tips so
 // 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;
   }
 
   return true;
 }
 
 // Main function. First gets a working dir, unpacks the resources and finally
 // executes setup.exe to do the install/upgrade.
 ProcessExitCode WMain(HMODULE module) {
@@ skipping 10 matching lines @@
   if (!configuration.Initialize(module))
     return GENERIC_INITIALIZATION_FAILURE;
 
   // If the --cleanup switch was specified on the command line, then that means
   // we should only do the cleanup and then exit.
   if (ProcessNonInstallOperations(configuration, &exit_code))
     return exit_code;
 
   // First get a path where we can extract payload
   PathString base_path;
   if (!GetWorkDir(module, &base_path))

[grt (UTC plus 2), 2015/07/28 02:30:06]

failures in here should be captured, too

     return UNABLE_TO_GET_WORK_DIRECTORY;
 
 #if defined(GOOGLE_CHROME_BUILD)
   // Set the magic suffix in registry to try full installer next time. We ignore
   // any errors here and we try to set the suffix for user level unless
   // --system-level is on the command line in which case we set it for system
   // level instead. This only applies to the Google Chrome distribution.
   SetInstallerFlags(configuration);
 #endif
 
   PathString archive_path;
   PathString setup_path;
   exit_code = UnpackBinaryResources(configuration, module, base_path.get(),
                                     &archive_path, &setup_path);
 
   // While unpacking the binaries, we paged in a whole bunch of memory that
   // we don't need anymore.  Let's give it back to the pool before running
   // setup.
   ::SetProcessWorkingSetSize(::GetCurrentProcess(), -1, -1);
 
   if (exit_code == SUCCESS_EXIT_CODE)
     exit_code = RunSetup(configuration, archive_path.get(), setup_path.get());
 
   if (ShouldDeleteExtractedFiles())
     DeleteExtractedFiles(base_path.get(), archive_path.get(), setup_path.get());
 
+  WriteInstallResults(configuration, exit_code, LastWindowsError);
   return exit_code;
 }
 
 }  // namespace mini_installer
 
 int MainEntryPoint() {
   mini_installer::ProcessExitCode result =
       mini_installer::WMain(::GetModuleHandle(NULL));
+
   ::ExitProcess(result);
 }
 
 // VC Express editions don't come with the memset CRT obj file and linking to
 // the obj files between versions becomes a bit problematic. Therefore,
 // simply implement memset.
 //
 // This also avoids having to explicitly set the __sse2_available hack when
 // linking with both the x64 and x86 obj files which is required when not
 // linking with the std C lib in certain instances (including Chromium) with
 // MSVC.  __sse2_available determines whether to use SSE2 intructions with
 // std C lib routines, and is set by MSVC's std C lib implementation normally.
 extern "C" {
 #pragma function(memset)
 void* memset(void* dest, int c, size_t count) {
   void* start = dest;
   while (count--) {
     *reinterpret_cast<char*>(dest) = static_cast<char>(c);
     dest = reinterpret_cast<char*>(dest) + 1;
   }
   return start;
 }
 }  // extern "C"