Make the windows_version.h functions threadsafe by using a singleton. Add accessors to the singleton for more values that various code wants, then convert almost everyone using OSVERSIONINFO or SYSTEM_INFO structs to calling these accessors. Declare an AtExitManager in the out-of-process test runner since it didn't have one and that breaks singleton-using code in the test executable (as opposed to in chrome.dll).

A few other minor cleanups along the way (binding of "*", shorter code, etc.).  Because I ran into problems with it while modifying gcapi.cc, I cleaned up our usage of strsafe.h a bit, so that files that don't need it don't include it and files that do use STRSAFE_NO_DEPRECATE instead of a modified #include order.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/6713107

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80819 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 91 insertions, 96 deletions

diff --git a/chrome/browser/bug_report_util.cc b/chrome/browser/bug_report_util.cc
index 8ba0e2c..ebb8e24 100644
--- a/chrome/browser/bug_report_util.cc
+++ b/chrome/browser/bug_report_util.cc
@@ -13,6 +13,7 @@
 #include "base/memory/singleton.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "base/win/windows_version.h"
 #include "chrome/browser/browser_process_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
@@ -135,19 +136,19 @@ void BugReportUtil::PostCleanup::OnURLFetchComplete(
 
     // Process the error for debug output
     if (response_code == kHttpPostFailNoConnection) {
-        error_stream << "No connection to server.";
+      error_stream << "No connection to server.";
     } else if ((response_code > kHttpPostFailClientError) &&
-      (response_code < kHttpPostFailServerError)) {
-        error_stream << "Client error: HTTP response code " << response_code;
+        (response_code < kHttpPostFailServerError)) {
+      error_stream << "Client error: HTTP response code " << response_code;
     } else if (response_code > kHttpPostFailServerError) {
-        error_stream << "Server error: HTTP response code " << response_code;
+      error_stream << "Server error: HTTP response code " << response_code;
     } else {
-        error_stream << "Unknown error: HTTP response code " << response_code;
+      error_stream << "Unknown error: HTTP response code " << response_code;
     }
   }
 
-  LOG(WARNING) << "FEEDBACK: Submission to feedback server (" << url <<
-      ") status: " << error_stream.str() << std::endl;
+  LOG(WARNING) << "FEEDBACK: Submission to feedback server (" << url
+               << ") status: " << error_stream.str();
 
   // Delete the URLFetcher.
   delete source;
@@ -156,21 +157,15 @@ void BugReportUtil::PostCleanup::OnURLFetchComplete(
 }
 
 // static
-void BugReportUtil::SetOSVersion(std::string *os_version) {
+void BugReportUtil::SetOSVersion(std::string* os_version) {
 #if defined(OS_WIN)
-  OSVERSIONINFO osvi;
-  ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
-  osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
-
-  if (GetVersionEx(&osvi)) {
-    *os_version = StringPrintf("%d.%d.%d %S",
-      osvi.dwMajorVersion,
-      osvi.dwMinorVersion,
-      osvi.dwBuildNumber,
-      osvi.szCSDVersion);
-  } else {
-    *os_version = "unknown";
-  }
+  base::win::OSInfo* os_info = base::win::OSInfo::GetInstance();
+  base::win::OSInfo::VersionNumber version_number = os_info->version_number();
+  *os_version = StringPrintf("%d.%d.%d", version_number.major,
+                             version_number.minor, version_number.build);
+  int service_pack = os_info->service_pack().major;
+  if (service_pack > 0)
+    os_version->append(StringPrintf("Service Pack %d", service_pack));
 #elif defined(OS_MACOSX)
   int32 major;
   int32 minor;
diff --git a/chrome/browser/diagnostics/recon_diagnostics.cc b/chrome/browser/diagnostics/recon_diagnostics.cc
index 86666f6..4ff348e 100644
--- a/chrome/browser/diagnostics/recon_diagnostics.cc
+++ b/chrome/browser/diagnostics/recon_diagnostics.cc
@@ -47,27 +47,20 @@ class OperatingSystemTest : public DiagnosticTest {
   virtual int GetId() { return 0; }
 
   virtual bool ExecuteImpl(DiagnosticsModel::Observer* observer) {
-    int version = 0;
-    int major = 0;
-    int minor = 0;
 #if defined(OS_WIN)
-    version = base::win::GetVersion();
-    if (version < base::win::VERSION_XP) {
-      RecordFailure(ASCIIToUTF16("Windows 2000 or earlier"));
-      return false;
-    }
-    base::win::GetServicePackLevel(&major, &minor);
-    if ((version == base::win::VERSION_XP) && (major < 2)) {
-      RecordFailure(ASCIIToUTF16("XP Service Pack 1 or earlier"));
+    base::win::Version version = base::win::GetVersion();
+    if ((version < base::win::VERSION_XP) ||
+        ((version == base::win::VERSION_XP) &&
+         (base::win::OSInfo::GetInstance()->service_pack().major < 2))) {
+      RecordFailure(ASCIIToUTF16("Must have Windows XP SP2 or later"));
       return false;
     }
 #else
     // TODO(port): define the OS criteria for Linux and Mac.
 #endif  // defined(OS_WIN)
-    RecordSuccess(ASCIIToUTF16(StringPrintf("%s %s (%d [%d:%d])",
+    RecordSuccess(ASCIIToUTF16(StringPrintf("%s %s",
         base::SysInfo::OperatingSystemName().c_str(),
-        base::SysInfo::OperatingSystemVersion().c_str(),
-        version, major, minor)));
+        base::SysInfo::OperatingSystemVersion().c_str())));
     return true;
   }
 
diff --git a/chrome/browser/importer/importer_unittest.cc b/chrome/browser/importer/importer_unittest.cc
index b0ac166..9fe3e12 100644
--- a/chrome/browser/importer/importer_unittest.cc
+++ b/chrome/browser/importer/importer_unittest.cc
@@ -205,7 +205,7 @@ class TestObserver : public ProfileWriter,
     EXPECT_EQ(arraysize(kIEBookmarks), bookmark_count_);
     EXPECT_EQ(1, history_count_);
 #if 0  // This part of the test is disabled. See bug #2466
-    if (base::win::GetVersion() < base::win::VERSION_VISTA)
+    if (base::win::GetVersion() >= base::win::VERSION_VISTA)
       EXPECT_EQ(0, password_count_);
     else
       EXPECT_EQ(1, password_count_);
diff --git a/chrome/browser/memory_details_win.cc b/chrome/browser/memory_details_win.cc
index c954a8e..576c74c 100644
--- a/chrome/browser/memory_details_win.cc
+++ b/chrome/browser/memory_details_win.cc
@@ -72,8 +72,8 @@ void MemoryDetails::CollectProcessData(
   for (unsigned int index = 0; index < process_data_.size(); index++)
     process_data_[index].processes.clear();
 
-  base::win::WindowsArchitecture windows_architecture =
-      base::win::GetWindowsArchitecture();
+  base::win::OSInfo::WindowsArchitecture windows_architecture =
+      base::win::OSInfo::GetInstance()->architecture();
 
   base::win::ScopedHandle snapshot(
       ::CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0));
@@ -93,10 +93,10 @@ void MemoryDetails::CollectProcessData(
     if (!process_handle.Get())
       continue;
     bool is_64bit_process =
-        ((windows_architecture == base::win::X64_ARCHITECTURE) ||
-         (windows_architecture == base::win::IA64_ARCHITECTURE)) &&
-        (base::win::GetWOW64StatusForProcess(process_handle) ==
-            base::win::WOW64_DISABLED);
+        ((windows_architecture == base::win::OSInfo::X64_ARCHITECTURE) ||
+         (windows_architecture == base::win::OSInfo::IA64_ARCHITECTURE)) &&
+        (base::win::OSInfo::GetWOW64StatusForProcess(process_handle) ==
+            base::win::OSInfo::WOW64_DISABLED);
     for (unsigned int index2 = 0; index2 < process_data_.size(); index2++) {
       if (_wcsicmp(process_data_[index2].process_name.c_str(),
                    process_entry.szExeFile) != 0)
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 528e380..d23b51c 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -56,7 +56,8 @@ NaClProcessHost::NaClProcessHost(const std::wstring& url)
       running_on_wow64_(false) {
   set_name(url);
 #if defined(OS_WIN)
-  running_on_wow64_ = (base::win::GetWOW64Status() == base::win::WOW64_ENABLED);
+  running_on_wow64_ = (base::win::OSInfo::GetInstance()->wow64_status() ==
+      base::win::OSInfo::WOW64_ENABLED);
 #endif
 }
 
diff --git a/chrome/browser/ui/views/about_chrome_view.cc b/chrome/browser/ui/views/about_chrome_view.cc
index 70505fb..93faa2a 100644
--- a/chrome/browser/ui/views/about_chrome_view.cc
+++ b/chrome/browser/ui/views/about_chrome_view.cc
@@ -532,10 +532,8 @@ void AboutChromeView::ViewHierarchyChanged(bool is_add,
       // on-demand updates. Silent updates (in the background) should still
       // work as before - enabling UAC or installing the latest service pack
       // for Vista is another option.
-      int service_pack_major = 0, service_pack_minor = 0;
-      base::win::GetServicePackLevel(&service_pack_major, &service_pack_minor);
       if (!(base::win::GetVersion() == base::win::VERSION_VISTA &&
-            (service_pack_major == 0) &&
+            (base::win::OSInfo::GetInstance()->service_pack().major == 0) &&
             !base::win::UserAccountControlIsEnabled())) {
         UpdateStatus(UPGRADE_CHECK_STARTED, GOOGLE_UPDATE_NO_ERROR);
         // CheckForUpdate(false, ...) means don't upgrade yet.
diff --git a/chrome/installer/gcapi/gcapi.cc b/chrome/installer/gcapi/gcapi.cc
index 5ba9251..dc38bbe 100644
--- a/chrome/installer/gcapi/gcapi.cc
+++ b/chrome/installer/gcapi/gcapi.cc
@@ -1,4 +1,4 @@
-// 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.
 
@@ -8,10 +8,12 @@
 #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 {
@@ -156,31 +158,31 @@ bool ReadValueFromRegistry(HKEY root_key, const wchar_t *sub_key,
 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));
+
+  // Windows Vista is version 6.0.
+  if (version_info.dwMajorVersion >= 6)
+    return VERSION_VISTA_OR_HIGHER;
+
+  // Windows XP is version 5.1.  (5.2 is Windows Server 2003/XP Pro x64.)
+  if ((version_info.dwMajorVersion < 5) || (version_info.dwMinorVersion < 1))
+    return VERSION_BELOW_XP_SP2;
+
+  // For XP itself, we only support SP2 and above.
+  return ((version_info.dwMinorVersion > 1) ||
+          (version_info.wServicePackMajor >= 2)) ?
+      VERSION_XP_SP2_UP_TO_VISTA : VERSION_BELOW_XP_SP2;
 }
 
 // Note this function should not be called on old Windows versions where these
@@ -230,9 +232,8 @@ bool VerifyHKLMAccess(const wchar_t* sub_key) {
 
 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;
@@ -287,9 +288,9 @@ 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))
@@ -300,7 +301,8 @@ DLLEXPORT BOOL __stdcall GoogleChromeCompatibilityCheck(BOOL set_flag,
 
   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;
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index 8b11835..7bdc9e5 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -512,7 +512,8 @@ void AddInstallWorkItems(const InstallationState& original_state,
   // Extra executable for 64 bit systems.
   // NOTE: We check for "not disabled" so that if the API call fails, we play it
   // safe and copy the executable anyway.
-  if (base::win::GetWOW64Status() != base::win::WOW64_DISABLED) {
+  if (base::win::OSInfo::GetInstance()->wow64_status() !=
+      base::win::OSInfo::WOW64_DISABLED) {
     install_list->AddMoveTreeWorkItem(
         src_path.Append(installer::kWowHelperExe).value(),
         target_path.Append(installer::kWowHelperExe).value(),
diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
index c2ee5eb..89dd58a 100644
--- a/chrome/installer/util/google_chrome_distribution.cc
+++ b/chrome/installer/util/google_chrome_distribution.cc
@@ -326,14 +326,10 @@ void GoogleChromeDistribution::DoPostUninstallOperations(
   // need to escape the string before using it in a URL.
   const std::wstring kVersionParam = L"crversion";
   const std::wstring kOSParam = L"os";
-  std::wstring os_version = L"na";
-  OSVERSIONINFO version_info;
-  version_info.dwOSVersionInfoSize = sizeof(version_info);
-  if (GetVersionEx(&version_info)) {
-    os_version = StringPrintf(L"%d.%d.%d", version_info.dwMajorVersion,
-                                           version_info.dwMinorVersion,
-                                           version_info.dwBuildNumber);
-  }
+  base::win::OSInfo::VersionNumber version_number =
+      base::win::OSInfo::GetInstance()->version_number();
+  std::wstring os_version = StringPrintf(L"%d.%d.%d", version_number.major,
+      version_number.minor, version_number.build);
 
   FilePath iexplore;
   if (!PathService::Get(base::DIR_PROGRAM_FILES, &iexplore))
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index 999d84a..d1110de 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -19,6 +19,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
+#include "base/sys_info.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "base/win/registry.h"
@@ -136,15 +137,13 @@ Version* InstallUtil::GetChromeVersion(BrowserDistribution* dist,
 }
 
 bool InstallUtil::IsOSSupported() {
-  int major, minor;
-  base::win::Version version = base::win::GetVersion();
-  base::win::GetServicePackLevel(&major, &minor);
-
   // We do not support Win2K or older, or XP without service pack 2.
-  VLOG(1) << "Windows Version: " << version
-          << ", Service Pack: " << major << "." << minor;
+  VLOG(1) << base::SysInfo::OperatingSystemName() << ' '
+          << base::SysInfo::OperatingSystemVersion();
+  base::win::Version version = base::win::GetVersion();
   return (version > base::win::VERSION_XP) ||
-      (version == base::win::VERSION_XP && major >= 2);
+      ((version == base::win::VERSION_XP) &&
+       (base::win::OSInfo::GetInstance()->service_pack().major >= 2));
 }
 
 void InstallUtil::WriteInstallerResult(bool system_install,
diff --git a/chrome/renderer/renderer_glue.cc b/chrome/renderer/renderer_glue.cc
index 3e40ea3..84a7ff8 100644
--- a/chrome/renderer/renderer_glue.cc
+++ b/chrome/renderer/renderer_glue.cc
@@ -43,9 +43,7 @@
 #include "native_client/src/trusted/plugin/nacl_entry_points.h"
 #endif
 
-#if defined(OS_WIN)
-#include <strsafe.h>  // note: per msdn docs, this must *follow* other includes
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
 #include "content/renderer/renderer_sandbox_support_linux.h"
 #endif
 
diff --git a/chrome/test/out_of_proc_test_runner.cc b/chrome/test/out_of_proc_test_runner.cc
index 6e4bc3b..5d23954 100644
--- a/chrome/test/out_of_proc_test_runner.cc
+++ b/chrome/test/out_of_proc_test_runner.cc
@@ -579,6 +579,18 @@ int main(int argc, char** argv) {
     return ChromeTestSuite(argc, argv).Run();
   }
 
+  // The exit manager is in charge of calling the dtors of singleton objects.
+  // On Windows, the call to ChromeMain() below will construct one for the
+  // chrome.dll module, but that global is not shared with this module, so if
+  // chrome.dll calls back out to this module and the called code uses a
+  // singleton, we'll need this.  On other platforms, ChromeMain() isn't called
+  // at all below, so this is the lone exit manager for any code after this
+  // point.
+  // NOTE: We can't init this atop main() because ChromeTestSuite, as a subclass
+  // of TestSuite, creates one.  So we wait until after the Run() call above to
+  // create the manager for the code path that _doesn't_ use ChromeTestSuite.
+  base::AtExitManager exit_manager;
+
 #if defined(OS_WIN)
   if (command_line->HasSwitch(switches::kProcessType)) {
     // This is a child process, call ChromeMain.