Create the crash key registration system and register some Mac-specific keys.

base/debug/crash_logging.cc

Index: base/debug/crash_logging.cc
diff --git a/base/debug/crash_logging.cc b/base/debug/crash_logging.cc
index 1e20677ec4a32f94408811f773e17f75eaf018ce..0817e5983564e31c38c8eb58ee0d13ecd54874ce 100644
--- a/base/debug/crash_logging.cc
+++ b/base/debug/crash_logging.cc
@@ -4,6 +4,8 @@
 
 #include "base/debug/crash_logging.h"
 
+#include <map>
+
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
 #include "base/string_util.h"
@@ -12,18 +14,62 @@
 namespace base {
 namespace debug {
 
-static SetCrashKeyValueFuncT g_set_key_func_ = NULL;
-static ClearCrashKeyValueFuncT g_clear_key_func_ = NULL;
+namespace {
+
+std::map<std::string, CrashKey>* g_crash_keys_;

[Scott Hess - ex-Googler, 2013/01/04 18:59:39]

Would prefer =NULL, if it's the default, the compiler can optimize it away.

[Robert Sesek, 2013/01/04 19:25:38]

Done.

+
+const char kChunkFormatString[] = "%s-%zu";
+
+SetCrashKeyValueFuncT g_set_key_func_ = NULL;
+ClearCrashKeyValueFuncT g_clear_key_func_ = NULL;
+
+}  // namespace

[Scott Hess - ex-Googler, 2013/01/04 18:59:39]

This should be outside the base::debug::, I think?

[Robert Sesek, 2013/01/04 19:25:38]

I don't think it matters.

 
 void SetCrashKeyValue(const base::StringPiece& key,
                       const base::StringPiece& value) {
-  if (g_set_key_func_)
+  if (!g_set_key_func_)
+    return;
+
+  const CrashKey* crash_key = LookupCrashKey(key);
+
+  // TODO(rsesek): Do this:
+  //DCHECK(crash_key) << "All crash keys must be registered before use "
+  //                  << "(key = " << key << ")";
+
+  // Handle the un-chunked case.
+  if (!crash_key || crash_key->num_chunks == 1) {
     g_set_key_func_(key, value);
+    return;
+  }
+
+  // Unset the unused chunks.
+  std::vector<std::string> chunks = ChunkCrashKeyValue(*crash_key, value);
+  for (size_t i = chunks.size(); i < crash_key->num_chunks; ++i) {
+    g_clear_key_func_(base::StringPrintf(kChunkFormatString, key.data(), i+1));
+  }
+
+  // Set the chunked keys.
+  for (size_t i = 0; i < chunks.size(); ++i) {
+    g_set_key_func_(base::StringPrintf(kChunkFormatString, key.data(), i+1),
+                    chunks[i]);
+  }
 }
 
 void ClearCrashKey(const base::StringPiece& key) {
-  if (g_clear_key_func_)
+  if (!g_clear_key_func_)
+    return;
+
+  const CrashKey* crash_key = LookupCrashKey(key);
+
+  // Handle the un-checked case.
+  if (!crash_key || crash_key->num_chunks == 1) {
     g_clear_key_func_(key);
+    return;
+  }
+
+  for (size_t i = 1; i <= crash_key->num_chunks; ++i) {
+    g_clear_key_func_(base::StringPrintf(kChunkFormatString, key.data(), i));
+  }
 }
 
 void SetCrashKeyToStackTrace(const base::StringPiece& key,
@@ -70,6 +116,42 @@ ScopedCrashKey::~ScopedCrashKey() {
   ClearCrashKey(key_);
 }
 
+void InitCrashKeys(const CrashKey* const keys, size_t count,
+                   InitCrashKeysCallbackFuncT completion_callback) {
+  if (!keys) {
+    delete g_crash_keys_;
+    g_crash_keys_ = NULL;
+    return;
+  }
+
+  g_crash_keys_ = new std::map<std::string, CrashKey>;

[Scott Hess - ex-Googler, 2013/01/04 18:59:39]

This won't work great for modules which want to manage their own crash keys.  If
chrome/ is the only embedder, it probably doesn't matter that much, and the
entire point is to prevent people from larding things up, so it's possible it
shouldn't be made generic.  Thinking.

If it's one-shot, DCHECK(!g_crash_keys_).  And maybe a thread assertion of some
sort would make sense.

[Robert Sesek, 2013/01/04 19:25:38]

Because Windows enforces the requirement of a fixed-size storage, yes this can
only be called once. Having modules manage their own crash keys sort of implies
that each module then has its own crash reporter.

+
+  std::vector<std::string> all_keys;
+  all_keys.reserve(count);
+  for (size_t i = 0; i < count; ++i) {
+    g_crash_keys_->insert(std::make_pair(keys[i].key_name, keys[i]));
+    if (keys[i].num_chunks == 1) {
+      all_keys.push_back(keys[i].key_name);
+    } else {
+      for (size_t j = 1; j <= keys[i].num_chunks; ++j) {
+        all_keys.push_back(
+            base::StringPrintf(kChunkFormatString, keys[i].key_name, j));
+      }
+    }
+  }
+
+  if (completion_callback)
+    completion_callback(all_keys);
+}
+
+const CrashKey* LookupCrashKey(const base::StringPiece& key) {
+  std::map<std::string, CrashKey>::const_iterator it =
+      g_crash_keys_->find(key.as_string());

[Scott Hess - ex-Googler, 2013/01/04 18:59:39]

Once we're calling as_string(), StringPiece isn't much of a gain any longer,
just lets you pass char* or std::string.  Sigh.  Can the keys be const char* and
forced to be static storage?  Hmm.

[Robert Sesek, 2013/01/04 19:25:38]

If we do it by const char* though, won't using them as keys in a map perform
pointer equality rather than string equality? That seems wonky.

[Scott Hess - ex-Googler, 2013/01/05 00:22:05]

That's actually what I meant - kKeyName would be the literal key, which just
happened to point to a string.

When I originally suggested StringPiece, I was still thinking in terms of a
flexible API - now you're locking it down so that the keys could potentially be
exactly the keys registered, no more, no less.  Keying off the contents rather
than the pointer might still be easier, though, since we'll have module-boundary
issues around using kKeyName (unless the module defines and chrome/ uses the
module's variable in registering?).

+  if (it == g_crash_keys_->end())
+    return NULL;
+  return &(it->second);
+}
+
 void SetCrashKeyReportingFunctions(
     SetCrashKeyValueFuncT set_key_func,
     ClearCrashKeyValueFuncT clear_key_func) {
@@ -77,5 +159,27 @@ void SetCrashKeyReportingFunctions(
   g_clear_key_func_ = clear_key_func;
 }
 
+std::vector<std::string> ChunkCrashKeyValue(const CrashKey& crash_key,
+                                            const base::StringPiece& value) {
+  std::string value_string = value.as_string();
+  std::vector<std::string> chunks;
+  size_t offset = 0;
+  for (size_t i = 0;
+       i < crash_key.num_chunks && offset < value_string.length();
+       ++i) {
+    std::string chunk = value_string.substr(offset, crash_key.max_length);
+    chunks.push_back(chunk);
+    offset += chunk.length();
+  }
+  return chunks;
+}
+
+void ResetCrashLoggingForTesting() {
+  delete g_crash_keys_;
+  g_crash_keys_ = NULL;
+  g_set_key_func_ = NULL;
+  g_clear_key_func_ = NULL;
+}
+
 }  // namespace debug
 }  // namespace base