Match only prefixes of histogram names.

base/test/histogram_tester.cc

 // Copyright 2014 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.
 
 #include "base/test/histogram_tester.h"
 
 #include <stddef.h>
 
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/metrics_hashes.h"
 #include "base/metrics/sample_map.h"
 #include "base/metrics/statistics_recorder.h"
+#include "base/strings/string_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
 HistogramTester::HistogramTester() {
   StatisticsRecorder::Initialize();  // Safe to call multiple times.
 
   // Record any histogram data that exists when the object is created so it can
   // be subtracted later.
   StatisticsRecorder::Histograms histograms;
@@ skipping 71 matching lines @@
       HistogramBase::Sample sample;
       HistogramBase::Count count;
       it->Get(&sample, nullptr, &count);
       samples.push_back(Bucket(sample, count));
     }
   }
   return samples;
 }
 
 HistogramTester::CountsMap HistogramTester::GetTotalCountsForPrefix(
     const std::string& query) const {

[Ilya Sherman, 2017/01/12 20:32:41]

nit: While you're fixing up this method, please update this parameter name to be
"prefix", both here and in the header file.  ("query" makes a bit more sense for
the StatisticsRecorder code, which is primarily dealing with HTML query
strings.)

[pkalinnikov, 2017/01/13 10:28:59]

Done.

   EXPECT_TRUE(query.find('.') != std::string::npos)
       << "|query| ought to contain at least one period, to avoid matching too"
       << " many histograms.";
 
   // Find matches by using the prefix-matching logic built into GetSnapshot().

[Paweł Hajdan Jr., 2017/01/11 20:45:00]

This suggests prefix-matching logic in GetSnapshot was supposed to handle things
correctly.

Could you consult other histogram owners to find the best place for the logic?

I'd be fine with this code, but there may be a better solution.

[Ilya Sherman, 2017/01/12 20:32:41]

This comment is not quite correct.  StatisticsRecorder::GetSnapshot()
intentionally filters by substring rather than by prefix.  So, let's update or
omit this comment.

[pkalinnikov, 2017/01/13 10:28:59]

Updated the comment.

   StatisticsRecorder::Histograms query_matches;
   StatisticsRecorder::GetSnapshot(query, &query_matches);
 
   CountsMap result;
   for (base::HistogramBase* histogram : query_matches) {

[Ilya Sherman, 2017/01/12 20:32:41]

Optional nit: While you're here, it would be nice to drop this unnecessary use
of the "base::" namespace.

[pkalinnikov, 2017/01/13 10:28:59]

Done here and throughout this file and its header.

+    if (!StartsWith(histogram->histogram_name(), query, CompareCase::SENSITIVE))
+      continue;
     std::unique_ptr<HistogramSamples> new_samples =
         GetHistogramSamplesSinceCreation(histogram->histogram_name());
     // Omit unchanged histograms from the result.
     if (new_samples->TotalCount()) {
       result[histogram->histogram_name()] = new_samples->TotalCount();
     }
   }
   return result;
 }
 
@@ skipping 53 matching lines @@
 
 bool Bucket::operator==(const Bucket& other) const {
   return min == other.min && count == other.count;
 }
 
 void PrintTo(const Bucket& bucket, std::ostream* os) {
   *os << "Bucket " << bucket.min << ": " << bucket.count;
 }
 
 }  // namespace base