Render/pepper hang debugging: generate dumps for the browser and renderer processes if users kill a…

chrome/browser/hang_monitor/hang_crash_dump_win.cc

Index: chrome/browser/hang_monitor/hang_crash_dump_win.cc
diff --git a/chrome/browser/hang_monitor/hang_crash_dump_win.cc b/chrome/browser/hang_monitor/hang_crash_dump_win.cc
index c327519c3e8e24d231a7a1fec5e3bd0ac40853a8..c271f1cb0799b84f8b549742ed584742b00ce67c 100644
--- a/chrome/browser/hang_monitor/hang_crash_dump_win.cc
+++ b/chrome/browser/hang_monitor/hang_crash_dump_win.cc
@@ -7,7 +7,6 @@
 #include "base/logging.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/public/common/result_codes.h"
-#include "ppapi/shared_impl/ppapi_message_tracker.h"
 
 namespace {
 
@@ -17,20 +16,6 @@ static const int kTerminateTimeoutMS = 2000;
 // How long do we wait for the crash to be generated (in ms).
 static const int kGenerateDumpTimeoutMS = 10000;
 
-DWORD WINAPI DumpIfHandlingPepper(void*) {
-  typedef void (__cdecl *DumpFunction)();
-  if (ppapi::PpapiMessageTracker::GetInstance()->IsHandlingMessage()) {
-    DumpFunction request_dump = reinterpret_cast<DumpFunction>(GetProcAddress(
-        GetModuleHandle(chrome::kBrowserProcessExecutableName),
-        "DumpProcessWithoutCrash"));
-    DCHECK(request_dump) << "Failed loading DumpProcessWithoutCrash: error " <<
-        GetLastError();
-    if (request_dump)
-      request_dump();
-  }
-  return 0;
-}
-
 }  // namespace
 
 void CrashDumpAndTerminateHungChildProcess(HANDLE hprocess) {
@@ -60,24 +45,31 @@ void CrashDumpAndTerminateHungChildProcess(HANDLE hprocess) {
   WaitForSingleObject(hprocess, kTerminateTimeoutMS);
 }
 
-void CrashDumpIfProcessHandlingPepper(HANDLE hprocess) {
-  // Unlike CrashDumpAndTerminateHungChildProcess() which creates a remote
-  // thread using function pointer relative to chrome.exe, here we create a
-  // remote thread using function pointer relative to chrome.dll. The reason is
-  // that there are separate PpapiMessageTracker singletons for chrome.dll and
-  // chrome.exe (in non-component build). We cannot access the information
-  // collected by PpapiMessageTracker of chrome.dll in chrome.exe.
-  //
-  // This is less safe, because chrome.dll may be loaded at different addresses
-  // in different processes. We could cause crash in that case. However, it
-  // should be rare and we are only doing this temporarily for debugging on the
-  // Canary channel.
-  HANDLE remote_thread = CreateRemoteThread(hprocess, NULL, 0,
-                                            DumpIfHandlingPepper, 0, 0, NULL);
-  DCHECK(remote_thread) << "Failed creating remote thread: error " <<
-      GetLastError();
-  if (remote_thread) {
-    WaitForSingleObject(remote_thread, kGenerateDumpTimeoutMS);
-    CloseHandle(remote_thread);
+void CrashDumpForHangDebugging(HANDLE hprocess) {
+  if (hprocess == GetCurrentProcess()) {
+    typedef void (__cdecl *DumpFunction)();
+    DumpFunction request_dump = reinterpret_cast<DumpFunction>(GetProcAddress(
+        GetModuleHandle(chrome::kBrowserProcessExecutableName),
+        "DumpProcessWithoutCrash"));
+    DCHECK(request_dump) << "Failed loading DumpProcessWithoutCrash: error " <<
+        GetLastError();
+    if (request_dump)
+      request_dump();
+  } else {
+    typedef HANDLE (__cdecl *DumpFunction)(HANDLE);
+    DumpFunction request_dump = reinterpret_cast<DumpFunction>(GetProcAddress(
+        GetModuleHandle(chrome::kBrowserProcessExecutableName),
+        "InjectDumpForHangDebugging"));
+    DCHECK(request_dump) << "Failed loading DumpForHangDebugging: error " <<

[eroman, 2012/09/25 20:10:19]

DumpForHangDebugging --> InjectDumpForHangDebugging

[yzshen1, 2012/09/25 20:24:09]

Done.

+        GetLastError();
+    if (request_dump) {
+      HANDLE remote_thread = request_dump(hprocess);
+      DCHECK(remote_thread) << "Failed creating remote thread: error " <<
+          GetLastError();
+      if (remote_thread) {
+        WaitForSingleObject(remote_thread, kGenerateDumpTimeoutMS);

[eroman, 2012/09/25 20:10:19]

[optional] Note that for better performance, you could make this function take a
list of hprocess, and start all the remotethreads before joining.

Probably best to leave it as-is for simplicity, but wanted to mention this
possibility if the timeouts become an issue.

[yzshen1, 2012/09/25 20:24:09]

Thanks for the suggestion!
If this code proves to be helpful and we want it to become some more general
debugging facility, I will follow the suggestion and improve it.

+        CloseHandle(remote_thread);
+      }
+    }
   }
 }