crash_keys: Convert "variations" and "num-experiments" to the new API.

Tbr: pkl@chromium.org
Bug: 598854
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ia842c49d115bdbc3b9109039d6eaf6862fd82ff4
Reviewed-on: https://chromium-review.googlesource.com/836207
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525515}

android_webview/common/crash_reporter/crash_keys.cc

@@ -19,17 +19,7 @@ const char kAppPackageVersionCode[] = "app-package-version-code";
 const char kAndroidSdkInt[] = "android-sdk-int";
 
 size_t RegisterWebViewCrashKeys() {
-  base::debug::CrashKey fixed_keys[] = {
-      {kNumVariations, kSmallSize},
-      {kVariations, kHugeSize},
-  };
-
-  // This dynamic set of keys is used for sets of key value pairs when gathering
-  // a collection of data, like command line switches or extension IDs.
-  std::vector<base::debug::CrashKey> keys(fixed_keys,
-                                          fixed_keys + arraysize(fixed_keys));
-
-  return base::debug::InitCrashKeys(&keys.at(0), keys.size(), kChunkMaxLength);
+  return 0;
 }
 
 // clang-format off

chrome/app/chrome_crash_reporter_client_win.cc

@@ -38,22 +38,7 @@ namespace {
 using namespace crash_keys;
 
 size_t RegisterCrashKeysHelper() {
-  // The following keys may be chunked by the underlying crash logging system,
-  // but ultimately constitute a single key-value pair.
-  //
-  // For now these need to be kept relatively up to date with those in
-  // chrome/common/crash_keys.cc::RegisterChromeCrashKeys().
-  static constexpr base::debug::CrashKey kFixedKeys[] = {
-      {kNumVariations, kSmallSize},
-      {kVariations, kHugeSize},
-  };
-
-  // This dynamic set of keys is used for sets of key value pairs when gathering
-  // a collection of data, like command line switches or extension IDs.
-  std::vector<base::debug::CrashKey> keys(std::begin(kFixedKeys),
-                                          std::end(kFixedKeys));
-
-  return base::debug::InitCrashKeys(&keys[0], keys.size(), kChunkMaxLength);
+  return 0;
 }
 
 }  // namespace

chrome/common/crash_keys.cc

@@ -26,25 +26,7 @@
 namespace crash_keys {
 
 size_t RegisterChromeCrashKeys() {
-  // The following keys may be chunked by the underlying crash logging system,
-  // but ultimately constitute a single key-value pair.
-  //
-  // If you're adding keys here, please also add them to the following lists:
-  // chrome/app/chrome_crash_reporter_client_win.cc::RegisterCrashKeysHelper(),
-  // android_webview/common/crash_reporter/crash_keys.cc::
-  //     RegisterWebViewCrashKeys(),
-  // chromecast/crash/cast_crash_keys.cc::RegisterCastCrashKeys().
-  base::debug::CrashKey fixed_keys[] = {
-    {kNumVariations, kSmallSize},
-    {kVariations, kHugeSize},
-  };
-
-  // This dynamic set of keys is used for sets of key value pairs when gathering
-  // a collection of data, like command line switches or extension IDs.
-  std::vector<base::debug::CrashKey> keys(
-      fixed_keys, fixed_keys + arraysize(fixed_keys));
-
-  return base::debug::InitCrashKeys(&keys.at(0), keys.size(), kChunkMaxLength);
+  return 0;
 }
 
 static bool IsBoringSwitch(const std::string& flag) {

chromecast/crash/cast_crash_keys.cc

@@ -11,17 +11,7 @@ namespace chromecast {
 namespace crash_keys {
 
 size_t RegisterCastCrashKeys() {
-  const base::debug::CrashKey fixed_keys[] = {
-      // TODO(sanfin): The following crash keys are copied from
-      // chrome/common/crash_keys.cc. When http://crbug.com/598854 is fixed,
-      // remove these and refactor as necessary.
-
-      {::crash_keys::kNumVariations, ::crash_keys::kSmallSize},
-      {::crash_keys::kVariations, ::crash_keys::kHugeSize},
-  };
-
-  return base::debug::InitCrashKeys(fixed_keys, arraysize(fixed_keys),
-                                    ::crash_keys::kChunkMaxLength);
+  return 0;
 }
 
 crash_reporter::CrashKeyString<64> last_app("last_app");

components/crash/core/common/crash_keys.cc

@@ -35,9 +35,6 @@ crash_reporter::CrashKeyString<40> client_id_key(kMetricsClientId);
 
 }  // namespace
 
-const char kNumVariations[] = "num-experiments";
-const char kVariations[] = "variations";
-
 void SetMetricsClientIdFromGUID(const std::string& metrics_client_guid) {
   std::string stripped_guid(metrics_client_guid);
   // Remove all instance of '-' char from the GUID. So BCD-WXY becomes BCDWXY.
@@ -68,22 +65,25 @@ void ClearMetricsClientId() {
 }
 
 void SetVariationsList(const std::vector<std::string>& variations) {
-  base::debug::SetCrashKeyValue(kNumVariations,
-      base::StringPrintf("%" PRIuS, variations.size()));
+  static crash_reporter::CrashKeyString<8> num_variations_key(
+      "num-experiments");
+  num_variations_key.Set(base::NumberToString(variations.size()));
+
+  static constexpr size_t kVariationsKeySize = 2048;
+  static crash_reporter::CrashKeyString<kVariationsKeySize> crash_key(
+      "variations");
 
   std::string variations_string;
-  variations_string.reserve(kHugeSize);
+  variations_string.reserve(kVariationsKeySize);
 
-  for (size_t i = 0; i < variations.size(); ++i) {
-    const std::string& variation = variations[i];
+  for (const auto& variation : variations) {
     // Do not truncate an individual experiment.
-    if (variations_string.size() + variation.size() >= kHugeSize)
+    if (variations_string.size() + variation.size() >= kVariationsKeySize)
       break;
     variations_string += variation;
     variations_string += ",";
   }
-
-  base::debug::SetCrashKeyValue(kVariations, variations_string);
+  crash_key.Set(variations_string);
 }
 
 using SwitchesCrashKey = crash_reporter::CrashKeyString<64>;

components/crash/core/common/crash_keys.h

@@ -69,15 +69,6 @@ const size_t kLargeSize = kSmallSize * 16;
 // be used very sparingly.
 const size_t kHugeSize = kLargeSize * 2;
 
-// Crash Key Name Constants ////////////////////////////////////////////////////
-
-// The total number of experiments the instance has.
-extern const char kNumVariations[];
-
-// The experiments chunk. Hashed experiment names separated by |,|. This is
-// typically set by SetExperimentList.
-extern const char kVariations[];
-
 }  // namespace crash_keys
 
 #endif  // COMPONENTS_CRASH_CORE_COMMON_CRASH_KEYS_H_

components/crash/core/common/crash_keys_unittest.cc

@@ -4,9 +4,6 @@
 
 #include "components/crash/core/common/crash_keys.h"
 
-#include <stddef.h>
-
-#include <map>
 #include <string>
 
 #include "base/command_line.h"
@@ -27,65 +24,20 @@ class CrashKeysTest : public testing::Test {
  public:
   void SetUp() override {
     ResetData();
-
     crash_reporter::InitializeCrashKeys();
-    self_ = this;
-    base::debug::SetCrashKeyReportingFunctions(
-        &SetCrashKeyValue, &ClearCrashKey);
-  }
-
-  bool InitVariationsCrashKeys() {
-    std::vector<base::debug::CrashKey> keys = {
-        {crash_keys::kNumVariations, crash_keys::kSmallSize},
-        {crash_keys::kVariations, crash_keys::kHugeSize}};
-    return InitCrashKeys(keys);
   }
 
   void TearDown() override {
-    base::debug::ResetCrashLoggingForTesting();
     ResetData();
-    self_ = nullptr;
-  }
-
-  bool HasCrashKey(const std::string& key) {
-    return keys_.find(key) != keys_.end();
-  }
-
-  std::string GetKeyValue(const std::string& key) {
-    std::map<std::string, std::string>::const_iterator it = keys_.find(key);
-    if (it == keys_.end())
-      return std::string();
-    return it->second;
   }
 
  private:
-  bool InitCrashKeys(const std::vector<base::debug::CrashKey>& keys) {
-    base::debug::InitCrashKeys(keys.data(), keys.size(),
-                               crash_keys::kChunkMaxLength);
-    return !keys.empty();
-  }
-
-  static void SetCrashKeyValue(const base::StringPiece& key,
-                               const base::StringPiece& value) {
-    self_->keys_[key.as_string()] = value.as_string();
-  }
-
-  static void ClearCrashKey(const base::StringPiece& key) {
-    self_->keys_.erase(key.as_string());
-  }
-
   void ResetData() {
     crash_keys::ResetCommandLineForTesting();
     crash_reporter::ResetCrashKeysForTesting();
   }
-
-  static CrashKeysTest* self_;
-
-  std::map<std::string, std::string> keys_;
 };
 
-CrashKeysTest* CrashKeysTest::self_ = nullptr;
-
 TEST_F(CrashKeysTest, Switches) {
   // Set three switches.
   {
@@ -159,35 +111,3 @@ TEST_F(CrashKeysTest, FilterFlags) {
         << "switch_name is " << switch_name;
   }
 }
-
-TEST_F(CrashKeysTest, VariationsCapacity) {
-  ASSERT_TRUE(InitVariationsCrashKeys());
-
-  // Variation encoding: two 32bit numbers encorded as hex with a '-' separator.
-  const char kSampleVariation[] = "12345678-12345678";
-  const size_t kVariationLen = std::strlen(kSampleVariation);
-  const size_t kSeparatedVariationLen = kVariationLen + 1U;
-  ASSERT_EQ(17U, kVariationLen);
-
-  // The expected capacity factors in a separator (',').
-  const size_t kExpectedCapacity = 112U;
-  ASSERT_EQ(kExpectedCapacity,
-            crash_keys::kHugeSize / (kSeparatedVariationLen));
-
-  // Create some variations and set the crash keys.
-  std::vector<std::string> variations;
-  for (size_t i = 0; i < kExpectedCapacity + 2; ++i)
-    variations.push_back(kSampleVariation);
-  crash_keys::SetVariationsList(variations);
-
-  // Validate crash keys.
-  ASSERT_TRUE(HasCrashKey(crash_keys::kNumVariations));
-  EXPECT_EQ("114", GetKeyValue(crash_keys::kNumVariations));
-
-  const size_t kExpectedChunks = (kSeparatedVariationLen * kExpectedCapacity) /
-                                 crash_keys::kChunkMaxLength;
-  for (size_t i = 0; i < kExpectedChunks; ++i) {
-    ASSERT_TRUE(HasCrashKey(
-        base::StringPrintf("%s-%" PRIuS, crash_keys::kVariations, i + 1)));
-  }
-}

ios/chrome/browser/crash_report/crash_keys.cc

@@ -11,13 +11,5 @@
 #include "components/crash/core/common/crash_keys.h"
 
 size_t RegisterChromeIOSCrashKeys() {
-  // The following keys may be chunked by the underlying crash logging system,
-  // but ultimately constitute a single key-value pair.
-  base::debug::CrashKey fixed_keys[] = {
-      {crash_keys::kNumVariations, crash_keys::kSmallSize},
-      {crash_keys::kVariations, crash_keys::kHugeSize},
-  };
-
-  return base::debug::InitCrashKeys(fixed_keys, arraysize(fixed_keys),
-                                    crash_keys::kChunkMaxLength);
+  return 0;
 }