Integrate Crashpad UMA

third_party/crashpad/crashpad/handler/handler_main.cc

Index: third_party/crashpad/crashpad/handler/handler_main.cc
diff --git a/third_party/crashpad/crashpad/handler/handler_main.cc b/third_party/crashpad/crashpad/handler/handler_main.cc
index 3240cd38b37d79d6c915560aa41f325702cf00cd..e825f1113f9196f9777dc73865da9a21080a1148 100644
--- a/third_party/crashpad/crashpad/handler/handler_main.cc
+++ b/third_party/crashpad/crashpad/handler/handler_main.cc
@@ -27,6 +27,7 @@
 #include "base/files/file_path.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
+#include "base/metrics/persistent_metrics_file_util.h"
 #include "base/scoped_generic.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -313,6 +314,19 @@ int HandlerMain(int argc, char* argv[]) {
     return EXIT_FAILURE;
   }
 
+  std::unique_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(
+      base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType(
+          options.database))));
+  if (!database) {
+    return EXIT_FAILURE;
+  }
+
+  const base::FilePath metrics_dir(
+      ToolSupport::CommandLineArgumentToFilePathStringType(options.database));

[Mark Mentovai, 2016/09/13 21:35:56]

Hmm, this will delay the handshake with the browser, and the browser’s in turn
blocked on that.

[scottmg, 2016/09/13 21:46:24]

It will. :( It "only" creates the directory if it doesn't exist and sets a
couple in-memory variables, but on first run startup we have the cost of
creating that directory. All we need is a place to put the file where the
browser can find them -- I guess we should just put it in the User Data dir
directly instead of inside the Crashpad subdirectory since we can rely on the
installer to have created that one for us?

[Mark Mentovai, 2016/09/13 21:52:01]

Well, crashpad_handler won’t run at all unless the parent of database_dir
already exists. We’d hit the EXIT_FAILURE just above in that case.

But now that you bring this up, there’s another concern: we’ve never promised
that options.database is a directory. Some future database implementation might
stick a file there instead. I don’t think we should litter an arbitrary database
implementation’s innards with files it’s not expecting, either.

So there’s another argument in favor of just the user-data-dir.

+  const char kMetricsName[] = "CrashpadMetrics";
+  base::InitializeGlobalPersistentMetricsStorage(
+      metrics_dir, kMetricsName, 1 << 20);
+
 #if defined(OS_MACOSX)
   if (options.mach_service.empty()) {
     // Don’t do this when being run by launchd. See launchd.plist(5).
@@ -386,13 +400,6 @@ int HandlerMain(int argc, char* argv[]) {
   }
 #endif  // OS_MACOSX
 
-  std::unique_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(
-      base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType(
-          options.database))));
-  if (!database) {
-    return EXIT_FAILURE;
-  }
-
   // TODO(scottmg): options.rate_limit should be removed when we have a
   // configurable database setting to control upload limiting.
   // See https://crashpad.chromium.org/bug/23.
@@ -412,6 +419,8 @@ int HandlerMain(int argc, char* argv[]) {
   upload_thread.Stop();
   prune_thread.Stop();
 
+  base::CleanUpGlobalPersistentHistogramStorage();
+
   return EXIT_SUCCESS;
 }