[Setup]: Refactor error code.

This refactors UnPackArchive to return UnPackStatus instead
of GetLaastError(). Some errors don't have a clear DWORD mapping
and it's more straigthforward to return UnPackStatus as the main result,
and use it to decide success/failure at call sites.

Change-Id: I0aef82ba65a7cd134a2ecd552cee9b600f188e89
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1846312Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709208}

chrome/installer/setup/archive_patch_helper.cc

@@ -8,6 +8,7 @@
 
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/optional.h"
 #include "chrome/installer/setup/buildflags.h"
 #include "chrome/installer/util/lzma_util.h"
 #include "components/zucchini/zucchini.h"
@@ -48,12 +49,13 @@ bool ArchivePatchHelper::Uncompress(base::FilePath* last_uncompressed_file) {
 
   // UnPackArchive takes care of logging.
   base::FilePath output_file;
-  UnPackStatus unpack_status = UNPACK_NO_ERROR;
-  int32_t ntstatus = 0;
-  DWORD lzma_result = UnPackArchive(compressed_archive_, working_directory_,
-                                    &output_file, &unpack_status, &ntstatus);
-  RecordUnPackMetrics(unpack_status, ntstatus, lzma_result, consumer_);
-  if (lzma_result != ERROR_SUCCESS)
+  base::Optional<DWORD> error_code;
+  base::Optional<int32_t> ntstatus;
+  UnPackStatus unpack_status =
+      UnPackArchive(compressed_archive_, working_directory_, &output_file,
+                    &error_code, &ntstatus);
+  RecordUnPackMetrics(unpack_status, ntstatus, error_code, consumer_);
+  if (unpack_status != UNPACK_NO_ERROR)
     return false;
 
   last_uncompressed_file_ = output_file;

chrome/installer/setup/setup_main.cc

@@ -26,6 +26,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/persistent_histogram_storage.h"
 #include "base/numerics/safe_conversions.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/process/memory.h"
@@ -1098,13 +1099,13 @@ InstallStatus InstallProductsHelper(const InstallationState& original_state,
   // --- Background ---
   //   ~14s (50%ile) / >3m (99%ile)
   installer_state.SetStage(UNPACKING);
-  UnPackStatus unpack_status = UNPACK_NO_ERROR;
-  int32_t ntstatus = 0;
-  DWORD lzma_result = UnPackArchive(uncompressed_archive, unpack_path, NULL,
-                                    &unpack_status, &ntstatus);
-  RecordUnPackMetrics(unpack_status, ntstatus, lzma_result,
+  base::Optional<DWORD> error_code;
+  base::Optional<int32_t> ntstatus;
+  UnPackStatus unpack_status = UnPackArchive(uncompressed_archive, unpack_path,
+                                             nullptr, &error_code, &ntstatus);
+  RecordUnPackMetrics(unpack_status, ntstatus, error_code,
                       UnPackConsumer::UNCOMPRESSED_CHROME_ARCHIVE);
-  if (lzma_result) {
+  if (unpack_status != UNPACK_NO_ERROR) {
     installer_state.WriteInstallerResult(
         UNPACKING_FAILED,
         IDS_INSTALL_UNCOMPRESSION_FAILED_BASE,

chrome/installer/setup/setup_util.cc

@@ -701,8 +701,8 @@ int GetInstallAge(const InstallerState& installer_state) {
 }
 
 void RecordUnPackMetrics(UnPackStatus unpack_status,
-                         int32_t status,
-                         DWORD lzma_result,
+                         base::Optional<int32_t> ntstatus,
+                         base::Optional<DWORD> error_code,
                          UnPackConsumer consumer) {
   std::string consumer_name = "";
 
@@ -725,11 +725,16 @@ void RecordUnPackMetrics(UnPackStatus unpack_status,
       std::string(std::string(kUnPackStatusMetricsName) + "_" + consumer_name),
       unpack_status, UNPACK_STATUS_COUNT);
 
+  if (error_code.has_value()) {
     base::UmaHistogramSparse(
-      std::string(kUnPackResultMetricsName) + "_" + consumer_name, lzma_result);
-
+        std::string(kUnPackResultMetricsName) + "_" + consumer_name,
+        *error_code);
+  }
+  if (ntstatus.has_value()) {
     base::UmaHistogramSparse(
-      std::string(kUnPackNTSTATUSMetricsName) + "_" + consumer_name, status);
+        std::string(kUnPackNTSTATUSMetricsName) + "_" + consumer_name,
+        *ntstatus);
+  }
 }
 
 void RegisterEventLogProvider(const base::FilePath& install_directory,

chrome/installer/setup/setup_util.h

@@ -123,8 +123,8 @@ int GetInstallAge(const InstallerState& installer_state);
 
 // Records UMA metrics for unpack result.
 void RecordUnPackMetrics(UnPackStatus unpack_status,
-                         int32_t status,
-                         DWORD lzma_result,
+                         base::Optional<int32_t> ntstatus,
+                         base::Optional<DWORD> error_code,
                          UnPackConsumer consumer);
 
 // Register Chrome's EventLog message provider dll.

chrome/installer/test/alternate_version_generator.cc

@@ -649,7 +649,7 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
   // Unpack chrome.packed.7z (static build only).
   if (!chrome_packed_7z.empty()) {
     if (UnPackArchive(chrome_packed_7z, work_dir.directory(), &chrome_7z,
-                      nullptr, nullptr) != ERROR_SUCCESS) {
+                      nullptr, nullptr) != UNPACK_NO_ERROR) {
       LOG(DFATAL) << "Failed unpacking \"" << chrome_packed_7z.value() << "\"";
       return false;
     }
@@ -658,7 +658,7 @@ bool GenerateAlternateVersion(const base::FilePath& original_installer_path,
 
   // Unpack chrome.7z
   if (UnPackArchive(chrome_7z, work_dir.directory(), nullptr, nullptr,
-                    nullptr) != ERROR_SUCCESS) {
+                    nullptr) != UNPACK_NO_ERROR) {
     LOG(DFATAL) << "Failed unpacking \"" << chrome_7z.value() << "\"";
     return false;
   }

chrome/installer/util/lzma_util.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/installer/util/lzma_util.h"
 
+#include <windows.h>
+
 #include <stddef.h>
 
 #include <vector>
@@ -102,71 +104,62 @@ DWORD FilterPageError(const LzmaFileAllocator& file_allocator,
 
 }  // namespace
 
-DWORD UnPackArchive(const base::FilePath& archive,
+UnPackStatus UnPackArchive(const base::FilePath& archive,
                            const base::FilePath& output_dir,
                            base::FilePath* output_file,
-                    UnPackStatus* unpack_status,
-                    int32_t* ntstatus) {
+                           base::Optional<DWORD>* error_code,
+                           base::Optional<int32_t>* ntstatus) {
   VLOG(1) << "Opening archive " << archive.value();
   LzmaUtilImpl lzma_util;
-  DWORD ret;
-  if ((ret = lzma_util.OpenArchive(archive)) != ERROR_SUCCESS) {
+  UnPackStatus status;
+  if ((status = lzma_util.OpenArchive(archive)) != UNPACK_NO_ERROR) {
     PLOG(ERROR) << "Unable to open install archive: " << archive.value();
   } else {
     VLOG(1) << "Uncompressing archive to path " << output_dir.value();
-    if ((ret = lzma_util.UnPack(output_dir, output_file)) != ERROR_SUCCESS)
+    if ((status = lzma_util.UnPack(output_dir, output_file)) != UNPACK_NO_ERROR)
       PLOG(ERROR) << "Unable to uncompress archive: " << archive.value();
-    lzma_util.CloseArchive();
   }
-  if (unpack_status)
-    *unpack_status = lzma_util.GetUnPackStatus();
+  if (error_code)
+    *error_code = lzma_util.GetErrorCode();
   if (ntstatus)
     *ntstatus = lzma_util.GetNTSTATUSCode();
-  return ret;
+  return status;
 }
 
-LzmaUtilImpl::LzmaUtilImpl() {}
-LzmaUtilImpl::~LzmaUtilImpl() {
-  CloseArchive();
-}
+LzmaUtilImpl::LzmaUtilImpl() = default;
+LzmaUtilImpl::~LzmaUtilImpl() = default;
 
-DWORD LzmaUtilImpl::OpenArchive(const base::FilePath& archivePath) {
+UnPackStatus LzmaUtilImpl::OpenArchive(const base::FilePath& archivePath) {
   // Make sure file is not already open.
   CloseArchive();
 
-  DWORD ret = ERROR_SUCCESS;
-  archive_handle_ =
-      CreateFile(archivePath.value().c_str(), GENERIC_READ, FILE_SHARE_READ,
-                 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-  if (archive_handle_ == INVALID_HANDLE_VALUE) {
-    archive_handle_ = NULL;  // The rest of the code only checks for NULL.
-    ret = GetLastError();
-    if (ret == ERROR_FILE_NOT_FOUND)
-      unpack_status_ = UNPACK_ARCHIVE_NOT_FOUND;
-    else
-      unpack_status_ = UNPACK_ARCHIVE_CANNOT_OPEN;
-  }
-  return ret;
+  archive_file_.Initialize(archivePath, base::File::FLAG_OPEN |
+                                            base::File::FLAG_READ |
+                                            base::File::FLAG_EXCLUSIVE_WRITE);
+  if (archive_file_.IsValid())
+    return UNPACK_NO_ERROR;
+  error_code_ = ::GetLastError();
+  return archive_file_.error_details() == base::File::FILE_ERROR_NOT_FOUND
+             ? UNPACK_ARCHIVE_NOT_FOUND
+             : UNPACK_ARCHIVE_CANNOT_OPEN;
 }
 
-DWORD LzmaUtilImpl::UnPack(const base::FilePath& location) {
+UnPackStatus LzmaUtilImpl::UnPack(const base::FilePath& location) {
   return UnPack(location, NULL);
 }
 
-DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
+UnPackStatus LzmaUtilImpl::UnPack(const base::FilePath& location,
                                   base::FilePath* output_file) {
-  if (!archive_handle_)
-    return ERROR_INVALID_HANDLE;
+  DCHECK(archive_file_.IsValid());
 
   CFileInStream archiveStream;
   CLookToRead lookStream;
   CSzArEx db;
   ISzAlloc allocImp;
   ISzAlloc allocTempImp;
-  DWORD ret = ERROR_SUCCESS;
   SRes sz_res = SZ_OK;
 
-  archiveStream.file.handle = archive_handle_;
+  archiveStream.file.handle = archive_file_.GetPlatformFile();
   archiveStream.s.Read = SzFileReadImp;
   archiveStream.s.Seek = SzFileSeekImp;
   LookToRead_CreateVTable(&lookStream, false);
@@ -179,12 +172,14 @@ DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
 
   CrcGenerateTable();
   SzArEx_Init(&db);
+  ::SetLastError(ERROR_SUCCESS);
   if ((sz_res = SzArEx_Open(&db, &lookStream.s, &allocImp, &allocTempImp)) !=
       SZ_OK) {
     LOG(ERROR) << L"Error returned by SzArchiveOpen: " << sz_res;
-
-    unpack_status_ = UNPACK_SZAREX_OPEN_ERROR;
-    return ERROR_INVALID_HANDLE;
+    auto error_code = ::GetLastError();
+    if (error_code != ERROR_SUCCESS)
+      error_code_ = error_code;
+    return UNPACK_SZAREX_OPEN_ERROR;
   }
 
   Byte* outBuffer = 0;  // it must be 0 before first call for each new archive
@@ -193,41 +188,40 @@ DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
 
   LzmaFileAllocator fileAllocator(location);
 
-  unpack_status_ = UNPACK_NO_ERROR;
-
+  UnPackStatus status = UNPACK_NO_ERROR;
   for (unsigned int i = 0; i < db.NumFiles; i++) {
     DWORD written;
     size_t offset = 0;
     size_t outSizeProcessed = 0;
 
-    int32_t status = 0;  // STATUS_SUCCESS
+    int32_t ntstatus = 0;  // STATUS_SUCCESS
+    ::SetLastError(ERROR_SUCCESS);
     __try {
       if ((sz_res =
                SzArEx_Extract(&db, &lookStream.s, i, &blockIndex, &outBuffer,
                               &outBufferSize, &offset, &outSizeProcessed,
                               &fileAllocator, &allocTempImp)) != SZ_OK) {
         LOG(ERROR) << L"Error returned by SzExtract: " << sz_res;
-
-        ret = ERROR_INVALID_HANDLE;
-        unpack_status_ = UNPACK_EXTRACT_ERROR;
+        auto error_code = ::GetLastError();
+        if (error_code != ERROR_SUCCESS)
+          error_code_ = error_code;
+        status = UNPACK_EXTRACT_ERROR;
       }
     } __except(FilterPageError(fileAllocator, GetExceptionCode(),
-                                GetExceptionInformation(), &status)) {
-      ret = ERROR_IO_DEVICE;
-      ntstatus_ = status;
+                                GetExceptionInformation(), &ntstatus)) {
+      ntstatus_ = ntstatus;
+      status = UNPACK_EXTRACT_EXCEPTION;
       LOG(ERROR) << L"EXCEPTION_IN_PAGE_ERROR while accessing mapped memory; "
                     L"NTSTATUS = "
-                 << ntstatus_;
-      unpack_status_ = UNPACK_EXTRACT_EXCEPTION;
+                 << ntstatus;
     }
-    if (ret != ERROR_SUCCESS)
+    if (status != UNPACK_NO_ERROR)
       break;
 
     size_t file_name_length = SzArEx_GetFileNameUtf16(&db, i, NULL);
     if (file_name_length < 1) {
       LOG(ERROR) << L"Couldn't get file name";
-      ret = ERROR_INVALID_HANDLE;
-      unpack_status_ = UNPACK_NO_FILENAME_ERROR;
+      status = UNPACK_NO_FILENAME_ERROR;
       break;
     }
 
@@ -242,7 +236,11 @@ DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
 
     // If archive entry is directory create it and move on to the next entry.
     if (SzArEx_IsDir(&db, i)) {
-      CreateDirectory(file_path);
+      if (!CreateDirectory(file_path)) {
+        error_code_ = ::GetLastError();
+        status = UNPACK_CREATE_FILE_ERROR;
+        break;
+      }
       continue;
     }
 
@@ -252,9 +250,9 @@ DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
     hFile = CreateFile(file_path.value().c_str(), GENERIC_WRITE, 0, NULL,
                        CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
     if (hFile == INVALID_HANDLE_VALUE) {
-      ret = GetLastError();
-      LOG(ERROR) << L"Error returned by CreateFile: " << ret;
-      unpack_status_ = UNPACK_CREATE_FILE_ERROR;
+      error_code_ = ::GetLastError();
+      status = UNPACK_CREATE_FILE_ERROR;
+      PLOG(ERROR) << L"CreateFile failed";
       break;
     }
 
@@ -268,57 +266,56 @@ DWORD LzmaUtilImpl::UnPack(const base::FilePath& location,
       if ((!WriteFile(hFile, outBuffer + offset + total_written,
                       static_cast<DWORD>(write_amount), &written, nullptr)) ||
           (written != write_amount)) {
-        ret = GetLastError();
+        error_code_ = ::GetLastError();
+        status = UNPACK_WRITE_FILE_ERROR;
         PLOG(ERROR) << L"Error returned by WriteFile";
         CloseHandle(hFile);
-        unpack_status_ = UNPACK_WRITE_FILE_ERROR;
         break;
       }
       total_written += write_amount;
     }
 
     // Break out of the file loop if the write loop failed.
-    if (unpack_status_ != UNPACK_NO_ERROR)
+    if (status != UNPACK_NO_ERROR)
       break;
 
     if (SzBitWithVals_Check(&db.MTime, i)) {
       if (!SetFileTime(hFile, NULL, NULL,
                        (const FILETIME*)(&db.MTime.Vals[i]))) {
-        ret = GetLastError();
+        error_code_ = ::GetLastError();
+        status = UNPACK_SET_FILE_TIME_ERROR;
         PLOG(ERROR) << L"Error returned by SetFileTime";
         CloseHandle(hFile);
-        unpack_status_ = UNPACK_SET_FILE_TIME_ERROR;
         break;
       }
     }
     if (!CloseHandle(hFile)) {
-      ret = GetLastError();
+      error_code_ = ::GetLastError();
+      status = UNPACK_CLOSE_FILE_ERROR;
       PLOG(ERROR) << L"Error returned by CloseHandle";
-      unpack_status_ = UNPACK_CLOSE_FILE_ERROR;
       break;
     }
   }  // for loop
   IAlloc_Free(&fileAllocator, outBuffer);
   SzArEx_Free(&db, &allocImp);
-  DCHECK_EQ(ret == static_cast<DWORD>(ERROR_SUCCESS),
-            unpack_status_ == UNPACK_NO_ERROR);
+  DCHECK(status != UNPACK_NO_ERROR || !error_code_.has_value());
+  DCHECK(status != UNPACK_NO_ERROR || !ntstatus_.has_value());
 
-  return ret;
+  return status;
 }
 
 void LzmaUtilImpl::CloseArchive() {
-  if (archive_handle_) {
-    CloseHandle(archive_handle_);
-    archive_handle_ = NULL;
-  }
+  archive_file_.Close();
+  error_code_ = base::nullopt;
+  ntstatus_ = base::nullopt;
 }
 
 bool LzmaUtilImpl::CreateDirectory(const base::FilePath& dir) {
-  bool ret = true;
-  if (directories_created_.find(dir.value()) == directories_created_.end()) {
-    ret = base::CreateDirectory(dir);
-    if (ret)
-      directories_created_.insert(dir.value());
+  bool result = true;
+  if (directories_created_.find(dir) == directories_created_.end()) {
+    result = base::CreateDirectory(dir);
+    if (result)
+      directories_created_.insert(dir);
   }
-  return ret;
+  return result;
 }

chrome/installer/util/lzma_util.h

@@ -5,13 +5,13 @@
 #ifndef CHROME_INSTALLER_UTIL_LZMA_UTIL_H_
 #define CHROME_INSTALLER_UTIL_LZMA_UTIL_H_
 
-#include <windows.h>
-
 #include <set>
 
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
-#include "base/strings/string16.h"
+#include "base/optional.h"
+#include "base/win/windows_types.h"
 
 // The error status of LzmaUtil::Unpack which is used to publish metrics. Do not
 // change the order.
@@ -32,14 +32,15 @@ enum UnPackStatus {
 
 // Unpacks the contents of |archive| into |output_dir|. |output_file|, if not
 // null, is populated with the name of the last (or only) member extracted from
-// the archive. Returns ERROR_SUCCESS on success. Otherwise, returns a Windows
-// error code and populates |unpack_status| (if not null) with a status value
-// indicating the operation that failed.
-DWORD UnPackArchive(const base::FilePath& archive,
+// the archive. Returns UNPACK_NO_ERROR on success. Otherwise, returns a status
+// value indicating the operation that failed, populates |error_code| (if not
+// null) with a Windows error code and |ntstatus| with an exception code, if
+// any.
+UnPackStatus UnPackArchive(const base::FilePath& archive,
                            const base::FilePath& output_dir,
                            base::FilePath* output_file,
-                    UnPackStatus* unpack_status,
-                    int32_t* ntstatus);
+                           base::Optional<DWORD>* error_code,
+                           base::Optional<int32_t>* ntstatus);
 
 // A utility class that wraps LZMA SDK library. Prefer UnPackArchive over using
 // this class directly.
@@ -48,29 +49,30 @@ class LzmaUtilImpl {
   LzmaUtilImpl();
   ~LzmaUtilImpl();
 
-  DWORD OpenArchive(const base::FilePath& archivePath);
+  UnPackStatus OpenArchive(const base::FilePath& archivePath);
 
   // Unpacks the archive to the given location
-  DWORD UnPack(const base::FilePath& location);
-
-  void CloseArchive();
+  UnPackStatus UnPack(const base::FilePath& location);
 
   // Unpacks the archive to the given location and returns the last file
   // extracted from archive.
-  DWORD UnPack(const base::FilePath& location, base::FilePath* output_file);
+  UnPackStatus UnPack(const base::FilePath& location,
+                      base::FilePath* output_file);
+
+  void CloseArchive();
 
-  UnPackStatus GetUnPackStatus() { return unpack_status_; }
-  int32_t GetNTSTATUSCode() { return ntstatus_; }
+  base::Optional<DWORD> GetErrorCode() { return error_code_; }
+  base::Optional<int32_t> GetNTSTATUSCode() { return ntstatus_; }
 
  protected:
   bool CreateDirectory(const base::FilePath& dir);
 
  private:
-  HANDLE archive_handle_ = nullptr;
-  std::set<base::string16> directories_created_;
-  UnPackStatus unpack_status_ = UNPACK_NO_ERROR;
+  base::File archive_file_;
+  std::set<base::FilePath> directories_created_;
+  base::Optional<DWORD> error_code_;
   // Can't include ntstatus.h as it's conflicted with winnt.h
-  int32_t ntstatus_ = 0;  // STATUS_SUCCESS.
+  base::Optional<int32_t> ntstatus_;
 
   DISALLOW_COPY_AND_ASSIGN(LzmaUtilImpl);
 };

chrome/installer/util/lzma_util_unittest.cc

@@ -36,22 +36,21 @@ class LzmaUtilTest : public testing::Test {
 TEST_F(LzmaUtilTest, OpenArchiveTest) {
   base::FilePath archive = data_dir_.AppendASCII("archive1.7z");
   LzmaUtilImpl lzma_util;
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
 
   // We allow opening another archive (which will automatically close the first
   // archive).
   archive = data_dir_.AppendASCII("archive2.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
 
   // Explicitly close and open the first archive again.
   lzma_util.CloseArchive();
   archive = data_dir_.AppendASCII("archive1.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
 
   // Make sure non-existent archive returns error.
   archive = data_dir_.AppendASCII("archive.non_existent.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_FILE_NOT_FOUND),
-            lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_ARCHIVE_NOT_FOUND, lzma_util.OpenArchive(archive));
 }
 
 // Test that we can extract archives successfully.
@@ -64,38 +63,33 @@ TEST_F(LzmaUtilTest, UnPackTest) {
 
   base::FilePath archive = data_dir_.AppendASCII("archive1.7z");
   LzmaUtilImpl lzma_util;
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
   base::FilePath unpacked_file;
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            lzma_util.UnPack(extract_dir, &unpacked_file));
-  EXPECT_EQ(lzma_util.GetUnPackStatus(), UNPACK_NO_ERROR);
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.UnPack(extract_dir, &unpacked_file));
+  EXPECT_FALSE(lzma_util.GetErrorCode());
 
   EXPECT_TRUE(base::PathExists(extract_dir.AppendASCII("a.exe")));
   EXPECT_TRUE(unpacked_file == extract_dir.AppendASCII("a.exe"));
 
   archive = data_dir_.AppendASCII("archive2.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            lzma_util.UnPack(extract_dir, &unpacked_file));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.UnPack(extract_dir, &unpacked_file));
   EXPECT_TRUE(base::PathExists(extract_dir.AppendASCII("b.exe")));
   EXPECT_TRUE(unpacked_file == extract_dir.AppendASCII("b.exe"));
-  EXPECT_EQ(lzma_util.GetUnPackStatus(), UNPACK_NO_ERROR);
+  EXPECT_FALSE(lzma_util.GetErrorCode());
 
   lzma_util.CloseArchive();
   archive = data_dir_.AppendASCII("invalid_archive.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_INVALID_HANDLE),
-            lzma_util.UnPack(extract_dir, &unpacked_file));
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
-  EXPECT_EQ(static_cast<DWORD>(ERROR_INVALID_HANDLE),
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_SZAREX_OPEN_ERROR,
             lzma_util.UnPack(extract_dir, &unpacked_file));
 
-  EXPECT_EQ(static_cast<DWORD>(ERROR_INVALID_HANDLE),
+  EXPECT_EQ(UNPACK_SZAREX_OPEN_ERROR,
             lzma_util.UnPack(extract_dir, &unpacked_file));
 
   archive = data_dir_.AppendASCII("archive3.7z");
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS), lzma_util.OpenArchive(archive));
-  EXPECT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            lzma_util.UnPack(extract_dir, &unpacked_file));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.OpenArchive(archive));
+  EXPECT_EQ(UNPACK_NO_ERROR, lzma_util.UnPack(extract_dir, &unpacked_file));
   EXPECT_TRUE(base::PathExists(extract_dir.AppendASCII("archive\\a.exe")));
   EXPECT_TRUE(base::PathExists(
       extract_dir.AppendASCII("archive\\sub_dir\\text.txt")));
@@ -112,23 +106,21 @@ TEST_F(LzmaUtilTest, UnPackArchiveTest) {
   base::FilePath archive = data_dir_.AppendASCII("archive1.7z");
   base::FilePath unpacked_file;
 
-  EXPECT_EQ(
-      static_cast<DWORD>(ERROR_SUCCESS),
-      UnPackArchive(archive, extract_dir, &unpacked_file, nullptr, nullptr));
+  EXPECT_EQ(UNPACK_NO_ERROR, UnPackArchive(archive, extract_dir, &unpacked_file,
+                                           nullptr, nullptr));
 
   EXPECT_TRUE(base::PathExists(extract_dir.AppendASCII("a.exe")));
   EXPECT_TRUE(unpacked_file == extract_dir.AppendASCII("a.exe"));
 
   archive = data_dir_.AppendASCII("archive2.7z");
-  EXPECT_EQ(
-      static_cast<DWORD>(ERROR_SUCCESS),
-      UnPackArchive(archive, extract_dir, &unpacked_file, nullptr, nullptr));
+  EXPECT_EQ(UNPACK_NO_ERROR, UnPackArchive(archive, extract_dir, &unpacked_file,
+                                           nullptr, nullptr));
 
   EXPECT_TRUE(base::PathExists(extract_dir.AppendASCII("b.exe")));
   EXPECT_TRUE(unpacked_file == extract_dir.AppendASCII("b.exe"));
 
   archive = data_dir_.AppendASCII("invalid_archive.7z");
   EXPECT_EQ(
-      static_cast<DWORD>(ERROR_INVALID_HANDLE),
+      UNPACK_SZAREX_OPEN_ERROR,
       UnPackArchive(archive, extract_dir, &unpacked_file, nullptr, nullptr));
 }

chrome/updater/win/installer/installer.cc

@@ -423,20 +423,18 @@ ProcessExitResult WMain(HMODULE module) {
     return ProcessExitResult(static_cast<DWORD>(installer::TEMP_DIR_FAILED));
 
   // Unpack the compressed archive to extract the uncompressed archive file.
-  UnPackStatus unpack_status = UNPACK_NO_ERROR;
-  int32_t ntstatus = 0;
-  auto lzma_result =
+  UnPackStatus unpack_status =
       UnPackArchive(base::FilePath(compressed_archive.get()), unpack_path,
-                    nullptr, &unpack_status, &ntstatus);
-  if (lzma_result)
+                    nullptr, nullptr, nullptr);
+  if (unpack_status != UNPACK_NO_ERROR)
     return ProcessExitResult(static_cast<DWORD>(installer::UNPACKING_FAILED));
 
   // Unpack the uncompressed archive to extract the updater files.
   base::FilePath uncompressed_archive =
       unpack_path.Append(FILE_PATH_LITERAL("updater.7z"));
-  lzma_result = UnPackArchive(uncompressed_archive, unpack_path, nullptr,
-                              &unpack_status, &ntstatus);
-  if (lzma_result)
+  unpack_status = UnPackArchive(uncompressed_archive, unpack_path, nullptr,
+                                nullptr, nullptr);
+  if (unpack_status != UNPACK_NO_ERROR)
     return ProcessExitResult(static_cast<DWORD>(installer::UNPACKING_FAILED));
 
   // While unpacking the binaries, we paged in a whole bunch of memory that