Add histograms to compare GetVersionEx() with VerQueryValue() of kernel32

base/win/windows_version.cc

Index: base/win/windows_version.cc
diff --git a/base/win/windows_version.cc b/base/win/windows_version.cc
index 7a8b8fdbdbaad6958759cdf603858d2387606bc5..60f102fbe79720a2ae0939c5b6e45858c275f809 100644
--- a/base/win/windows_version.cc
+++ b/base/win/windows_version.cc
@@ -6,17 +6,109 @@
 
 #include <windows.h>
 
+#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/win/registry.h"
 
 namespace {
+
 typedef BOOL (WINAPI *GetProductInfoPtr)(DWORD, DWORD, DWORD, DWORD, PDWORD);
+
+// Retrieve the type and version information from a given module. This function
+// calls GetFileVersionInfo() which can implicitly call LoadLibrary().
+bool GetModuleVersionAndType(const base::FilePath& path,

[grt (UTC plus 2), 2016/03/10 19:43:44]

i propose avoiding the FilePath for this private function. it's only used to
extract the string it's constructed with.

[scottmg, 2016/03/10 23:51:39]

Done.

+                             VS_FIXEDFILEINFO* vs_fixedfileinfo) {
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  DWORD size = ::GetFileVersionInfoSize(path.value().c_str(), nullptr);
+  if (!size) {
+    PLOG_IF(WARNING, GetLastError() != ERROR_RESOURCE_TYPE_NOT_FOUND)
+        << "GetFileVersionInfoSize: " << base::UTF16ToUTF8(path.value());

[grt (UTC plus 2), 2016/03/10 19:43:44]

in the future, base::UTF16ToUTF8 isn't needed in cases like this; path.value()
works by itself. for cross-platformness, i believe path.AsUTF8Unsafe() is the
right choice when you do need to turn a path into UTF-8.

[scottmg, 2016/03/10 23:51:39]

It works in an ugly way (injecting into operator<< into std::), so I'd rather be
explicit.
Noted, thanks.

+    return false;
+  }
+
+  scoped_ptr<uint8_t[]> data(new uint8_t[size]);
+  if (!::GetFileVersionInfo(path.value().c_str(), 0, size, data.get())) {
+    PLOG(WARNING) << "GetFileVersionInfo: " << base::UTF16ToUTF8(path.value());
+    return false;
+  }
+
+  VS_FIXEDFILEINFO* fixed_file_info;
+  UINT ffi_size;

[robliao, 2016/03/10 19:28:51]

Nit: Given that we don't actually care about this, you could go with UINT
ignored;

[scottmg, 2016/03/10 23:51:40]

Done.

+  if (!::VerQueryValue(data.get(), L"\\",
+                       reinterpret_cast<void**>(&fixed_file_info), &ffi_size)) {
+    PLOG(WARNING) << "VerQueryValue";
+    return false;
+  }
+
+  *vs_fixedfileinfo = *fixed_file_info;
+  vs_fixedfileinfo->dwFileFlags &= vs_fixedfileinfo->dwFileFlagsMask;
+  return true;
 }
 
+}  // namespace
+
 namespace base {
 namespace win {
 
+namespace {
+
+// Helper to map a major.minor.x.build version (e.g. 6.1) to a Windows release.
+base::win::Version MajorMinorBuildToVersion(int major, int minor, int build) {

[grt (UTC plus 2), 2016/03/10 19:43:44]

is base::win:: needed to disambiguate? this is within the namespace already.

[scottmg, 2016/03/10 23:51:39]

Done.

+  if ((major == 5) && (minor > 0)) {
+    // Treat XP Pro x64, Home Server, and Server 2003 R2 as Server 2003.
+    return (minor == 1) ? VERSION_XP : VERSION_SERVER_2003;
+  } else if (major == 6) {
+    switch (minor) {
+      case 0:
+        // Treat Windows Server 2008 the same as Windows Vista.
+        return VERSION_VISTA;
+      case 1:
+        // Treat Windows Server 2008 R2 the same as Windows 7.
+        return VERSION_WIN7;
+      case 2:
+        // Treat Windows Server 2012 the same as Windows 8.
+        return VERSION_WIN8;
+      default:
+        DCHECK_EQ(minor, 3);
+        return VERSION_WIN8_1;
+    }
+  } else if (major == 10) {
+    if (build < 10586) {
+      return VERSION_WIN10;
+    } else {
+      return VERSION_WIN10_TH2;
+    }
+  } else if (major > 6) {
+    NOTREACHED();
+    return VERSION_WIN_LAST;
+  }
+
+  NOTREACHED();
+  return VERSION_WIN_LAST;
+}
+
+// Retrieve a version from kernel32. This is useful because when running in
+// compatibility mode for a down-level version of the OS, the file version of
+// kernel32 will still be the "real" version.
+base::win::Version GetVersionFromKernel32() {
+  const wchar_t kSystemDll[] = L"kernel32.dll";

[grt (UTC plus 2), 2016/03/10 19:43:44]

nit: static const wchar_t to avoid the weight of putting the string on the
stack.

[scottmg, 2016/03/10 23:51:39]

Done.

+  VS_FIXEDFILEINFO ffi;

[robliao, 2016/03/10 19:28:51]

Nit: fixed_file_info. This looks very hungarian :-).

[scottmg, 2016/03/10 23:51:39]

Done.

+  if (::GetModuleVersionAndType(base::FilePath(kSystemDll), &ffi)) {

[grt (UTC plus 2), 2016/03/10 19:43:44]

can you use FileVersionInfoWin to get rid of the duplication of code here?

[scottmg, 2016/03/10 23:51:39]

Done.

+    const int major = ffi.dwFileVersionMS >> 16;

[grt (UTC plus 2), 2016/03/10 19:43:44]

HIWORD and LOWORD?

[scottmg, 2016/03/10 23:51:39]

Done.

+    const int minor = ffi.dwFileVersionMS & 0xffff;
+    const int build = ffi.dwFileVersionLS >> 16;
+    return MajorMinorBuildToVersion(major, minor, build);
+  }
+
+  NOTREACHED();
+  return VERSION_WIN_LAST;
+}
+
+}  // namespace
+
 // static
 OSInfo* OSInfo::GetInstance() {
   // Note: we don't use the Singleton class because it depends on AtExitManager,
@@ -35,6 +127,8 @@ OSInfo* OSInfo::GetInstance() {
 
 OSInfo::OSInfo()
     : version_(VERSION_PRE_XP),
+      kernel32_version_(VERSION_PRE_XP),
+      got_kernel32_version_(false),
       architecture_(OTHER_ARCHITECTURE),
       wow64_status_(GetWOW64StatusForProcess(GetCurrentProcess())) {
   OSVERSIONINFOEX version_info = { sizeof version_info };
@@ -42,38 +136,8 @@ OSInfo::OSInfo()
   version_number_.major = version_info.dwMajorVersion;
   version_number_.minor = version_info.dwMinorVersion;
   version_number_.build = version_info.dwBuildNumber;
-  if ((version_number_.major == 5) && (version_number_.minor > 0)) {
-    // Treat XP Pro x64, Home Server, and Server 2003 R2 as Server 2003.
-    version_ = (version_number_.minor == 1) ? VERSION_XP : VERSION_SERVER_2003;
-  } else if (version_number_.major == 6) {
-    switch (version_number_.minor) {
-      case 0:
-        // Treat Windows Server 2008 the same as Windows Vista.
-        version_ = VERSION_VISTA;
-        break;
-      case 1:
-        // Treat Windows Server 2008 R2 the same as Windows 7.
-        version_ = VERSION_WIN7;
-        break;
-      case 2:
-        // Treat Windows Server 2012 the same as Windows 8.
-        version_ = VERSION_WIN8;
-        break;
-      default:
-        DCHECK_EQ(version_number_.minor, 3);
-        version_ = VERSION_WIN8_1;
-        break;
-    }
-  } else if (version_number_.major == 10) {
-    if (version_number_.build < 10586) {
-      version_ = VERSION_WIN10;
-    } else {
-      version_ = VERSION_WIN10_TH2;
-    }
-  } else if (version_number_.major > 6) {
-    NOTREACHED();
-    version_ = VERSION_WIN_LAST;
-  }
+  version_ = MajorMinorBuildToVersion(
+      version_number_.major, version_number_.minor, version_number_.build);
   service_pack_.major = version_info.wServicePackMajor;
   service_pack_.minor = version_info.wServicePackMinor;
 
@@ -149,6 +213,14 @@ OSInfo::OSInfo()
 OSInfo::~OSInfo() {
 }
 
+Version OSInfo::Kernel32Version() const {
+  if (!got_kernel32_version_) {
+    kernel32_version_ = GetVersionFromKernel32();
+    got_kernel32_version_ = true;
+  }
+  return kernel32_version_;
+}
+
 std::string OSInfo::processor_model_name() {
   if (processor_model_name_.empty()) {
     const wchar_t kProcessorNameString[] =