macOS: Add option for forwarding browser process crashes to the system crash reporter (issue #1995)

include/capi/cef_crash_util_capi.h

@@ -61,6 +61,9 @@ extern "C" {
 //           information; default to "CEF">
 //  ExternalHandler=<Windows only; Name of the external handler exe to use
 //                   instead of re-launching the main exe; default to empty>
+//  BrowserCrashForwardingEnabled=<macOS only; True if browser process crashes
+//                                 should be forwarded to the system crash
+//                                 reporter; default to false>
 //  ServerURL=<crash server URL; default to empty>
 //  RateLimitEnabled=<True if uploads should be rate limited; default to true>
 //  MaxUploadsPerDay=<Max uploads per 24 hours, used if rate limit is enabled;
@@ -92,6 +95,12 @@ extern "C" {
 // directory. On Linux the CefSettings.browser_subprocess_path value will be
 // used. On macOS the existing subprocess app bundle will be used.
 //
+// If "BrowserCrashForwardingEnabled" is set to true (1) on macOS then browser
+// process crashes will be forwarded to the system crash reporter. This results
+// in the crash UI dialog being displayed to the user and crash reports being
+// logged under "~/Library/Logs/DiagnosticReports". Forwarding of crash reports
+// from non-browser processes and Debug builds is always disabled.
+//
 // If "ServerURL" is set then crashes will be uploaded as a multi-part POST
 // request to the specified URL. Otherwise, reports will only be stored locally
 // on disk.

include/cef_crash_util.h

@@ -54,6 +54,9 @@
 //           information; default to "CEF">
 //  ExternalHandler=<Windows only; Name of the external handler exe to use
 //                   instead of re-launching the main exe; default to empty>
+//  BrowserCrashForwardingEnabled=<macOS only; True if browser process crashes
+//                                 should be forwarded to the system crash
+//                                 reporter; default to false>
 //  ServerURL=<crash server URL; default to empty>
 //  RateLimitEnabled=<True if uploads should be rate limited; default to true>
 //  MaxUploadsPerDay=<Max uploads per 24 hours, used if rate limit is enabled;
@@ -85,6 +88,12 @@
 // directory. On Linux the CefSettings.browser_subprocess_path value will be
 // used. On macOS the existing subprocess app bundle will be used.
 //
+// If "BrowserCrashForwardingEnabled" is set to true on macOS then browser
+// process crashes will be forwarded to the system crash reporter. This results
+// in the crash UI dialog being displayed to the user and crash reports being
+// logged under "~/Library/Logs/DiagnosticReports". Forwarding of crash reports
+// from non-browser processes and Debug builds is always disabled.
+//
 // If "ServerURL" is set then crashes will be uploaded as a multi-part POST
 // request to the specified URL. Otherwise, reports will only be stored locally
 // on disk.

libcef/common/crash_reporter_client.cc

@@ -380,6 +380,12 @@ bool CefCrashReporterClient::ReadCrashConfigFile() {
         if (!val_str.empty())
           app_name_ = sanitizePathComponent(val_str);
       }
+#elif defined(OS_MACOSX)
+      else if (name_str == "BrowserCrashForwardingEnabled") {
+        enable_browser_crash_forwarding_ =
+            (base::EqualsCaseInsensitiveASCII(val_str, "true") ||
+             val_str == "1");
+      }
 #endif
     } else if (current_section == kCrashKeysSection) {
       size_t max_size = 0;
@@ -629,3 +635,9 @@ bool CefCrashReporterClient::HasCrashExternalHandler() const {
 }
 
 #endif  // defined(OS_WIN)
+
+#if defined(OS_MACOSX)
+bool CefCrashReporterClient::EnableBrowserCrashForwarding() {
+  return enable_browser_crash_forwarding_;
+}
+#endif

libcef/common/crash_reporter_client.h

@@ -85,6 +85,10 @@ class CefCrashReporterClient : public crash_reporter::CrashReporterClient {
   bool HasCrashExternalHandler() const;
 #endif
 
+#if defined(OS_MACOSX)
+  bool EnableBrowserCrashForwarding() override;
+#endif
+
 private:
   bool has_crash_config_file_ = false;
 
@@ -109,6 +113,10 @@ private:
   std::string external_handler_;
 #endif
 
+#if defined(OS_MACOSX)
+  bool enable_browser_crash_forwarding_ = false;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(CefCrashReporterClient);
 };
 

patch/patches/crashpad_1995.patch

@@ -362,7 +362,7 @@ index 3316fa0..df90dbd 100644
  
  const char kWebViewSingleProcessType[] = "webview";
 diff --git components/crash/content/app/crash_reporter_client.cc components/crash/content/app/crash_reporter_client.cc
-index 3dfbd99..80e5b66 100644
+index 3dfbd99..cb9344b 100644
 --- components/crash/content/app/crash_reporter_client.cc
 +++ components/crash/content/app/crash_reporter_client.cc
 @@ -88,11 +88,12 @@ int CrashReporterClient::GetResultCodeRespawnFailed() {
@@ -387,7 +387,7 @@ index 3dfbd99..80e5b66 100644
  
  #if defined(OS_WIN)
  bool CrashReporterClient::GetCrashDumpLocation(base::string16* crash_dir) {
-@@ -141,6 +143,26 @@ bool CrashReporterClient::ReportingIsEnforcedByPolicy(bool* breakpad_enabled) {
+@@ -141,6 +143,32 @@ bool CrashReporterClient::ReportingIsEnforcedByPolicy(bool* breakpad_enabled) {
  }
  #endif
  
@@ -410,11 +410,17 @@ index 3dfbd99..80e5b66 100644
 +  return exe_dir + L"\\crashpad_handler.exe";
 +}
 +#endif
++
++#if defined(OS_MACOSX)
++bool CrashReporterClient::EnableBrowserCrashForwarding() {
++  return true;
++}
++#endif
 +
  #if defined(OS_ANDROID)
  int CrashReporterClient::GetAndroidMinidumpDescriptor() {
    return 0;
-@@ -165,9 +187,4 @@ bool CrashReporterClient::ShouldEnableBreakpadMicrodumps() {
+@@ -165,9 +193,4 @@ bool CrashReporterClient::ShouldEnableBreakpadMicrodumps() {
  }
  #endif
  
@@ -425,7 +431,7 @@ index 3dfbd99..80e5b66 100644
 -
  }  // namespace crash_reporter
 diff --git components/crash/content/app/crash_reporter_client.h components/crash/content/app/crash_reporter_client.h
-index 25ae505..d923b39 100644
+index 25ae505..f563761 100644
 --- components/crash/content/app/crash_reporter_client.h
 +++ components/crash/content/app/crash_reporter_client.h
 @@ -8,6 +8,7 @@
@@ -459,7 +465,7 @@ index 25ae505..d923b39 100644
  
    // The location where minidump files should be written. Returns true if
    // |crash_dir| was set. Windows has to use base::string16 because this code
-@@ -176,6 +179,17 @@ class CrashReporterClient {
+@@ -176,6 +179,23 @@ class CrashReporterClient {
  
    // Returns true if breakpad should run in the given process type.
    virtual bool EnableBreakpadForProcess(const std::string& process_type);
@@ -473,10 +479,30 @@ index 25ae505..d923b39 100644
 +#if defined(OS_WIN)
 +  // Returns the absolute path to the external crash handler exe.
 +  virtual base::string16 GetCrashExternalHandler(const base::string16& exe_dir);
++#endif
++
++#if defined(OS_MACOSX)
++  // Returns true if forwarding of crashes to the system crash reporter is
++  // enabled for the browser process.
++  virtual bool EnableBrowserCrashForwarding();
 +#endif
  };
  
  }  // namespace crash_reporter
+diff --git components/crash/content/app/crashpad.cc components/crash/content/app/crashpad.cc
+index 76f6734..f6f8000 100644
+--- components/crash/content/app/crashpad.cc
++++ components/crash/content/app/crashpad.cc
+@@ -142,7 +142,8 @@ void InitializeCrashpadImpl(bool initial_client,
+   // fallback. Forwarding is turned off for debug-mode builds even for the
+   // browser process, because the system's crash reporter can take a very long
+   // time to chew on symbols.
+-  if (!browser_process || is_debug_build) {
++  if (!browser_process || is_debug_build ||
++      !crash_reporter_client->EnableBrowserCrashForwarding()) {
+     crashpad_info->set_system_crash_reporter_forwarding(
+         crashpad::TriState::kDisabled);
+   }
 diff --git components/crash/content/app/crashpad_mac.mm components/crash/content/app/crashpad_mac.mm
 index 7df66ea..4ee709a 100644
 --- components/crash/content/app/crashpad_mac.mm
@@ -496,7 +522,7 @@ index 7df66ea..4ee709a 100644
  #include "third_party/crashpad/crashpad/client/crash_report_database.h"
  #include "third_party/crashpad/crashpad/client/crashpad_client.h"
  #include "third_party/crashpad/crashpad/client/crashpad_info.h"
-@@ -40,9 +43,10 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
+@@ -40,9 +43,10 @@
  
    if (initial_client) {
      @autoreleasepool {
@@ -510,7 +536,7 @@ index 7df66ea..4ee709a 100644
  
        // Is there a way to recover if this fails?
        CrashReporterClient* crash_reporter_client = GetCrashReporterClient();
-@@ -54,16 +58,27 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
+@@ -54,16 +58,27 @@
        // crash server won't have symbols for any other build types.
        std::string url = "https://clients2.google.com/cr/report";
  #else
@@ -543,7 +569,7 @@ index 7df66ea..4ee709a 100644
  
  #if defined(GOOGLE_CHROME_BUILD)
        NSString* channel = base::mac::ObjCCast<NSString>(
-@@ -73,12 +88,16 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
+@@ -73,12 +88,16 @@
        }
  #endif
  
@@ -565,7 +591,7 @@ index 7df66ea..4ee709a 100644
  
        std::vector<std::string> arguments;
        if (!browser_process) {
-@@ -90,6 +109,12 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
+@@ -90,6 +109,12 @@
              "--reset-own-crash-exception-port-to-system-default");
        }