Use factory to create histograms, and refcounts to track lifetimes

This is CL patch 377028 by Raman Tenneti, with minor changes to
make the try-bots happier.

It is cleanup that better ensures lifetimes of histograms (making it harder
for users to abuse them).  

bug=16495 (repairs leak induced by the first landing)
bug=18840 (should make leaks less possible)
tbr=raman.tenneti
Review URL: http://codereview.chromium.org/462027

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

5 files changed, 53 insertions, 7 deletions

diff --git a/net/disk_cache/histogram_macros.h b/net/disk_cache/histogram_macros.h
index 6ff66ea..6d63dc8 100644
--- a/net/disk_cache/histogram_macros.h
+++ b/net/disk_cache/histogram_macros.h
@@ -26,9 +26,11 @@
     UMA_HISTOGRAM_TIMES(name, Time::Now() - initial_time)
 
 #define UMA_HISTOGRAM_CACHE_ERROR(name, sample) do { \
-    static LinearHistogram counter((name), 0, 49, 50); \
-    counter.SetFlags(kUmaTargetedHistogramFlag); \
-    counter.Add(sample); \
+    static scoped_refptr<Histogram> counter = \
+        LinearHistogram::LinearHistogramFactoryGet(\
+            (name), 1, 49, 50); \
+    counter->SetFlags(kUmaTargetedHistogramFlag); \
+    counter->Add(sample); \
   } while (0)
 
 #ifdef NET_DISK_CACHE_BACKEND_IMPL_CC_
diff --git a/net/disk_cache/stats.cc b/net/disk_cache/stats.cc
index a351a6c..e69ea00 100644
--- a/net/disk_cache/stats.cc
+++ b/net/disk_cache/stats.cc
@@ -138,7 +138,8 @@ bool Stats::Init(BackendImpl* backend, uint32* storage_addr) {
         backend->ShouldReportAgain()) {
       // Stats may be reused when the cache is re-created, but we want only one
       // histogram at any given time.
-      size_histogram_.reset(new StatsHistogram("DiskCache.SizeStats"));
+      size_histogram_ =
+          StatsHistogram::StatsHistogramFactoryGet("DiskCache.SizeStats");
       size_histogram_->Init(this);
     }
   }
diff --git a/net/disk_cache/stats.h b/net/disk_cache/stats.h
index 1dcd39f..13536b1 100644
--- a/net/disk_cache/stats.h
+++ b/net/disk_cache/stats.h
@@ -84,7 +84,7 @@ class Stats {
   uint32 storage_addr_;
   int data_sizes_[kDataSizesLength];
   int64 counters_[MAX_COUNTER];
-  scoped_ptr<StatsHistogram> size_histogram_;
+  scoped_refptr<StatsHistogram> size_histogram_;
 
   DISALLOW_COPY_AND_ASSIGN(Stats);
 };
diff --git a/net/disk_cache/stats_histogram.cc b/net/disk_cache/stats_histogram.cc
index f605934..e6eaf90 100644
--- a/net/disk_cache/stats_histogram.cc
+++ b/net/disk_cache/stats_histogram.cc
@@ -12,6 +12,37 @@ namespace disk_cache {
 // Static.
 const Stats* StatsHistogram::stats_ = NULL;
 
+scoped_refptr<StatsHistogram> StatsHistogram::StatsHistogramFactoryGet(
+    const std::string& name) {
+  scoped_refptr<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.get() != NULL);
+  } else {
+    histogram = new StatsHistogram(name, minimum, maximum, bucket_count);
+    scoped_refptr<Histogram> registered_histogram(NULL);
+    StatisticsRecorder::FindHistogram(name, &registered_histogram);
+    if (registered_histogram.get() != NULL &&
+        registered_histogram.get() != histogram.get())
+      histogram = registered_histogram;
+  }
+
+  DCHECK(HISTOGRAM == histogram->histogram_type());
+  DCHECK(histogram->HasConstructorArguments(minimum, maximum, bucket_count));
+
+  // We're preparing for an otherwise unsafe upcast by ensuring we have the
+  // proper class type.
+  Histogram* temp_histogram = histogram.get();
+  StatsHistogram* temp_stats_histogram =
+      static_cast<StatsHistogram*>(temp_histogram);
+  scoped_refptr<StatsHistogram> return_histogram = temp_stats_histogram;
+  return return_histogram;
+}
+
 bool StatsHistogram::Init(const Stats* stats) {
   DCHECK(stats);
   if (stats_)
diff --git a/net/disk_cache/stats_histogram.h b/net/disk_cache/stats_histogram.h
index 8db3bb3..995d486 100644
--- a/net/disk_cache/stats_histogram.h
+++ b/net/disk_cache/stats_histogram.h
@@ -5,6 +5,8 @@
 #ifndef NET_DISK_CACHE_STATS_HISTOGRAM_H_
 #define NET_DISK_CACHE_STATS_HISTOGRAM_H_
 
+#include <string>
+
 #include "base/histogram.h"
 
 namespace disk_cache {
@@ -14,6 +16,10 @@ class Stats;
 // This class provides support for sending the disk cache size stats as a UMA
 // histogram. We'll provide our own storage and management for the data, and a
 // SampleSet with a copy of our data.
+//
+// Class derivation of Histogram "deprecated," and should not be copied, and
+// may eventually go away.
+//
 class StatsHistogram : public Histogram {
  public:
   class StatsSamples : public SampleSet {
@@ -23,10 +29,14 @@ class StatsHistogram : public Histogram {
     }
   };
 
-  explicit StatsHistogram(const char* name)
-      : Histogram(name, 1, 1, 2), init_(false) {}
+  explicit StatsHistogram(const std::string& name, Sample minimum,
+                          Sample maximum, size_t bucket_count)
+      : Histogram(name, minimum, maximum, bucket_count), init_(false) {}
   ~StatsHistogram();
 
+  static scoped_refptr<StatsHistogram>
+      StatsHistogramFactoryGet(const std::string& name);
+
   // We'll be reporting data from the given set of cache stats.
   bool Init(const Stats* stats);
 
@@ -35,6 +45,8 @@ class StatsHistogram : public Histogram {
   virtual void SnapshotSample(SampleSet* sample) const;
 
  private:
+  friend class Histogram;
+
   bool init_;
   static const Stats* stats_;
   DISALLOW_COPY_AND_ASSIGN(StatsHistogram);