diff --git a/BUILD.gn b/BUILD.gn index 657e5efe2428..dd6a69b96e42 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -445,21 +445,13 @@ static_library("electron_lib") { if (is_mac) { deps += [ "//components/remote_cocoa/app_shim", - "//third_party/crashpad/crashpad/client", "//ui/accelerated_widget_mac", ] sources += [ "atom/browser/ui/views/autofill_popup_view.cc", "atom/browser/ui/views/autofill_popup_view.h", ] - include_dirs += [ - # NOTE(nornagon): other chromium files use the full path to include - # crashpad; this is just here for compatibility between GN and GYP, so that - # the #includes can be agnostic about where crashpad is vendored. - "//third_party/crashpad", - ] if (is_mas_build) { - deps -= [ "//third_party/crashpad/crashpad/client" ] sources += [ "atom/browser/api/atom_api_app_mas.mm" ] sources -= [ "atom/browser/auto_updater_mac.mm", @@ -489,10 +481,14 @@ static_library("electron_lib") { "//chrome/browser/ui/libgtkui", "//dbus", "//device/bluetooth", + "//third_party/breakpad:client", "//ui/events/devices/x11", "//ui/events/platform/x11", + "//ui/views/controls/webview", + "//ui/wm", ] configs += [ ":gio_unix" ] + include_dirs += [ "//third_party/breakpad" ] defines += [ # Disable warnings for g_settings_list_schemas. "GLIB_DISABLE_DEPRECATION_WARNINGS", @@ -503,20 +499,23 @@ static_library("electron_lib") { if (is_win) { libs += [ "dwmapi.lib" ] deps += [ - "//third_party/breakpad:breakpad_handler", - "//third_party/breakpad:breakpad_sender", "//ui/native_theme:native_theme_browser", - "//ui/wm/public", - ] - public_deps += [ "//sandbox/win:sandbox" ] - } - if (is_linux || is_win) { - deps += [ - "//third_party/breakpad:client", "//ui/views/controls/webview", "//ui/wm", + "//ui/wm/public", ] - include_dirs += [ "//third_party/breakpad" ] + public_deps += [ + "//sandbox/win:sandbox", + "//third_party/crashpad/crashpad/handler", + ] + } + + if ((is_mac && !is_mas_build) || is_win) { + sources += [ + "atom/common/crash_reporter/crash_reporter_crashpad.cc", + "atom/common/crash_reporter/crash_reporter_crashpad.h", + ] + deps += [ "//third_party/crashpad/crashpad/client" ] } if (enable_pdf) { @@ -736,6 +735,7 @@ if (is_mac) { } defines = [ "HELPER_EXECUTABLE" ] sources = filenames.app_sources + sources += [ "atom/common/atom_constants.cc" ] include_dirs = [ "." ] info_plist = "atom/renderer/resources/mac/Info.plist" extra_substitutions = [ "ATOM_BUNDLE_ID=$electron_mac_bundle_id.helper" ] @@ -841,6 +841,7 @@ if (is_mac) { mac_app_bundle("electron_app") { output_name = electron_product_name sources = filenames.app_sources + sources += [ "atom/common/atom_constants.cc" ] include_dirs = [ "." ] deps = [ ":electron_app_framework_bundle_data", diff --git a/DEPS b/DEPS index d1ef38c023ef..6e5d1268bdd6 100644 --- a/DEPS +++ b/DEPS @@ -12,7 +12,7 @@ vars = { 'chromium_version': 'f200986dfaabd6aad6a4b37dad7aae42fec349e9', 'node_version': - 'b823596192bb790f9ea2a61022b55bf50e6daa83', + '229bd3245b2f54c12ea9ad0abcadbc209f8023dc', 'nan_version': '960dd6c70fc9eb136efdf37b4bef18fadbc3436f', diff --git a/appveyor.yml b/appveyor.yml index dae315754cb0..2b3fca16df4f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -32,7 +32,6 @@ build_cloud: libcc-20 image: libcc-20-vs2017-15.9 environment: GIT_CACHE_PATH: C:\Users\electron\libcc_cache - DISABLE_CRASH_REPORTER_TESTS: true ELECTRON_OUT_DIR: Default ELECTRON_ENABLE_STACK_DUMPING: 1 notifications: diff --git a/atom/app/atom_main.cc b/atom/app/atom_main.cc index 49a77c842b0d..beef50e64f03 100644 --- a/atom/app/atom_main.cc +++ b/atom/app/atom_main.cc @@ -4,6 +4,7 @@ #include "atom/app/atom_main.h" +#include #include #include #include @@ -39,6 +40,7 @@ #include "atom/app/node_main.h" #include "atom/common/atom_command_line.h" +#include "atom/common/atom_constants.h" #include "base/at_exit.h" #include "base/i18n/icu_util.h" #include "electron/buildflags/buildflags.h" @@ -49,10 +51,6 @@ namespace { -#if BUILDFLAG(ENABLE_RUN_AS_NODE) -const char kRunAsNode[] = "ELECTRON_RUN_AS_NODE"; -#endif - ALLOW_UNUSED_TYPE bool IsEnvSet(const char* name) { #if defined(OS_WIN) size_t required_size; @@ -86,6 +84,11 @@ void FixStdioStreams() { } // namespace #if defined(OS_WIN) + +namespace crash_reporter { +extern const char kCrashpadProcess[]; +} + int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { struct Arguments { int argc = 0; @@ -122,7 +125,7 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { #endif #if BUILDFLAG(ENABLE_RUN_AS_NODE) - bool run_as_node = IsEnvSet(kRunAsNode); + bool run_as_node = IsEnvSet(atom::kRunAsNode); #else bool run_as_node = false; #endif @@ -148,13 +151,11 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { atexit([]() { OnThreadExit(nullptr, DLL_THREAD_DETACH, nullptr); }); #endif + std::vector argv(arguments.argc); + std::transform(arguments.argv, arguments.argv + arguments.argc, argv.begin(), + [](auto& a) { return _strdup(base::WideToUTF8(a).c_str()); }); #if BUILDFLAG(ENABLE_RUN_AS_NODE) if (run_as_node) { - std::vector argv(arguments.argc); - std::transform( - arguments.argv, arguments.argv + arguments.argc, argv.begin(), - [](auto& a) { return _strdup(base::WideToUTF8(a).c_str()); }); - base::AtExitManager atexit_manager; base::i18n::InitializeICU(); auto ret = atom::NodeMain(argv.size(), argv.data()); @@ -163,8 +164,11 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { } #endif - if (IsEnvSet("ELECTRON_INTERNAL_CRASH_SERVICE")) { - return crash_service::Main(cmd); + base::CommandLine::Init(argv.size(), argv.data()); + const base::CommandLine& cmd_line = *base::CommandLine::ForCurrentProcess(); + if (cmd_line.GetSwitchValueASCII("type") == + crash_reporter::kCrashpadProcess) { + return crash_service::Main(&argv); } if (!atom::CheckCommandLineArguments(arguments.argc, arguments.argv)) @@ -187,7 +191,7 @@ int main(int argc, char* argv[]) { FixStdioStreams(); #if BUILDFLAG(ENABLE_RUN_AS_NODE) - if (IsEnvSet(kRunAsNode)) { + if (IsEnvSet(atom::kRunAsNode)) { base::i18n::InitializeICU(); base::AtExitManager atexit_manager; return atom::NodeMain(argc, argv); @@ -208,7 +212,7 @@ int main(int argc, char* argv[]) { FixStdioStreams(); #if BUILDFLAG(ENABLE_RUN_AS_NODE) - if (IsEnvSet(kRunAsNode)) { + if (IsEnvSet(atom::kRunAsNode)) { return AtomInitializeICUandStartNode(argc, argv); } #endif diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 7dc9186d2abb..95c655999add 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -43,6 +43,9 @@ #if defined(OS_WIN) #include "base/win/win_util.h" +#if defined(_WIN64) +#include "atom/common/crash_reporter/crash_reporter_win.h" +#endif #endif namespace atom { @@ -127,6 +130,10 @@ bool AtomMainDelegate::BasicStartupComplete(int* exit_code) { logging::LoggingSettings settings; #if defined(OS_WIN) +#if defined(_WIN64) + crash_reporter::CrashReporterWin::SetUnhandledExceptionFilter(); +#endif + // On Windows the terminal returns immediately, so we add a new line to // prevent output in the same line as the prompt. if (IsBrowserProcess(command_line)) diff --git a/atom/app/node_main.cc b/atom/app/node_main.cc index 5636c51023a6..3b0d7b23c492 100644 --- a/atom/app/node_main.cc +++ b/atom/app/node_main.cc @@ -25,6 +25,10 @@ #include "gin/v8_initializer.h" #include "native_mate/dictionary.h" +#if defined(_WIN64) +#include "atom/common/crash_reporter/crash_reporter_win.h" +#endif + namespace atom { int NodeMain(int argc, char* argv[]) { @@ -52,6 +56,9 @@ int NodeMain(int argc, char* argv[]) { // Initialize gin::IsolateHolder. JavascriptEnvironment gin_env(loop); +#if defined(_WIN64) + crash_reporter::CrashReporterWin::SetUnhandledExceptionFilter(); +#endif // Explicitly register electron's builtin modules. NodeBindings::RegisterBuiltinModules(); diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index f8ba3a095e41..736b0d6d69d3 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -81,6 +81,10 @@ #include "ui/base/resource/resource_bundle.h" #include "v8/include/v8.h" +#if defined(OS_WIN) +#include "sandbox/win/src/sandbox_policy.h" +#endif + #if defined(USE_NSS_CERTS) #include "net/ssl/client_cert_store_nss.h" #elif defined(OS_WIN) @@ -961,6 +965,18 @@ bool AtomBrowserClient::WillCreateURLLoaderFactory( return true; } +#if defined(OS_WIN) +bool AtomBrowserClient::PreSpawnRenderer(sandbox::TargetPolicy* policy) { + // Allow crashpad to communicate via named pipe. + sandbox::ResultCode result = policy->AddRule( + sandbox::TargetPolicy::SUBSYS_FILES, + sandbox::TargetPolicy::FILES_ALLOW_ANY, L"\\??\\pipe\\crashpad_*"); + if (result != sandbox::SBOX_ALL_OK) + return false; + return true; +} +#endif // defined(OS_WIN) + std::string AtomBrowserClient::GetApplicationLocale() { if (BrowserThread::CurrentlyOn(BrowserThread::IO)) return g_io_thread_application_locale.Get(); diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index a29c5dbdd162..4cc72e139b15 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -178,6 +178,9 @@ class AtomBrowserClient : public content::ContentBrowserClient, network::mojom::URLLoaderFactoryRequest* factory_request, network::mojom::TrustedURLLoaderHeaderClientPtrInfo* header_client, bool* bypass_redirect_checks) override; +#if defined(OS_WIN) + bool PreSpawnRenderer(sandbox::TargetPolicy* policy) override; +#endif // content::RenderProcessHostObserver: void RenderProcessHostDestroyed(content::RenderProcessHost* host) override; diff --git a/atom/browser/ui/inspectable_web_contents.h b/atom/browser/ui/inspectable_web_contents.h index fe4c2c3bd0d7..7d6d6ea72a67 100644 --- a/atom/browser/ui/inspectable_web_contents.h +++ b/atom/browser/ui/inspectable_web_contents.h @@ -25,6 +25,8 @@ namespace atom { class InspectableWebContentsDelegate; class InspectableWebContentsView; +// TODO(zcbenz): Remove this abstract wrapper and rename +// InspectableWebContentsImpl to InspectableWebContents instead. class InspectableWebContents { public: // The returned InspectableWebContents takes ownership of the passed-in diff --git a/atom/browser/ui/inspectable_web_contents_impl.cc b/atom/browser/ui/inspectable_web_contents_impl.cc index c0efe548c2a5..48dfea30f376 100644 --- a/atom/browser/ui/inspectable_web_contents_impl.cc +++ b/atom/browser/ui/inspectable_web_contents_impl.cc @@ -3,11 +3,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE-CHROMIUM file. +#include "atom/browser/ui/inspectable_web_contents_impl.h" + #include #include -#include "atom/browser/ui/inspectable_web_contents_impl.h" - #include "atom/browser/ui/inspectable_web_contents_delegate.h" #include "atom/browser/ui/inspectable_web_contents_view.h" #include "atom/browser/ui/inspectable_web_contents_view_delegate.h" @@ -75,6 +75,9 @@ const char kTitleFormat[] = "Developer Tools - %s"; const size_t kMaxMessageChunkSize = IPC::Channel::kMaximumMessageSize / 4; +// Stores all instances of InspectableWebContentsImpl. +InspectableWebContentsImpl::List g_web_contents_instances_; + base::Value RectToDictionary(const gfx::Rect& bounds) { base::Value dict(base::Value::Type::DICTIONARY); dict.SetKey("x", base::Value(bounds.x())); @@ -227,6 +230,12 @@ class InspectableWebContentsImpl::NetworkResourceLoader InspectableWebContentsView* CreateInspectableContentsView( InspectableWebContentsImpl* inspectable_web_contents_impl); +// static +const InspectableWebContentsImpl::List& InspectableWebContentsImpl::GetAll() { + return g_web_contents_instances_; +} + +// static void InspectableWebContentsImpl::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterDictionaryPref(kDevToolsBoundsPref, RectToDictionary(gfx::Rect(0, 0, 800, 600))); @@ -270,9 +279,11 @@ InspectableWebContentsImpl::InspectableWebContentsImpl( display.y() + (display.height() - devtools_bounds_.height()) / 2); } } + g_web_contents_instances_.push_back(this); } InspectableWebContentsImpl::~InspectableWebContentsImpl() { + g_web_contents_instances_.remove(this); // Unsubscribe from devtools and Clean up resources. if (GetDevToolsWebContents()) { if (managed_devtools_web_contents_) diff --git a/atom/browser/ui/inspectable_web_contents_impl.h b/atom/browser/ui/inspectable_web_contents_impl.h index 569597cf05ff..c75a05b6ec5b 100644 --- a/atom/browser/ui/inspectable_web_contents_impl.h +++ b/atom/browser/ui/inspectable_web_contents_impl.h @@ -6,6 +6,7 @@ #ifndef ATOM_BROWSER_UI_INSPECTABLE_WEB_CONTENTS_IMPL_H_ #define ATOM_BROWSER_UI_INSPECTABLE_WEB_CONTENTS_IMPL_H_ +#include #include #include #include @@ -38,6 +39,9 @@ class InspectableWebContentsImpl public content::WebContentsDelegate, public DevToolsEmbedderMessageDispatcher::Delegate { public: + using List = std::list; + + static const List& GetAll(); static void RegisterPrefs(PrefRegistrySimple* pref_registry); InspectableWebContentsImpl(content::WebContents* web_contents, diff --git a/atom/common/api/api.mojom b/atom/common/api/api.mojom index 5ea41b68a79d..b37f8e75da94 100644 --- a/atom/common/api/api.mojom +++ b/atom/common/api/api.mojom @@ -12,6 +12,8 @@ interface ElectronRenderer { mojo_base.mojom.ListValue arguments, int32 sender_id); + UpdateCrashpadPipeName(string pipe_name); + TakeHeapSnapshot(handle file) => (bool success); }; diff --git a/atom/common/atom_constants.cc b/atom/common/atom_constants.cc index 19b2bde65a01..4d4c596ced5c 100644 --- a/atom/common/atom_constants.cc +++ b/atom/common/atom_constants.cc @@ -27,6 +27,14 @@ const char kSecureProtocolDescription[] = "The connection to this site is using a strong protocol version " "and cipher suite."; +#if defined(OS_WIN) +const char kCrashpadPipeName[] = "ELECTRON_CRASHPAD_PIPE_NAME"; +#endif + +#if BUILDFLAG(ENABLE_RUN_AS_NODE) +const char kRunAsNode[] = "ELECTRON_RUN_AS_NODE"; +#endif + #if BUILDFLAG(ENABLE_PDF_VIEWER) const char kPdfPluginMimeType[] = "application/x-google-chrome-pdf"; const char kPdfPluginPath[] = "chrome://pdf-viewer/"; diff --git a/atom/common/atom_constants.h b/atom/common/atom_constants.h index 3b86a2cf9079..463d654d86ef 100644 --- a/atom/common/atom_constants.h +++ b/atom/common/atom_constants.h @@ -5,6 +5,7 @@ #ifndef ATOM_COMMON_ATOM_CONSTANTS_H_ #define ATOM_COMMON_ATOM_CONSTANTS_H_ +#include "build/build_config.h" #include "electron/buildflags/buildflags.h" namespace atom { @@ -26,6 +27,15 @@ extern const char kValidCertificateDescription[]; extern const char kSecureProtocol[]; extern const char kSecureProtocolDescription[]; +#if defined(OS_WIN) +// Crashpad pipe name. +extern const char kCrashpadPipeName[]; +#endif + +#if BUILDFLAG(ENABLE_RUN_AS_NODE) +extern const char kRunAsNode[]; +#endif + #if BUILDFLAG(ENABLE_PDF_VIEWER) // The MIME type used for the PDF plugin. extern const char kPdfPluginMimeType[]; diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index ecb4a8473748..ca404969db41 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -4,11 +4,15 @@ #include "atom/common/crash_reporter/crash_reporter.h" +#include + #include "atom/browser/browser.h" +#include "atom/common/atom_constants.h" #include "atom/common/atom_version.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/map_converter.h" #include "base/command_line.h" +#include "base/environment.h" #include "base/files/file_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" @@ -17,13 +21,26 @@ namespace crash_reporter { +const char kCrashpadProcess[] = "crash-handler"; +const char kCrashesDirectoryKey[] = "crashes-directory"; + CrashReporter::CrashReporter() { - auto* cmd = base::CommandLine::ForCurrentProcess(); - is_browser_ = cmd->GetSwitchValueASCII(switches::kProcessType).empty(); + std::unique_ptr env = base::Environment::Create(); + if (env->HasVar(atom::kRunAsNode)) { + process_type_ = "node"; + } else { + auto* cmd = base::CommandLine::ForCurrentProcess(); + process_type_ = cmd->GetSwitchValueASCII(switches::kProcessType); + } + // process_type_ will be empty for browser process } CrashReporter::~CrashReporter() {} +bool CrashReporter::IsInitialized() { + return is_initialized_; +} + void CrashReporter::Start(const std::string& product_name, const std::string& company_name, const std::string& submit_url, @@ -31,15 +48,19 @@ void CrashReporter::Start(const std::string& product_name, bool upload_to_server, bool skip_system_crash_handler, const StringMap& extra_parameters) { + is_initialized_ = true; SetUploadParameters(extra_parameters); - InitBreakpad(product_name, ATOM_VERSION_STRING, company_name, submit_url, - crashes_dir, upload_to_server, skip_system_crash_handler); + Init(product_name, company_name, submit_url, crashes_dir, upload_to_server, + skip_system_crash_handler); } void CrashReporter::SetUploadParameters(const StringMap& parameters) { upload_parameters_ = parameters; - upload_parameters_["process_type"] = is_browser_ ? "browser" : "renderer"; + upload_parameters_["process_type"] = + process_type_.empty() ? "browser" : process_type_; + upload_parameters_["prod"] = ATOM_PRODUCT_NAME; + upload_parameters_["ver"] = ATOM_VERSION_STRING; // Setting platform dependent parameters. SetUploadParameters(); @@ -75,13 +96,12 @@ CrashReporter::GetUploadedReports(const base::FilePath& crashes_dir) { return result; } -void CrashReporter::InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool auto_submit, - bool skip_system_crash_handler) {} +void CrashReporter::Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool auto_submit, + bool skip_system_crash_handler) {} void CrashReporter::SetUploadParameters() {} diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index 99c4c9818d8c..357d91e8f991 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -16,6 +16,9 @@ namespace crash_reporter { +extern const char kCrashpadProcess[]; +extern const char kCrashesDirectoryKey[]; + class CrashReporter { public: typedef std::map StringMap; @@ -24,6 +27,7 @@ class CrashReporter { static CrashReporter* GetInstance(); static void StartInstance(const mate::Dictionary& options); + bool IsInitialized(); void Start(const std::string& product_name, const std::string& company_name, const std::string& submit_url, @@ -46,19 +50,19 @@ class CrashReporter { CrashReporter(); virtual ~CrashReporter(); - virtual void InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler); + virtual void Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler); virtual void SetUploadParameters(); StringMap upload_parameters_; - bool is_browser_; + std::string process_type_; private: + bool is_initialized_ = false; void SetUploadParameters(const StringMap& parameters); DISALLOW_COPY_AND_ASSIGN(CrashReporter); diff --git a/atom/common/crash_reporter/crash_reporter_crashpad.cc b/atom/common/crash_reporter/crash_reporter_crashpad.cc new file mode 100644 index 000000000000..87265ed0b0b4 --- /dev/null +++ b/atom/common/crash_reporter/crash_reporter_crashpad.cc @@ -0,0 +1,118 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/common/crash_reporter/crash_reporter_crashpad.h" + +#include +#include + +#include "base/files/file_util.h" +#include "base/strings/string_piece.h" +#include "base/strings/stringprintf.h" +#include "base/strings/sys_string_conversions.h" +#include "base/threading/thread_restrictions.h" +#include "third_party/crashpad/crashpad/client/settings.h" + +namespace crash_reporter { + +CrashReporterCrashpad::CrashReporterCrashpad() {} + +CrashReporterCrashpad::~CrashReporterCrashpad() {} + +bool CrashReporterCrashpad::GetUploadToServer() { + bool enabled = true; + if (database_) { + database_->GetSettings()->GetUploadsEnabled(&enabled); + } + return enabled; +} + +void CrashReporterCrashpad::SetUploadToServer(const bool upload_to_server) { + if (database_) { + database_->GetSettings()->SetUploadsEnabled(upload_to_server); + } +} + +void CrashReporterCrashpad::SetCrashKeyValue(const base::StringPiece& key, + const base::StringPiece& value) { + simple_string_dictionary_->SetKeyValue(key.data(), value.data()); +} + +void CrashReporterCrashpad::SetInitialCrashKeyValues() { + for (const auto& upload_parameter : upload_parameters_) + SetCrashKeyValue(upload_parameter.first, upload_parameter.second); +} + +void CrashReporterCrashpad::AddExtraParameter(const std::string& key, + const std::string& value) { + if (simple_string_dictionary_) { + SetCrashKeyValue(key, value); + } else { + upload_parameters_[key] = value; + } +} + +void CrashReporterCrashpad::RemoveExtraParameter(const std::string& key) { + if (simple_string_dictionary_) + simple_string_dictionary_->RemoveKey(key.data()); + else + upload_parameters_.erase(key); +} + +std::map CrashReporterCrashpad::GetParameters() + const { + if (simple_string_dictionary_) { + std::map ret; + crashpad::SimpleStringDictionary::Iterator iter(*simple_string_dictionary_); + for (;;) { + auto* const entry = iter.Next(); + if (!entry) + break; + ret[entry->key] = entry->value; + } + return ret; + } + return upload_parameters_; +} + +std::vector +CrashReporterCrashpad::GetUploadedReports(const base::FilePath& crashes_dir) { + std::vector uploaded_reports; + + { + base::ThreadRestrictions::ScopedAllowIO allow_io; + if (!base::PathExists(crashes_dir)) { + return uploaded_reports; + } + } + // Load crashpad database. + std::unique_ptr database = + crashpad::CrashReportDatabase::Initialize(crashes_dir); + DCHECK(database); + + std::vector completed_reports; + crashpad::CrashReportDatabase::OperationStatus status = + database->GetCompletedReports(&completed_reports); + if (status != crashpad::CrashReportDatabase::kNoError) { + return uploaded_reports; + } + + for (const crashpad::CrashReportDatabase::Report& completed_report : + completed_reports) { + if (completed_report.uploaded) { + uploaded_reports.push_back( + UploadReportResult(static_cast(completed_report.creation_time), + completed_report.id)); + } + } + + auto sort_by_time = [](const UploadReportResult& a, + const UploadReportResult& b) { + return a.first >= b.first; + }; + std::sort(uploaded_reports.begin(), uploaded_reports.end(), sort_by_time); + return uploaded_reports; +} + +} // namespace crash_reporter diff --git a/atom/common/crash_reporter/crash_reporter_crashpad.h b/atom/common/crash_reporter/crash_reporter_crashpad.h new file mode 100644 index 000000000000..bd721d533717 --- /dev/null +++ b/atom/common/crash_reporter/crash_reporter_crashpad.h @@ -0,0 +1,50 @@ +// Copyright (c) 2013 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_COMMON_CRASH_REPORTER_CRASH_REPORTER_CRASHPAD_H_ +#define ATOM_COMMON_CRASH_REPORTER_CRASH_REPORTER_CRASHPAD_H_ + +#include +#include +#include +#include + +#include "atom/common/crash_reporter/crash_reporter.h" +#include "base/compiler_specific.h" +#include "base/strings/string_piece.h" +#include "third_party/crashpad/crashpad/client/crash_report_database.h" +#include "third_party/crashpad/crashpad/client/simple_string_dictionary.h" + +namespace crash_reporter { + +class CrashReporterCrashpad : public CrashReporter { + public: + void SetUploadToServer(bool upload_to_server) override; + bool GetUploadToServer() override; + void AddExtraParameter(const std::string& key, + const std::string& value) override; + void RemoveExtraParameter(const std::string& key) override; + std::map GetParameters() const override; + + protected: + CrashReporterCrashpad(); + ~CrashReporterCrashpad() override; + + void SetUploadsEnabled(bool enable_uploads); + void SetCrashKeyValue(const base::StringPiece& key, + const base::StringPiece& value); + void SetInitialCrashKeyValues(); + + std::vector GetUploadedReports( + const base::FilePath& crashes_dir) override; + + std::unique_ptr simple_string_dictionary_; + std::unique_ptr database_; + + DISALLOW_COPY_AND_ASSIGN(CrashReporterCrashpad); +}; + +} // namespace crash_reporter + +#endif // ATOM_COMMON_CRASH_REPORTER_CRASH_REPORTER_CRASHPAD_H_ diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index 7928c64fb8da..c293cdcbdff4 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -51,22 +51,18 @@ CrashReporterLinux::CrashReporterLinux() : pid_(getpid()) { CrashReporterLinux::~CrashReporterLinux() {} -void CrashReporterLinux::InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) { +void CrashReporterLinux::Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) { EnableCrashDumping(crashes_dir); - crash_keys_.reset(new CrashKeyStorage()); - - crash_keys_->SetKeyValue("prod", ATOM_PRODUCT_NAME); - crash_keys_->SetKeyValue("ver", version.c_str()); upload_url_ = submit_url; upload_to_server_ = upload_to_server; + crash_keys_.reset(new CrashKeyStorage()); for (StringMap::const_iterator iter = upload_parameters_.begin(); iter != upload_parameters_.end(); ++iter) crash_keys_->SetKeyValue(iter->first.c_str(), iter->second.c_str()); diff --git a/atom/common/crash_reporter/crash_reporter_linux.h b/atom/common/crash_reporter/crash_reporter_linux.h index ce9d85fa4b8c..0f22d553c1c6 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.h +++ b/atom/common/crash_reporter/crash_reporter_linux.h @@ -28,13 +28,12 @@ class CrashReporterLinux : public CrashReporter { public: static CrashReporterLinux* GetInstance(); - void InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) override; + void Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) override; void SetUploadToServer(bool upload_to_server) override; void SetUploadParameters() override; bool GetUploadToServer() override; diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index 7a68a4389b1e..d1b4bcbc337d 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -5,16 +5,10 @@ #ifndef ATOM_COMMON_CRASH_REPORTER_CRASH_REPORTER_MAC_H_ #define ATOM_COMMON_CRASH_REPORTER_CRASH_REPORTER_MAC_H_ -#include #include #include -#include -#include "atom/common/crash_reporter/crash_reporter.h" -#include "base/compiler_specific.h" -#include "base/strings/string_piece.h" -#include "crashpad/client/crash_report_database.h" -#include "crashpad/client/simple_string_dictionary.h" +#include "atom/common/crash_reporter/crash_reporter_crashpad.h" namespace base { template @@ -23,24 +17,17 @@ struct DefaultSingletonTraits; namespace crash_reporter { -class CrashReporterMac : public CrashReporter { +class CrashReporterMac : public CrashReporterCrashpad { public: static CrashReporterMac* GetInstance(); - void InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) override; + void Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) override; void SetUploadParameters() override; - void SetUploadToServer(bool upload_to_server) override; - bool GetUploadToServer() override; - void AddExtraParameter(const std::string& key, - const std::string& value) override; - void RemoveExtraParameter(const std::string& key) override; - std::map GetParameters() const override; private: friend struct base::DefaultSingletonTraits; @@ -48,16 +35,6 @@ class CrashReporterMac : public CrashReporter { CrashReporterMac(); ~CrashReporterMac() override; - void SetUploadsEnabled(bool enable_uploads); - void SetCrashKeyValue(const base::StringPiece& key, - const base::StringPiece& value); - - std::vector GetUploadedReports( - const base::FilePath& crashes_dir) override; - - std::unique_ptr simple_string_dictionary_; - std::unique_ptr database_; - DISALLOW_COPY_AND_ASSIGN(CrashReporterMac); }; diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index be8925b08a5c..3da73d0944b2 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -10,17 +10,11 @@ #include #include -#include "base/files/file_util.h" #include "base/mac/bundle_locations.h" #include "base/mac/mac_util.h" #include "base/memory/singleton.h" -#include "base/strings/string_piece.h" -#include "base/strings/stringprintf.h" -#include "base/strings/sys_string_conversions.h" -#include "base/threading/thread_restrictions.h" -#include "crashpad/client/crashpad_client.h" -#include "crashpad/client/crashpad_info.h" -#include "crashpad/client/settings.h" +#include "third_party/crashpad/crashpad/client/crashpad_client.h" +#include "third_party/crashpad/crashpad/client/crashpad_info.h" namespace crash_reporter { @@ -28,19 +22,18 @@ CrashReporterMac::CrashReporterMac() {} CrashReporterMac::~CrashReporterMac() {} -void CrashReporterMac::InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) { +void CrashReporterMac::Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) { // check whether crashpad has been initialized. // Only need to initialize once. if (simple_string_dictionary_) return; - if (is_browser_) { + if (process_type_.empty()) { // browser process @autoreleasepool { base::FilePath framework_bundle_path = base::mac::FrameworkBundlePath(); base::FilePath handler_path = @@ -67,112 +60,17 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, simple_string_dictionary_.reset(new crashpad::SimpleStringDictionary()); crashpad_info->set_simple_annotations(simple_string_dictionary_.get()); - SetCrashKeyValue("prod", ATOM_PRODUCT_NAME); - SetCrashKeyValue("process_type", is_browser_ ? "browser" : "renderer"); - SetCrashKeyValue("ver", version); - - for (const auto& upload_parameter : upload_parameters_) { - SetCrashKeyValue(upload_parameter.first, upload_parameter.second); - } - if (is_browser_) { + SetInitialCrashKeyValues(); + if (process_type_.empty()) { // browser process database_ = crashpad::CrashReportDatabase::Initialize(crashes_dir); SetUploadToServer(upload_to_server); } } -bool CrashReporterMac::GetUploadToServer() { - bool enabled = true; - if (database_) { - database_->GetSettings()->GetUploadsEnabled(&enabled); - } - return enabled; -} - -void CrashReporterMac::SetUploadToServer(const bool upload_to_server) { - if (database_) { - database_->GetSettings()->SetUploadsEnabled(upload_to_server); - } -} - void CrashReporterMac::SetUploadParameters() { upload_parameters_["platform"] = "darwin"; } -void CrashReporterMac::SetCrashKeyValue(const base::StringPiece& key, - const base::StringPiece& value) { - simple_string_dictionary_->SetKeyValue(key.data(), value.data()); -} - -void CrashReporterMac::AddExtraParameter(const std::string& key, - const std::string& value) { - if (simple_string_dictionary_) { - SetCrashKeyValue(key, value); - } else { - upload_parameters_[key] = value; - } -} - -void CrashReporterMac::RemoveExtraParameter(const std::string& key) { - if (simple_string_dictionary_) - simple_string_dictionary_->RemoveKey(key.data()); - else - upload_parameters_.erase(key); -} - -std::map CrashReporterMac::GetParameters() const { - if (simple_string_dictionary_) { - std::map ret; - crashpad::SimpleStringDictionary::Iterator iter(*simple_string_dictionary_); - for (;;) { - auto* const entry = iter.Next(); - if (!entry) - break; - ret[entry->key] = entry->value; - } - return ret; - } - return upload_parameters_; -} - -std::vector -CrashReporterMac::GetUploadedReports(const base::FilePath& crashes_dir) { - std::vector uploaded_reports; - - { - base::ThreadRestrictions::ScopedAllowIO allow_io; - if (!base::PathExists(crashes_dir)) { - return uploaded_reports; - } - } - // Load crashpad database. - std::unique_ptr database = - crashpad::CrashReportDatabase::Initialize(crashes_dir); - DCHECK(database); - - std::vector completed_reports; - crashpad::CrashReportDatabase::OperationStatus status = - database->GetCompletedReports(&completed_reports); - if (status != crashpad::CrashReportDatabase::kNoError) { - return uploaded_reports; - } - - for (const crashpad::CrashReportDatabase::Report& completed_report : - completed_reports) { - if (completed_report.uploaded) { - uploaded_reports.push_back( - UploadReportResult(static_cast(completed_report.creation_time), - completed_report.id)); - } - } - - auto sort_by_time = [](const UploadReportResult& a, - const UploadReportResult& b) { - return a.first >= b.first; - }; - std::sort(uploaded_reports.begin(), uploaded_reports.end(), sort_by_time); - return uploaded_reports; -} - // static CrashReporterMac* CrashReporterMac::GetInstance() { return base::Singleton::get(); diff --git a/atom/common/crash_reporter/crash_reporter_win.cc b/atom/common/crash_reporter/crash_reporter_win.cc index bbb586dec621..6d48b9533c2e 100644 --- a/atom/common/crash_reporter/crash_reporter_win.cc +++ b/atom/common/crash_reporter/crash_reporter_win.cc @@ -4,276 +4,128 @@ #include "atom/common/crash_reporter/crash_reporter_win.h" -#include +#include +#include -#include "base/files/file_util.h" -#include "base/logging.h" +#include "atom/browser/ui/inspectable_web_contents_impl.h" +#include "atom/common/atom_constants.h" +#include "base/environment.h" #include "base/memory/singleton.h" -#include "base/strings/string_util.h" +#include "base/path_service.h" +#include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" -#include "content/public/common/result_codes.h" +#include "electron/atom/common/api/api.mojom.h" +#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h" +#include "third_party/crashpad/crashpad/client/crashpad_client.h" +#include "third_party/crashpad/crashpad/client/crashpad_info.h" + +#if defined(_WIN64) #include "gin/public/debug.h" -#include "sandbox/win/src/nt_internals.h" - -#pragma intrinsic(_AddressOfReturnAddress) -#pragma intrinsic(_ReturnAddress) - -#ifdef _WIN64 -// See http://msdn.microsoft.com/en-us/library/ddssxxy8.aspx -typedef struct _UNWIND_INFO { - unsigned char Version : 3; - unsigned char Flags : 5; - unsigned char SizeOfProlog; - unsigned char CountOfCodes; - unsigned char FrameRegister : 4; - unsigned char FrameOffset : 4; - ULONG ExceptionHandler; -} UNWIND_INFO, *PUNWIND_INFO; #endif -namespace crash_reporter { - namespace { -// Minidump with stacks, PEB, TEB, and unloaded module list. -const MINIDUMP_TYPE kSmallDumpType = static_cast( - 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"; - -// Matches breakpad/src/client/windows/common/ipc_protocol.h. -const int kNameMaxLength = 64; -const int kValueMaxLength = 64; - -typedef NTSTATUS(WINAPI* NtTerminateProcessPtr)(HANDLE ProcessHandle, - NTSTATUS ExitStatus); -char* g_real_terminate_process_stub = NULL; - -void TerminateProcessWithoutDump() { - // Patched stub exists based on conditions (See InitCrashReporter). - // As a side note this function also gets called from - // WindowProcExceptionFilter. - if (g_real_terminate_process_stub == NULL) { - ::TerminateProcess(::GetCurrentProcess(), content::RESULT_CODE_KILLED); - } else { - NtTerminateProcessPtr real_terminate_proc = - reinterpret_cast( - static_cast(g_real_terminate_process_stub)); - real_terminate_proc(::GetCurrentProcess(), content::RESULT_CODE_KILLED); - } +#if defined(_WIN64) +int CrashForException(EXCEPTION_POINTERS* info) { + auto* reporter = crash_reporter::CrashReporterWin::GetInstance(); + if (reporter->IsInitialized()) + reporter->GetCrashpadClient().DumpAndCrash(info); + return EXCEPTION_CONTINUE_SEARCH; } - -#ifdef _WIN64 -int CrashForExceptionInNonABICompliantCodeRange( - PEXCEPTION_RECORD ExceptionRecord, - ULONG64 EstablisherFrame, - PCONTEXT ContextRecord, - PDISPATCHER_CONTEXT DispatcherContext) { - EXCEPTION_POINTERS info = {ExceptionRecord, ContextRecord}; - if (!CrashReporter::GetInstance()) - return EXCEPTION_CONTINUE_SEARCH; - return static_cast(CrashReporter::GetInstance()) - ->CrashForException(&info); -} - -struct ExceptionHandlerRecord { - RUNTIME_FUNCTION runtime_function; - UNWIND_INFO unwind_info; - unsigned char thunk[12]; -}; - -bool RegisterNonABICompliantCodeRange(void* start, size_t size_in_bytes) { - ExceptionHandlerRecord* record = - reinterpret_cast(start); - - // We assume that the first page of the code range is executable and - // committed and reserved for breakpad. What could possibly go wrong? - - // All addresses are 32bit relative offsets to start. - record->runtime_function.BeginAddress = 0; -#if defined(_M_ARM64) - record->runtime_function.FunctionLength = - base::checked_cast(size_in_bytes); -#else - record->runtime_function.EndAddress = - base::checked_cast(size_in_bytes); #endif - record->runtime_function.UnwindData = - offsetof(ExceptionHandlerRecord, unwind_info); - - // Create unwind info that only specifies an exception handler. - record->unwind_info.Version = 1; - record->unwind_info.Flags = UNW_FLAG_EHANDLER; - record->unwind_info.SizeOfProlog = 0; - record->unwind_info.CountOfCodes = 0; - record->unwind_info.FrameRegister = 0; - record->unwind_info.FrameOffset = 0; - record->unwind_info.ExceptionHandler = - offsetof(ExceptionHandlerRecord, thunk); - - // Hardcoded thunk. - // mov imm64, rax - record->thunk[0] = 0x48; - record->thunk[1] = 0xb8; - void* handler = - reinterpret_cast(&CrashForExceptionInNonABICompliantCodeRange); - memcpy(&record->thunk[2], &handler, 8); - - // jmp rax - record->thunk[10] = 0xff; - record->thunk[11] = 0xe0; - - // Protect reserved page against modifications. - DWORD old_protect; - return VirtualProtect(start, sizeof(ExceptionHandlerRecord), - PAGE_EXECUTE_READ, &old_protect) && - RtlAddFunctionTable(&record->runtime_function, 1, - reinterpret_cast(start)); -} - -void UnregisterNonABICompliantCodeRange(void* start) { - ExceptionHandlerRecord* record = - reinterpret_cast(start); - - RtlDeleteFunctionTable(&record->runtime_function); -} - -#endif // _WIN64 } // namespace +namespace crash_reporter { + CrashReporterWin::CrashReporterWin() {} CrashReporterWin::~CrashReporterWin() {} -void CrashReporterWin::InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) { - skip_system_crash_handler_ = skip_system_crash_handler; +#if defined(_WIN64) +void CrashReporterWin::SetUnhandledExceptionFilter() { + gin::Debug::SetUnhandledExceptionCallback(&CrashForException); +} +#endif - base::string16 pipe_name = base::ReplaceStringPlaceholders( - kPipeNameFormat, base::UTF8ToUTF16(product_name), NULL); - base::string16 wait_name = base::ReplaceStringPlaceholders( - kWaitEventFormat, base::UTF8ToUTF16(product_name), NULL); +void CrashReporterWin::Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) { + // check whether crashpad has been initialized. + // Only need to initialize once. + if (simple_string_dictionary_) + return; + if (process_type_.empty()) { // browser process + base::FilePath handler_path; + base::PathService::Get(base::FILE_EXE, &handler_path); - // Wait until the crash service is started. - HANDLE wait_event = ::CreateEventW(NULL, TRUE, FALSE, wait_name.c_str()); - if (wait_event != NULL) { - WaitForSingleObject(wait_event, 1000); - CloseHandle(wait_event); - } - - // ExceptionHandler() attaches our handler and ~ExceptionHandler() detaches - // it, so we must explicitly reset *before* we instantiate our new handler - // to allow any previous handler to detach in the correct order. - breakpad_.reset(); - - breakpad_.reset(new google_breakpad::ExceptionHandler( - crashes_dir.DirName().value(), FilterCallback, MinidumpCallback, this, - google_breakpad::ExceptionHandler::HANDLER_ALL, kSmallDumpType, - pipe_name.c_str(), - GetCustomInfo(product_name, version, company_name, upload_to_server))); - - if (!breakpad_->IsOutOfProcess()) - LOG(ERROR) << "Cannot initialize out-of-process crash handler"; - -#ifdef _WIN64 - // Hook up V8 to breakpad. - if (!code_range_registered_) { - code_range_registered_ = true; - // gin::Debug::SetCodeRangeCreatedCallback only runs the callback when - // Isolate is just created, so we have to manually run following code here. - void* code_range = nullptr; - size_t size = 0; - v8::Isolate::GetCurrent()->GetCodeRange(&code_range, &size); - if (code_range && size && - RegisterNonABICompliantCodeRange(code_range, size)) { - // FIXME(nornagon): This broke with https://crrev.com/c/1474703 - gin::Debug::SetCodeRangeDeletedCallback( - UnregisterNonABICompliantCodeRange); + std::vector args = { + "--no-rate-limit", + "--no-upload-gzip", // not all servers accept gzip + }; + args.push_back(base::StringPrintf("--type=%s", kCrashpadProcess)); + args.push_back( + base::StringPrintf("--%s=%s", kCrashesDirectoryKey, + base::UTF16ToUTF8(crashes_dir.value()).c_str())); + crashpad_client_.StartHandler(handler_path, crashes_dir, crashes_dir, + submit_url, StringMap(), args, true, false); + UpdatePipeName(); + } else { + std::unique_ptr env(base::Environment::Create()); + std::string pipe_name_utf8; + if (env->GetVar(atom::kCrashpadPipeName, &pipe_name_utf8)) { + base::string16 pipe_name = base::UTF8ToUTF16(pipe_name_utf8); + if (!crashpad_client_.SetHandlerIPCPipe(pipe_name)) + LOG(ERROR) << "Failed to set handler IPC pipe name: " << pipe_name; + } else { + LOG(ERROR) << "Unable to get pipe name for crashpad"; } } -#endif + crashpad::CrashpadInfo* crashpad_info = + crashpad::CrashpadInfo::GetCrashpadInfo(); + if (skip_system_crash_handler) { + crashpad_info->set_system_crash_reporter_forwarding( + crashpad::TriState::kDisabled); + } + simple_string_dictionary_.reset(new crashpad::SimpleStringDictionary()); + crashpad_info->set_simple_annotations(simple_string_dictionary_.get()); + + SetInitialCrashKeyValues(); + if (process_type_.empty()) { // browser process + database_ = crashpad::CrashReportDatabase::Initialize(crashes_dir); + SetUploadToServer(upload_to_server); + } } void CrashReporterWin::SetUploadParameters() { upload_parameters_["platform"] = "win32"; } -int CrashReporterWin::CrashForException(EXCEPTION_POINTERS* info) { - if (breakpad_) { - breakpad_->WriteMinidumpForException(info); - if (skip_system_crash_handler_) - TerminateProcessWithoutDump(); - else - RaiseFailFastException(info->ExceptionRecord, info->ContextRecord, 0); - } - return EXCEPTION_CONTINUE_SEARCH; +crashpad::CrashpadClient& CrashReporterWin::GetCrashpadClient() { + return crashpad_client_; } -// static -bool CrashReporterWin::FilterCallback(void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion) { - return true; -} +void CrashReporterWin::UpdatePipeName() { + std::string pipe_name = + base::UTF16ToUTF8(crashpad_client_.GetHandlerIPCPipe()); + std::unique_ptr env(base::Environment::Create()); + env->SetVar(atom::kCrashpadPipeName, pipe_name); -// static -bool CrashReporterWin::MinidumpCallback(const wchar_t* dump_path, - const wchar_t* minidump_id, - void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion, - bool succeeded) { - CrashReporterWin* self = static_cast(context); - if (succeeded && self->skip_system_crash_handler_) - return true; - else - return false; -} + // Notify all WebContents of the pipe name. + const auto& pages = atom::InspectableWebContentsImpl::GetAll(); + for (auto* page : pages) { + auto* frame_host = page->GetWebContents()->GetMainFrame(); + if (!frame_host) + continue; -google_breakpad::CustomClientInfo* CrashReporterWin::GetCustomInfo( - const std::string& product_name, - const std::string& version, - const std::string& company_name, - bool upload_to_server) { - custom_info_entries_.clear(); - custom_info_entries_.reserve(3 + upload_parameters_.size()); - - custom_info_entries_.push_back( - google_breakpad::CustomInfoEntry(L"prod", L"Electron")); - custom_info_entries_.push_back(google_breakpad::CustomInfoEntry( - L"ver", base::UTF8ToWide(version).c_str())); - if (!upload_to_server) { - custom_info_entries_.push_back( - google_breakpad::CustomInfoEntry(L"skip_upload", L"1")); + atom::mojom::ElectronRendererAssociatedPtr electron_ptr; + frame_host->GetRemoteAssociatedInterfaces()->GetInterface( + mojo::MakeRequest(&electron_ptr)); + electron_ptr->UpdateCrashpadPipeName(pipe_name); } - - for (StringMap::const_iterator iter = upload_parameters_.begin(); - iter != upload_parameters_.end(); ++iter) { - // breakpad has hardcoded the length of name/value, and doesn't truncate - // the values itself, so we have to truncate them here otherwise weird - // things may happen. - std::wstring name = base::UTF8ToWide(iter->first); - std::wstring value = base::UTF8ToWide(iter->second); - if (name.length() > kNameMaxLength - 1) - name.resize(kNameMaxLength - 1); - if (value.length() > kValueMaxLength - 1) - value.resize(kValueMaxLength - 1); - - custom_info_entries_.push_back( - google_breakpad::CustomInfoEntry(name.c_str(), value.c_str())); - } - - custom_info_.entries = &custom_info_entries_.front(); - custom_info_.count = custom_info_entries_.size(); - return &custom_info_; } // static diff --git a/atom/common/crash_reporter/crash_reporter_win.h b/atom/common/crash_reporter/crash_reporter_win.h index 89a2a5966d6c..f41385de3720 100644 --- a/atom/common/crash_reporter/crash_reporter_win.h +++ b/atom/common/crash_reporter/crash_reporter_win.h @@ -7,11 +7,9 @@ #include #include -#include -#include "atom/common/crash_reporter/crash_reporter.h" -#include "base/compiler_specific.h" -#include "breakpad/src/client/windows/handler/exception_handler.h" +#include "atom/common/crash_reporter/crash_reporter_crashpad.h" +#include "third_party/crashpad/crashpad/client/crashpad_client.h" namespace base { template @@ -20,55 +18,31 @@ struct DefaultSingletonTraits; namespace crash_reporter { -class CrashReporterWin : public CrashReporter { +class CrashReporterWin : public CrashReporterCrashpad { public: static CrashReporterWin* GetInstance(); +#if defined(_WIN64) + static void SetUnhandledExceptionFilter(); +#endif - void InitBreakpad(const std::string& product_name, - const std::string& version, - const std::string& company_name, - const std::string& submit_url, - const base::FilePath& crashes_dir, - bool upload_to_server, - bool skip_system_crash_handler) override; + void Init(const std::string& product_name, + const std::string& company_name, + const std::string& submit_url, + const base::FilePath& crashes_dir, + bool upload_to_server, + bool skip_system_crash_handler) override; void SetUploadParameters() override; - // Crashes the process after generating a dump for the provided exception. - int CrashForException(EXCEPTION_POINTERS* info); + crashpad::CrashpadClient& GetCrashpadClient(); private: friend struct base::DefaultSingletonTraits; - CrashReporterWin(); ~CrashReporterWin() override; - static bool FilterCallback(void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion); + void UpdatePipeName(); - static bool MinidumpCallback(const wchar_t* dump_path, - const wchar_t* minidump_id, - void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion, - bool succeeded); - - // Returns the custom info structure based on parameters. - google_breakpad::CustomClientInfo* GetCustomInfo( - const std::string& product_name, - const std::string& version, - const std::string& company_name, - bool upload_to_server); - - // Custom information to be passed to crash handler. - std::vector custom_info_entries_; - google_breakpad::CustomClientInfo custom_info_; - - bool skip_system_crash_handler_ = false; -#ifdef _WIN64 - bool code_range_registered_ = false; -#endif - std::unique_ptr breakpad_; + crashpad::CrashpadClient crashpad_client_; DISALLOW_COPY_AND_ASSIGN(CrashReporterWin); }; diff --git a/atom/common/crash_reporter/win/crash_service_main.cc b/atom/common/crash_reporter/win/crash_service_main.cc index c25ee858de0b..99caaaa391fd 100644 --- a/atom/common/crash_reporter/win/crash_service_main.cc +++ b/atom/common/crash_reporter/win/crash_service_main.cc @@ -4,21 +4,22 @@ #include "atom/common/crash_reporter/win/crash_service_main.h" +#include + +#include "atom/common/crash_reporter/crash_reporter.h" #include "atom/common/crash_reporter/win/crash_service.h" #include "base/at_exit.h" #include "base/command_line.h" #include "base/files/file_util.h" #include "base/logging.h" #include "base/strings/string_util.h" +#include "base/strings/utf_string_conversions.h" +#include "third_party/crashpad/crashpad/handler/handler_main.h" namespace crash_service { namespace { -const char kApplicationName[] = "application-name"; -const char kCrashesDirectory[] = "crashes-directory"; - -const wchar_t kPipeNameFormat[] = L"\\\\.\\pipe\\$1 Crash Service"; const wchar_t kStandardLogFile[] = L"operation_log.txt"; void InvalidParameterHandler(const wchar_t*, @@ -37,35 +38,29 @@ bool CreateCrashServiceDirectory(const base::FilePath& temp_dir) { return true; } +void RemoveArgs(std::vector* args) { + args->erase( + std::remove_if(args->begin(), args->end(), [](const std::string& str) { + return base::StartsWith(str, "--type", base::CompareCase::SENSITIVE) || + base::StartsWith( + str, + std::string("--") + crash_reporter::kCrashesDirectoryKey, + base::CompareCase::INSENSITIVE_ASCII); + })); +} + } // namespace. -int Main(const wchar_t* cmd) { +int Main(std::vector* args) { // Ignore invalid parameter errors. _set_invalid_parameter_handler(InvalidParameterHandler); // Initialize all Chromium things. base::AtExitManager exit_manager; - base::CommandLine::Init(0, NULL); - base::CommandLine& cmd_line = *base::CommandLine::ForCurrentProcess(); - - // Use the application's name as pipe name and output directory. - if (!cmd_line.HasSwitch(kApplicationName)) { - LOG(ERROR) << "Application's name must be specified with --" - << kApplicationName; - return 1; - } - std::wstring application_name = - cmd_line.GetSwitchValueNative(kApplicationName); - - if (!cmd_line.HasSwitch(kCrashesDirectory)) { - LOG(ERROR) << "Crashes directory path must be specified with --" - << kCrashesDirectory; - return 1; - } - + base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess(); // We use/create a directory under the user's temp folder, for logging. base::FilePath operating_dir( - cmd_line.GetSwitchValueNative(kCrashesDirectory)); + cmd_line->GetSwitchValueNative(crash_reporter::kCrashesDirectoryKey)); CreateCrashServiceDirectory(operating_dir); base::FilePath log_file = operating_dir.Append(kStandardLogFile); @@ -78,27 +73,9 @@ int Main(const wchar_t* cmd) { // Logging with pid, tid and timestamp. logging::SetLogItems(true, true, true, false); - VLOG(1) << "Session start. cmdline is [" << cmd << "]"; - - // Setting the crash reporter. - base::string16 pipe_name = - base::ReplaceStringPlaceholders(kPipeNameFormat, application_name, NULL); - cmd_line.AppendSwitch("no-window"); - cmd_line.AppendSwitchASCII("max-reports", "128"); - cmd_line.AppendSwitchASCII("reporter", ATOM_PROJECT_NAME "-crash-service"); - cmd_line.AppendSwitchNative("pipe-name", pipe_name); - - breakpad::CrashService crash_service; - if (!crash_service.Initialize(application_name, operating_dir, operating_dir)) - return 2; - - VLOG(1) << "Ready to process crash requests"; - - // Enter the message loop. - int retv = crash_service.ProcessingLoop(); - // Time to exit. - VLOG(1) << "Session end. return code is " << retv; - return retv; + // Crashpad cannot handle unknown arguments, so we need to remove it + RemoveArgs(args); + return crashpad::HandlerMain(args->size(), args->data(), nullptr); } } // namespace crash_service diff --git a/atom/common/crash_reporter/win/crash_service_main.h b/atom/common/crash_reporter/win/crash_service_main.h index b536313dbc32..4481fb50ae66 100644 --- a/atom/common/crash_reporter/win/crash_service_main.h +++ b/atom/common/crash_reporter/win/crash_service_main.h @@ -5,10 +5,12 @@ #ifndef ATOM_COMMON_CRASH_REPORTER_WIN_CRASH_SERVICE_MAIN_H_ #define ATOM_COMMON_CRASH_REPORTER_WIN_CRASH_SERVICE_MAIN_H_ +#include + namespace crash_service { // Program entry, should be called by main(); -int Main(const wchar_t* cmd_line); +int Main(std::vector* args); } // namespace crash_service diff --git a/atom/renderer/electron_api_service_impl.cc b/atom/renderer/electron_api_service_impl.cc index 380cc6755a46..99bbcdbbe29e 100644 --- a/atom/renderer/electron_api_service_impl.cc +++ b/atom/renderer/electron_api_service_impl.cc @@ -8,8 +8,10 @@ #include #include +#include "atom/common/atom_constants.h" #include "atom/common/heap_snapshot.h" #include "atom/common/native_mate_converters/value_converter.h" +#include "base/environment.h" #include "base/macros.h" #include "base/threading/thread_restrictions.h" #include "electron/atom/common/api/event_emitter_caller.h" @@ -147,6 +149,14 @@ void ElectronApiServiceImpl::Message(bool internal, } } +void ElectronApiServiceImpl::UpdateCrashpadPipeName( + const std::string& pipe_name) { +#if defined(OS_WIN) + std::unique_ptr env(base::Environment::Create()); + env->SetVar(kCrashpadPipeName, pipe_name); +#endif +} + void ElectronApiServiceImpl::TakeHeapSnapshot( mojo::ScopedHandle file, TakeHeapSnapshotCallback callback) { diff --git a/atom/renderer/electron_api_service_impl.h b/atom/renderer/electron_api_service_impl.h index 19c51b81091d..4c198a9cccce 100644 --- a/atom/renderer/electron_api_service_impl.h +++ b/atom/renderer/electron_api_service_impl.h @@ -29,7 +29,7 @@ class ElectronApiServiceImpl : public mojom::ElectronRenderer, const std::string& channel, base::Value arguments, int32_t sender_id) override; - + void UpdateCrashpadPipeName(const std::string& pipe_name) override; void TakeHeapSnapshot(mojo::ScopedHandle file, TakeHeapSnapshotCallback callback) override; diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index ae8dab702537..74e1e0c4d293 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -66,26 +66,7 @@ reports temporarily. You can test this out by calling `process.crash()` to crash first call `start` you can call `addExtraParameter` on macOS or call `start` again with the new/updated `extra` parameters on Linux and Windows. -**Note:** To collect crash reports from child process in Windows, you need to add this extra code as well. -This will start the process that will monitor and send the crash reports. Replace `submitURL`, `productName` -and `crashesDirectory` with appropriate values. - -```js -const args = [ - `--reporter-url=${submitURL}`, - `--application-name=${productName}`, - `--crashes-directory=${crashesDirectory}` -] -const env = { - ELECTRON_INTERNAL_CRASH_SERVICE: 1 -} -spawn(process.execPath, args, { - env: env, - detached: true -}) -``` - -**Note:** On macOS, Electron uses a new `crashpad` client for crash collection and reporting. +**Note:** On macOS and windows, Electron uses a new `crashpad` client for crash collection and reporting. If you want to enable crash reporting, initializing `crashpad` from the main process using `crashReporter.start` is required regardless of which process you want to collect crashes from. Once initialized this way, the crashpad handler collects crashes from all processes. You still have to call `crashReporter.start` from the renderer or child process, otherwise crashes from @@ -104,14 +85,14 @@ Returns [`CrashReport[]`](structures/crash-report.md): Returns all uploaded crash reports. Each report contains the date and uploaded ID. -### `crashReporter.getUploadToServer()` _Linux_ _macOS_ +### `crashReporter.getUploadToServer()` Returns `Boolean` - Whether reports should be submitted to the server. Set through the `start` method or `setUploadToServer`. **Note:** This API can only be called from the main process. -### `crashReporter.setUploadToServer(uploadToServer)` _Linux_ _macOS_ +### `crashReporter.setUploadToServer(uploadToServer)` * `uploadToServer` Boolean _macOS_ - Whether reports should be submitted to the server. @@ -120,15 +101,15 @@ called before `start` is called. **Note:** This API can only be called from the main process. -### `crashReporter.addExtraParameter(key, value)` _macOS_ +### `crashReporter.addExtraParameter(key, value)` _macOS_ _Windows_ * `key` String - Parameter key, must be less than 64 characters long. * `value` String - Parameter value, must be less than 64 characters long. Set an extra parameter to be sent with the crash report. The values -specified here will be sent in addition to any values set via the `extra` option when `start` was called. This API is only available on macOS, if you need to add/update extra parameters on Linux and Windows after your first call to `start` you can call `start` again with the updated `extra` options. +specified here will be sent in addition to any values set via the `extra` option when `start` was called. This API is only available on macOS and windows, if you need to add/update extra parameters on Linux after your first call to `start` you can call `start` again with the updated `extra` options. -### `crashReporter.removeExtraParameter(key)` _macOS_ +### `crashReporter.removeExtraParameter(key)` _macOS_ _Windows_ * `key` String - Parameter key, must be less than 64 characters long. diff --git a/filenames.gni b/filenames.gni index 8800316630c2..963cc453663b 100644 --- a/filenames.gni +++ b/filenames.gni @@ -497,12 +497,10 @@ filenames = { "atom/common/crash_reporter/crash_reporter_linux.h", "atom/common/crash_reporter/crash_reporter_mac.h", "atom/common/crash_reporter/crash_reporter_mac.mm", - "atom/common/crash_reporter/crash_reporter_win.cc", "atom/common/crash_reporter/crash_reporter_win.h", + "atom/common/crash_reporter/crash_reporter_win.cc", "atom/common/crash_reporter/linux/crash_dump_handler.cc", "atom/common/crash_reporter/linux/crash_dump_handler.h", - "atom/common/crash_reporter/win/crash_service.cc", - "atom/common/crash_reporter/win/crash_service.h", "atom/common/crash_reporter/win/crash_service_main.cc", "atom/common/crash_reporter/win/crash_service_main.h", "atom/common/gin_util.h", diff --git a/lib/browser/crash-reporter-init.js b/lib/browser/crash-reporter-init.js index 0c7d501691e6..4df564e8b3a2 100644 --- a/lib/browser/crash-reporter-init.js +++ b/lib/browser/crash-reporter-init.js @@ -16,31 +16,10 @@ const getTempDirectory = function () { exports.crashReporterInit = function (options) { const productName = options.productName || app.name const crashesDirectory = path.join(getTempDirectory(), `${productName} Crashes`) - let crashServicePid - - if (process.platform === 'win32') { - const env = { - ELECTRON_INTERNAL_CRASH_SERVICE: 1 - } - const args = [ - '--reporter-url=' + options.submitURL, - '--application-name=' + productName, - '--crashes-directory=' + crashesDirectory, - '--v=1' - ] - - const crashServiceProcess = require('child_process').spawn(process.helperExecPath, args, { - env, - detached: true - }) - - crashServicePid = crashServiceProcess.pid - } return { productName, crashesDirectory, - crashServicePid, appVersion: app.getVersion() } } diff --git a/lib/common/crash-reporter.js b/lib/common/crash-reporter.js index fe62bab5fa87..ae651f9523ba 100644 --- a/lib/common/crash-reporter.js +++ b/lib/common/crash-reporter.js @@ -34,7 +34,6 @@ class CrashReporter { this.productName = ret.productName this.crashesDirectory = ret.crashesDirectory - this.crashServicePid = ret.crashServicePid if (extra._productName == null) extra._productName = ret.productName if (extra._companyName == null) extra._companyName = companyName diff --git a/patches/chromium/.patches b/patches/chromium/.patches index a4f987f6e5b0..f7053cd83884 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -67,7 +67,6 @@ fix_disable_usage_of_setapplicationisdaemon_and.patch disable_network_services_by_default.patch unsandboxed_ppapi_processes_skip_zygote.patch patch_the_ensure_gn_version_py_script_to_work_on_mac_ci.patch -partially_revert_fb4ab3be3fc0024d00358d5b7816d3215a8ff20c.patch build_add_electron_tracing_category.patch add_contentgpuclient_precreatemessageloop_callback.patch disable_custom_libcxx_on_windows.patch @@ -77,3 +76,4 @@ fix_breakpad_symbol_generation_on_linux_arm.patch frame_host_manager.patch cross_site_document_resource_handler.patch woa_compiler_workaround.patch +crashpad_pid_check.patch diff --git a/patches/chromium/crashpad_pid_check.patch b/patches/chromium/crashpad_pid_check.patch new file mode 100644 index 000000000000..f3e8ac39f697 --- /dev/null +++ b/patches/chromium/crashpad_pid_check.patch @@ -0,0 +1,38 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Cheng Zhao +Date: Tue Jun 4 11:30:12 JST 2019 +Subject: crashpad_pid_check.patch + +When both browser process and renderer process are connecting to the pipe, +the API may return the PID of browser process as real_pid, which is different +from the PID of renderer process. + +This is caused by the crashReporter getting started after the sanbox, after +we redesign crashReporter's API to make it alwasy start before the +sanbox, we can remove this patch. + +See following links for more: +https://github.com/electron/electron/pull/18483#discussion_r292703588 +https://github.com/electron/electron/pull/18483#issuecomment-501090683 + +diff --git a/third_party/crashpad/crashpad/util/win/exception_handler_server.cc b/third_party/crashpad/crashpad/util/win/exception_handler_server.cc +index 2593ff2de032..e89b8ff675be 100644 +--- a/third_party/crashpad/crashpad/util/win/exception_handler_server.cc ++++ b/third_party/crashpad/crashpad/util/win/exception_handler_server.cc +@@ -448,9 +448,16 @@ bool ExceptionHandlerServer::ServiceClientConnection( + DWORD real_pid = 0; + if (get_named_pipe_client_process_id(service_context.pipe(), &real_pid) && + message.registration.client_process_id != real_pid) { ++ // Electron: When both browser process and renderer process are connecting ++ // to the pipe, the API may return the PID of browser process as real_pid, ++ // which is different from the PID of renderer process. ++ // ++ // I don't understand why Chromium does not have this issue. ++#if 0 + LOG(ERROR) << "forged client pid, real pid: " << real_pid + << ", got: " << message.registration.client_process_id; + return false; ++#endif + } + } + diff --git a/patches/chromium/partially_revert_fb4ab3be3fc0024d00358d5b7816d3215a8ff20c.patch b/patches/chromium/partially_revert_fb4ab3be3fc0024d00358d5b7816d3215a8ff20c.patch deleted file mode 100644 index 900d4f8c2e38..000000000000 --- a/patches/chromium/partially_revert_fb4ab3be3fc0024d00358d5b7816d3215a8ff20c.patch +++ /dev/null @@ -1,141 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Nitish Sakhawalkar -Date: Fri, 19 Apr 2019 14:01:12 -0700 -Subject: Partially revert fb4ab3be3fc0024d00358d5b7816d3215a8ff20c - -This change partially reverts the changes made in gin in fb4ab3be3fc0024d00358d5b7816d3215a8ff20c. Electron uses breakpad for windows, while chromium has moved to crashpad. Once electron uses crashpad for windows, we can get rid of this change. - -diff --git a/gin/debug_impl.cc b/gin/debug_impl.cc -index ca0577ea4caabce42bb4ec1aad8062b59eaaa8e4..5c3b7ffc932f063c8ad458d92643564edba393dc 100644 ---- a/gin/debug_impl.cc -+++ b/gin/debug_impl.cc -@@ -8,6 +8,10 @@ namespace gin { - - namespace { - v8::JitCodeEventHandler g_jit_code_event_handler = NULL; -+#if defined(OS_WIN) -+Debug::CodeRangeCreatedCallback g_code_range_created_callback = NULL; -+Debug::CodeRangeDeletedCallback g_code_range_deleted_callback = NULL; -+#endif - } // namespace - - // static -@@ -17,9 +21,13 @@ void Debug::SetJitCodeEventHandler(v8::JitCodeEventHandler event_handler) { - - #if defined(OS_WIN) - // static --void Debug::SetUnhandledExceptionCallback( -- v8::UnhandledExceptionCallback callback) { -- v8::V8::SetUnhandledExceptionCallback(callback); -+void Debug::SetCodeRangeCreatedCallback(CodeRangeCreatedCallback callback) { -+ g_code_range_created_callback = callback; -+} -+ -+// static -+void Debug::SetCodeRangeDeletedCallback(CodeRangeDeletedCallback callback) { -+ g_code_range_deleted_callback = callback; - } - #endif - -@@ -28,4 +36,16 @@ v8::JitCodeEventHandler DebugImpl::GetJitCodeEventHandler() { - return g_jit_code_event_handler; - } - -+#if defined(OS_WIN) -+// static -+Debug::CodeRangeCreatedCallback DebugImpl::GetCodeRangeCreatedCallback() { -+ return g_code_range_created_callback; -+} -+ -+// static -+Debug::CodeRangeDeletedCallback DebugImpl::GetCodeRangeDeletedCallback() { -+ return g_code_range_deleted_callback; -+} -+#endif -+ - } // namespace gin -diff --git a/gin/debug_impl.h b/gin/debug_impl.h -index b0b7931f3e10592577858f048eab37d4eff4c18f..b88c0b6c0896f60c6421023fe5783b614d596769 100644 ---- a/gin/debug_impl.h -+++ b/gin/debug_impl.h -@@ -13,6 +13,10 @@ namespace gin { - class DebugImpl { - public: - static v8::JitCodeEventHandler GetJitCodeEventHandler(); -+#if defined(OS_WIN) -+ static Debug::CodeRangeCreatedCallback GetCodeRangeCreatedCallback(); -+ static Debug::CodeRangeDeletedCallback GetCodeRangeDeletedCallback(); -+#endif - }; - - } // namespace gin -diff --git a/gin/isolate_holder.cc b/gin/isolate_holder.cc -index 4dd48f6a871cccb374c58adacb8ad9a80da89a5a..ec61b873d4e2dcdca833c8503beabb88d0798f2d 100644 ---- a/gin/isolate_holder.cc -+++ b/gin/isolate_holder.cc -@@ -91,9 +91,31 @@ IsolateHolder::IsolateHolder( - - isolate_memory_dump_provider_.reset( - new V8IsolateMemoryDumpProvider(this, task_runner)); -+#if defined(OS_WIN) -+ { -+ void* code_range; -+ size_t size; -+ isolate_->GetCodeRange(&code_range, &size); -+ Debug::CodeRangeCreatedCallback callback = -+ DebugImpl::GetCodeRangeCreatedCallback(); -+ if (code_range && size && callback) -+ callback(code_range, size); -+ } -+#endif - } - - IsolateHolder::~IsolateHolder() { -+#if defined(OS_WIN) -+ { -+ void* code_range; -+ size_t size; -+ isolate_->GetCodeRange(&code_range, &size); -+ Debug::CodeRangeDeletedCallback callback = -+ DebugImpl::GetCodeRangeDeletedCallback(); -+ if (code_range && callback) -+ callback(code_range); -+ } -+#endif - isolate_memory_dump_provider_.reset(); - isolate_data_.reset(); - isolate_->Dispose(); -diff --git a/gin/public/debug.h b/gin/public/debug.h -index 4e567876f7ac0f010783eded5e57f8b2293542d8..8c2eee341c3bc64331926e258e1fd20080373a80 100644 ---- a/gin/public/debug.h -+++ b/gin/public/debug.h -@@ -24,11 +24,25 @@ class GIN_EXPORT Debug { - static void SetJitCodeEventHandler(v8::JitCodeEventHandler event_handler); - - #if defined(OS_WIN) -- /* Sets a callback that is invoked for exceptions that arise in V8-generated -- * code (jitted code or embedded builtins). -+ typedef void (__cdecl *CodeRangeCreatedCallback)(void*, size_t); -+ -+ /* Sets a callback that is invoked for every new code range being created. -+ * -+ * On Win64, exception handling in jitted code is broken due to the fact -+ * that JS stack frames are not ABI compliant. It is possible to install -+ * custom handlers for the code range which holds the jitted code to work -+ * around this issue. -+ * -+ * https://code.google.com/p/v8/issues/detail?id=3598 -+ */ -+ static void SetCodeRangeCreatedCallback(CodeRangeCreatedCallback callback); -+ -+ typedef void (__cdecl *CodeRangeDeletedCallback)(void*); -+ -+ /* Sets a callback that is invoked for every previously registered code range -+ * when it is deleted. - */ -- static void SetUnhandledExceptionCallback( -- v8::UnhandledExceptionCallback callback); -+ static void SetCodeRangeDeletedCallback(CodeRangeDeletedCallback callback); - #endif - }; - diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js index 2f3a386df9f9..2aaaefdd58eb 100644 --- a/spec/api-crash-reporter-spec.js +++ b/spec/api-crash-reporter-spec.js @@ -47,10 +47,6 @@ describe('crashReporter module', () => { afterEach(() => closeWindow(w).then(() => { w = null })) - afterEach(() => { - stopCrashService() - }) - afterEach((done) => { if (stopServer != null) { stopServer(done) @@ -60,9 +56,6 @@ describe('crashReporter module', () => { }) it('should send minidump when renderer crashes', function (done) { - // TODO(alexeykuzmin): Skip the test instead of marking it as passed. - if (process.env.APPVEYOR === 'True') return done() - this.timeout(180000) stopServer = startServer({ @@ -75,34 +68,17 @@ describe('crashReporter module', () => { }) it('should send minidump when node processes crash', function (done) { - // TODO(alexeykuzmin): Skip the test instead of marking it as passed. - if (process.env.APPVEYOR === 'True') return done() - this.timeout(180000) stopServer = startServer({ callback (port) { - const crashesDir = path.join(app.getPath('temp'), `${process.platform === 'win32' ? 'Zombies' : app.name} Crashes`) + const crashesDir = path.join(app.getPath('temp'), `${app.name} Crashes`) const version = app.getVersion() const crashPath = path.join(fixtures, 'module', 'crash.js') - if (process.platform === 'win32') { - const crashServiceProcess = childProcess.spawn(process.execPath, [ - `--reporter-url=http://127.0.0.1:${port}`, - '--application-name=Zombies', - `--crashes-directory=${crashesDir}` - ], { - env: { - ELECTRON_INTERNAL_CRASH_SERVICE: 1 - }, - detached: true - }) - remote.process.crashServicePid = crashServiceProcess.pid - } - childProcess.fork(crashPath, [port, version, crashesDir], { silent: true }) }, - processType: 'browser', + processType: 'node', done: done }) }) @@ -113,14 +89,18 @@ describe('crashReporter module', () => { let dumpFile let crashesDir = crashReporter.getCrashesDirectory() const existingDumpFiles = new Set() - if (process.platform === 'darwin') { + if (process.platform !== 'linux') { // crashpad puts the dump files in the "completed" subdirectory - crashesDir = path.join(crashesDir, 'completed') + if (process.platform === 'darwin') { + crashesDir = path.join(crashesDir, 'completed') + } else { + crashesDir = path.join(crashesDir, 'reports') + } crashReporter.setUploadToServer(false) } const testDone = (uploaded) => { if (uploaded) return done(new Error('Uploaded crash report')) - if (process.platform === 'darwin') crashReporter.setUploadToServer(true) + if (process.platform !== 'linux') crashReporter.setUploadToServer(true) expect(fs.existsSync(dumpFile)).to.be.true() done() } @@ -170,9 +150,6 @@ describe('crashReporter module', () => { }) it('should send minidump with updated extra parameters', function (done) { - // TODO(alexeykuzmin): Skip the test instead of marking it as passed. - if (process.env.APPVEYOR === 'True') return done() - this.timeout(180000) stopServer = startServer({ @@ -212,7 +189,7 @@ describe('crashReporter module', () => { describe('getProductName', () => { it('returns the product name if one is specified', () => { const name = crashReporter.getProductName() - const expectedName = (process.platform === 'darwin') ? 'Electron Test' : 'Zombies' + const expectedName = 'Electron Test' expect(name).to.equal(expectedName) }) }) @@ -244,12 +221,7 @@ describe('crashReporter module', () => { describe('getCrashesDirectory', () => { it('correctly returns the directory', () => { const crashesDir = crashReporter.getCrashesDirectory() - let dir - if (process.platform === 'win32') { - dir = `${app.getPath('temp')}/Zombies Crashes` - } else { - dir = `${app.getPath('temp')}/Electron Test Crashes` - } + const dir = path.join(app.getPath('temp'), 'Electron Test Crashes') expect(crashesDir).to.equal(dir) }) }) @@ -294,7 +266,7 @@ describe('crashReporter module', () => { expect(() => require('electron').crashReporter.getUploadToServer()).to.throw() }) it('returns true when uploadToServer is set to true', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -308,7 +280,7 @@ describe('crashReporter module', () => { expect(crashReporter.getUploadToServer()).to.be.true() }) it('returns false when uploadToServer is set to false', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -329,7 +301,7 @@ describe('crashReporter module', () => { expect(() => require('electron').crashReporter.setUploadToServer('arg')).to.throw() }) it('sets uploadToServer false when called with false', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -344,7 +316,7 @@ describe('crashReporter module', () => { expect(crashReporter.getUploadToServer()).to.be.false() }) it('sets uploadToServer true when called with true', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -371,7 +343,7 @@ describe('crashReporter module', () => { expect(parameters).to.be.an('object') }) it('adds a parameter to current parameters', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -386,7 +358,7 @@ describe('crashReporter module', () => { expect(crashReporter.getParameters()).to.have.a.property('hello') }) it('removes a parameter from current parameters', function () { - if (process.platform !== 'darwin') { + if (process.platform === 'linux') { // FIXME(alexeykuzmin): Skip the test. // this.skip() return @@ -467,7 +439,7 @@ const startServer = ({ callback, processType, done }) => { server.listen(port, '127.0.0.1', () => { port = server.address().port remote.process.port = port - if (process.platform === 'darwin') { + if (process.platform !== 'linux') { crashReporter.start({ companyName: 'Umbrella Corporation', submitURL: 'http://127.0.0.1:' + port @@ -485,17 +457,3 @@ const startServer = ({ callback, processType, done }) => { }) } } - -const stopCrashService = () => { - const { crashServicePid } = remote.process - if (crashServicePid) { - remote.process.crashServicePid = 0 - try { - process.kill(crashServicePid) - } catch (error) { - if (error.code !== 'ESRCH') { - throw error - } - } - } -} diff --git a/spec/fixtures/api/crash-restart.html b/spec/fixtures/api/crash-restart.html index 7dec07ad8819..0ee4ad53503b 100644 --- a/spec/fixtures/api/crash-restart.html +++ b/spec/fixtures/api/crash-restart.html @@ -18,12 +18,8 @@ crashReporter.start({ } }) -if (process.platform === 'win32') { - ipcRenderer.sendSync('crash-service-pid', crashReporter.crashServicePid) -} - setImmediate(() => { - if (process.platform === 'darwin') { + if (process.platform !== 'linux') { crashReporter.addExtraParameter('extra2', 'extra2') crashReporter.removeExtraParameter('extra3') } else { diff --git a/spec/fixtures/api/crash.html b/spec/fixtures/api/crash.html index 588737c79299..bcce825426c9 100644 --- a/spec/fixtures/api/crash.html +++ b/spec/fixtures/api/crash.html @@ -19,10 +19,6 @@ } }) - if (process.platform === 'win32') { - ipcRenderer.sendSync('crash-service-pid', crashReporter.crashServicePid) - } - if (!uploadToServer) { ipcRenderer.sendSync('list-existing-dumps') } @@ -31,4 +27,4 @@ - \ No newline at end of file + diff --git a/spec/static/main.js b/spec/static/main.js index 499345b9d217..d1ddd3637bec 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -19,7 +19,6 @@ let window = null // will be used by crash-reporter spec. process.port = 0 -process.crashServicePid = 0 v8.setFlagsFromString('--expose_gc') app.commandLine.appendSwitch('js-flags', '--expose_gc') @@ -299,11 +298,6 @@ ipcMain.on('handle-unhandled-rejection', (event, message) => { }) }) -ipcMain.on('crash-service-pid', (event, pid) => { - process.crashServicePid = pid - event.returnValue = null -}) - // Suspend listeners until the next event and then restore them const suspendListeners = (emitter, eventName, callback) => { const listeners = emitter.listeners(eventName)