Serialize/Deserialize support in HistogramBase

base/metrics/histogram.h

Index: base/metrics/histogram.h
diff --git a/base/metrics/histogram.h b/base/metrics/histogram.h
index 24585b7ab1dee6679106c85aff17dea87ed46b14..56b01307d1aa223e9f1fecb64ea3ed4af0f5caab 100644
--- a/base/metrics/histogram.h
+++ b/base/metrics/histogram.h
@@ -371,11 +371,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
     NEVER_EXCEEDED_VALUE = 0x10
   };
 
-  struct DescriptionPair {
-    Sample sample;
-    const char* description;  // Null means end of a list of pairs.
-  };
-
   //----------------------------------------------------------------------------
   // For a valid histogram, input should follow these restrictions:
   // minimum > 0 (if a minimum below 1 is specified, it will implicitly be
@@ -414,25 +409,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
     Add(static_cast<int>(time.InMilliseconds()));
   }
 
-  void AddSamples(const HistogramSamples& samples);
-  bool AddSamplesFromPickle(PickleIterator* iter);
-
-  // 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
-  // accumulates the browser-side shadow copy of histograms (that mirror
-  // histograms created in the renderer).
-
-  // Serialize the given snapshot of a Histogram into a String. Uses
-  // Pickle class to flatten the object.
-  static std::string SerializeHistogramInfo(const Histogram& histogram,
-                                            const HistogramSamples& snapshot);
-
-  // The following method accepts a list of pickled histograms and
-  // builds a histogram and updates shadow copy of histogram data in the
-  // browser process.
-  static bool DeserializeHistogramInfo(const std::string& histogram_info);
-
   // This constant if for FindCorruption. Since snapshots of histograms are
   // taken asynchronously relative to sampling, and our counting code currently
   // does not prevent race conditions, it is pretty likely that we'll catch a
@@ -476,6 +452,8 @@ class BASE_EXPORT Histogram : public HistogramBase {
                                         size_t bucket_count) const OVERRIDE;
   virtual void Add(Sample value) OVERRIDE;
   virtual scoped_ptr<HistogramSamples> SnapshotSamples() const OVERRIDE;
+  virtual void AddSamples(const HistogramSamples& samples) OVERRIDE;
+  virtual bool AddSamplesFromPickle(PickleIterator* iter) OVERRIDE;
   virtual void WriteHTMLGraph(std::string* output) const OVERRIDE;
   virtual void WriteAscii(std::string* output) const OVERRIDE;
 
@@ -490,11 +468,8 @@ class BASE_EXPORT Histogram : public HistogramBase {
 
   virtual ~Histogram();
 
-  // 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
-  // serialized parameters.
-  virtual bool SerializeRanges(Pickle* pickle) const;
+  // HistogramBase implementation:
+  virtual bool SerializeInfoImpl(Pickle* pickle) const OVERRIDE;
 
   // Method to override to skip the display of the i'th bucket if it's empty.
   virtual bool PrintEmptyBucket(size_t index) const;
@@ -517,6 +492,10 @@ class BASE_EXPORT Histogram : public HistogramBase {
 
   friend class StatisticsRecorder;  // To allow it to delete duplicates.
   friend class StatisticsRecorderTest;
+  friend HistogramBase* HistogramBase::DeserializeHistogramInfo(
+      PickleIterator* iter);

[Ilya Sherman, 2012/12/29 00:17:30]

Why does HistogramBase need to know about the Histogram class?  That seems like
a bad dependency...

[kaiwang, 2013/01/08 00:51:40]

I need a function to know all kind of histograms to deserialize (like the old
Histogram::Deserialize...).

Another option is a global function DeserializeHistogramInfo. Which do you
prefer?

[Ilya Sherman, 2013/01/08 22:31:53]

If you keep roughly the current design, I'd prefer to make the deserialization
functions public.  HistogramBase should not know about private portions of these
classes.

+
+  static HistogramBase* DeserializeHistogramInfo(PickleIterator* iter);
 
   // Implementation of SnapshotSamples function.
   scoped_ptr<SampleVector> SnapshotSampleVector() const;
@@ -597,6 +576,10 @@ class BASE_EXPORT LinearHistogram : public Histogram {
   // |descriptions| can be NULL, which means no special descriptions to set. If
   // it's not NULL, the last element in the array must has a NULL in its
   // "description" field.
+  struct DescriptionPair {
+    Sample sample;
+    const char* description;  // Null means end of a list of pairs.
+  };

[Ilya Sherman, 2012/12/29 00:17:30]

nit: Please move this to be above the comment that's describing
FactoryGetWithRangeDescription.

[kaiwang, 2013/01/08 00:51:40]

Done.

   static Histogram* FactoryGetWithRangeDescription(
       const std::string& name,
       Sample minimum,
@@ -631,6 +614,10 @@ class BASE_EXPORT LinearHistogram : public Histogram {
   virtual bool PrintEmptyBucket(size_t index) const OVERRIDE;
 
  private:
+  friend HistogramBase* HistogramBase::DeserializeHistogramInfo(
+      PickleIterator* iter);
+  static HistogramBase* DeserializeHistogramInfo(PickleIterator* iter);
+
   // For some ranges, we store a printable description of a bucket range.
   // If there is no desciption, then GetAsciiBucketRange() uses parent class
   // to provide a description.
@@ -654,6 +641,10 @@ class BASE_EXPORT BooleanHistogram : public LinearHistogram {
  private:
   BooleanHistogram(const std::string& name, const BucketRanges* ranges);
 
+  friend HistogramBase* HistogramBase::DeserializeHistogramInfo(
+      PickleIterator* iter);
+  static HistogramBase* DeserializeHistogramInfo(PickleIterator* iter);
+
   DISALLOW_COPY_AND_ASSIGN(BooleanHistogram);
 };
 
@@ -681,20 +672,20 @@ class BASE_EXPORT CustomHistogram : public Histogram {
   // TODO(kaiwang): Change name to ArrayToCustomEnumRanges.
   static std::vector<Sample> ArrayToCustomRanges(const Sample* values,
                                                  size_t num_values);
-
-  // Helper for deserializing CustomHistograms.  |*ranges| should already be
-  // correctly sized before this call.  Return true on success.
-  static bool DeserializeRanges(PickleIterator* iter,
-                                std::vector<Sample>* ranges);
  protected:
   CustomHistogram(const std::string& name,
                   const BucketRanges* ranges);
 
-  virtual bool SerializeRanges(Pickle* pickle) const OVERRIDE;
+  // HistogramBase implementation:
+  virtual bool SerializeInfoImpl(Pickle* pickle) const OVERRIDE;
 
   virtual double GetBucketSize(Count current, size_t i) const OVERRIDE;
 
  private:
+  friend HistogramBase* HistogramBase::DeserializeHistogramInfo(
+      PickleIterator* iter);
+  static HistogramBase* DeserializeHistogramInfo(PickleIterator* iter);
+
   static bool ValidateCustomRanges(const std::vector<Sample>& custom_ranges);
   static BucketRanges* CreateBucketRangesFromCustomRanges(
       const std::vector<Sample>& custom_ranges);