Cleanup.

base/debug/gdi_debug_util_win.cc

Index: base/debug/gdi_debug_util_win.cc
diff --git a/base/debug/gdi_debug_util_win.cc b/base/debug/gdi_debug_util_win.cc
index 4bac759d8c78568e86e374deb6247dcd5fbc4508..02095ecddbbfac20a586075d7e87900e858ac152 100644
--- a/base/debug/gdi_debug_util_win.cc
+++ b/base/debug/gdi_debug_util_win.cc
@@ -15,9 +15,8 @@
 namespace {
 
 void CollectChildGDIUsageAndDie(DWORD parent_pid) {
-  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) ;
-  if(snapshot == INVALID_HANDLE_VALUE)
-    CHECK(false);
+  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+  CHECK_NE(INVALID_HANDLE_VALUE, snapshot);
 
   int child_count = 0;
   base::debug::Alias(&child_count);
@@ -29,9 +28,8 @@ void CollectChildGDIUsageAndDie(DWORD parent_pid) {
   base::debug::Alias(&sum_user_count);
 
   PROCESSENTRY32 proc_entry = {0};
-  proc_entry.dwSize = sizeof(PROCESSENTRY32) ;
-  if(!Process32First(snapshot, &proc_entry))
-    CHECK(false);
+  proc_entry.dwSize = sizeof(PROCESSENTRY32);
+  CHECK(Process32First(snapshot, &proc_entry));
 
   do {
     if (parent_pid != proc_entry.th32ParentProcessID)
@@ -54,9 +52,9 @@ void CollectChildGDIUsageAndDie(DWORD parent_pid) {
     if (peak_gdi_count < num_gdi_handles)
       peak_gdi_count = num_gdi_handles;
 
-  } while(Process32Next(snapshot, &proc_entry));
+  } while (Process32Next(snapshot, &proc_entry));
 

[cpu_(ooo_6.6-7.5), 2014/09/26 01:17:50]

there is an inconsistency in this file wrt windows api calls, the :: versus
nothing. I figure you might want to correct that while we are here.

[Peter Kasting, 2014/09/29 19:29:45]

Done.

-  ::CloseHandle(snapshot) ;
+  ::CloseHandle(snapshot);
   CHECK(false);

[cpu_(ooo_6.6-7.5), 2014/09/26 01:17:50]

I have no idea why we are closing the snapshot handle if we are going to go up
in flames. Did I coded that?

Anyhow remove the close.

[Peter Kasting, 2014/09/26 01:21:33]

It wasn't me :)
Is it really wrong?  If it's correct and just unnecessary I'd kinda rather leave
it in...

[cpu_(ooo_6.6-7.5), 2014/09/29 19:01:48]

Its best to not do cleanup if we are intent in generating a dump. We want to
preserve more state, not less.

Not hung on this, so fee free to skip. Just spelling the rationale.

[Peter Kasting, 2014/09/29 19:29:45]

OK.  I think I'll leave the code as-is rather than make a functional change
here.

 }
 
@@ -92,21 +90,16 @@ void GDIBitmapAllocFailure(BITMAPINFOHEADER* header, HANDLE shared_section) {
   base::debug::Alias(&num_user_handles);
 
   const DWORD kLotsOfHandles = 9990;
-  if (num_gdi_handles > kLotsOfHandles)
-    CHECK(false);
+  CHECK_LE(num_gdi_handles, kLotsOfHandles);
 
   PROCESS_MEMORY_COUNTERS_EX pmc;
   pmc.cb = sizeof(pmc);
-  if (!GetProcessMemoryInfo(GetCurrentProcess(),
-                            reinterpret_cast<PROCESS_MEMORY_COUNTERS*>(&pmc),
-                            sizeof(pmc))) {
-    CHECK(false);
-  }
+  CHECK(GetProcessMemoryInfo(GetCurrentProcess(),
+                             reinterpret_cast<PROCESS_MEMORY_COUNTERS*>(&pmc),
+                             sizeof(pmc)));
   const size_t kLotsOfMemory = 1500 * 1024 * 1024; // 1.5GB
-  if (pmc.PagefileUsage > kLotsOfMemory)
-    CHECK(false);
-  if (pmc.PrivateUsage > kLotsOfMemory)
-    CHECK(false);
+  CHECK_LE(pmc.PagefileUsage, kLotsOfMemory);
+  CHECK_LE(pmc.PrivateUsage, kLotsOfMemory);
 
   void* small_data = NULL;
   base::debug::Alias(&small_data);
@@ -120,6 +113,8 @@ void GDIBitmapAllocFailure(BITMAPINFOHEADER* header, HANDLE shared_section) {
     HBITMAP small_bitmap = CreateDIBSection(
         NULL, reinterpret_cast<BITMAPINFO*>(&header),
         0, &small_data, shared_section, 0);
+    CHECK(small_bitmap != NULL);
+    DeleteObject(small_bitmap);
   }
   // Maybe the child processes are the ones leaking GDI or USER resouces.
   CollectChildGDIUsageAndDie(::GetCurrentProcessId());