Monitor crashpad_handler for crashes [sometimes]

When Crashpad self-monitoring is enabled, crashes in crashpad_handler
will be written to the crash report database and uploaded according to
the user's preference. Self-monitoring dedicates an additional
crashpad_handler process to serve as the handler for the original
instance. Having a dedicated process is somewhat expensive, so
self-monitoring is not enabled at all times.

An existing mechanism, the "fallback crash handler," is already in place
on Windows. The fallback crash handler does not create any additional
processes until a crashpad_handler crash occurs. The fallback handler
remains effective in cases where Crashpad's own self-monitoring is
disabled. The fallback handler and Crashpad self-monitoring have
different attributes, so there are no plans to move Windows entirely
onto Crashpad self-monitoring.

For branded Chrome, the dev channel gets Crashpad self-monitoring in 25%
of browser starts, and the beta channel gets it in 10% of browser
starts. On macOS, the stable channel gets self-monitoring in 1% of
browser starts, and the canary and Chromium builds get it all the time.
On Windows, the Chrome stable channel never gets self-monitoring because
the fallback crash handler is sufficient, and the canary and Chromium
builds get it 50% of the time, allowing for some use of the fallback
crash handler in those cases.

While reports will be collected for crashes that occur while Crashpad is
attempting to upload another report, it's unlikley that these reports
will be uploaded automatically under the current one-attempt-per-hour,
no-retry rate limiting strategy. See
https://crashpad.chromium.org/bug/23.

BUG=crashpad:143
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2799013002
Cr-Commit-Position: refs/heads/master@{#463352}

chrome/app/chrome_crash_reporter_client.h

@@ -53,6 +53,10 @@ class ChromeCrashReporterClient : public crash_reporter::CrashReporterClient {
   int GetAndroidMinidumpDescriptor() override;
 #endif
 
+#if defined(OS_MACOSX)
+  bool ShouldMonitorCrashHandlerExpensively() override;
+#endif
+
   bool EnableBreakpadForProcess(const std::string& process_type) override;
 
  private:

chrome/app/chrome_crash_reporter_client_mac.mm

@@ -8,9 +8,12 @@
 
 #include "base/mac/scoped_cftyperef.h"
 #include "base/path_service.h"
+#include "base/rand_util.h"
 #include "base/strings/sys_string_conversions.h"
+#include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_paths.h"
 #include "components/policy/policy_constants.h"
+#include "components/version_info/version_info.h"
 
 #if !defined(DISABLE_NACL)
 #include "base/command_line.h"
@@ -38,3 +41,29 @@ bool ChromeCrashReporterClient::ReportingIsEnforcedByPolicy(
   }
   return false;
 }
+
+bool ChromeCrashReporterClient::ShouldMonitorCrashHandlerExpensively() {
+  // This mechanism dedicates a process to be crashpad_handler's own
+  // crashpad_handler. In Google Chrome, scale back on this in the more stable
+  // channels. Other builds are of more of a developmental nature, so always
+  // enable the additional crash reporting.
+  double probability;
+  switch (chrome::GetChannel()) {
+    case version_info::Channel::STABLE:
+      probability = 0.01;
+      break;
+
+    case version_info::Channel::BETA:
+      probability = 0.1;
+      break;
+
+    case version_info::Channel::DEV:
+      probability = 0.25;
+      break;
+
+    default:
+      return true;
+  }
+
+  return base::RandDouble() < probability;
+}

chrome/app/chrome_crash_reporter_client_win.cc

@@ -19,12 +19,13 @@
 #include "base/debug/crash_logging.h"
 #include "base/debug/leak_annotations.h"
 #include "base/format_macros.h"
+#include "base/rand_util.h"
 #include "chrome/common/chrome_result_codes.h"
-#include "chrome/install_static/install_details.h"
 #include "chrome/install_static/install_util.h"
 #include "chrome/install_static/user_data_dir.h"
 #include "components/crash/content/app/crashpad.h"
 #include "components/crash/core/common/crash_keys.h"
+#include "components/version_info/channel.h"
 
 namespace {
 
@@ -340,14 +341,13 @@ bool ChromeCrashReporterClient::GetDeferredUploadsSupported(
 }
 
 bool ChromeCrashReporterClient::GetIsPerUserInstall() {
-  return !install_static::InstallDetails::Get().system_level();
+  return !install_static::IsSystemInstall();
 }
 
 bool ChromeCrashReporterClient::GetShouldDumpLargerDumps() {
-  // Capture larger dumps for Google Chrome "beta", "dev", and "canary"
-  // channels. Stable channel and Chromium builds are on channel "", and use
-  // smaller dumps.
-  return !install_static::InstallDetails::Get().channel().empty();
+  // Capture larger dumps for Google Chrome beta, dev, and canary channels, and
+  // Chromium builds. The Google Chrome stable channel uses smaller dumps.
+  return install_static::GetChromeChannel() != version_info::Channel::STABLE;
 }
 
 int ChromeCrashReporterClient::GetResultCodeRespawnFailed() {
@@ -405,6 +405,34 @@ bool ChromeCrashReporterClient::GetCollectStatsInSample() {
   return install_static::GetCollectStatsInSample();
 }
 
+bool ChromeCrashReporterClient::ShouldMonitorCrashHandlerExpensively() {
+  // The expensive mechanism dedicates a process to be crashpad_handler's own
+  // crashpad_handler. In Google Chrome, scale back on this in the more stable
+  // channels. There's a fallback crash handler that can catch crashes when this
+  // expensive mechanism isn't used, although the fallback crash handler has
+  // different characteristics so it's desirable to use the expensive mechanism
+  // at least some of the time.
+  double probability;
+  switch (install_static::GetChromeChannel()) {
+    case version_info::Channel::STABLE:
+      return false;
+
+    case version_info::Channel::BETA:
+      probability = 0.1;
+      break;
+
+    case version_info::Channel::DEV:
+      probability = 0.25;
+      break;
+
+    default:
+      probability = 0.5;
+      break;
+  }
+
+  return base::RandDouble() < probability;
+}
+
 bool ChromeCrashReporterClient::EnableBreakpadForProcess(
     const std::string& process_type) {
   // This is not used by Crashpad (at least on Windows).

chrome/app/chrome_crash_reporter_client_win.h

@@ -49,6 +49,8 @@ class ChromeCrashReporterClient : public crash_reporter::CrashReporterClient {
 
   bool ReportingIsEnforcedByPolicy(bool* breakpad_enabled) override;
 
+  bool ShouldMonitorCrashHandlerExpensively() override;
+
   bool EnableBreakpadForProcess(const std::string& process_type) override;
 
  private:

chrome/app/chrome_exe_main_win.cc

@@ -245,7 +245,7 @@ int main() {
   if (process_type == crash_reporter::switches::kCrashpadHandler) {
     crash_reporter::SetupFallbackCrashHandling(*command_line);
     return crash_reporter::RunAsCrashpadHandler(
-        *base::CommandLine::ForCurrentProcess());
+        *base::CommandLine::ForCurrentProcess(), switches::kProcessType);
   } else if (process_type == crash_reporter::switches::kFallbackCrashHandler) {
     return RunFallbackCrashHandler(*command_line);
   }

chrome/common/channel_info_win.cc

@@ -8,8 +8,7 @@
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/install_static/install_details.h"
-#include "components/version_info/version_info.h"
+#include "chrome/install_static/install_util.h"
 
 namespace chrome {
 
@@ -21,7 +20,7 @@ std::string GetChannelString() {
           "422460 VersionInfo::GetVersionStringModifier"));
 
 #if defined(GOOGLE_CHROME_BUILD)
-  base::string16 channel(install_static::InstallDetails::Get().channel());
+  base::string16 channel(install_static::GetChromeChannelName());
 #if defined(SYZYASAN)
   if (base::debug::IsBinaryInstrumented())
     channel += L" SyzyASan";
@@ -33,21 +32,7 @@ std::string GetChannelString() {
 }
 
 version_info::Channel GetChannel() {
-#if defined(GOOGLE_CHROME_BUILD)
-  base::string16 channel(install_static::InstallDetails::Get().channel());
-
-  if (channel.empty()) {
-    return version_info::Channel::STABLE;
-  } else if (channel == L"beta") {
-    return version_info::Channel::BETA;
-  } else if (channel == L"dev") {
-    return version_info::Channel::DEV;
-  } else if (channel == L"canary") {
-    return version_info::Channel::CANARY;
-  }
-#endif
-
-  return version_info::Channel::UNKNOWN;
+  return install_static::GetChromeChannel();
 }
 
 }  // namespace chrome

chrome/install_static/BUILD.gn

@@ -11,6 +11,7 @@ assert(is_win)
 # Please don't add dependencies on other system libraries.
 static_library("install_static_util") {
   deps = [
+    "//components/version_info:channel",
     "//components/version_info:generate_version_info",
   ]
 

chrome/install_static/DEPS

@@ -11,6 +11,8 @@ include_rules = [
   "+chrome/install_static",
   # All registry access should go through nt_registry.
   "+chrome_elf/nt_registry/nt_registry.h",
+  # For the version_info::Channel enum.
+  "+components/version_info/channel.h",
   # For the compile-time PRODUCT_VERSION macro.
   "+components/version_info/version_info_values.h",
 ]

chrome/install_static/install_util.cc

@@ -20,6 +20,7 @@
 #include "chrome/install_static/policy_path_parser.h"
 #include "chrome/install_static/user_data_dir.h"
 #include "chrome_elf/nt_registry/nt_registry.h"
+#include "components/version_info/channel.h"
 
 namespace install_static {
 
@@ -561,6 +562,26 @@ void GetExecutableVersionDetails(const std::wstring& exe_path,
   *channel_name = GetChromeChannelName();
 }
 
+version_info::Channel GetChromeChannel() {
+#if defined(GOOGLE_CHROME_BUILD)
+  std::wstring channel_name(GetChromeChannelName());
+  if (channel_name.empty()) {
+    return version_info::Channel::STABLE;
+  }
+  if (channel_name == L"beta") {
+    return version_info::Channel::BETA;
+  }
+  if (channel_name == L"dev") {
+    return version_info::Channel::DEV;
+  }
+  if (channel_name == L"canary") {
+    return version_info::Channel::CANARY;
+  }
+#endif
+
+  return version_info::Channel::UNKNOWN;
+}
+
 std::wstring GetChromeChannelName() {
   return InstallDetails::Get().channel();
 }

chrome/install_static/install_util.h

@@ -13,6 +13,10 @@
 #include <string>
 #include <vector>
 
+namespace version_info {
+enum class Channel;
+}
+
 namespace install_static {
 
 struct InstallConstants;
@@ -191,7 +195,8 @@ void GetExecutableVersionDetails(const std::wstring& exe_path,
                                  std::wstring* special_build,
                                  std::wstring* channel_name);
 
-// Gets the channel name for the current Chrome process.
+// Gets the channel or channel name for the current Chrome process.
+version_info::Channel GetChromeChannel();
 std::wstring GetChromeChannelName();
 
 // Returns true if the |source| string matches the |pattern|. The pattern

chrome/installer/setup/setup_main.cc

@@ -1329,7 +1329,7 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
 
   if (process_type == crash_reporter::switches::kCrashpadHandler) {
     return crash_reporter::RunAsCrashpadHandler(
-        *base::CommandLine::ForCurrentProcess());
+        *base::CommandLine::ForCurrentProcess(), switches::kProcessType);
   }
 
   // install_util uses chrome paths.

chrome_elf/BUILD.gn

@@ -150,6 +150,7 @@ static_library("crash") {
     "//chrome/install_static:install_static_util",
     "//components/crash/content/app:app",
     "//components/crash/core/common",  # crash_keys
+    "//components/version_info:channel",
     "//content/public/common:result_codes",
     "//third_party/crashpad/crashpad/client:client",  # DumpWithoutCrash
   ]

components/crash/content/app/crash_reporter_client.cc

@@ -173,6 +173,12 @@ bool CrashReporterClient::ShouldEnableBreakpadMicrodumps() {
 }
 #endif
 
+#if defined(OS_MACOSX) || defined(OS_WIN)
+bool CrashReporterClient::ShouldMonitorCrashHandlerExpensively() {
+  return false;
+}
+#endif
+
 bool CrashReporterClient::EnableBreakpadForProcess(
     const std::string& process_type) {
   return false;

components/crash/content/app/crash_reporter_client.h

@@ -179,6 +179,22 @@ class CrashReporterClient {
   virtual bool ShouldEnableBreakpadMicrodumps();
 #endif
 
+#if defined(OS_MACOSX) || defined(OS_WIN)
+  // This method should return true to configure a crash reporter capable of
+  // monitoring itself for its own crashes to do so, even if self-monitoring
+  // would be expensive. "Expensive" self-monitoring dedicates an additional
+  // crash handler process to handle the crashes of the initial crash handler
+  // process.
+  //
+  // In some cases, inexpensive self-monitoring may also be available. When it
+  // is, it may be used when this method returns false. If only expensive
+  // self-monitoring is available, returning false from this function will
+  // prevent the crash handler process from being monitored for crashes at all.
+  //
+  // The default implementation returns false.
+  virtual bool ShouldMonitorCrashHandlerExpensively();
+#endif
+
   // Returns true if breakpad should run in the given process type.
   virtual bool EnableBreakpadForProcess(const std::string& process_type);
 };

components/crash/content/app/crashpad_mac.mm

@@ -66,12 +66,18 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
           base::SysNSStringToUTF8(product).append("_Mac");
 
 #if defined(GOOGLE_CHROME_BUILD)
+      // Empty means stable.
+      const bool allow_empty_channel = true;
+#else
+      const bool allow_empty_channel = false;
+#endif
       NSString* channel = base::mac::ObjCCast<NSString>(
           [outer_bundle objectForInfoDictionaryKey:@"KSChannelID"]);
       if (channel) {
         process_annotations["channel"] = base::SysNSStringToUTF8(channel);
+      } else if (allow_empty_channel) {
+        process_annotations["channel"] = "";
       }
-#endif
 
       NSString* version =
           base::mac::ObjCCast<NSString>([base::mac::FrameworkBundle()
@@ -81,6 +87,16 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
       process_annotations["plat"] = std::string("OS X");
 
       std::vector<std::string> arguments;
+
+      if (crash_reporter_client->ShouldMonitorCrashHandlerExpensively()) {
+        arguments.push_back("--monitor-self");
+      }
+
+      // Set up --monitor-self-annotation even in the absence of --monitor-self
+      // so that minidumps produced by Crashpad's generate_dump tool will
+      // contain these annotations.
+      arguments.push_back("--monitor-self-annotation=ptype=crashpad-handler");
+
       if (!browser_process) {
         // If this is an initial client that's not the browser process, it's
         // important that the new Crashpad handler also not be connected to any

components/crash/content/app/crashpad_win.cc

@@ -35,7 +35,14 @@ void GetPlatformCrashpadAnnotations(
       exe_file, &product_name, &version, &special_build, &channel_name);
   (*annotations)["prod"] = base::UTF16ToUTF8(product_name);
   (*annotations)["ver"] = base::UTF16ToUTF8(version);
-  (*annotations)["channel"] = base::UTF16ToUTF8(channel_name);
+#if defined(GOOGLE_CHROME_BUILD)
+  // Empty means stable.
+  const bool allow_empty_channel = true;
+#else
+  const bool allow_empty_channel = false;
+#endif
+  if (allow_empty_channel || !channel_name.empty())
+    (*annotations)["channel"] = base::UTF16ToUTF8(channel_name);
   if (!special_build.empty())
     (*annotations)["special"] = base::UTF16ToUTF8(special_build);
 #if defined(ARCH_CPU_X86)
@@ -96,18 +103,35 @@ base::FilePath PlatformCrashpadInitialization(bool initial_client,
     // If the handler is embedded in the binary (e.g. chrome, setup), we
     // reinvoke it with --type=crashpad-handler. Otherwise, we use the
     // standalone crashpad_handler.exe (for tests, etc.).
-    std::vector<std::string> arguments;
+    std::vector<std::string> start_arguments;
     if (embedded_handler) {
-      arguments.push_back(std::string("--type=") + switches::kCrashpadHandler);
+      start_arguments.push_back(std::string("--type=") +
+                                switches::kCrashpadHandler);
       // The prefetch argument added here has to be documented in
       // chrome_switches.cc, below the kPrefetchArgument* constants. A constant
       // can't be used here because crashpad can't depend on Chrome.
-      arguments.push_back("/prefetch:7");
+      start_arguments.push_back("/prefetch:7");
     } else {
       base::FilePath exe_dir = exe_file.DirName();
       exe_file = exe_dir.Append(FILE_PATH_LITERAL("crashpad_handler.exe"));
     }
 
+    std::vector<std::string> arguments(start_arguments);
+
+    if (crash_reporter_client->ShouldMonitorCrashHandlerExpensively()) {
+      arguments.push_back("--monitor-self");
+      for (const std::string& start_argument : start_arguments) {
+        arguments.push_back(std::string("--monitor-self-argument=") +
+                            start_argument);
+      }
+    }
+
+    // Set up --monitor-self-annotation even in the absence of --monitor-self so
+    // that minidumps produced by Crashpad's generate_dump tool will contain
+    // these annotations.
+    arguments.push_back(std::string("--monitor-self-annotation=ptype=") +
+                        switches::kCrashpadHandler);
+
     GetCrashpadClient().StartHandler(exe_file, database_path, metrics_path, url,
                                      process_annotations, arguments, false,
                                      false);

components/crash/content/app/run_as_crashpad_handler_win.cc

@@ -14,25 +14,46 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "third_party/crashpad/crashpad/client/crashpad_info.h"
+#include "third_party/crashpad/crashpad/client/simple_string_dictionary.h"
 #include "third_party/crashpad/crashpad/handler/handler_main.h"
 
 namespace crash_reporter {
 
-int RunAsCrashpadHandler(const base::CommandLine& command_line) {
+int RunAsCrashpadHandler(const base::CommandLine& command_line,
+                         const char* process_type_switch) {
   // Make sure this process terminates on OOM in the same mode as other Chrome
   // processes.
   base::EnableTerminationOnOutOfMemory();
 
+  // If the handler is started with --monitor-self, it'll need a ptype
+  // annotation set. It'll normally set one itself by being invoked with
+  // --monitor-self-annotation=ptype=crashpad-handler, but that leaves a window
+  // during self-monitoring initialization when the ptype is not set at all, so
+  // provide one here.
+  const std::string process_type =
+      command_line.GetSwitchValueASCII(process_type_switch);
+  if (!process_type.empty()) {
+    crashpad::SimpleStringDictionary* annotations =
+        new crashpad::SimpleStringDictionary();
+    annotations->SetKeyValue("ptype", process_type.c_str());
+    crashpad::CrashpadInfo* crashpad_info =
+        crashpad::CrashpadInfo::GetCrashpadInfo();
+    DCHECK(!crashpad_info->simple_annotations());
+    crashpad_info->set_simple_annotations(annotations);
+  }
+
   std::vector<base::string16> argv = command_line.argv();
-  const base::string16 process_type = L"--type=";
-  argv.erase(std::remove_if(argv.begin(), argv.end(),
-                            [&process_type](const base::string16& str) {
-                              return base::StartsWith(
-                                         str, process_type,
-                                         base::CompareCase::SENSITIVE) ||
-                                     (!str.empty() && str[0] == L'/');
-                            }),
-             argv.end());
+  const base::string16 process_type_arg_prefix =
+      base::string16(L"--") + base::UTF8ToUTF16(process_type_switch) + L"=";
+  argv.erase(
+      std::remove_if(argv.begin(), argv.end(),
+                     [&process_type_arg_prefix](const base::string16& str) {
+                       return base::StartsWith(str, process_type_arg_prefix,
+                                               base::CompareCase::SENSITIVE) ||
+                              (!str.empty() && str[0] == L'/');
+                     }),
+      argv.end());
 
   std::unique_ptr<char* []> argv_as_utf8(new char*[argv.size() + 1]);
   std::vector<std::string> storage;

components/crash/content/app/run_as_crashpad_handler_win.h

@@ -12,10 +12,15 @@ class CommandLine;
 namespace crash_reporter {
 
 // Helper for running an embedded copy of crashpad_handler. Searches for and
-// removes --switches::kProcessType=xyz arguments in the command line, and all
+// removes --(process_type_switch)=xyz arguments in the command line, and all
 // options starting with '/' (for "/prefetch:N"), and then runs
 // crashpad::HandlerMain with the remaining arguments.
-int RunAsCrashpadHandler(const base::CommandLine& command_line);
+//
+// Normally, pass switches::kProcessType for process_type_switch. It's accepted
+// as a parameter because this component does not have access to content/, where
+// that variable lives.
+int RunAsCrashpadHandler(const base::CommandLine& command_line,
+                         const char* process_type_switch);
 
 }  // namespace crash_reporter
 

components/version_info/BUILD.gn

@@ -22,12 +22,22 @@ static_library("version_info") {
     "//components/strings",
   ]
 
+  public_deps = [
+    ":channel",
+  ]
+
   if (use_unofficial_version_number) {
     defines = [ "USE_UNOFFICIAL_VERSION_NUMBER" ]
     deps += [ "//ui/base" ]
   }
 }
 
+source_set("channel") {
+  sources = [
+    "channel.h",
+  ]
+}
+
 process_version("generate_version_info") {
   template_file = "version_info_values.h.version"
   sources = [

components/version_info/channel.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_VERSION_INFO_CHANNEL_H_
+#define COMPONENTS_VERSION_INFO_CHANNEL_H_
+
+namespace version_info {
+
+// The possible channels for an installation, from most fun to most stable.
+enum class Channel { UNKNOWN = 0, CANARY, DEV, BETA, STABLE };
+
+}  // namespace version_info
+
+#endif  // COMPONENTS_VERSION_INFO_CHANNEL_H_

components/version_info/version_info.h

@@ -7,10 +7,9 @@
 
 #include <string>
 
-namespace version_info {
+#include "components/version_info/channel.h"
 
-// The possible channels for an installation, from most fun to most stable.
-enum class Channel { UNKNOWN = 0, CANARY, DEV, BETA, STABLE };
+namespace version_info {
 
 // Returns the product name and version information for UserAgent header,
 // e.g. "Chrome/a.b.c.d".