Changing whitelist to allowlist in histogram expiry code.

This will need a follow up cleanup cl once the parameter is changed in
the variations config.

BUG=b/160157778

Change-Id: I6f29d98223eeb04de880e302870af642e6e73292
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507340Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Commit-Queue: Jesse Doherty <jwd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823228}

components/metrics/expired_histogram_util.cc

@@ -15,20 +15,27 @@ namespace {
 const base::Feature kExpiredHistogramLogicFeature{
     "ExpiredHistogramLogic", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// TODO(jwd): Remove when the study config is updated with the new param.
 const base::FeatureParam<std::string> kWhitelistParam{
     &kExpiredHistogramLogicFeature, "whitelist", ""};
 
+const base::FeatureParam<std::string> kAllowlistParam{
+    &kExpiredHistogramLogicFeature, "allowlist", ""};
+
 }  // namespace
 
 void EnableExpiryChecker(const uint64_t* expired_histograms_hashes,
                          size_t num_expired_histograms) {
   DCHECK(base::FeatureList::GetInstance());
   if (base::FeatureList::IsEnabled(kExpiredHistogramLogicFeature)) {
+    std::string allowlist = kAllowlistParam.Get();
+    // TODO(jwd): Remove when the study config is updated with the new param.
+    if (allowlist.empty())
+      allowlist = kWhitelistParam.Get();
     base::StatisticsRecorder::SetRecordChecker(
-        std::make_unique<ExpiredHistogramsChecker>(expired_histograms_hashes,
-                                                   num_expired_histograms,
-                                                   kWhitelistParam.Get()));
+        std::make_unique<ExpiredHistogramsChecker>(
+            expired_histograms_hashes, num_expired_histograms, allowlist));
   }
 }
 
-}  // namespace metrics
\ No newline at end of file
+}  // namespace metrics

components/metrics/expired_histograms_checker.cc

@@ -15,27 +15,28 @@
 namespace metrics {
 
 ExpiredHistogramsChecker::ExpiredHistogramsChecker(
-    const uint64_t* array,
+    const uint64_t* expired_histogram_hashes,
     size_t size,
-    const std::string& whitelist_str)
-    : array_(array), size_(size) {
-  InitWhitelist(whitelist_str);
+    const std::string& allowlist_str)
+    : expired_histogram_hashes_(expired_histogram_hashes), size_(size) {
+  InitAllowlist(allowlist_str);
 }
 
 ExpiredHistogramsChecker::~ExpiredHistogramsChecker() {}
 
 bool ExpiredHistogramsChecker::ShouldRecord(uint64_t histogram_hash) const {
-  // If histogram is whitelisted then it should always be recorded.
-  if (base::Contains(whitelist_, histogram_hash))
+  // If histogram is explicitly allowed then it should always be recorded.
+  if (base::Contains(allowlist_, histogram_hash))
     return true;
-  return !std::binary_search(array_, array_ + size_, histogram_hash);
+  return !std::binary_search(expired_histogram_hashes_,
+                             expired_histogram_hashes_ + size_, histogram_hash);
 }
 
-void ExpiredHistogramsChecker::InitWhitelist(const std::string& whitelist_str) {
-  std::vector<base::StringPiece> whitelist_names = base::SplitStringPiece(
-      whitelist_str, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  for (base::StringPiece name : whitelist_names)
-    whitelist_.insert(base::HashMetricName(name));
+void ExpiredHistogramsChecker::InitAllowlist(const std::string& allowlist_str) {
+  std::vector<base::StringPiece> allowlist_names = base::SplitStringPiece(
+      allowlist_str, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  for (base::StringPiece name : allowlist_names)
+    allowlist_.insert(base::HashMetricName(name));
 }
 
 }  // namespace metrics

components/metrics/expired_histograms_checker.h

@@ -18,30 +18,31 @@ namespace metrics {
 // to avoid recording expired metrics.
 class ExpiredHistogramsChecker final : public base::RecordHistogramChecker {
  public:
-  // Takes sorted in nondecreasing order array of histogram hashes, its size and
-  // list of whitelisted histogram names concatenated as a comma-separated
-  // string.
-  ExpiredHistogramsChecker(const uint64_t* array,
+  // Takes a sorted array of histogram hashes in ascending order, its size and a
+  // list of explicitly allowed histogram names as a comma-separated string.
+  // Histograms in the |allowlist_str| are logged even if their hash is in the
+  // |expired_histograms_hashes|.
+  ExpiredHistogramsChecker(const uint64_t* expired_histogram_hashes,
                            size_t size,
-                           const std::string& whitelist_str);
+                           const std::string& allowlist_str);
   ~ExpiredHistogramsChecker() override;
 
   // Checks if the given |histogram_hash| corresponds to an expired histogram.
   bool ShouldRecord(uint64_t histogram_hash) const override;
 
  private:
-  // Initializes the |whitelist_| array of histogram hashes that should be
+  // Initializes the |allowlist_| array of histogram hashes that should be
   // recorded regardless of their expiration.
-  void InitWhitelist(const std::string& whitelist_str);
+  void InitAllowlist(const std::string& allowlist_str);
 
   // Array of expired histogram hashes.
-  const uint64_t* const array_;
+  const uint64_t* const expired_histogram_hashes_;
 
-  // Size of the |array_|.
+  // Size of the |expired_histogram_hashes_|.
   const size_t size_;
 
-  // List of expired histogram hashes that should be recorded.
-  std::set<uint64_t> whitelist_;
+  // Set of expired histogram hashes that should be recorded.
+  std::set<uint64_t> allowlist_;
 
   DISALLOW_COPY_AND_ASSIGN(ExpiredHistogramsChecker);
 };

components/metrics/expired_histograms_checker_unittest.cc

@@ -12,14 +12,14 @@ namespace metrics {
 TEST(ExpiredHistogramsCheckerTests, BasicTest) {
   uint64_t expired_hashes[] = {1, 2, 3};
   size_t size = 3;
-  std::string whitelist_str = "";
-  ExpiredHistogramsChecker checker(expired_hashes, size, whitelist_str);
+  std::string allowlist_str = "";
+  ExpiredHistogramsChecker checker(expired_hashes, size, allowlist_str);
 
   EXPECT_TRUE(checker.ShouldRecord(0));
   EXPECT_FALSE(checker.ShouldRecord(3));
 }
 
-TEST(ExpiredHistogramsCheckerTests, WhitelistTest) {
+TEST(ExpiredHistogramsCheckerTests, AllowlistTest) {
   std::string hist1 = "hist1";
   std::string hist2 = "hist2";
   std::string hist3 = "hist3";
@@ -28,8 +28,8 @@ TEST(ExpiredHistogramsCheckerTests, WhitelistTest) {
   uint64_t expired_hashes[] = {base::HashMetricName(hist1),
                                base::HashMetricName(hist2)};
   size_t size = 2;
-  std::string whitelist_str = hist2 + "," + hist4;
-  ExpiredHistogramsChecker checker(expired_hashes, size, whitelist_str);
+  std::string allowlist_str = hist2 + "," + hist4;
+  ExpiredHistogramsChecker checker(expired_hashes, size, allowlist_str);
 
   EXPECT_FALSE(checker.ShouldRecord(base::HashMetricName(hist1)));
   EXPECT_TRUE(checker.ShouldRecord(base::HashMetricName(hist2)));
@@ -37,4 +37,4 @@ TEST(ExpiredHistogramsCheckerTests, WhitelistTest) {
   EXPECT_TRUE(checker.ShouldRecord(base::HashMetricName(hist4)));
 }
 
-}  // namespace metrics
\ No newline at end of file
+}  // namespace metrics