Add comments to HistogramFlatterner interface and rename a function

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

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

base/metrics/histogram_flattener.h

@@ -14,18 +14,28 @@
 
 namespace base {
 
-// HistogramFlattener is an interface for the logistics of gathering up
-// available histograms for recording. The implementors handle the exact lower
-// level recording mechanism, or error report mechanism.
+// HistogramFlattener is an interface used by HistogramSnapshotManager, which
+// handles the logistics of gathering up available histograms for recording.
+// The implementors handle the exact lower level recording mechanism, or
+// error report mechanism.
 class BASE_EXPORT HistogramFlattener {
  public:
   virtual void RecordDelta(const base::Histogram& histogram,
                            const base::Histogram::SampleSet& snapshot) = 0;
 
-  // Record various errors found during attempts to record histograms.
-  virtual void InconsistencyDetected(int problem) = 0;
-  virtual void UniqueInconsistencyDetected(int problem) = 0;
-  virtual void SnapshotProblemResolved(int amount) = 0;
+  // Will be called each time a type of Inconsistenies is seen on a histogram,
+  // during inspections done internally in HistogramSnapshotManager class.
+  virtual void InconsistencyDetected(Histogram::Inconsistencies problem) = 0;
+
+  // Will be called when a type of Inconsistenies is seen for the first time
+  // on a histogram.
+  virtual void UniqueInconsistencyDetected(
+      Histogram::Inconsistencies problem) = 0;
+
+  // Will be called when the total logged sample count of a histogram
+  // differs from the sum of logged sample count in all the buckets.  The
+  // argument |amount| is the non-zero discrepancy.
+  virtual void InconsistencyDetectedInLoggedCount(int amount) = 0;
 
  protected:
   HistogramFlattener() {}

base/metrics/histogram_snapshot_manager.cc

@@ -55,9 +55,13 @@ void HistogramSnapshotManager::PrepareDelta(const Histogram& histogram) {
   // Checksum corruption might not have caused order corruption.
   CHECK_EQ(0, Histogram::RANGE_CHECKSUM_ERROR & corruption);
 
+  // Note, at this point corruption can only be COUNT_HIGH_ERROR or
+  // COUNT_LOW_ERROR and they never arise together, so we don't need to extract
+  // bits from corruption.
   if (corruption) {
     NOTREACHED();
-    histogram_flattener_->InconsistencyDetected(corruption);
+    histogram_flattener_->InconsistencyDetected(
+        static_cast<Histogram::Inconsistencies>(corruption));
     // Don't record corrupt data to metrics survices.
     if (NULL == inconsistencies_.get())
       inconsistencies_.reset(new ProblemMap);
@@ -65,7 +69,8 @@ void HistogramSnapshotManager::PrepareDelta(const Histogram& histogram) {
     if (old_corruption == (corruption | old_corruption))
       return;  // We've already seen this corruption for this histogram.
     (*inconsistencies_)[histogram_name] |= corruption;
-    histogram_flattener_->UniqueInconsistencyDetected(corruption);
+    histogram_flattener_->UniqueInconsistencyDetected(
+        static_cast<Histogram::Inconsistencies>(corruption));
     return;
   }
 
@@ -87,7 +92,7 @@ void HistogramSnapshotManager::PrepareDelta(const Histogram& histogram) {
       int problem = static_cast<int>(discrepancy);
       if (problem != discrepancy)
         problem = INT_MAX;
-      histogram_flattener_->SnapshotProblemResolved(problem);
+      histogram_flattener_->InconsistencyDetectedInLoggedCount(problem);
       // With no valid baseline, we'll act like we've recorded everything in our
       // snapshot.
       already_logged->Subtract(*already_logged);

chrome/common/metrics/metrics_service_base.cc

@@ -41,17 +41,19 @@ void MetricsServiceBase::RecordDelta(
   log_manager_.current_log()->RecordHistogramDelta(histogram, snapshot);
 }
 
-void MetricsServiceBase::InconsistencyDetected(int problem) {
+void MetricsServiceBase::InconsistencyDetected(
+    Histogram::Inconsistencies problem) {
   UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowser",
                             problem, Histogram::NEVER_EXCEEDED_VALUE);
 }
 
-void MetricsServiceBase::UniqueInconsistencyDetected(int problem) {
+void MetricsServiceBase::UniqueInconsistencyDetected(
+    Histogram::Inconsistencies problem) {
   UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowserUnique",
                             problem, Histogram::NEVER_EXCEEDED_VALUE);
 }
 
-void MetricsServiceBase::SnapshotProblemResolved(int amount) {
+void MetricsServiceBase::InconsistencyDetectedInLoggedCount(int amount) {
   UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotBrowser",
                        std::abs(amount));
 }

chrome/common/metrics/metrics_service_base.h

@@ -19,9 +19,11 @@ class MetricsServiceBase : public base::HistogramFlattener {
   // HistogramFlattener interface (override) methods.
   virtual void RecordDelta(const base::Histogram& histogram,
                            const base::Histogram::SampleSet& snapshot) OVERRIDE;
-  virtual void InconsistencyDetected(int problem) OVERRIDE;
-  virtual void UniqueInconsistencyDetected(int problem) OVERRIDE;
-  virtual void SnapshotProblemResolved(int amount) OVERRIDE;
+  virtual void InconsistencyDetected(
+      base::Histogram::Inconsistencies problem) OVERRIDE;
+  virtual void UniqueInconsistencyDetected(
+      base::Histogram::Inconsistencies problem) OVERRIDE;
+  virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE;
 
  protected:
   MetricsServiceBase();

content/common/child_histogram_message_filter.cc

@@ -81,17 +81,20 @@ void ChildHistogramMessageFilter::RecordDelta(
   pickled_histograms_.push_back(histogram_info);
 }
 
-void ChildHistogramMessageFilter::InconsistencyDetected(int problem) {
+void ChildHistogramMessageFilter::InconsistencyDetected(
+    base::Histogram::Inconsistencies problem) {
   UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcess",
                             problem, base::Histogram::NEVER_EXCEEDED_VALUE);
 }
 
-void ChildHistogramMessageFilter::UniqueInconsistencyDetected(int problem) {
+void ChildHistogramMessageFilter::UniqueInconsistencyDetected(
+    base::Histogram::Inconsistencies problem) {
   UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcessUnique",
                             problem, base::Histogram::NEVER_EXCEEDED_VALUE);
 }
 
-void ChildHistogramMessageFilter::SnapshotProblemResolved(int amount) {
+void ChildHistogramMessageFilter::InconsistencyDetectedInLoggedCount(
+    int amount) {
   UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotChildProcess",
                        std::abs(amount));
 }

content/common/child_histogram_message_filter.h

@@ -33,9 +33,11 @@ class ChildHistogramMessageFilter : public base::HistogramFlattener,
   // HistogramFlattener interface (override) methods.
   virtual void RecordDelta(const base::Histogram& histogram,
                            const base::Histogram::SampleSet& snapshot) OVERRIDE;
-  virtual void InconsistencyDetected(int problem) OVERRIDE;
-  virtual void UniqueInconsistencyDetected(int problem) OVERRIDE;
-  virtual void SnapshotProblemResolved(int amount) OVERRIDE;
+  virtual void InconsistencyDetected(
+      base::Histogram::Inconsistencies problem) OVERRIDE;
+  virtual void UniqueInconsistencyDetected(
+      base::Histogram::Inconsistencies problem) OVERRIDE;
+  virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE;
 
  private:
   typedef std::vector<std::string> HistogramPickledList;