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

base/debug/crash_logging.h

 // Copyright (c) 2012 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 BASE_DEBUG_CRASH_LOGGING_H_
 #define BASE_DEBUG_CRASH_LOGGING_H_
 
 #include <string>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
+#include "base/callback.h"

[Scott Hess - ex-Googler, 2013/01/08 00:34:37]

I assume this was here for using base::Callback, which is now gone?

[Robert Sesek, 2013/01/08 00:56:23]

Done.

 #include "base/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 {
@@ skipping 22 matching lines @@
  public:
   ScopedCrashKey(const base::StringPiece& key, const base::StringPiece& value);
   ~ScopedCrashKey();
 
  private:
   std::string key_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedCrashKey);
 };
 
+// Before setting values for a key, all the keys must be registered.
+struct BASE_EXPORT CrashKey {
+  // The name of the crash key, used in the above functions.
+  const char* const key_name;
+
+  // For values longer than max_length, the value can be chunked into values
+  // named "key_name-1", "key_name-2", etc. This is the maximum number of
+  // numbered chunks to use before the value is truncated.
+  size_t num_chunks;
+
+  // The maximum length of a single chunk. Truncation happens if the value is
+  // greater than |num_chunks * max_length|.
+  size_t max_length;
+};
+
+// Before the crash key logging mechanism can be used, all crash keys must be
+// registered with this function. The crash reporting implementation should
+// allocate space for exactly |out_key_storage_size| crash keys.
+BASE_EXPORT void InitCrashKeys(const CrashKey* const keys, size_t count,
+                               size_t* out_key_storage_size);

[Scott Hess - ex-Googler, 2013/01/08 00:34:37]

I guess there must be something I'm missing, here.  The crash keys are being
initialized by some caller who presumably already knows the out_key_storage_size
info, more-or-less?  My main thought about the num_chunks/max_length in CrashKey
is that they are useful for when a caller calls Set/Clear, not for calculating
the implementation space requirements.

If you just need to return a count, you could always have it be the return value
:-).

[Robert Sesek, 2013/01/08 00:56:23]

My assumption is that when the caller registers the key, they register the space
for it and should know that.

Windows requires all space for crash keys be allocated at Breakpad
initialization. I went the route of calculating the exact amount of space
required. The other option is to allocate a "safe" number. But how do you
determine that safe number? Ideally you don't register crash keys at all, I
think (like how Mac worked before), but I did it because of the Windows
limitation. So I guess that leaves these options:

1) Calculate exact space requirements as done here.
2) Remove key registration requirement and guess at a "safe" number of keys (64,
128, 256?).
3) Keep the key registration requirement and use the size of the registration
list * some factor (1.25?) to guess the number of keys for which space should be
allocated.

I implemented (1) and chunking/length properties are determined at registration.
(2) and (3) could push chunking/length down to the Set() callsite.

[Scott Hess - ex-Googler, 2013/01/08 01:06:06]

I think you're over-analyzing.  Mostly, I'm thinking "The caller passed me a wad
of crap, and I'm passing back a count of the number of items in your wad of
crap".  But I would have assumed that the caller already knew how much crap it
had.  Is that right?

I can see why it might be right.  I'm just wondering if it is right.  It kind of
assumes that the caller can be sure it can allocate the space after calling this
function, which may-or-may-not be true (it could have to allocate before
calling, for instance).

I guess maybe you could decouple implementation from this API, and just say
"returns the number of crash keys needed to handle |keys|".  Then I don't feel
like the function is instructing me to do things a certain way.  The caller can
do whatever it wants with that information, either allocate the space now, or
verify that previously-allocated space was big enough, or on Mac/Linux ignore it
completely.

+
+// Returns the correspnding crash key object or NULL for a given key.
+BASE_EXPORT const CrashKey* LookupCrashKey(const base::StringPiece& key);
+
 typedef void (*SetCrashKeyValueFuncT)(const base::StringPiece&,
                                       const base::StringPiece&);
 typedef void (*ClearCrashKeyValueFuncT)(const base::StringPiece&);
 
 // Sets the function pointers that are used to integrate with the platform-
 // specific crash reporting libraries.
 BASE_EXPORT void SetCrashKeyReportingFunctions(
     SetCrashKeyValueFuncT set_key_func,
     ClearCrashKeyValueFuncT clear_key_func);
 
+// Helper function that breaks up a value according to the parameters
+// specified by the crash key object.
+BASE_EXPORT std::vector<std::string> ChunkCrashKeyValue(
+    const CrashKey& crash_key, const base::StringPiece& value);
+
+// Resets the crash key system so it can be reinitialized. For testing only.
+BASE_EXPORT void ResetCrashLoggingForTesting();
+
 }  // namespace debug
 }  // namespace base
 
 #endif  // BASE_DEBUG_CRASH_LOGGING_H_