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

This is a reland of 5bf2df67
Original change's description:
> 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/795230
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520333}

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

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;              \
+      break;                                                                 \
+    case base::debug::CrashKeySize::Size64:                                  \
+      operation_prefix BaseCrashKeyString<64> operation_suffix;              \
+      break;                                                                 \
+    case base::debug::CrashKeySize::Size256:                                 \
+      operation_prefix BaseCrashKeyString<256> operation_suffix;             \
+      break;                                                                 \
+  }
+
+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