SampleSet -> HistogramSamples (will be reused by SparseHistogram)

net/disk_cache/stats_histogram.h

Index: net/disk_cache/stats_histogram.h
===================================================================
--- net/disk_cache/stats_histogram.h	(revision 155400)
+++ net/disk_cache/stats_histogram.h	(working copy)
@@ -7,8 +7,15 @@
 
 #include <string>
 
+#include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 
+namespace base {
+class BucketRanges;
+class SampleVector;
+class HistogramSamples;
+};

[Ilya Sherman, 2012/09/12 03:20:58]

nit: No need for this semicolon

[kaiwang, 2012/09/20 22:54:59]

Done.

+
 namespace disk_cache {
 
 class Stats;
@@ -22,34 +29,25 @@
 //
 class StatsHistogram : public base::Histogram {
  public:
-  class StatsSamples : public SampleSet {
-   public:
-    Counts* GetCounts() {
-      return &counts_;
-    }
-  };
-
   StatsHistogram(const std::string& name,
                  Sample minimum,
                  Sample maximum,
-                 size_t bucket_count)
-      : Histogram(name, minimum, maximum, bucket_count, NULL), init_(false) {}
+                 size_t bucket_count,
+                 base::BucketRanges* ranges,
+                 const Stats* stats);

[Ilya Sherman, 2012/09/12 03:20:58]

Can this have private visibility, since clients are seemingly expected to use
the FactoryGet() factory instead?

[Ilya Sherman, 2012/09/21 08:52:12]

Ping.

   virtual ~StatsHistogram();
 
-  static StatsHistogram* FactoryGet(const std::string& name);
+  static void InitializeBucketRanges(const Stats* stats,
+                                     base::BucketRanges* ranges);

[Ilya Sherman, 2012/09/12 03:20:58]

nit: Can this be tucked into an anonymous namespace in the implementation file?

[Ilya Sherman, 2012/09/21 08:52:12]

Ping.

+  static StatsHistogram* FactoryGet(const std::string& name,
+                                    const Stats* stats);
 
-  // We'll be reporting data from the given set of cache stats.
-  bool Init(const Stats* stats);
-
-  virtual Sample ranges(size_t i) const OVERRIDE;
-  virtual size_t bucket_count() const OVERRIDE;
-  virtual void SnapshotSample(SampleSet* sample) const OVERRIDE;
+  virtual scoped_ptr<base::SampleVector> SnapshotSamples() const OVERRIDE;
   virtual Inconsistencies FindCorruption(
-      const SampleSet& snapshot) const OVERRIDE;
+      const base::HistogramSamples& samples) const OVERRIDE;
 
  private:
-  bool init_;
-  static const Stats* stats_;
+  const Stats* stats_;

[Ilya Sherman, 2012/09/12 03:20:58]

nit: const Stats* const stats_?

[Ilya Sherman, 2012/09/21 08:52:12]

Ping.

[kaiwang, 2012/09/21 19:51:18]

For above 3 comments, I tried to follow the original code (public and const),
and code in base/metricts(Initialize). I'm trying to limit my change only to
SampleSet -> HistogramSamples, so it doesnot make sense to change more in this
CL.
Current OWNER looked good. If you feel these code is a problem, feel free to
file another CL.

   DISALLOW_COPY_AND_ASSIGN(StatsHistogram);
 };