Crash when we notice a corruption of the histogram range-vector...

base/metrics/histogram.h

Index: base/metrics/histogram.h
===================================================================
--- base/metrics/histogram.h	(revision 76117)
+++ base/metrics/histogram.h	(working copy)
@@ -411,7 +411,7 @@
   Sample declared_min() const { return declared_min_; }
   Sample declared_max() const { return declared_max_; }
   virtual Sample ranges(size_t i) const;
-  Sample range_checksum() const { return range_checksum_; }
+  uint32 range_checksum() const { return range_checksum_; }
   virtual size_t bucket_count() const;
   // Snapshot the current complete set of sample data.
   // Override with atomic/locked snapshot if needed.
@@ -423,6 +423,8 @@
   virtual bool HasConstructorTimeDeltaArguments(TimeDelta minimum,
                                                 TimeDelta maximum,
                                                 size_t bucket_count);
+  // Return true iff the range_checksum_ matches current ranges_ vector.
+  bool HasValidRangeChecksum() const;
 
  protected:
   friend class base::RefCountedThreadSafe<Histogram>;
@@ -469,19 +471,21 @@
   // values relate properly to the declared_min_ and declared_max_)..
   bool ValidateBucketRanges() const;
 
+  virtual uint32 CalculateRangeChecksum() const;
+
  private:
   // Allow tests to corrupt our innards for testing purposes.
   FRIEND_TEST(HistogramTest, CorruptBucketBounds);
   FRIEND_TEST(HistogramTest, CorruptSampleCounts);
+  FRIEND_TEST(HistogramTest, Crc32SampleHash);
+  FRIEND_TEST(HistogramTest, Crc32TableTest);
 
   // Post constructor initialization.
   void Initialize();
 
-  // Return true iff the range_checksum_ matches current ranges_ vector.
-  bool HasValidRangeChecksum() const;
+  // Checksum function for accumulating range values into a checksum.
+  static uint32 Crc32(uint32 sum, Sample range);
 
-  Sample CalculateRangeChecksum() const;
-
   //----------------------------------------------------------------------------
   // Helpers for emitting Ascii graphic.  Each method appends data to output.
 
@@ -508,6 +512,9 @@
                              std::string* output) const;
 
   //----------------------------------------------------------------------------
+  // Table for generating Crc32 values.
+  static const uint32 kCrcTable[256];
+  //----------------------------------------------------------------------------
   // Invariant values set at/near construction time
 
   // ASCII version of original name given to the constructor.  All identically
@@ -526,10 +533,10 @@
   // The dimension of ranges_ is bucket_count + 1.
   Ranges ranges_;
 
-  // For redundancy, we store the sum of all the sample ranges when ranges are
-  // generated.  If ever there is ever a difference, then the histogram must
+  // For redundancy, we store a checksum of all the sample ranges when ranges
+  // are generated.  If ever there is ever a difference, then the histogram must
   // have been corrupted.
-  Sample range_checksum_;
+  uint32 range_checksum_;
 
   // Finally, provide the state that changes with the addition of each new
   // sample.
@@ -625,13 +632,9 @@
                   const std::vector<Sample>& custom_ranges);
 
   // Initialize ranges_ mapping.
-  virtual void InitializeBucketRange();
+  virtual void InitializeBucketRange(const std::vector<Sample>& custom_ranges);
   virtual double GetBucketSize(Count current, size_t i) const;
 
- private:
-  // Temporary pointer used during construction/initialization, and then NULLed.
-  const std::vector<Sample>* ranges_vector_;
-
   DISALLOW_COPY_AND_ASSIGN(CustomHistogram);
 };
 
@@ -651,8 +654,10 @@
   // Find out if histograms can now be registered into our list.
   static bool IsActive();
 
-  // Register, or add a new histogram to the collection of statistics.
-  static void Register(Histogram* histogram);
+  // Register, or add a new histogram to the collection of statistics. If an
+  // identically named histogram is already registered, then the argument
+  // |histogram| will be replaced by the previously registered value.
+  static void Register(scoped_refptr<Histogram>* histogram);

[rvargas (doing something else), 2011/02/28 19:50:53]

In general, I would consider this to be unexpected behavior given the function
name. I suggest changing the name to reflecting the new behavior.

[jar (doing other things), 2011/02/28 22:11:44]

I landed the code this morning... so.... 
I've changed the name in a separate CL, 6591052, along with
a few other small items.  I'll send you a review request shortly.

 
   // Methods for printing histograms.  Only histograms which have query as
   // a substring are written to output (an empty string will process all