Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule()

chrome/installer/util/module_util_win.cc

Index: chrome/installer/util/module_util_win.cc
diff --git a/chrome/installer/util/module_util_win.cc b/chrome/installer/util/module_util_win.cc
index 2ed551319da19b086ece7e40c443b95e5da2d37e..1cec074bb30ecba6428c09eb7ada036a1bea8d27 100644
--- a/chrome/installer/util/module_util_win.cc
+++ b/chrome/installer/util/module_util_win.cc
@@ -4,35 +4,77 @@
 
 #include "chrome/installer/util/module_util_win.h"
 
-#include <memory>
+#include <Windows.h>
+
+#include <string.h>
 
 #include "base/base_paths.h"
-#include "base/file_version_info.h"
 #include "base/files/file.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/version.h"
 #include "base/win/current_module.h"
+#include "base/win/resource_util.h"
 
 namespace installer {
 
 namespace {
 
+// This structure is not declared anywhere in the SDK, but is documented at

[fdoray, 2016/06/06 14:40:58]

I copied this from
third_party/crashpad/crashpad/snapshot/win/pe_image_reader.cc. Would it be
appropriate to declare in in base/win/* so that it can be reused here and in
crashpad?

[grt (UTC plus 2), 2016/06/06 15:42:56]

Many current uses of CreateFileVersionInfoForModule and CreateFileVersionInfo
look to me like they could be serviced with this impl or something like it (some
require other fields from VS_FIXEDFILEINFO). Please survey them all to see how
practical it is to make a single fn in base/win that would serve most/all
callers. Thanks.

[fdoray, 2016/06/17 20:18:45]

Done.

The documentation of ::VerQueryValue() [1] says that its first argument must
come from ::GetFileVersionInfo. However, these pages [2] imply that it is also
valid to use the VERSIONINFO resource as argument. And unit tests don't fail :)

[1]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms647464(v=vs.85).aspx
[2]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms646981(v=vs.85).aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/aa381058(v=vs.85).aspx

+// https://msdn.microsoft.com/en-us/library/windows/desktop/ms647001.aspx.
+struct VS_VERSIONINFO {
+  WORD wLength;
+  WORD wValueLength;
+  WORD wType;
+
+  // The structure documentation on MSDN doesn’t show the [16], but it does
+  // say that it’s supposed to be L"VS_VERSION_INFO", which is is in fact a
+  // 16-character string (including its NUL terminator).
+  WCHAR szKey[16];
+
+  WORD Padding1;
+  VS_FIXEDFILEINFO Value;
+
+  // Don’t include Children or the Padding2 that precedes it, because they may
+  // not be present.
+  // WORD Padding2;
+  // WORD Children;
+};
+
 // Returns the version in the current executable's version resource.
 base::string16 GetCurrentExecutableVersion() {
-  std::unique_ptr<FileVersionInfo> file_version_info(
-      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE()));
-  DCHECK(file_version_info.get());
-  base::string16 version_string(file_version_info->file_version());
+  VS_VERSIONINFO* version_info;
+  size_t version_info_length;
+  const bool has_version_resource = base::win::GetResourceFromModule(
+      CURRENT_MODULE(), VS_VERSION_INFO, RT_VERSION,
+      reinterpret_cast<void**>(&version_info), &version_info_length);
+  DCHECK(has_version_resource);
+  DCHECK_LE(sizeof(*version_info), version_info_length);
+  DCHECK_LE(sizeof(*version_info), version_info->wLength);
+  DCHECK_EQ(version_info_length, version_info->wLength);
+  DCHECK_EQ(version_info->wValueLength, sizeof(version_info->Value));
+  DCHECK_EQ(version_info->wType, 0);
+  DCHECK_EQ(wcsncmp(version_info->szKey, L"VS_VERSION_INFO",
+                    arraysize(version_info->szKey)),
+            0);
+
+  const uint16_t version_0 = version_info->Value.dwFileVersionMS >> 16;
+  const uint16_t version_1 = version_info->Value.dwFileVersionMS & 0xffff;
+  const uint16_t version_2 = version_info->Value.dwFileVersionLS >> 16;
+  const uint16_t version_3 = version_info->Value.dwFileVersionLS & 0xffff;
+
+  const base::string16 version_string(base::StringPrintf(
+      L"%hu.%hu.%hu.%hu", version_0, version_1, version_2, version_3));
   DCHECK(base::Version(base::UTF16ToASCII(version_string)).IsValid());
   return version_string;
 }
 
 // Indicates whether a file can be opened using the same flags that
 // ::LoadLibrary() uses to open modules.
-bool ModuleCanBeRead(const base::FilePath file_path) {
+bool ModuleCanBeRead(const base::FilePath& file_path) {
   return base::File(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ)
       .IsValid();
 }