Make the windows_version.h functions threadsafe by using a singleton.

chrome/installer/gcapi/gcapi.cc

-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
 #include "chrome/installer/gcapi/gcapi.h"
 
 #include <atlbase.h>
 #include <atlcom.h>
 #include <windows.h>
 #include <sddl.h>
-#include <stdlib.h>
+#define STRSAFE_NO_DEPRECATE
 #include <strsafe.h>
 #include <tlhelp32.h>
 
+#include <cstdlib>
+
 #include "google_update_idl.h"
 
 namespace {
 
 const wchar_t kChromeRegClientsKey[] =
     L"Software\\Google\\Update\\Clients\\"
     L"{8A69D345-D564-463c-AFF1-A69D9E530F96}";
 const wchar_t kChromeRegClientStateKey[] =
     L"Software\\Google\\Update\\ClientState\\"
     L"{8A69D345-D564-463c-AFF1-A69D9E530F96}";
@@ skipping 124 matching lines @@
                          reinterpret_cast<LPDWORD>(size)) == ERROR_SUCCESS)) {
     ::RegCloseKey(key);
     return true;
   }
   return false;
 }
 
 bool IsChromeInstalled(HKEY root_key) {
   wchar_t version[64];
   size_t size = _countof(version);
-  if (ReadValueFromRegistry(root_key, kChromeRegClientsKey, kChromeRegVersion,
-                            version, &size))
-    return true;
-  return false;
+  return ReadValueFromRegistry(root_key, kChromeRegClientsKey,
+                               kChromeRegVersion, version, &size);
 }
 
-bool IsWinXPSp2OrLater(bool* is_vista_or_later) {
-  OSVERSIONINFOEX osviex = { sizeof(OSVERSIONINFOEX) };
-  int r = ::GetVersionEx((LPOSVERSIONINFO)&osviex);
-  // If this failed we're on Win9X or a pre NT4SP6 OS.
-  if (!r)
-    return false;
-
-  if (osviex.dwMajorVersion < 5)
-    return false;
-
-  if (osviex.dwMajorVersion > 5) {
-    *is_vista_or_later = true;
-    return true;    // way beyond Windows XP;
-  }
-
-  if (osviex.dwMinorVersion >= 1 && osviex.wServicePackMajor >= 2)
-    return true;    // Windows XP SP2 or better.
-
-  return false;     // Windows 2000, WinXP no Service Pack.
+enum WindowsVersion {
+  VERSION_BELOW_XP_SP2,
+  VERSION_XP_SP2_UP_TO_VISTA, // "but not including"
+  VERSION_VISTA_OR_HIGHER,
+};
+WindowsVersion GetWindowsVersion() {
+  OSVERSIONINFOEX version_info = { sizeof version_info };
+  GetVersionEx(reinterpret_cast<OSVERSIONINFO*>(&version_info));
+  if (version_info.dwMajorVersion > 5)
+    return VERSION_VISTA_OR_HIGHER;
+  return (version_info.dwMajorVersion < 5) ||

[brettw, 2011/04/06 04:27:47]

I can't follow this at all, and I found the old version much easier. Can you
unroll this a lot?

[Peter Kasting, 2011/04/06 04:52:43]

Um... OK... the old code happened to be wrong, though, which is part of what
made it simpler (it erroneously treated Windows Server 2003 <SP2 as
unsupported).

I will give it a shot.

+      (version_info.dwMinorVersion == 0) ||
+      ((version_info.dwMinorVersion == 1) &&
+          (version_info.wServicePackMajor < 2)) ?
+      VERSION_BELOW_XP_SP2 : VERSION_XP_SP2_UP_TO_VISTA;
 }
 
 // Note this function should not be called on old Windows versions where these
 // Windows API are not available. We always invoke this function after checking
 // that current OS is Vista or later.
 bool VerifyAdminGroup() {
   SID_IDENTIFIER_AUTHORITY NtAuthority = SECURITY_NT_AUTHORITY;
   PSID Group;
   BOOL check = ::AllocateAndInitializeSid(&NtAuthority, 2,
                                           SECURITY_BUILTIN_DOMAIN_RID,
@@ skipping 29 matching lines @@
       RegDeleteKey(key, NULL);
 
     RegCloseKey(key);
   }
 
   return result;
 }
 
 bool IsRunningElevated() {
   // This method should be called only for Vista or later.
-  bool is_vista_or_later = false;
-  IsWinXPSp2OrLater(&is_vista_or_later);
-  if (!is_vista_or_later || !VerifyAdminGroup())
+  if ((GetWindowsVersion() < VERSION_VISTA_OR_HIGHER) ||
+      !VerifyAdminGroup())
     return false;
 
   HANDLE process_token;
   if (!::OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &process_token))
     return false;
 
   TOKEN_ELEVATION_TYPE elevation_type = TokenElevationTypeDefault;
   DWORD size_returned = 0;
   if (!::GetTokenInformation(process_token, TokenElevationType,
                              &elevation_type, sizeof(elevation_type),
@@ skipping 34 matching lines @@
   ::CloseHandle(process_handle);
   return result;
 }
 }  // namespace
 
 #pragma comment(linker, "/EXPORT:GoogleChromeCompatibilityCheck=_GoogleChromeCompatibilityCheck@8,PRIVATE")
 DLLEXPORT BOOL __stdcall GoogleChromeCompatibilityCheck(BOOL set_flag,
                                                         DWORD *reasons) {
   DWORD local_reasons = 0;
 
-  bool is_vista_or_later = false;
+  WindowsVersion windows_version = GetWindowsVersion();
   // System requirements?
-  if (!IsWinXPSp2OrLater(&is_vista_or_later))
+  if (windows_version == VERSION_BELOW_XP_SP2)
     local_reasons |= GCCC_ERROR_OSNOTSUPPORTED;
 
   if (IsChromeInstalled(HKEY_LOCAL_MACHINE))
     local_reasons |= GCCC_ERROR_SYSTEMLEVELALREADYPRESENT;
 
   if (IsChromeInstalled(HKEY_CURRENT_USER))
     local_reasons |= GCCC_ERROR_USERLEVELALREADYPRESENT;
 
   if (!VerifyHKLMAccess(kChromeRegClientsKey)) {
     local_reasons |= GCCC_ERROR_ACCESSDENIED;
-  } else if (is_vista_or_later && !VerifyAdminGroup()) {
+  } else if ((windows_version == VERSION_VISTA_OR_HIGHER) &&
+             !VerifyAdminGroup()) {
     // For Vista or later check for elevation since even for admin user we could
     // be running in non-elevated mode. We require integrity level High.
     local_reasons |= GCCC_ERROR_INTEGRITYLEVEL;
   }
 
   // Then only check whether we can re-offer, if everything else is OK.
   if (local_reasons == 0 && !CanReOfferChrome(set_flag))
     local_reasons |= GCCC_ERROR_ALREADYOFFERED;
 
   // Done. Copy/return results.
@@ skipping 128 matching lines @@
   // This loop iterates through all of the top-level Windows named
   // Chrome_WindowImpl_0, and looks for the first one with any children.
   while (handle && !FindWindowEx(handle, NULL, L"Chrome_WindowImpl_0", NULL)) {
     // Get the next top-level Chrome window.
     handle = FindWindowEx(NULL, handle, L"Chrome_WindowImpl_0", NULL);
   }
 
   return (handle &&
           SetWindowPos(handle, 0, x, y, width, height, SWP_NOZORDER));
 }