Bug 1615401 - Part 2: Keep Chromium's file_version_info_win.cpp updated r=bobowen
authorToshihito Kikuchi <tkikuchi@mozilla.com>
Fri, 06 Mar 2020 22:24:01 +0200
changeset 517413 dc9d71fb3bac807a37dbfba35d609ac4ffff1980
parent 517412 032c00b0ab3967111f686a45ce1b1beec97c5ba1
child 517414 b6a09f304f86fdc9d6a19f85e64a422c3d30b8bc
push id37190
push useraciure@mozilla.com
push dateSat, 07 Mar 2020 21:33:39 +0000
treeherdermozilla-central@d2ac41047c10 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1615401
milestone75.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1615401 - Part 2: Keep Chromium's file_version_info_win.cpp updated r=bobowen Cherry-picking the following commits from Chromium to keep file_version_info_win.cpp up-to-date. 1. Use StringPiece rather than std::string for Version parsing. https://source.chromium.org/chromium/chromium/src/+/15a9d1733f4129fb18a7fe57708d351caf42638e 2. [Cleanup] Un-const the result of base::Version::GetString() https://source.chromium.org/chromium/chromium/src/+/fde745d058a8a50019c01c838a7efa5be2f6268c 3. Export Windows file version as base::Version https://source.chromium.org/chromium/chromium/src/+/e93de3a37d21cd2ef61890d8acafb4b3f033781b 4. Tidy FileVersionInfoWin. https://source.chromium.org/chromium/chromium/src/+/4bb23ded3a6612274bb16b347ae7a387ddc51c29 Differential Revision: https://phabricator.services.mozilla.com/D65802 Depends on D65744
security/sandbox/chromium-shim/base/file_version_info_win.cpp
security/sandbox/chromium-shim/base/file_version_info_win.h
security/sandbox/chromium/base/version.cc
security/sandbox/chromium/base/version.h
security/sandbox/chromium/base/win/windows_version.cc
--- a/security/sandbox/chromium-shim/base/file_version_info_win.cpp
+++ b/security/sandbox/chromium-shim/base/file_version_info_win.cpp
@@ -28,24 +28,22 @@ LanguageAndCodePage* GetTranslate(const 
   static constexpr wchar_t kTranslation[] = L"\\VarFileInfo\\Translation";
   LPVOID translate = nullptr;
   UINT dummy_size;
   if (::VerQueryValue(data, kTranslation, &translate, &dummy_size))
     return static_cast<LanguageAndCodePage*>(translate);
   return nullptr;
 }
 
-VS_FIXEDFILEINFO* GetVsFixedFileInfo(const void* data) {
-  VS_FIXEDFILEINFO* fixed_file_info = nullptr;
-  UINT length;
-  if (::VerQueryValue(data, L"\\", reinterpret_cast<void**>(&fixed_file_info),
-                      &length)) {
-    return fixed_file_info;
-  }
-  return nullptr;
+const VS_FIXEDFILEINFO& GetVsFixedFileInfo(const void* data) {
+  static constexpr wchar_t kRoot[] = L"\\";
+  LPVOID fixed_file_info = nullptr;
+  UINT dummy_size;
+  CHECK(::VerQueryValue(data, kRoot, &fixed_file_info, &dummy_size));
+  return *static_cast<VS_FIXEDFILEINFO*>(fixed_file_info);
 }
 
 }  // namespace
 
 // static
 std::unique_ptr<FileVersionInfoWin>
 FileVersionInfoWin::CreateFileVersionInfoWin(const base::FilePath& file_path) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
@@ -65,16 +63,23 @@ FileVersionInfoWin::CreateFileVersionInf
   const LanguageAndCodePage* translate = GetTranslate(data.data());
   if (!translate)
     return nullptr;
 
   return base::WrapUnique(new FileVersionInfoWin(
       std::move(data), translate->language, translate->code_page));
 }
 
+base::Version FileVersionInfoWin::GetFileVersion() const {
+  return base::Version({HIWORD(fixed_file_info_.dwFileVersionMS),
+                        LOWORD(fixed_file_info_.dwFileVersionMS),
+                        HIWORD(fixed_file_info_.dwFileVersionLS),
+                        LOWORD(fixed_file_info_.dwFileVersionLS)});
+}
+
 FileVersionInfoWin::FileVersionInfoWin(std::vector<uint8_t>&& data,
                                        WORD language,
                                        WORD code_page)
     : owned_data_(std::move(data)),
       data_(owned_data_.data()),
       language_(language),
       code_page_(code_page),
       fixed_file_info_(GetVsFixedFileInfo(data_)) {
--- a/security/sandbox/chromium-shim/base/file_version_info_win.h
+++ b/security/sandbox/chromium-shim/base/file_version_info_win.h
@@ -9,45 +9,46 @@
 
 #ifndef BASE_FILE_VERSION_INFO_WIN_H_
 #define BASE_FILE_VERSION_INFO_WIN_H_
 
 #include <memory>
 #include <vector>
 
 #include "base/macros.h"
+#include "base/version.h"
 
 #include "mozilla/Assertions.h"
 
 struct tagVS_FIXEDFILEINFO;
 typedef tagVS_FIXEDFILEINFO VS_FIXEDFILEINFO;
 
 namespace base {
 class FilePath;
 }
 
 class FileVersionInfoWin {
  public:
   static std::unique_ptr<FileVersionInfoWin> CreateFileVersionInfoWin(
       const base::FilePath& file_path);
 
-  // Get the fixed file info if it exists. Otherwise NULL
-  const VS_FIXEDFILEINFO* fixed_file_info() const { return fixed_file_info_; }
+  // Get file version number in dotted version format.
+  base::Version GetFileVersion() const;
 
  private:
   // |data| is a VS_VERSION_INFO resource. |language| and |code_page| are
   // extracted from the \VarFileInfo\Translation value of |data|.
   FileVersionInfoWin(std::vector<uint8_t>&& data,
                      WORD language,
                      WORD code_page);
 
   const std::vector<uint8_t> owned_data_;
   const void* const data_;
   const WORD language_;
   const WORD code_page_;
 
-  // This is a pointer into |data_| if it exists. Otherwise nullptr.
-  const VS_FIXEDFILEINFO* const fixed_file_info_;
+  // This is a reference for a portion of |data_|.
+  const VS_FIXEDFILEINFO& fixed_file_info_;
 
   DISALLOW_COPY_AND_ASSIGN(FileVersionInfoWin);
 };
 
 #endif  // BASE_FILE_VERSION_INFO_WIN_H_
--- a/security/sandbox/chromium/base/version.cc
+++ b/security/sandbox/chromium/base/version.cc
@@ -17,33 +17,33 @@ namespace base {
 
 namespace {
 
 // Parses the |numbers| vector representing the different numbers
 // inside the version string and constructs a vector of valid integers. It stops
 // when it reaches an invalid item (including the wildcard character). |parsed|
 // is the resulting integer vector. Function returns true if all numbers were
 // parsed successfully, false otherwise.
-bool ParseVersionNumbers(const std::string& version_str,
+bool ParseVersionNumbers(StringPiece version_str,
                          std::vector<uint32_t>* parsed) {
   std::vector<StringPiece> numbers =
       SplitStringPiece(version_str, ".", KEEP_WHITESPACE, SPLIT_WANT_ALL);
   if (numbers.empty())
     return false;
 
   for (auto it = numbers.begin(); it != numbers.end(); ++it) {
     if (StartsWith(*it, "+", CompareCase::SENSITIVE))
       return false;
 
     unsigned int num;
     if (!StringToUint(*it, &num))
       return false;
 
     // This throws out leading zeros for the first item only.
-    if (it == numbers.begin() && UintToString(num) != *it)
+    if (it == numbers.begin() && NumberToString(num) != *it)
       return false;
 
     // StringToUint returns unsigned int but Version fields are uint32_t.
     static_assert(sizeof (uint32_t) == sizeof (unsigned int),
         "uint32_t must be same as unsigned int");
     parsed->push_back(num);
   }
   return true;
@@ -78,42 +78,42 @@ int CompareVersionComponents(const std::
 }  // namespace
 
 Version::Version() = default;
 
 Version::Version(const Version& other) = default;
 
 Version::~Version() = default;
 
-Version::Version(const std::string& version_str) {
+Version::Version(StringPiece version_str) {
   std::vector<uint32_t> parsed;
   if (!ParseVersionNumbers(version_str, &parsed))
     return;
 
   components_.swap(parsed);
 }
 
 Version::Version(std::vector<uint32_t> components)
     : components_(std::move(components)) {}
 
 bool Version::IsValid() const {
   return (!components_.empty());
 }
 
 // static
-bool Version::IsValidWildcardString(const std::string& wildcard_string) {
-  std::string version_string = wildcard_string;
+bool Version::IsValidWildcardString(StringPiece wildcard_string) {
+  StringPiece version_string = wildcard_string;
   if (EndsWith(version_string, ".*", CompareCase::SENSITIVE))
-    version_string.resize(version_string.size() - 2);
+    version_string = version_string.substr(0, version_string.size() - 2);
 
   Version version(version_string);
   return version.IsValid();
 }
 
-int Version::CompareToWildcardString(const std::string& wildcard_string) const {
+int Version::CompareToWildcardString(StringPiece wildcard_string) const {
   DCHECK(IsValid());
   DCHECK(Version::IsValidWildcardString(wildcard_string));
 
   // Default behavior if the string doesn't end with a wildcard.
   if (!EndsWith(wildcard_string, ".*", CompareCase::SENSITIVE)) {
     Version version(wildcard_string);
     DCHECK(version.IsValid());
     return CompareTo(version);
@@ -146,25 +146,25 @@ int Version::CompareToWildcardString(con
 }
 
 int Version::CompareTo(const Version& other) const {
   DCHECK(IsValid());
   DCHECK(other.IsValid());
   return CompareVersionComponents(components_, other.components_);
 }
 
-const std::string Version::GetString() const {
+std::string Version::GetString() const {
   DCHECK(IsValid());
   std::string version_str;
   size_t count = components_.size();
   for (size_t i = 0; i < count - 1; ++i) {
-    version_str.append(UintToString(components_[i]));
+    version_str.append(NumberToString(components_[i]));
     version_str.append(".");
   }
-  version_str.append(UintToString(components_[count - 1]));
+  version_str.append(NumberToString(components_[count - 1]));
   return version_str;
 }
 
 bool operator==(const Version& v1, const Version& v2) {
   return v1.CompareTo(v2) == 0;
 }
 
 bool operator!=(const Version& v1, const Version& v2) {
--- a/security/sandbox/chromium/base/version.h
+++ b/security/sandbox/chromium/base/version.h
@@ -7,60 +7,61 @@
 
 #include <stdint.h>
 
 #include <iosfwd>
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
+#include "base/strings/string_piece.h"
 
 namespace base {
 
 // Version represents a dotted version number, like "1.2.3.4", supporting
 // parsing and comparison.
 class BASE_EXPORT Version {
  public:
   // The only thing you can legally do to a default constructed
   // Version object is assign to it.
   Version();
 
   Version(const Version& other);
 
   // Initializes from a decimal dotted version number, like "0.1.1".
   // Each component is limited to a uint16_t. Call IsValid() to learn
   // the outcome.
-  explicit Version(const std::string& version_str);
+  explicit Version(StringPiece version_str);
 
   // Initializes from a vector of components, like {1, 2, 3, 4}. Call IsValid()
   // to learn the outcome.
   explicit Version(std::vector<uint32_t> components);
 
   ~Version();
 
   // Returns true if the object contains a valid version number.
   bool IsValid() const;
 
   // Returns true if the version wildcard string is valid. The version wildcard
   // string may end with ".*" (e.g. 1.2.*, 1.*). Any other arrangement with "*"
   // is invalid (e.g. 1.*.3 or 1.2.3*). This functions defaults to standard
   // Version behavior (IsValid) if no wildcard is present.
-  static bool IsValidWildcardString(const std::string& wildcard_string);
+  static bool IsValidWildcardString(StringPiece wildcard_string);
 
   // Returns -1, 0, 1 for <, ==, >.
   int CompareTo(const Version& other) const;
 
   // Given a valid version object, compare if a |wildcard_string| results in a
   // newer version. This function will default to CompareTo if the string does
   // not end in wildcard sequence ".*". IsValidWildcard(wildcard_string) must be
   // true before using this function.
-  int CompareToWildcardString(const std::string& wildcard_string) const;
+  int CompareToWildcardString(StringPiece wildcard_string) const;
 
   // Return the string representation of this version.
-  const std::string GetString() const;
+  std::string GetString() const;
 
   const std::vector<uint32_t>& components() const { return components_; }
 
  private:
   std::vector<uint32_t> components_;
 };
 
 BASE_EXPORT bool operator==(const Version& v1, const Version& v2);
--- a/security/sandbox/chromium/base/win/windows_version.cc
+++ b/security/sandbox/chromium/base/win/windows_version.cc
@@ -215,25 +215,17 @@ base::Version OSInfo::Kernel32BaseVersio
     if (!file_version_info) {
       // crbug.com/912061: on some systems it seems kernel32.dll might be
       // corrupted or not in a state to get version info. In this case try
       // kernelbase.dll as a fallback.
       file_version_info = FileVersionInfoWin::CreateFileVersionInfoWin(
           FilePath(FILE_PATH_LITERAL("kernelbase.dll")));
     }
     CHECK(file_version_info);
-    const uint32_t major =
-        HIWORD(file_version_info->fixed_file_info()->dwFileVersionMS);
-    const uint32_t minor =
-        LOWORD(file_version_info->fixed_file_info()->dwFileVersionMS);
-    const uint32_t build =
-        HIWORD(file_version_info->fixed_file_info()->dwFileVersionLS);
-    const uint32_t patch =
-        LOWORD(file_version_info->fixed_file_info()->dwFileVersionLS);
-    return base::Version(std::vector<uint32_t>{major, minor, build, patch});
+    return file_version_info->GetFileVersion();
   }());
   return *version;
 }
 
 std::string OSInfo::processor_model_name() {
   if (processor_model_name_.empty()) {
     const char16 kProcessorNameString[] =
         STRING16_LITERAL("HARDWARE\\DESCRIPTION\\System\\CentralProcessor\\0");