Histogram argument related change

1. HistogramBase supports HasConstructionArguments.
2. Move Histogram::InspectConstructionArguments() to public, so chromeos
does not need to extend Histogram to do arguments checking.

Also grouped OVERRIDE functions in histogram.cc/h together.

TBR=zellidrag@chromium.org


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

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

5 files changed, 91 insertions, 77 deletions

diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index ec1afdd..2ccedc1 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -174,17 +174,6 @@ void Histogram::InitializeBucketRanges(Sample minimum,
   ranges->ResetChecksum();
 }
 
-void Histogram::Add(int value) {
-  DCHECK_EQ(0, ranges(0));
-  DCHECK_EQ(kSampleType_MAX, ranges(bucket_count_));
-
-  if (value > kSampleType_MAX - 1)
-    value = kSampleType_MAX - 1;
-  if (value < 0)
-    value = 0;
-  samples_->Accumulate(value, 1);
-}
-
 void Histogram::AddBoolean(bool value) {
   DCHECK(false);
 }
@@ -201,18 +190,6 @@ void Histogram::SetRangeDescriptions(const DescriptionPair descriptions[]) {
   DCHECK(false);
 }
 
-// The following methods provide a graphical histogram display.
-void Histogram::WriteHTMLGraph(string* output) const {
-  // TBD(jar) Write a nice HTML bar chart, with divs an mouse-overs etc.
-  output->append("<PRE>");
-  WriteAsciiImpl(true, "<br>", output);
-  output->append("</PRE>");
-}
-
-void Histogram::WriteAscii(string* output) const {
-  WriteAsciiImpl(true, "\n", output);
-}
-
 // static
 string Histogram::SerializeHistogramInfo(const Histogram& histogram,
                                          const HistogramSamples& snapshot) {
@@ -367,17 +344,69 @@ size_t Histogram::bucket_count() const {
   return bucket_count_;
 }
 
-scoped_ptr<HistogramSamples> Histogram::SnapshotSamples() const {
-  return SnapshotSampleVector().PassAs<HistogramSamples>();
+// static
+bool Histogram::InspectConstructionArguments(const string& name,
+                                             Sample* minimum,
+                                             Sample* maximum,
+                                             size_t* bucket_count) {
+  // Defensive code for backward compatibility.
+  if (*minimum < 1) {
+    DVLOG(1) << "Histogram: " << name << " has bad minimum: " << *minimum;
+    *minimum = 1;
+  }
+  if (*maximum >= kSampleType_MAX) {
+    DVLOG(1) << "Histogram: " << name << " has bad maximum: " << *maximum;
+    *maximum = kSampleType_MAX - 1;
+  }
+  if (*bucket_count >= kBucketCount_MAX) {
+    DVLOG(1) << "Histogram: " << name << " has bad bucket_count: "
+             << *bucket_count;
+    *bucket_count = kBucketCount_MAX - 1;
+  }
+
+  if (*minimum >= *maximum)
+    return false;
+  if (*bucket_count < 3)
+    return false;
+  if (*bucket_count > static_cast<size_t>(*maximum - *minimum + 2))
+    return false;
+  return true;
 }
 
 bool Histogram::HasConstructionArguments(Sample minimum,
                                          Sample maximum,
-                                         size_t bucket_count) {
+                                         size_t bucket_count) const {
   return ((minimum == declared_min_) && (maximum == declared_max_) &&
           (bucket_count == bucket_count_));
 }
 
+void Histogram::Add(int value) {
+  DCHECK_EQ(0, ranges(0));
+  DCHECK_EQ(kSampleType_MAX, ranges(bucket_count_));
+
+  if (value > kSampleType_MAX - 1)
+    value = kSampleType_MAX - 1;
+  if (value < 0)
+    value = 0;
+  samples_->Accumulate(value, 1);
+}
+
+scoped_ptr<HistogramSamples> Histogram::SnapshotSamples() const {
+  return SnapshotSampleVector().PassAs<HistogramSamples>();
+}
+
+// The following methods provide a graphical histogram display.
+void Histogram::WriteHTMLGraph(string* output) const {
+  // TBD(jar) Write a nice HTML bar chart, with divs an mouse-overs etc.
+  output->append("<PRE>");
+  WriteAsciiImpl(true, "<br>", output);
+  output->append("</PRE>");
+}
+
+void Histogram::WriteAscii(string* output) const {
+  WriteAsciiImpl(true, "\n", output);
+}
+
 Histogram::Histogram(const string& name,
                      Sample minimum,
                      Sample maximum,
@@ -400,35 +429,6 @@ Histogram::~Histogram() {
   }
 }
 
-// static
-bool Histogram::InspectConstructionArguments(const string& name,
-                                             Sample* minimum,
-                                             Sample* maximum,
-                                             size_t* bucket_count) {
-  // Defensive code for backward compatibility.
-  if (*minimum < 1) {
-    DVLOG(1) << "Histogram: " << name << " has bad minimum: " << *minimum;
-    *minimum = 1;
-  }
-  if (*maximum >= kSampleType_MAX) {
-    DVLOG(1) << "Histogram: " << name << " has bad maximum: " << *maximum;
-    *maximum = kSampleType_MAX - 1;
-  }
-  if (*bucket_count >= kBucketCount_MAX) {
-    DVLOG(1) << "Histogram: " << name << " has bad bucket_count: "
-             << *bucket_count;
-    *bucket_count = kBucketCount_MAX - 1;
-  }
-
-  if (*minimum >= *maximum)
-    return false;
-  if (*bucket_count < 3)
-    return false;
-  if (*bucket_count > static_cast<size_t>(*maximum - *minimum + 2))
-    return false;
-  return true;
-}
-
 bool Histogram::SerializeRanges(Pickle* pickle) const {
   return true;
 }
diff --git a/base/metrics/histogram.h b/base/metrics/histogram.h
index 8c19be1..f3104eb 100644
--- a/base/metrics/histogram.h
+++ b/base/metrics/histogram.h
@@ -417,8 +417,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
                                      size_t bucket_count,
                                      BucketRanges* ranges);
 
-  virtual void Add(Sample value) OVERRIDE;
-
   // This method is an interface, used only by BooleanHistogram.
   virtual void AddBoolean(bool value);
 
@@ -433,10 +431,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
   // This method is an interface, used only by LinearHistogram.
   virtual void SetRangeDescriptions(const DescriptionPair descriptions[]);
 
-  // The following methods provide graphical histogram displays.
-  virtual void WriteHTMLGraph(std::string* output) const OVERRIDE;
-  virtual void WriteAscii(std::string* output) const OVERRIDE;
-
   // Convenience methods for serializing/deserializing the histograms.
   // Histograms from Renderer process are serialized and sent to the browser.
   // Browser process reconstructs the histogram from the pickled version
@@ -479,13 +473,26 @@ class BASE_EXPORT Histogram : public HistogramBase {
   virtual size_t bucket_count() const;
   const BucketRanges* bucket_ranges() const { return bucket_ranges_; }
 
-  // Snapshot the current complete set of sample data.
-  // Override with atomic/locked snapshot if needed.
-  virtual scoped_ptr<HistogramSamples> SnapshotSamples() const OVERRIDE;
+  // This function validates histogram construction arguments. It returns false
+  // if some of the arguments are totally bad.
+  // Note. Currently it allow some bad input, e.g. 0 as minimum, but silently
+  // converts it to good input: 1.
+  // TODO(kaiwang): Be more restrict and return false for any bad input, and
+  // make this a readonly validating function.
+  static bool InspectConstructionArguments(const std::string& name,
+                                           Sample* minimum,
+                                           Sample* maximum,
+                                           size_t* bucket_count);
 
+  // HistogramBase implementation:
   virtual bool HasConstructionArguments(Sample minimum,
                                         Sample maximum,
-                                        size_t bucket_count);
+                                        size_t bucket_count) const OVERRIDE;
+  virtual void Add(Sample value) OVERRIDE;
+  virtual scoped_ptr<HistogramSamples> SnapshotSamples() const OVERRIDE;
+  virtual void WriteHTMLGraph(std::string* output) const OVERRIDE;
+  virtual void WriteAscii(std::string* output) const OVERRIDE;
+
  protected:
   // |bucket_count| and |ranges| should contain the underflow and overflow
   // buckets. See top comments for example.
@@ -497,17 +504,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
 
   virtual ~Histogram();
 
-  // This function validates histogram construction arguments. It returns false
-  // if some of the arguments are totally bad.
-  // Note. Currently it allow some bad input, e.g. 0 as minimum, but silently
-  // converts it to good input: 1.
-  // TODO(kaiwang): Be more restrict and return false for any bad input, and
-  // make this a readonly validating function.
-  static bool InspectConstructionArguments(const std::string& name,
-                                           Sample* minimum,
-                                           Sample* maximum,
-                                           size_t* bucket_count);
-
   // Serialize the histogram's ranges to |*pickle|, returning true on success.
   // Most subclasses can leave this no-op implementation, but some will want to
   // override it, especially if the ranges cannot be re-derived from other
diff --git a/base/metrics/histogram_base.h b/base/metrics/histogram_base.h
index 211606a..5e27e1e 100644
--- a/base/metrics/histogram_base.h
+++ b/base/metrics/histogram_base.h
@@ -51,8 +51,17 @@ class BASE_EXPORT HistogramBase {
   void SetFlags(int32 flags);
   void ClearFlags(int32 flags);
 
+  // Whether the histogram has construction arguments as parameters specified.
+  // For histograms that don't have the concept of minimum, maximum or
+  // bucket_count, this function always returns false.
+  virtual bool HasConstructionArguments(Sample minimum,
+                                        Sample maximum,
+                                        size_t bucket_count) const = 0;
+
   virtual void Add(Sample value) = 0;
 
+  // Snapshot the current complete set of sample data.
+  // Override with atomic/locked snapshot if needed.
   virtual scoped_ptr<HistogramSamples> SnapshotSamples() const = 0;
 
   // The following methods provide graphical histogram displays.
diff --git a/base/metrics/sparse_histogram.cc b/base/metrics/sparse_histogram.cc
index bb9061c..e4e16e8 100644
--- a/base/metrics/sparse_histogram.cc
+++ b/base/metrics/sparse_histogram.cc
@@ -26,6 +26,13 @@ HistogramBase* SparseHistogram::FactoryGet(const string& name, int32 flags) {
 
 SparseHistogram::~SparseHistogram() {}
 
+bool SparseHistogram::HasConstructionArguments(Sample minimum,
+                                               Sample maximum,
+                                               size_t bucket_count) const {
+  // SparseHistogram never has min/max/bucket_count limit.
+  return false;
+}
+
 void SparseHistogram::Add(Sample value) {
   base::AutoLock auto_lock(lock_);
   sample_counts_[value]++;
diff --git a/base/metrics/sparse_histogram.h b/base/metrics/sparse_histogram.h
index d209fd8..cd127ee 100644
--- a/base/metrics/sparse_histogram.h
+++ b/base/metrics/sparse_histogram.h
@@ -28,10 +28,12 @@ class BASE_EXPORT_PRIVATE SparseHistogram : public HistogramBase {
   virtual ~SparseHistogram();
 
   // HistogramBase implementation:
+  virtual bool HasConstructionArguments(Sample minimum,
+                                        Sample maximum,
+                                        size_t bucket_count) const OVERRIDE;
   virtual void Add(Sample value) OVERRIDE;
   virtual void WriteHTMLGraph(std::string* output) const OVERRIDE;
   virtual void WriteAscii(std::string* output) const OVERRIDE;
-
   virtual scoped_ptr<HistogramSamples> SnapshotSamples() const OVERRIDE;
 
  private: