[go: nahoru, domu]

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));