Provide a new //base-level API for crash keys v2.

The new, preferred crash key API is located at
//components/crash/core/common:crash_key, but not all clients of the
crash key system can have a direct dependency on that.

In this CL, a new base/debug/crash_logging.h API is created to indirect
to the new v2 API. It is less featured and flexible than the crash
component API. This will fully replace the existing API still located
in that file.

Bug: 598854
Change-Id: Ifabd12f80d1a858ff61b999eba956d6cd5113784
Reviewed-on: https://chromium-review.googlesource.com/795230Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520333}

base/debug/crash_logging.cc

@@ -21,6 +21,8 @@ namespace debug {
 
 namespace {
 
+CrashKeyImplementation* g_crash_key_impl = nullptr;
+
 // Global map of crash key names to registration entries.
 typedef std::unordered_map<base::StringPiece, CrashKey, base::StringPieceHash>
     CrashKeyMap;
@@ -49,6 +51,33 @@ const size_t kLargestValueAllowed = 2048;
 
 }  // namespace
 
+CrashKeyString* AllocateCrashKeyString(const char name[],
+                                       CrashKeySize value_length) {
+  if (!g_crash_key_impl)
+    return nullptr;
+
+  return g_crash_key_impl->Allocate(name, value_length);
+}
+
+void SetCrashKeyString(CrashKeyString* crash_key, base::StringPiece value) {
+  if (!g_crash_key_impl || !crash_key)
+    return;
+
+  g_crash_key_impl->Set(crash_key, value);
+}
+
+void ClearCrashKeyString(CrashKeyString* crash_key) {
+  if (!g_crash_key_impl || !crash_key)
+    return;
+
+  g_crash_key_impl->Clear(crash_key);
+}
+
+void SetCrashKeyImplementation(std::unique_ptr<CrashKeyImplementation> impl) {
+  delete g_crash_key_impl;
+  g_crash_key_impl = impl.release();
+}
+
 void SetCrashKeyValue(const base::StringPiece& key,
                       const base::StringPiece& value) {
   if (!g_set_key_func_ || !g_crash_keys_)

base/debug/crash_logging.h

@@ -7,6 +7,7 @@
 
 #include <stddef.h>
 
+#include <memory>
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -15,15 +16,81 @@
 #include "base/macros.h"
 #include "base/strings/string_piece.h"
 
-// These functions add metadata to the upload payload when sending crash reports
-// to the crash server.
-//
-// IMPORTANT: On OS X and Linux, the key/value pairs are only sent as part of
-// the upload and are not included in the minidump!
-
 namespace base {
 namespace debug {
 
+// A crash key is an annotation that is carried along with a crash report, to
+// provide additional debugging information beyond a stack trace. Crash keys
+// have a name and a string value.
+//
+// The preferred API is //components/crash/core/common:crash_key, however not
+// all clients can hold a direct dependency on that target. The API provided
+// in this file indirects the dependency.
+//
+// Example usage:
+//   static CrashKeyString* crash_key =
+//       AllocateCrashKeyString("name", CrashKeySize::Size32);
+//   SetCrashKeyString(crash_key, "value");
+//   ClearCrashKeyString(crash_key);
+
+// The maximum length for a crash key's value must be one of the following
+// pre-determined values.
+enum class CrashKeySize {
+  Size32 = 32,
+  Size64 = 64,
+  Size256 = 256,
+};
+
+struct CrashKeyString;
+
+// Allocates a new crash key with the specified |name| with storage for a
+// value up to length |size|. This will return null if the crash key system is
+// not initialized.
+BASE_EXPORT CrashKeyString* AllocateCrashKeyString(const char name[],
+                                                   CrashKeySize size);
+
+// Stores |value| into the specified |crash_key|. The |crash_key| may be null
+// if AllocateCrashKeyString() returned null. If |value| is longer than the
+// size with which the key was allocated, it will be truncated.
+BASE_EXPORT void SetCrashKeyString(CrashKeyString* crash_key,
+                                   base::StringPiece value);
+
+// Clears any value that was stored in |crash_key|. The |crash_key| may be
+// null.
+BASE_EXPORT void ClearCrashKeyString(CrashKeyString* crash_key);
+
+////////////////////////////////////////////////////////////////////////////////
+// The following declarations are used to initialize the crash key system
+// in //base by providing implementations for the above functions.
+
+// The virtual interface that provides the implementation for the crash key
+// API. This is implemented by a higher-layer component, and the instance is
+// set using the function below.
+class CrashKeyImplementation {
+ public:
+  virtual ~CrashKeyImplementation() {}
+
+  virtual CrashKeyString* Allocate(const char name[], CrashKeySize size) = 0;
+  virtual void Set(CrashKeyString* crash_key, base::StringPiece value) = 0;
+  virtual void Clear(CrashKeyString* crash_key) = 0;
+};
+
+// Initializes the crash key system in base by replacing the existing
+// implementation, if it exists, with |impl|. The |impl| is copied into base.
+BASE_EXPORT void SetCrashKeyImplementation(
+    std::unique_ptr<CrashKeyImplementation> impl);
+
+// The base structure for a crash key, storing the allocation metadata.
+struct CrashKeyString {
+  constexpr CrashKeyString(const char name[], CrashKeySize size)
+      : name(name), size(size) {}
+  const char* const name;
+  const CrashKeySize size;
+};
+
+// The API below is deprecated.
+////////////////////////////////////////////////////////////////////////////////
+
 class StackTrace;
 
 // Sets or clears a specific key-value pair from the crash metadata. Keys and

base/debug/crash_logging_unittest.cc

@@ -183,3 +183,13 @@ TEST_F(CrashLoggingTest, ChunkRounding) {
   base::debug::CrashKey key = { "round", 12 };
   EXPECT_EQ(3u, base::debug::InitCrashKeys(&key, 1, 5));
 }
+
+TEST_F(CrashLoggingTest, UninitializedCrashKeyStringSupport) {
+  auto* crash_key = base::debug::AllocateCrashKeyString(
+      "test", base::debug::CrashKeySize::Size32);
+  EXPECT_FALSE(crash_key);
+
+  base::debug::SetCrashKeyString(crash_key, "value");
+
+  base::debug::ClearCrashKeyString(crash_key);
+}

components/crash/core/common/BUILD.gn

@@ -17,6 +17,8 @@ static_library("crash_key") {
   sources = [
     "crash_key.cc",
     "crash_key.h",
+    "crash_key_base_support.cc",
+    "crash_key_base_support.h",
   ]
 
   deps = [

components/crash/core/common/crash_key_base_support.cc

+// 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.
+
+#include "components/crash/core/common/crash_key_base_support.h"
+
+#include <memory>
+
+#include "base/debug/crash_logging.h"
+#include "components/crash/core/common/crash_key.h"
+
+namespace crash_reporter {
+
+namespace {
+
+// This stores the value for a crash key allocated through the //base API.
+template <uint32_t ValueSize>
+struct BaseCrashKeyString : public base::debug::CrashKeyString {
+  BaseCrashKeyString(const char name[], base::debug::CrashKeySize size)
+      : base::debug::CrashKeyString(name, size), impl(name) {
+    DCHECK_EQ(static_cast<uint32_t>(size), ValueSize);
+  }
+  crash_reporter::CrashKeyString<ValueSize> impl;
+};
+
+#define SIZE_CLASS_OPERATION(size_class, operation_prefix, operation_suffix) \
+  switch (size_class) {                                                      \
+    case base::debug::CrashKeySize::Size32:                                  \
+      operation_prefix BaseCrashKeyString<32> operation_suffix;              \
+    case base::debug::CrashKeySize::Size64:                                  \
+      operation_prefix BaseCrashKeyString<64> operation_suffix;              \
+    case base::debug::CrashKeySize::Size256:                                 \
+      operation_prefix BaseCrashKeyString<256> operation_suffix;             \
+  }
+
+class CrashKeyBaseSupport : public base::debug::CrashKeyImplementation {
+ public:
+  CrashKeyBaseSupport() = default;
+
+  ~CrashKeyBaseSupport() override = default;
+
+  base::debug::CrashKeyString* Allocate(
+      const char name[],
+      base::debug::CrashKeySize size) override {
+    SIZE_CLASS_OPERATION(size, return new, (name, size));
+    return nullptr;
+  }
+
+  void Set(base::debug::CrashKeyString* crash_key,
+           base::StringPiece value) override {
+    SIZE_CLASS_OPERATION(crash_key->size,
+                         reinterpret_cast<, *>(crash_key)->impl.Set(value));
+  }
+
+  void Clear(base::debug::CrashKeyString* crash_key) override {
+    SIZE_CLASS_OPERATION(crash_key->size,
+                         reinterpret_cast<, *>(crash_key)->impl.Clear());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CrashKeyBaseSupport);
+};
+
+#undef SIZE_CLASS_OPERATION
+
+}  // namespace
+
+void InitializeCrashKeyBaseSupport() {
+  base::debug::SetCrashKeyImplementation(
+      std::make_unique<CrashKeyBaseSupport>());
+}
+
+}  // namespace crash_reporter

components/crash/core/common/crash_key_base_support.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_CRASH_CORE_COMMON_CRASH_KEY_BASE_SUPPORT_H_
+#define COMPONENTS_CRASH_CORE_COMMON_CRASH_KEY_BASE_SUPPORT_H_
+
+namespace crash_reporter {
+
+// This initializes //base to support crash keys via the interface in
+// base/debug/crash_logging.h.
+void InitializeCrashKeyBaseSupport();
+
+}  // namespace crash_reporter
+
+#endif  // COMPONENTS_CRASH_CORE_COMMON_CRASH_KEY_BASE_SUPPORT_H_

components/crash/core/common/crash_key_breakpad.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
+#include "components/crash/core/common/crash_key_base_support.h"
 #include "components/crash/core/common/crash_key_internal.h"
 
 #if (defined(OS_MACOSX) && !defined(OS_IOS)) || defined(OS_WIN)
@@ -99,6 +100,7 @@ bool CrashKeyStringImpl::is_set() const {
 void InitializeCrashKeys() {
   if (!internal::g_storage) {
     internal::g_storage = new internal::TransitionalCrashKeyStorage();
+    InitializeCrashKeyBaseSupport();
   }
 }
 

components/crash/core/common/crash_key_crashpad.cc

@@ -7,12 +7,14 @@
 
 #include "components/crash/core/common/crash_key.h"
 
+#include "components/crash/core/common/crash_key_base_support.h"
 #include "third_party/crashpad/crashpad/client/annotation_list.h"
 
 namespace crash_reporter {
 
 void InitializeCrashKeys() {
   crashpad::AnnotationList::Register();
+  InitializeCrashKeyBaseSupport();
 }
 
 }  // namespace crash_reporter

components/crash/core/common/crash_key_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "components/crash/core/common/crash_key.h"
 
+#include "base/debug/crash_logging.h"
 #include "base/debug/stack_trace.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -78,5 +79,20 @@ TEST_F(CrashKeyStringTest, SetStackTrace) {
   EXPECT_TRUE(key.is_set());
 }
 
+TEST_F(CrashKeyStringTest, BaseSupport) {
+  static base::debug::CrashKeyString* crash_key =
+      base::debug::AllocateCrashKeyString("base-support",
+                                          base::debug::CrashKeySize::Size64);
+
+  EXPECT_TRUE(crash_key);
+
+  base::debug::SetCrashKeyString(crash_key, "this is a test");
+
+  base::debug::ClearCrashKeyString(crash_key);
+
+  base::debug::SetCrashKeyString(crash_key, std::string(128, 'b'));
+  base::debug::SetCrashKeyString(crash_key, std::string(64, 'a'));
+}
+
 }  // namespace
 }  // namespace crash_reporter