Change the FindHistogram interface of StatisticsRecorder

Review URL: https://chromiumcodereview.appspot.com/10802002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@147269 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 79 insertions, 77 deletions

diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index e176e8f..97069cc 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -79,8 +79,6 @@ Histogram* Histogram::FactoryGet(const std::string& name,
                                  Sample maximum,
                                  size_t bucket_count,
                                  Flags flags) {
-  Histogram* histogram(NULL);
-
   // Defensive code.
   if (minimum < 1)
     minimum = 1;
@@ -91,7 +89,8 @@ Histogram* Histogram::FactoryGet(const std::string& name,
   DCHECK_GT((Sample) bucket_count, 2);
   DCHECK_LE((Sample) bucket_count, maximum - minimum + 2);
 
-  if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
+  Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+  if (!histogram) {
     // Extra variable is not needed... but this keeps this section basically
     // identical to other derived classes in this file (and compiler will
     // optimize away the extra variable.
@@ -806,8 +805,6 @@ Histogram* LinearHistogram::FactoryGet(const std::string& name,
                                        Sample maximum,
                                        size_t bucket_count,
                                        Flags flags) {
-  Histogram* histogram(NULL);
-
   if (minimum < 1)
     minimum = 1;
   if (maximum > kSampleType_MAX - 1)
@@ -817,7 +814,8 @@ Histogram* LinearHistogram::FactoryGet(const std::string& name,
   DCHECK_GT((Sample) bucket_count, 2);
   DCHECK_LE((Sample) bucket_count, maximum - minimum + 2);
 
-  if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
+  Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+  if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     LinearHistogram* tentative_histogram =
         new LinearHistogram(name, minimum, maximum, bucket_count);
@@ -907,9 +905,8 @@ bool LinearHistogram::PrintEmptyBucket(size_t index) const {
 //------------------------------------------------------------------------------
 
 Histogram* BooleanHistogram::FactoryGet(const std::string& name, Flags flags) {
-  Histogram* histogram(NULL);
-
-  if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
+  Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+  if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     BooleanHistogram* tentative_histogram = new BooleanHistogram(name);
     tentative_histogram->InitializeBucketRange();
@@ -941,8 +938,6 @@ BooleanHistogram::BooleanHistogram(const std::string& name)
 Histogram* CustomHistogram::FactoryGet(const std::string& name,
                                        const std::vector<Sample>& custom_ranges,
                                        Flags flags) {
-  Histogram* histogram(NULL);
-
   // Remove the duplicates in the custom ranges array.
   std::vector<int> ranges = custom_ranges;
   ranges.push_back(0);  // Ensure we have a zero value.
@@ -956,7 +951,8 @@ Histogram* CustomHistogram::FactoryGet(const std::string& name,
 
   DCHECK_LT(ranges.back(), kSampleType_MAX);
 
-  if (!StatisticsRecorder::FindHistogram(name, &histogram)) {
+  Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+  if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     CustomHistogram* tentative_histogram = new CustomHistogram(name, ranges);
     tentative_histogram->InitializedCustomBucketRange(ranges);
diff --git a/base/metrics/statistics_recorder.cc b/base/metrics/statistics_recorder.cc
index 37b6f43..384c34f 100644
--- a/base/metrics/statistics_recorder.cc
+++ b/base/metrics/statistics_recorder.cc
@@ -243,18 +243,17 @@ void StatisticsRecorder::GetHistograms(Histograms* output) {
   }
 }
 
-bool StatisticsRecorder::FindHistogram(const std::string& name,
-                                       Histogram** histogram) {
+// static
+Histogram* StatisticsRecorder::FindHistogram(const std::string& name) {
   if (lock_ == NULL)
-    return false;
+    return NULL;
   base::AutoLock auto_lock(*lock_);
   if (!histograms_)
-    return false;
+    return NULL;
   HistogramMap::iterator it = histograms_->find(name);
   if (histograms_->end() == it)
-    return false;
-  *histogram = it->second;
-  return true;
+    return NULL;
+  return it->second;
 }
 
 // private static
diff --git a/base/metrics/statistics_recorder.h b/base/metrics/statistics_recorder.h
index b947c53..ee03f24 100644
--- a/base/metrics/statistics_recorder.h
+++ b/base/metrics/statistics_recorder.h
@@ -63,9 +63,8 @@ class BASE_EXPORT StatisticsRecorder {
   static void GetHistograms(Histograms* output);
 
   // Find a histogram by name. It matches the exact name. This method is thread
-  // safe.  If a matching histogram is not found, then the |histogram| is
-  // not changed.
-  static bool FindHistogram(const std::string& query, Histogram** histogram);
+  // safe.  It returns NULL if a matching histogram is not found.
+  static Histogram* FindHistogram(const std::string& name);
 
   static bool dump_on_exit() { return dump_on_exit_; }
 
diff --git a/chrome/browser/net/http_pipelining_compatibility_client_unittest.cc b/chrome/browser/net/http_pipelining_compatibility_client_unittest.cc
index 83598f9..1834783 100644
--- a/chrome/browser/net/http_pipelining_compatibility_client_unittest.cc
+++ b/chrome/browser/net/http_pipelining_compatibility_client_unittest.cc
@@ -192,9 +192,9 @@ class HttpPipeliningCompatibilityClientTest : public testing::Test {
  private:
   base::Histogram::SampleSet GetHistogram(const char* name) {
     base::Histogram::SampleSet sample;
-    base::Histogram* current_histogram = NULL;
     base::Histogram* cached_histogram = NULL;
-    base::StatisticsRecorder::FindHistogram(name, &current_histogram);
+    base::Histogram* current_histogram =
+        base::StatisticsRecorder::FindHistogram(name);
     if (ContainsKey(histograms_, name)) {
       cached_histogram = histograms_[name];
     }
diff --git a/chrome/browser/protector/default_search_provider_change_browsertest.cc b/chrome/browser/protector/default_search_provider_change_browsertest.cc
index 32d987e..321491b 100644
--- a/chrome/browser/protector/default_search_provider_change_browsertest.cc
+++ b/chrome/browser/protector/default_search_provider_change_browsertest.cc
@@ -128,8 +128,8 @@ class DefaultSearchProviderChangeTest : public InProcessBrowserTest {
   void ExpectHistogramCount(const std::string& name,
                             size_t bucket,
                             base::Histogram::Count count) {
-    base::Histogram* histogram;
-    EXPECT_TRUE(base::StatisticsRecorder::FindHistogram(name, &histogram));
+    base::Histogram* histogram = base::StatisticsRecorder::FindHistogram(name);
+    EXPECT_TRUE(histogram != NULL);
     base::Histogram::SampleSet sample;
     histogram->SnapshotSample(&sample);
     EXPECT_EQ(count, sample.counts(bucket)) <<
diff --git a/net/disk_cache/stats_histogram.cc b/net/disk_cache/stats_histogram.cc
index 22343d5..66c8e50 100644
--- a/net/disk_cache/stats_histogram.cc
+++ b/net/disk_cache/stats_histogram.cc
@@ -24,15 +24,12 @@ StatsHistogram::~StatsHistogram() {
 }
 
 StatsHistogram* StatsHistogram::FactoryGet(const std::string& name) {
-  Histogram* histogram(NULL);
-
   Sample minimum = 1;
   Sample maximum = disk_cache::Stats::kDataSizesLength - 1;
   size_t bucket_count = disk_cache::Stats::kDataSizesLength;
 
-  if (StatisticsRecorder::FindHistogram(name, &histogram)) {
-    DCHECK(histogram != NULL);
-  } else {
+  Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+  if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     StatsHistogram* stats_histogram =
         new StatsHistogram(name, minimum, maximum, bucket_count);
diff --git a/net/socket_stream/socket_stream_metrics_unittest.cc b/net/socket_stream/socket_stream_metrics_unittest.cc
index e0e11d6..d4daf1e 100644
--- a/net/socket_stream/socket_stream_metrics_unittest.cc
+++ b/net/socket_stream/socket_stream_metrics_unittest.cc
@@ -17,14 +17,13 @@ using base::StatisticsRecorder;
 namespace net {
 
 TEST(SocketStreamMetricsTest, ProtocolType) {
-  Histogram* histogram;
-
   // First we'll preserve the original values. We need to do this
   // as histograms can get affected by other tests. In particular,
   // SocketStreamTest and WebSocketTest can affect the histograms.
   Histogram::SampleSet original;
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.ProtocolType", &histogram)) {
+  Histogram* histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ProtocolType");
+  if (histogram) {
     histogram->SnapshotSample(&original);
   }
 
@@ -35,8 +34,9 @@ TEST(SocketStreamMetricsTest, ProtocolType) {
   SocketStreamMetrics wss2(GURL("wss://www.example.com/"));
   SocketStreamMetrics wss3(GURL("wss://www.example.com/"));
 
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ProtocolType", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ProtocolType");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
   Histogram::SampleSet sample;
@@ -49,12 +49,11 @@ TEST(SocketStreamMetricsTest, ProtocolType) {
 }
 
 TEST(SocketStreamMetricsTest, ConnectionType) {
-  Histogram* histogram;
-
   // First we'll preserve the original values.
   Histogram::SampleSet original;
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.ConnectionType", &histogram)) {
+  Histogram* histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ConnectionType");
+  if (histogram) {
     histogram->SnapshotSample(&original);
   }
 
@@ -68,8 +67,10 @@ TEST(SocketStreamMetricsTest, ConnectionType) {
   for (int i = 0; i < 4; ++i)
     metrics.OnCountConnectionType(SocketStreamMetrics::SSL_CONNECTION);
 
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ConnectionType", &histogram));
+
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ConnectionType");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
   Histogram::SampleSet sample;
@@ -83,12 +84,11 @@ TEST(SocketStreamMetricsTest, ConnectionType) {
 }
 
 TEST(SocketStreamMetricsTest, WireProtocolType) {
-  Histogram* histogram;
-
   // First we'll preserve the original values.
   Histogram::SampleSet original;
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.WireProtocolType", &histogram)) {
+  Histogram* histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.WireProtocolType");
+  if (histogram) {
     histogram->SnapshotSample(&original);
   }
 
@@ -99,8 +99,9 @@ TEST(SocketStreamMetricsTest, WireProtocolType) {
   for (int i = 0; i < 7; ++i)
     metrics.OnCountWireProtocolType(SocketStreamMetrics::WIRE_PROTOCOL_SPDY);
 
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.WireProtocolType", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.WireProtocolType");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
   Histogram::SampleSet sample;
@@ -112,8 +113,6 @@ TEST(SocketStreamMetricsTest, WireProtocolType) {
 }
 
 TEST(SocketStreamMetricsTest, OtherNumbers) {
-  Histogram* histogram;
-
   // First we'll preserve the original values.
   int64 original_received_bytes = 0;
   int64 original_received_counts = 0;
@@ -121,23 +120,28 @@ TEST(SocketStreamMetricsTest, OtherNumbers) {
   int64 original_sent_counts = 0;
 
   Histogram::SampleSet original;
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.ReceivedBytes", &histogram)) {
+
+  Histogram* histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ReceivedBytes");
+  if (histogram) {
     histogram->SnapshotSample(&original);
     original_received_bytes = original.sum();
   }
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.ReceivedCounts", &histogram)) {
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ReceivedCounts");
+  if (histogram) {
     histogram->SnapshotSample(&original);
     original_received_counts = original.sum();
   }
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.SentBytes", &histogram)) {
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.SentBytes");
+  if (histogram) {
     histogram->SnapshotSample(&original);
     original_sent_bytes = original.sum();
   }
-  if (StatisticsRecorder::FindHistogram(
-          "Net.SocketStream.SentCounts", &histogram)) {
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.SentCounts");
+  if (histogram) {
     histogram->SnapshotSample(&original);
     original_sent_counts = original.sum();
   }
@@ -156,47 +160,54 @@ TEST(SocketStreamMetricsTest, OtherNumbers) {
   Histogram::SampleSet sample;
 
   // ConnectionLatency.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ConnectionLatency", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ConnectionLatency");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // ConnectionEstablish.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ConnectionEstablish", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ConnectionEstablish");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // Duration.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.Duration", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.Duration");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   // We don't check the contents of the histogram as it's time sensitive.
 
   // ReceivedBytes.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ReceivedBytes", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ReceivedBytes");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(11, sample.sum() - original_received_bytes);  // 11 bytes read.
 
   // ReceivedCounts.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.ReceivedCounts", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.ReceivedCounts");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(2, sample.sum() - original_received_counts);  // 2 read requests.
 
   // SentBytes.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.SentBytes", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.SentBytes");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(222, sample.sum() - original_sent_bytes);  // 222 bytes sent.
 
   // SentCounts.
-  ASSERT_TRUE(StatisticsRecorder::FindHistogram(
-      "Net.SocketStream.SentCounts", &histogram));
+  histogram =
+      StatisticsRecorder::FindHistogram("Net.SocketStream.SentCounts");
+  ASSERT_TRUE(histogram != NULL);
   EXPECT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
   histogram->SnapshotSample(&sample);
   EXPECT_EQ(3, sample.sum() - original_sent_counts);  // 3 write requests.
diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc
index a082779..4721d4c 100644
--- a/net/url_request/url_request_throttler_unittest.cc
+++ b/net/url_request/url_request_throttler_unittest.cc
@@ -208,8 +208,8 @@ void URLRequestThrottlerEntryTest::SetUp() {
     // as other tests may affect them.
     const char* name = kHistogramNames[i];
     Histogram::SampleSet& original = original_samples_[name];
-    Histogram* histogram;
-    if (StatisticsRecorder::FindHistogram(name, &histogram)) {
+    Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+    if (histogram) {
       histogram->SnapshotSample(&original);
     }
   }
@@ -221,8 +221,8 @@ void URLRequestThrottlerEntryTest::CalculateHistogramDeltas() {
     Histogram::SampleSet& original = original_samples_[name];
     Histogram::SampleSet& sample = samples_[name];
 
-    Histogram* histogram;
-    if (StatisticsRecorder::FindHistogram(name, &histogram)) {
+    Histogram* histogram = StatisticsRecorder::FindHistogram(name);
+    if (histogram) {
       ASSERT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
       histogram->SnapshotSample(&sample);