When chunking crash keys with Breakpad, let the collector re-stitch the values.

After internal cl/170262399, the crash collector will automatically stitch
together values named "key__1"..."key__N". Chromium's existing "key-1".."key-N"
syntax is special-cased in the processor on a per-key basis.

By switching to the key syntax handled by the collector, it will make converting
to the new crash key API easier for values that have to be chunked in Breakpad
(like the active URL and variations hashes). When the crash key reaches the
processor, it will already be assembled into a single value, rather than needing
to be joined by the processor.

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

android_webview/common/crash_reporter/crash_keys.cc

@@ -80,16 +80,18 @@ const char* const kWebViewCrashKeyWhiteList[] = {
     "gpu-psver",
     "gpu-vsver",
     "gpu-gl-vendor",
+    "gpu-gl-vendor__1",
+    "gpu-gl-vendor__2",
     "gpu-gl-renderer",
 
     // content/:
     "bad_message_reason",
     "discardable-memory-allocated",
     "discardable-memory-free",
-    "mojo-message-error-1",
-    "mojo-message-error-2",
-    "mojo-message-error-3",
-    "mojo-message-error-4",
+    "mojo-message-error__1",
+    "mojo-message-error__2",
+    "mojo-message-error__3",
+    "mojo-message-error__4",
     "total-discardable-memory-allocated",
     nullptr};
 // clang-format on

components/crash/core/common/crash_key_breakpad.cc

@@ -23,8 +23,10 @@ namespace internal {
 
 namespace {
 
-// String used to format chunked key names.
-const char kChunkFormatString[] = "%s-%" PRIuS;
+// String used to format chunked key names. The __1 through __N syntax is
+// recognized by the crash collector, which will then stitch the numbered
+// parts back into a single string value.
+const char kChunkFormatString[] = "%s__%" PRIuS;
 
 static TransitionalCrashKeyStorage* g_storage = nullptr;
 

components/crash/core/common/crash_key_breakpad_unittest.cc

@@ -98,12 +98,12 @@ TEST_F(CrashKeyBreakpadTest, SetChunked) {
   // Since chunk1 through chunk3 are the same size as a storage slot,
   // and the storage NUL-terminates the value, ensure no bytes are
   // lost when chunking.
-  EXPECT_EQ(std::string(127, 'A'), storage()->GetValueForKey("chunky-1"));
+  EXPECT_EQ(std::string(127, 'A'), storage()->GetValueForKey("chunky__1"));
   EXPECT_EQ(std::string("A") + std::string(126, 'B'),
-            storage()->GetValueForKey("chunky-2"));
+            storage()->GetValueForKey("chunky__2"));
   EXPECT_EQ(std::string(2, 'B') + std::string(125, 'C'),
-            storage()->GetValueForKey("chunky-3"));
-  EXPECT_EQ(std::string(3, 'C'), storage()->GetValueForKey("chunky-4"));
+            storage()->GetValueForKey("chunky__3"));
+  EXPECT_EQ(std::string(3, 'C'), storage()->GetValueForKey("chunky__4"));
 
   std::string chunk4(240, 'D');
 
@@ -111,9 +111,10 @@ TEST_F(CrashKeyBreakpadTest, SetChunked) {
 
   ASSERT_EQ(2u, storage()->GetCount());
 
-  EXPECT_EQ(std::string(127, 'D'), storage()->GetValueForKey("chunky-1"));
-  EXPECT_EQ(std::string(240 - 127, 'D'), storage()->GetValueForKey("chunky-2"));
-  EXPECT_FALSE(storage()->GetValueForKey("chunky-3"));
+  EXPECT_EQ(std::string(127, 'D'), storage()->GetValueForKey("chunky__1"));
+  EXPECT_EQ(std::string(240 - 127, 'D'),
+            storage()->GetValueForKey("chunky__2"));
+  EXPECT_FALSE(storage()->GetValueForKey("chunky__3"));
 
   key.Clear();
 
@@ -130,41 +131,41 @@ TEST_F(CrashKeyBreakpadTest, SetTwoChunked) {
 
   ASSERT_EQ(2u, storage()->GetCount());
 
-  EXPECT_EQ(std::string(127, '1'), storage()->GetValueForKey("big-1"));
-  EXPECT_EQ(std::string(73, '1'), storage()->GetValueForKey("big-2"));
+  EXPECT_EQ(std::string(127, '1'), storage()->GetValueForKey("big__1"));
+  EXPECT_EQ(std::string(73, '1'), storage()->GetValueForKey("big__2"));
 
   key2.Set(std::string(256, '2').c_str());
 
   ASSERT_EQ(5u, storage()->GetCount());
 
-  EXPECT_EQ(std::string(127, '1'), storage()->GetValueForKey("big-1"));
-  EXPECT_EQ(std::string(73, '1'), storage()->GetValueForKey("big-2"));
-  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small-1"));
-  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small-2"));
-  EXPECT_EQ(std::string(2, '2'), storage()->GetValueForKey("small-3"));
+  EXPECT_EQ(std::string(127, '1'), storage()->GetValueForKey("big__1"));
+  EXPECT_EQ(std::string(73, '1'), storage()->GetValueForKey("big__2"));
+  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small__1"));
+  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small__2"));
+  EXPECT_EQ(std::string(2, '2'), storage()->GetValueForKey("small__3"));
 
   key1.Set(std::string(510, '3').c_str());
 
   ASSERT_EQ(8u, storage()->GetCount());
 
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-1"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-2"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-3"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-4"));
-  EXPECT_EQ(std::string(2, '3'), storage()->GetValueForKey("big-5"));
-  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small-1"));
-  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small-2"));
-  EXPECT_EQ(std::string(2, '2'), storage()->GetValueForKey("small-3"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__1"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__2"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__3"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__4"));
+  EXPECT_EQ(std::string(2, '3'), storage()->GetValueForKey("big__5"));
+  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small__1"));
+  EXPECT_EQ(std::string(127, '2'), storage()->GetValueForKey("small__2"));
+  EXPECT_EQ(std::string(2, '2'), storage()->GetValueForKey("small__3"));
 
   key2.Clear();
 
   ASSERT_EQ(5u, storage()->GetCount());
 
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-1"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-2"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-3"));
-  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big-4"));
-  EXPECT_EQ(std::string(2, '3'), storage()->GetValueForKey("big-5"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__1"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__2"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__3"));
+  EXPECT_EQ(std::string(127, '3'), storage()->GetValueForKey("big__4"));
+  EXPECT_EQ(std::string(2, '3'), storage()->GetValueForKey("big__5"));
 }
 
 }  // namespace crash_reporter