Fix CRITICAL_SECTION test

client/crashpad_client_win.cc

Index: client/crashpad_client_win.cc
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
index 7a1e547a4c8bfdbda64af09d36aee477e0b777ac..01c9151041028c7e809dc5f84d109c9aa689057a 100644
--- a/client/crashpad_client_win.cc
+++ b/client/crashpad_client_win.cc
@@ -23,6 +23,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "util/file/file_io.h"
+#include "util/win/critical_section_with_debug_info.h"
 #include "util/win/registration_protocol_win.h"
 #include "util/win/scoped_handle.h"
 
@@ -101,18 +102,6 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) {
   return EXCEPTION_CONTINUE_SEARCH;
 }
 
-BOOL CrashpadInitializeCriticalSectionEx(
-    CRITICAL_SECTION* critical_section,
-    DWORD spin_count,
-    DWORD flags) {
-  static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex =
-      reinterpret_cast<decltype(InitializeCriticalSectionEx)*>(GetProcAddress(
-          LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx"));
-  if (!initialize_critical_section_ex)
-    return false;
-  return initialize_critical_section_ex(critical_section, spin_count, flags);
-}
-
 }  // namespace
 
 namespace crashpad {
@@ -156,14 +145,12 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) {
   // debugger would instead inspect ntdll!RtlCriticalSectionList to get the head
   // of the list. But that is not an exported symbol, so on an arbitrary client
   // machine, we don't have a way of getting that pointer.
-  if (CrashpadInitializeCriticalSectionEx(
-          &g_critical_section_with_debug_info,
-          0,
-          RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO)) {
+  if (InitializeCriticalSectionWithDebugInfoIfPossible(
+          &g_critical_section_with_debug_info)) {
     message.registration.critical_section_address =
         reinterpret_cast<WinVMAddress>(&g_critical_section_with_debug_info);
   } else {
-    PLOG(ERROR) << "InitializeCriticalSectionEx";
+    PLOG(ERROR) << "InitializeCriticalSectionWithDebugInfoIfPossible";

[Mark Mentovai, 2015/10/16 19:42:47]

I think you should move this into the function (and document that it logs). The
reason is that InitializeCriticalSectionWithDebugInfoIfPossible() can return
false without setting GetLastError(). (It shouldn’t happen, but it’s logically
possible, and that case should have its own non-P LOG().)

[scottmg, 2015/10/16 20:04:11]

I don't think it could have ever returned false without setting GetLastError()
(either from GetProcAddress() or InitializeCriticalSectionEx()) but it makes
more sense inside anyway.

   }
 
   ServerToClientMessage response = {0};