Chromecast: optionally use the system's crash reporter.
If enabled, forward crashes to the system's crash handler, rather than
processing them internally via Breakpad/Crashpad.
This introduces a new GN argument, use_system_crash_handler, which
enables forwarding to the system crash handler. By default it instead
uses the builtin crash handling from Breakpad/Crashpad.
Only supports the external_mojo processes.
Also enable crash forwarding by default for the crash_uploader, since it
doesn't include internal crash handling.
Merge-With: eureka-internal/413496
Bug: internal b/156659049
Bug: internal b/157693574
Test: build
Change-Id: Iced0d8ce02f7367ac8c9f2ef46d232f2d4b27732
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2222971
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Commit-Queue: Joshua LeVasseur <jlevasseur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780502}
diff --git a/chromecast/chromecast.gni b/chromecast/chromecast.gni
index 3dc69fc..6b59e2b 100644
--- a/chromecast/chromecast.gni
+++ b/chromecast/chromecast.gni
@@ -134,6 +134,10 @@
# Set to true to enable media overlay for volume bar, etc.
enable_media_overlay = false
+
+ # Set to true to forward crashes to the system's crash handler instead of
+ # handling them internally. This disables the built-in crash handler.
+ use_system_crash_handler = false
}
declare_args() {
diff --git a/chromecast/crash/BUILD.gn b/chromecast/crash/BUILD.gn
index 6f1b958..f6b4fdf 100644
--- a/chromecast/crash/BUILD.gn
+++ b/chromecast/crash/BUILD.gn
@@ -94,6 +94,7 @@
"//chromecast/base:default_create_sys_info",
"//chromecast/public",
"//chromecast/system/reboot:reboot_util",
+ "//third_party/crashpad/crashpad/client",
]
}
diff --git a/chromecast/crash/linux/DEPS b/chromecast/crash/linux/DEPS
index 1739285..612f651 100644
--- a/chromecast/crash/linux/DEPS
+++ b/chromecast/crash/linux/DEPS
@@ -2,4 +2,5 @@
"+chromecast/system",
"+components/metrics",
"+components/prefs",
+ "+third_party/crashpad/crashpad/client",
]
diff --git a/chromecast/crash/linux/crash_uploader.cc b/chromecast/crash/linux/crash_uploader.cc
index a52b977..c96ce84 100644
--- a/chromecast/crash/linux/crash_uploader.cc
+++ b/chromecast/crash/linux/crash_uploader.cc
@@ -21,6 +21,7 @@
#include "chromecast/crash/linux/minidump_uploader.h"
#include "chromecast/public/cast_sys_info.h"
#include "chromecast/system/reboot/reboot_util.h"
+#include "third_party/crashpad/crashpad/client/crashpad_info.h"
namespace {
@@ -37,6 +38,10 @@
chromecast::RegisterPathProvider();
logging::InitLogging(logging::LoggingSettings());
+ // Allow the system crash handler to handle our own crashes.
+ crashpad::CrashpadInfo::GetCrashpadInfo()
+ ->set_system_crash_reporter_forwarding(crashpad::TriState::kEnabled);
+
LOG(INFO) << "Starting crash uploader...";
// Nice +19. Crash uploads are not time critical and we don't want to
diff --git a/chromecast/external_mojo/external_service_support/BUILD.gn b/chromecast/external_mojo/external_service_support/BUILD.gn
index a76ac50..9d6f3fc 100644
--- a/chromecast/external_mojo/external_service_support/BUILD.gn
+++ b/chromecast/external_mojo/external_service_support/BUILD.gn
@@ -34,21 +34,23 @@
]
if (!is_fuchsia) {
- sources += [
- "crash_reporter_client.cc",
- "crash_reporter_client.h",
- ]
- deps += [
- "//chromecast/crash",
- "//components/crash/core/app",
- ]
+ sources += [ "crash_reporter_client.h" ]
+
+ if (use_system_crash_handler) {
+ sources += [ "crash_reporter_system.cc" ]
+ deps += [ "//third_party/crashpad/crashpad/client" ]
+ } else {
+ sources += [ "crash_reporter_builtin.cc" ]
+ deps += [
+ "//chromecast/crash",
+ "//components/crash/core/app",
+ ]
+ }
}
}
source_set("tracing_client_hdr") {
- sources = [
- "tracing_client.h",
- ]
+ sources = [ "tracing_client.h" ]
}
source_set("tracing_client_dummy") {
@@ -56,9 +58,7 @@
"tracing_client_dummy.cc",
"tracing_client_dummy.h",
]
- deps = [
- ":tracing_client_hdr",
- ]
+ deps = [ ":tracing_client_hdr" ]
}
source_set("tracing_client_impl") {
@@ -80,9 +80,7 @@
}
group("tracing_client") {
- public_deps = [
- ":tracing_client_hdr",
- ]
+ public_deps = [ ":tracing_client_hdr" ]
if (enable_external_mojo_tracing) {
public_deps += [ ":tracing_client_impl" ]
} else {
@@ -91,15 +89,11 @@
}
source_set("service_process") {
- sources = [
- "service_process.h",
- ]
+ sources = [ "service_process.h" ]
}
source_set("standalone_service_main") {
- sources = [
- "standalone_service_main.cc",
- ]
+ sources = [ "standalone_service_main.cc" ]
deps = [
":external_service",
":process_setup",
@@ -108,9 +102,7 @@
"//chromecast/external_mojo/public/cpp:common",
"//mojo/core/embedder",
]
- public_deps = [
- ":service_process",
- ]
+ public_deps = [ ":service_process" ]
}
source_set("chromium_service") {
@@ -129,9 +121,7 @@
}
executable("standalone_mojo_broker") {
- sources = [
- "standalone_mojo_broker.cc",
- ]
+ sources = [ "standalone_mojo_broker.cc" ]
deps = [
":external_service",
":process_setup",
diff --git a/chromecast/external_mojo/external_service_support/DEPS b/chromecast/external_mojo/external_service_support/DEPS
index 7a876f7ed..b083153b 100644
--- a/chromecast/external_mojo/external_service_support/DEPS
+++ b/chromecast/external_mojo/external_service_support/DEPS
@@ -3,5 +3,6 @@
"+components/crash/core/app",
"+mojo/core/embedder",
"+services/tracing",
+ "+third_party/crashpad/crashpad/client",
"+third_party/perfetto/include/perfetto",
]
diff --git a/chromecast/external_mojo/external_service_support/crash_reporter_builtin.cc b/chromecast/external_mojo/external_service_support/crash_reporter_builtin.cc
new file mode 100644
index 0000000..bbcd54de
--- /dev/null
+++ b/chromecast/external_mojo/external_service_support/crash_reporter_builtin.cc
@@ -0,0 +1,72 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/debug/stack_trace.h"
+#include "base/no_destructor.h"
+#include "base/time/time.h"
+#include "build/build_config.h"
+#include "chromecast/crash/linux/crash_util.h"
+#include "chromecast/external_mojo/external_service_support/crash_reporter_client.h"
+#include "components/crash/core/app/breakpad_linux.h"
+#include "components/crash/core/app/crash_reporter_client.h"
+
+namespace chromecast {
+namespace external_service_support {
+
+namespace {
+
+class CrashReporterBuiltin : public crash_reporter::CrashReporterClient {
+ public:
+ CrashReporterBuiltin()
+ : start_time_ms_(base::TimeTicks::Now().since_origin().InMilliseconds()) {
+ }
+
+ ~CrashReporterBuiltin() override = default;
+
+ CrashReporterBuiltin(const CrashReporterBuiltin&) = delete;
+ CrashReporterBuiltin& operator=(const CrashReporterBuiltin&) = delete;
+
+ // crash_reporter::CrashReporterClient implementation:
+ bool EnableBreakpadForProcess(const std::string& process_type) override {
+ return true;
+ }
+ bool HandleCrashDump(const char* crashdump_filename,
+ uint64_t crash_pid) override {
+ chromecast::CrashUtil::RequestUploadCrashDump(crashdump_filename, crash_pid,
+ start_time_ms_);
+ // Always return true to indicate that this crash dump has been processed,
+ // so that it won't fallback to Chrome's default uploader.
+ return true;
+ }
+ bool GetCollectStatsConsent() override {
+ // Returning true allows writing the crash dump to disk, but not to
+ // upload. The uploader will check whether the device has opted in to crash
+ // uploading. It would be more optimal to avoid writing the crash dump if
+ // the device is opted out, but the complexity of checking that flag would
+ // increase the probability of a crash within the crash handler.
+ return true;
+ }
+
+ private:
+ const uint64_t start_time_ms_;
+};
+
+CrashReporterBuiltin* GetCrashReporterClient() {
+ static base::NoDestructor<CrashReporterBuiltin> crash_reporter_client;
+ return crash_reporter_client.get();
+}
+} // namespace
+
+// static
+void CrashReporterClient::Init() {
+#if !defined(OFFICIAL_BUILD)
+ base::debug::EnableInProcessStackDumping();
+#endif // !defined(OFFICIAL_BUILD)
+
+ crash_reporter::SetCrashReporterClient(GetCrashReporterClient());
+ breakpad::InitCrashReporter(/*process_type=*/"");
+}
+
+} // namespace external_service_support
+} // namespace chromecast
diff --git a/chromecast/external_mojo/external_service_support/crash_reporter_client.cc b/chromecast/external_mojo/external_service_support/crash_reporter_client.cc
deleted file mode 100644
index 3020911..0000000
--- a/chromecast/external_mojo/external_service_support/crash_reporter_client.cc
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-#include "chromecast/external_mojo/external_service_support/crash_reporter_client.h"
-
-#include "base/no_destructor.h"
-#include "base/time/time.h"
-#include "chromecast/crash/linux/crash_util.h"
-#include "components/crash/core/app/breakpad_linux.h"
-
-namespace chromecast {
-namespace external_service_support {
-
-namespace {
-CrashReporterClient* GetCrashReporterClient() {
- static base::NoDestructor<CrashReporterClient> crash_reporter_client;
- return crash_reporter_client.get();
-}
-} // namespace
-
-// static
-void CrashReporterClient::InitCrashReporter() {
- crash_reporter::SetCrashReporterClient(GetCrashReporterClient());
- breakpad::InitCrashReporter(/*process_type=*/"");
-}
-
-CrashReporterClient::CrashReporterClient()
- : start_time_ms_(
- (base::TimeTicks::Now() - base::TimeTicks()).InMilliseconds()) {}
-
-CrashReporterClient::~CrashReporterClient() = default;
-
-bool CrashReporterClient::EnableBreakpadForProcess(
- const std::string& process_type) {
- return true;
-}
-
-bool CrashReporterClient::HandleCrashDump(const char* crashdump_filename,
- uint64_t crash_pid) {
- chromecast::CrashUtil::RequestUploadCrashDump(crashdump_filename, crash_pid,
- start_time_ms_);
-
- // Always return true to indicate that this crash dump has been processed,
- // so that it won't fallback to Chrome's default uploader.
- return true;
-}
-
-bool CrashReporterClient::GetCollectStatsConsent() {
- // Returning true allows writing the crash dump to disk, but not to
- // upload. The uploader will check whether the device has opted in to crash
- // uploading. It would be more optimal to avoid writing the crash dump if the
- // device is opted out, but the complexity of checking that flag would
- // increase the probability of a crash within the crash handler.
- return true;
-}
-
-} // namespace external_service_support
-} // namespace chromecast
diff --git a/chromecast/external_mojo/external_service_support/crash_reporter_client.h b/chromecast/external_mojo/external_service_support/crash_reporter_client.h
index efe4c936..44bf3f9 100644
--- a/chromecast/external_mojo/external_service_support/crash_reporter_client.h
+++ b/chromecast/external_mojo/external_service_support/crash_reporter_client.h
@@ -4,32 +4,12 @@
#ifndef CHROMECAST_EXTERNAL_MOJO_EXTERNAL_SERVICE_SUPPORT_CRASH_REPORTER_CLIENT_H_
#define CHROMECAST_EXTERNAL_MOJO_EXTERNAL_SERVICE_SUPPORT_CRASH_REPORTER_CLIENT_H_
-#include <cstdint>
-#include <string>
-
-#include "base/macros.h"
-#include "components/crash/core/app/crash_reporter_client.h"
-
namespace chromecast {
namespace external_service_support {
-class CrashReporterClient : public crash_reporter::CrashReporterClient {
+class CrashReporterClient {
public:
- CrashReporterClient();
- ~CrashReporterClient() override;
-
- static void InitCrashReporter();
-
- // crash_reporter::CrashReporterClient implementation:
- bool EnableBreakpadForProcess(const std::string& process_type) override;
- bool HandleCrashDump(const char* crashdump_filename,
- uint64_t crash_pid) override;
- bool GetCollectStatsConsent() override;
-
- private:
- const uint64_t start_time_ms_;
-
- DISALLOW_COPY_AND_ASSIGN(CrashReporterClient);
+ static void Init();
};
} // namespace external_service_support
diff --git a/chromecast/external_mojo/external_service_support/crash_reporter_system.cc b/chromecast/external_mojo/external_service_support/crash_reporter_system.cc
new file mode 100644
index 0000000..217a301
--- /dev/null
+++ b/chromecast/external_mojo/external_service_support/crash_reporter_system.cc
@@ -0,0 +1,18 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/external_mojo/external_service_support/crash_reporter_client.h"
+#include "third_party/crashpad/crashpad/client/crashpad_info.h"
+
+namespace chromecast {
+namespace external_service_support {
+
+// static
+void CrashReporterClient::Init() {
+ crashpad::CrashpadInfo::GetCrashpadInfo()
+ ->set_system_crash_reporter_forwarding(crashpad::TriState::kEnabled);
+}
+
+} // namespace external_service_support
+} // namespace chromecast
diff --git a/chromecast/external_mojo/external_service_support/process_setup.cc b/chromecast/external_mojo/external_service_support/process_setup.cc
index 8b8b8d45..4a63a6c 100644
--- a/chromecast/external_mojo/external_service_support/process_setup.cc
+++ b/chromecast/external_mojo/external_service_support/process_setup.cc
@@ -9,7 +9,6 @@
#include "base/base_switches.h"
#include "base/command_line.h"
-#include "base/debug/stack_trace.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "build/build_config.h"
@@ -48,12 +47,9 @@
base::FeatureList::InitializeInstance(
command_line->GetSwitchValueASCII(switches::kEnableFeatures),
command_line->GetSwitchValueASCII(switches::kDisableFeatures));
-#if !defined(OFFICIAL_BUILD)
- base::debug::EnableInProcessStackDumping();
-#endif // !defined(OFFICIAL_BUILD)
#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
- CrashReporterClient::InitCrashReporter();
+ CrashReporterClient::Init();
#endif
CHECK_NE(SIG_ERR, signal(SIGPIPE, SIG_IGN));