From 8431e8f39edd5d7bff793f446035207fee6dacd1 Mon Sep 17 00:00:00 2001 From: erikwright Date: Thu, 23 Apr 2015 10:20:03 -0700 Subject: [PATCH] Send a crash report when a hung process is detected. BUG= Review URL: https://codereview.chromium.org/1060203004 Cr-Commit-Position: refs/heads/master@{#326559} --- chrome/app/client_util.cc | 5 +- chrome/chrome_watcher/BUILD.gn | 1 + chrome/chrome_watcher/DEPS | 1 + chrome/chrome_watcher/chrome_watcher.gypi | 1 + chrome/chrome_watcher/chrome_watcher_main.cc | 80 ++++++++++++++++++++++--- chrome/chrome_watcher/chrome_watcher_main_api.h | 5 +- 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/chrome/app/client_util.cc b/chrome/app/client_util.cc index 6cbca1a265a0..87b816370626 100644 --- a/chrome/app/client_util.cc +++ b/chrome/app/client_util.cc @@ -206,6 +206,9 @@ int MainDllLoader::Launch(HINSTANCE instance) { if (!PathService::Get(chrome::DIR_WATCHER_DATA, &watcher_data_directory)) return chrome::RESULT_CODE_MISSING_DATA; + base::string16 channel_name = GoogleUpdateSettings::GetChromeChannel( + !InstallUtil::IsPerUserInstall(cmd_line.GetProgram())); + // Intentionally leaked. HMODULE watcher_dll = Load(&version, &file); if (!watcher_dll) @@ -217,7 +220,7 @@ int MainDllLoader::Launch(HINSTANCE instance) { return watcher_main(chrome::kBrowserExitCodesRegistryPath, parent_process.Take(), on_initialized_event.Take(), watcher_data_directory.value().c_str(), - message_window_name.c_str()); + message_window_name.c_str(), channel_name.c_str()); } // Initialize the sandbox services. diff --git a/chrome/chrome_watcher/BUILD.gn b/chrome/chrome_watcher/BUILD.gn index db5f5e8cd37b..5a77ffd4500e 100644 --- a/chrome/chrome_watcher/BUILD.gn +++ b/chrome/chrome_watcher/BUILD.gn @@ -32,6 +32,7 @@ shared_library("chrome_watcher") { deps = [ ":chrome_watcher_resources", ":client", + ":installer_util", "//base", "//components/browser_watcher", ] diff --git a/chrome/chrome_watcher/DEPS b/chrome/chrome_watcher/DEPS index 9b1e8bde18fe..d7f54021a304 100644 --- a/chrome/chrome_watcher/DEPS +++ b/chrome/chrome_watcher/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+base", + "+chrome/installer/util", "+components/browser_watcher", "+syzygy/kasko/api", ] diff --git a/chrome/chrome_watcher/chrome_watcher.gypi b/chrome/chrome_watcher/chrome_watcher.gypi index 8a4eaf329ab5..9836501dee4d 100644 --- a/chrome/chrome_watcher/chrome_watcher.gypi +++ b/chrome/chrome_watcher/chrome_watcher.gypi @@ -61,6 +61,7 @@ 'dependencies': [ 'chrome_watcher_client', 'chrome_watcher_resources', + 'installer_util', '../base/base.gyp:base', '../components/components.gyp:browser_watcher', ], diff --git a/chrome/chrome_watcher/chrome_watcher_main.cc b/chrome/chrome_watcher/chrome_watcher_main.cc index 54da49cbd5eb..c6db791a41d1 100644 --- a/chrome/chrome_watcher/chrome_watcher_main.cc +++ b/chrome/chrome_watcher/chrome_watcher_main.cc @@ -8,6 +8,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/command_line.h" +#include "base/file_version_info.h" #include "base/files/file_path.h" #include "base/logging_win.h" #include "base/macros.h" @@ -17,12 +18,17 @@ #include "base/process/process.h" #include "base/run_loop.h" #include "base/sequenced_task_runner.h" +#include "base/strings/string16.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/string_piece.h" +#include "base/strings/utf_string_conversions.h" #include "base/synchronization/waitable_event.h" #include "base/template_util.h" #include "base/threading/thread.h" #include "base/time/time.h" #include "base/win/scoped_handle.h" #include "chrome/chrome_watcher/chrome_watcher_main_api.h" +#include "chrome/installer/util/util_constants.h" #include "components/browser_watcher/endsession_watcher_window_win.h" #include "components/browser_watcher/exit_code_watcher_win.h" #include "components/browser_watcher/exit_funnel_win.h" @@ -201,6 +207,7 @@ void BrowserMonitor::BrowserExited() { void OnWindowEvent( const base::string16& registry_path, base::Process process, + const base::Callback& on_hung_callback, browser_watcher::WindowHangMonitor::WindowEvent window_event) { browser_watcher::ExitFunnel exit_funnel; if (exit_funnel.Init(registry_path.c_str(), process.Handle())) { @@ -210,6 +217,8 @@ void OnWindowEvent( break; case browser_watcher::WindowHangMonitor::WINDOW_HUNG: exit_funnel.RecordEvent(L"MessageWindowHung"); + if (!on_hung_callback.is_null()) + on_hung_callback.Run(process); break; case browser_watcher::WindowHangMonitor::WINDOW_VANISHED: exit_funnel.RecordEvent(L"MessageWindowVanished"); @@ -221,6 +230,53 @@ void OnWindowEvent( } } +#ifdef KASKO +void DumpHungBrowserProcess(const base::string16& channel, + const base::Process& process) { + // TODO(erikwright): Rather than recreating these crash keys here, it would be + // ideal to read them directly from the browser process. + + // This is looking up the version of chrome_watcher.dll, which is equivalent + // for our purposes to chrome.dll. + scoped_ptr version_info( + FileVersionInfo::CreateFileVersionInfoForModule( + reinterpret_cast(&__ImageBase))); + using CrashKeyStrings = std::pair; + std::vector crash_key_strings; + if (version_info.get()) { + crash_key_strings.push_back( + CrashKeyStrings(L"prod", version_info->product_short_name())); + base::string16 version = version_info->product_version(); + if (!version_info->is_official_build()) + version.append(base::ASCIIToUTF16("-devel")); + crash_key_strings.push_back(CrashKeyStrings(L"ver", version)); + } else { + // No version info found. Make up the values. + crash_key_strings.push_back(CrashKeyStrings(L"prod", L"Chrome")); + crash_key_strings.push_back(CrashKeyStrings(L"ver", L"0.0.0.0-devel")); + } + crash_key_strings.push_back(CrashKeyStrings(L"channel", channel)); + crash_key_strings.push_back(CrashKeyStrings(L"plat", L"Win32")); + crash_key_strings.push_back(CrashKeyStrings(L"ptype", L"browser")); + crash_key_strings.push_back( + CrashKeyStrings(L"pid", base::IntToString16(process.Pid()))); + crash_key_strings.push_back(CrashKeyStrings(L"hung-process", L"1")); + + std::vector key_buffers; + std::vector value_buffers; + for (auto& strings : crash_key_strings) { + key_buffers.push_back(strings.first.c_str()); + value_buffers.push_back(strings.second.c_str()); + } + key_buffers.push_back(nullptr); + value_buffers.push_back(nullptr); + // TODO(erikwright): Make the dump-type channel-dependent. + kasko::api::SendReportForProcess(process.Handle(), + kasko::api::LARGER_DUMP_TYPE, + key_buffers.data(), value_buffers.data()); +} +#endif // KASKO + } // namespace // The main entry point to the watcher, declared as extern "C" to avoid name @@ -229,7 +285,8 @@ extern "C" int WatcherMain(const base::char16* registry_path, HANDLE process_handle, HANDLE on_initialized_event_handle, const base::char16* browser_data_directory, - const base::char16* message_window_name) { + const base::char16* message_window_name, + const base::char16* channel_name) { base::Process process(process_handle); base::win::ScopedHandle on_initialized_event(on_initialized_event_handle); @@ -244,6 +301,8 @@ extern "C" int WatcherMain(const base::char16* registry_path, // chrome.exe in order to report its exit status. ::SetProcessShutdownParameters(0x100, SHUTDOWN_NORETRY); + base::Callback on_hung_callback; + #ifdef KASKO bool launched_kasko = kasko::api::InitializeReporter( GetKaskoEndpoint(process.Pid()).c_str(), @@ -256,6 +315,10 @@ extern "C" int WatcherMain(const base::char16* registry_path, .Append(kPermanentlyFailedReportsSubdir) .value() .c_str()); + if (launched_kasko && + base::StringPiece16(channel_name) == installer::kChromeChannelCanary) { + on_hung_callback = base::Bind(&DumpHungBrowserProcess, channel_name); + } #endif // KASKO // Run a UI message loop on the main thread. @@ -269,13 +332,16 @@ extern "C" int WatcherMain(const base::char16* registry_path, return 1; } - browser_watcher::WindowHangMonitor hang_monitor( - base::TimeDelta::FromSeconds(60), base::TimeDelta::FromSeconds(20), - base::Bind(&OnWindowEvent, registry_path, - base::Passed(process.Duplicate()))); - hang_monitor.Initialize(process.Duplicate(), message_window_name); + { + // Scoped to force |hang_monitor| destruction before Kasko is shut down. + browser_watcher::WindowHangMonitor hang_monitor( + base::TimeDelta::FromSeconds(60), base::TimeDelta::FromSeconds(20), + base::Bind(&OnWindowEvent, registry_path, + base::Passed(process.Duplicate()), on_hung_callback)); + hang_monitor.Initialize(process.Duplicate(), message_window_name); - run_loop.Run(); + run_loop.Run(); + } #ifdef KASKO if (launched_kasko) diff --git a/chrome/chrome_watcher/chrome_watcher_main_api.h b/chrome/chrome_watcher/chrome_watcher_main_api.h index 82cebdb6a6f0..d070a577770d 100644 --- a/chrome/chrome_watcher/chrome_watcher_main_api.h +++ b/chrome/chrome_watcher/chrome_watcher_main_api.h @@ -26,12 +26,15 @@ extern const base::FilePath::CharType kPermanentlyFailedReportsSubdir[]; // reports. |on_initialized_event| will be signaled once the watcher process is // fully initialized. Takes ownership of |parent_process| and // |on_initialized_event|. +// |channel_name| is the current Chrome distribution channel (one of +// installer::kChromeChannelXXX). typedef int (*ChromeWatcherMainFunction)( const base::char16* registry_path, HANDLE parent_process, HANDLE on_initialized_event, const base::char16* browser_data_directory, - const base::char16* message_window_name); + const base::char16* message_window_name, + const base::char16* channel_name); // Returns an RPC endpoint name for the identified client process. This method // may be invoked in both the client and the watcher process with the PID of the -- 2.11.4.GIT