Detect corruption of previous snapshots in histograms

Having verified that histograms can be corrupted by
random memory smashers (or DRAM problems), this CL
looks at one last example of histograms that are
at rest for an extended period of time, and hence
vulnerable.  Between each UMA upload, we save
snapshots of the data we've already sent, so that
we can just send "new samples."  If those snapshots
are corrupted, the noise would be directly
injected into the UMA uploads.  This CL checks
for consistency in those snapshots, and if there
is any inconsistency, it skips over the
recent changes (since it has no baseline).

Since the code to do this was getting larger,
I factored it out a bit.  The hassle is that
I wanted to keep separate records of corruption
in the renderer vs browser, which complicates
the factoring a bit.

BUG=61281
r=mbelshe
Review URL: http://codereview.chromium.org/4733002

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

2 files changed, 41 insertions, 76 deletions

diff --git a/chrome/renderer/renderer_histogram_snapshots.cc b/chrome/renderer/renderer_histogram_snapshots.cc
index 6124e04..bc91647 100644
--- a/chrome/renderer/renderer_histogram_snapshots.cc
+++ b/chrome/renderer/renderer_histogram_snapshots.cc
@@ -20,7 +20,7 @@ using base::StatisticsRecorder;
 
 RendererHistogramSnapshots::RendererHistogramSnapshots()
     : ALLOW_THIS_IN_INITIALIZER_LIST(
-        renderer_histogram_snapshots_factory_(this)) {
+          renderer_histogram_snapshots_factory_(this)) {
 }
 
 RendererHistogramSnapshots::~RendererHistogramSnapshots() {
@@ -34,81 +34,43 @@ void RendererHistogramSnapshots::SendHistograms(int sequence_number) {
 }
 
 void RendererHistogramSnapshots::UploadAllHistrograms(int sequence_number) {
-  StatisticsRecorder::Histograms histograms;
-  StatisticsRecorder::GetHistograms(&histograms);
+  DCHECK_EQ(0u, pickled_histograms_.size());
 
-  HistogramPickledList pickled_histograms;
+  // Push snapshots into our pickled_histograms_ vector.
+  TransmitAllHistograms(Histogram::kIPCSerializationSourceFlag, false);
 
-  for (StatisticsRecorder::Histograms::iterator it = histograms.begin();
-       histograms.end() != it;
-       it++) {
-    (*it)->SetFlags(Histogram::kIPCSerializationSourceFlag);
-    UploadHistrogram(**it, &pickled_histograms);
-  }
   // Send the sequence number and list of pickled histograms over synchronous
-  // IPC.
+  // IPC, so we can clear pickled_histograms_ afterwards.
   RenderThread::current()->Send(
       new ViewHostMsg_RendererHistograms(
-          sequence_number, pickled_histograms));
-}
+          sequence_number, pickled_histograms_));
 
-// Extract snapshot data, remember what we've seen so far, and then send off the
-// delta to the browser.
-void RendererHistogramSnapshots::UploadHistrogram(
-    const Histogram& histogram,
-    HistogramPickledList* pickled_histograms) {
-  // Get up-to-date snapshot of sample stats.
-  Histogram::SampleSet snapshot;
-  histogram.SnapshotSample(&snapshot);
-  const std::string& histogram_name = histogram.histogram_name();
-
-  int corruption = histogram.FindCorruption(snapshot);
-  if (corruption) {
-    NOTREACHED();
-    // Don't send corrupt data to the browser.
-    UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRenderer",
-                              corruption, Histogram::NEVER_EXCEEDED_VALUE);
-    typedef std::map<std::string, int> ProblemMap;
-    static ProblemMap* inconsistencies = new ProblemMap;
-    int old_corruption = (*inconsistencies)[histogram_name];
-    if (old_corruption == (corruption | old_corruption))
-      return;  // We've already seen this corruption for this histogram.
-    (*inconsistencies)[histogram_name] |= corruption;
-    UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRendererUnique",
-                              corruption, Histogram::NEVER_EXCEEDED_VALUE);
-    return;
-  }
-
-  // Find the already sent stats, or create an empty set.
-  LoggedSampleMap::iterator it = logged_samples_.find(histogram_name);
-  Histogram::SampleSet* already_logged;
-  if (logged_samples_.end() == it) {
-    // Add new entry.
-    already_logged = &logged_samples_[histogram.histogram_name()];
-    already_logged->Resize(histogram);  // Complete initialization.
-  } else {
-    already_logged = &(it->second);
-    // Deduct any stats we've already logged from our snapshot.
-    snapshot.Subtract(*already_logged);
-  }
-
-  // Snapshot now contains only a delta to what we've already_logged.
-
-  if (snapshot.TotalCount() > 0) {
-    UploadHistogramDelta(histogram, snapshot, pickled_histograms);
-    // Add new data into our running total.
-    already_logged->Add(snapshot);
-  }
+  pickled_histograms_.clear();
 }
 
-void RendererHistogramSnapshots::UploadHistogramDelta(
-    const Histogram& histogram,
-    const Histogram::SampleSet& snapshot,
-    HistogramPickledList* pickled_histograms) {
-  DCHECK(0 != snapshot.TotalCount());
+void RendererHistogramSnapshots::TransmitHistogramDelta(
+      const base::Histogram& histogram,
+      const base::Histogram::SampleSet& snapshot) {
+  DCHECK_NE(0, snapshot.TotalCount());
   snapshot.CheckSize(histogram);
 
   std::string histogram_info =
       Histogram::SerializeHistogramInfo(histogram, snapshot);
-  pickled_histograms->push_back(histogram_info);
+  pickled_histograms_.push_back(histogram_info);
+}
+
+void RendererHistogramSnapshots::InconsistencyDetected(int problem) {
+  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRenderer",
+                            problem, Histogram::NEVER_EXCEEDED_VALUE);
+}
+
+void RendererHistogramSnapshots::UniqueInconsistencyDetected(int problem) {
+  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRendererUnique",
+                            problem, Histogram::NEVER_EXCEEDED_VALUE);
+}
+
+void RendererHistogramSnapshots::SnapshotProblemResolved(int amount) {
+  UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotRenderer",
+                       std::abs(amount));
 }
+
diff --git a/chrome/renderer/renderer_histogram_snapshots.h b/chrome/renderer/renderer_histogram_snapshots.h
index eff6f3e..5155b6f 100644
--- a/chrome/renderer/renderer_histogram_snapshots.h
+++ b/chrome/renderer/renderer_histogram_snapshots.h
@@ -14,8 +14,9 @@
 #include "base/metrics/histogram.h"
 #include "base/process.h"
 #include "base/task.h"
+#include "chrome/common/metrics_helpers.h"
 
-class RendererHistogramSnapshots {
+class RendererHistogramSnapshots : public HistogramSender {
  public:
   RendererHistogramSnapshots();
   ~RendererHistogramSnapshots();
@@ -23,26 +24,28 @@ class RendererHistogramSnapshots {
   // Send the histogram data.
   void SendHistograms(int sequence_number);
 
+ private:
   // Maintain a map of histogram names to the sample stats we've sent.
   typedef std::map<std::string, base::Histogram::SampleSet> LoggedSampleMap;
   typedef std::vector<std::string> HistogramPickledList;
 
- private:
   // Extract snapshot data and then send it off the the Browser process.
   // Send only a delta to what we have already sent.
   void UploadAllHistrograms(int sequence_number);
-  void UploadHistrogram(const base::Histogram& histogram,
-                        HistogramPickledList* histograms);
-  void UploadHistogramDelta(const base::Histogram& histogram,
-                            const base::Histogram::SampleSet& snapshot,
-                            HistogramPickledList* histograms);
 
   ScopedRunnableMethodFactory<RendererHistogramSnapshots>
       renderer_histogram_snapshots_factory_;
 
-  // For histograms, record what we've already logged (as a sample for each
-  // histogram) so that we can send only the delta with the next log.
-  LoggedSampleMap logged_samples_;
+  // HistogramSender interface (override) methods.
+  void TransmitHistogramDelta(
+      const base::Histogram& histogram,
+      const base::Histogram::SampleSet& snapshot);
+  void InconsistencyDetected(int problem);
+  void UniqueInconsistencyDetected(int problem);
+  void SnapshotProblemResolved(int amount);
+
+  // Collection of histograms to send to the browser.
+  HistogramPickledList pickled_histograms_;
 
   DISALLOW_COPY_AND_ASSIGN(RendererHistogramSnapshots);
 };