Merge pull request #2411 from atom/fix-crash-reporter

Fix crash reporter process not exiting if there is another one running

atom/browser/atom_browser_client.cc

@@ -4,6 +4,10 @@
 
 #include "atom/browser/atom_browser_client.h"
 
+#if defined(OS_WIN)
+#include <shlobj.h>
+#endif
+
 #include "atom/browser/atom_access_token_store.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
@@ -190,10 +194,20 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
   if (process_type != "renderer")
     return;
 
+  // The registered standard schemes.
   if (!g_custom_schemes.empty())
     command_line->AppendSwitchASCII(switches::kRegisterStandardSchemes,
                                     g_custom_schemes);
 
+#if defined(OS_WIN)
+  // Append --app-user-model-id.
+  PWSTR current_app_id;
+  if (SUCCEEDED(GetCurrentProcessExplicitAppUserModelID(&current_app_id))) {
+    command_line->AppendSwitchNative(switches::kAppUserModelId, current_app_id);
+    CoTaskMemFree(current_app_id);
+  }
+#endif
+
   NativeWindow* window;
   WebViewManager::WebViewInfo info;
   ProcessOwner owner = GetProcessOwner(process_id, &window, &info);

atom/browser/native_window.cc

@@ -8,10 +8,6 @@
 #include <utility>
 #include <vector>
 
-#if defined(OS_WIN)
-#include <shlobj.h>
-#endif
-
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
 #include "atom/browser/window_list.h"
@@ -384,15 +380,6 @@ void NativeWindow::AppendExtraCommandLineSwitches(
     command_line->AppendSwitchASCII(switches::kZoomFactor,
                                     base::DoubleToString(zoom_factor_));
 
-#if defined(OS_WIN)
-  // Append --app-user-model-id.
-  PWSTR current_app_id;
-  if (SUCCEEDED(GetCurrentProcessExplicitAppUserModelID(&current_app_id))) {
-    command_line->AppendSwitchNative(switches::kAppUserModelId, current_app_id);
-    CoTaskMemFree(current_app_id);
-  }
-#endif
-
   if (web_preferences_.IsEmpty())
     return;
 

atom/common/crash_reporter/crash_reporter_win.cc

@@ -21,6 +21,7 @@ const MINIDUMP_TYPE kSmallDumpType = static_cast<MINIDUMP_TYPE>(
     MiniDumpWithProcessThreadData |  // Get PEB and TEB.
     MiniDumpWithUnloadedModules);  // Get unloaded modules when available.
 
+const wchar_t kWaitEventFormat[] = L"$1CrashServiceWaitEvent";
 const wchar_t kPipeNameFormat[] = L"\\\\.\\pipe\\$1 Crash Service";
 
 }  // namespace
@@ -47,12 +48,15 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name,
 
   base::string16 pipe_name = ReplaceStringPlaceholders(
       kPipeNameFormat, base::UTF8ToUTF16(product_name), NULL);
+  base::string16 wait_name = ReplaceStringPlaceholders(
+      kWaitEventFormat, base::UTF8ToUTF16(product_name), NULL);
 
   // Wait until the crash service is started.
-  HANDLE waiting_event =
+  HANDLE wait_event = ::CreateEventW(NULL, TRUE, FALSE, wait_name.c_str());
-      ::CreateEventW(NULL, TRUE, FALSE, L"g_atom_shell_crash_service");
+  if (wait_event != NULL) {
-  if (waiting_event != INVALID_HANDLE_VALUE)
+    WaitForSingleObject(wait_event, 1000);
-    WaitForSingleObject(waiting_event, 1000);
+    CloseHandle(wait_event);
+  }
 
   // ExceptionHandler() attaches our handler and ~ExceptionHandler() detaches
   // it, so we must explicitly reset *before* we instantiate our new handler

atom/common/crash_reporter/win/crash_service.cc

@@ -14,6 +14,7 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "base/win/windows_version.h"
 #include "vendor/breakpad/src/client/windows/crash_generation/client_info.h"
@@ -24,6 +25,9 @@ namespace breakpad {
 
 namespace {
 
+const wchar_t kWaitEventFormat[] = L"$1CrashServiceWaitEvent";
+const wchar_t kClassNameFormat[] = L"$1CrashServiceWindow";
+
 const wchar_t kTestPipeName[] = L"\\\\.\\pipe\\ChromeCrashServices";
 
 const wchar_t kGoogleReportURL[] = L"https://clients2.google.com/cr/report";
@@ -111,13 +115,18 @@ LRESULT __stdcall CrashSvcWndProc(HWND hwnd, UINT message,
 // This is the main and only application window.
 HWND g_top_window = NULL;
 
-bool CreateTopWindow(HINSTANCE instance, bool visible) {
+bool CreateTopWindow(HINSTANCE instance,
+                     const base::string16& application_name,
+                     bool visible) {
+  base::string16 class_name = ReplaceStringPlaceholders(
+      kClassNameFormat, application_name, NULL);
+
   WNDCLASSEXW wcx = {0};
   wcx.cbSize = sizeof(wcx);
   wcx.style = CS_HREDRAW | CS_VREDRAW;
   wcx.lpfnWndProc = CrashSvcWndProc;
   wcx.hInstance = instance;
-  wcx.lpszClassName = L"crash_svc_class";
+  wcx.lpszClassName = class_name.c_str();
   ATOM atom = ::RegisterClassExW(&wcx);
   DWORD style = visible ? WS_POPUPWINDOW | WS_VISIBLE : WS_OVERLAPPED;
 
@@ -193,7 +202,8 @@ CrashService::~CrashService() {
   delete sender_;
 }
 
-bool CrashService::Initialize(const base::FilePath& operating_dir,
+bool CrashService::Initialize(const base::string16& application_name,
+                              const base::FilePath& operating_dir,
                               const base::FilePath& dumps_path) {
   using google_breakpad::CrashReportSender;
   using google_breakpad::CrashGenerationServer;
@@ -260,6 +270,7 @@ bool CrashService::Initialize(const base::FilePath& operating_dir,
   }
 
   if (!CreateTopWindow(::GetModuleHandleW(NULL),
+                       application_name,
                        !cmd_line.HasSwitch(kNoWindow))) {
     LOG(ERROR) << "could not create window";
     if (security_attributes.lpSecurityDescriptor)
@@ -298,11 +309,10 @@ bool CrashService::Initialize(const base::FilePath& operating_dir,
 
   // Create or open an event to signal the browser process that the crash
   // service is initialized.
-  HANDLE running_event =
+  base::string16 wait_name = ReplaceStringPlaceholders(
-      ::CreateEventW(NULL, TRUE, TRUE, L"g_atom_shell_crash_service");
+      kWaitEventFormat, application_name, NULL);
-  // If the browser already had the event open, the CreateEvent call did not
+  HANDLE wait_event = ::CreateEventW(NULL, TRUE, FALSE, wait_name.c_str());
-  // signal it. We need to do it manually.
+  ::SetEvent(wait_event);
-  ::SetEvent(running_event);
 
   return true;
 }

atom/common/crash_reporter/win/crash_service.h

@@ -37,7 +37,8 @@ class CrashService {
   // other members in that case. |operating_dir| is where the CrashService
   // should store breakpad's checkpoint file. |dumps_path| is the directory
   // where the crash dumps should be stored.
-  bool Initialize(const base::FilePath& operating_dir,
+  bool Initialize(const base::string16& application_name,
+                  const base::FilePath& operating_dir,
                   const base::FilePath& dumps_path);
 
   // Command line switches:

atom/common/crash_reporter/win/crash_service_main.cc

@@ -77,7 +77,8 @@ int Main(const wchar_t* cmd) {
   cmd_line.AppendSwitchNative("pipe-name", pipe_name);
 
   breakpad::CrashService crash_service;
-  if (!crash_service.Initialize(operating_dir, operating_dir))
+  if (!crash_service.Initialize(application_name, operating_dir,
+                                operating_dir))
     return 2;
 
   VLOG(1) << "Ready to process crash requests";