Support saving browser metrics to disk and reading them during next run.

base/metrics/histogram_snapshot_manager.h

Index: base/metrics/histogram_snapshot_manager.h
diff --git a/base/metrics/histogram_snapshot_manager.h b/base/metrics/histogram_snapshot_manager.h
index 42dbac225d830838db4f998020265ec8fe03e380..21b2beae7aa9a67aab7cedbce6359aad88ec24f5 100644
--- a/base/metrics/histogram_snapshot_manager.h
+++ b/base/metrics/histogram_snapshot_manager.h
@@ -62,6 +62,9 @@ class BASE_EXPORT HistogramSnapshotManager {
   // until FinishDeltas() completes. PrepareAbsolute() works the same
   // but assumes there were no previous logged values and no future deltas
   // will be created (and thus can work on read-only histograms).
+  // PrepareDifference() is works like PrepareDelta() except that it does

[Ilya Sherman, 2016/04/19 00:57:46]

nit: s/is works/works

[bcwhite, 2016/04/19 16:33:37]

Done.

+  // not update the previous logged values and can thus be used with
+  // read-only files.
   // Use Prepare*TakingOwnership() if it is desireable to have this class
   // automatically delete the histogram once it is "finished".
   void StartDeltas();
@@ -70,6 +73,9 @@ class BASE_EXPORT HistogramSnapshotManager {
   void PrepareAbsolute(const HistogramBase* histogram);
   void PrepareAbsoluteTakingOwnership(
       std::unique_ptr<const HistogramBase> histogram);
+  void PrepareDifference(const HistogramBase* histogram);

[Ilya Sherman, 2016/04/19 00:57:46]

It looks like this method might never be used.  Did I miss the use of it, or is
that true?

[bcwhite, 2016/04/19 16:33:37]

It's used in FileMetricsProvider for atomic files.  It previously called
"PrepareAbsolute()" but that won't be correct for browser metrics stored to disk
because browser metrics will certainly have had uploads done of them and we
don't want to include what was previously sent.

Before this, only setup metrics were saved to a file. They were never uploaded
so "previously sent" would always be zero.  For them, Difference and Absolute
are equivalent.

Absolute will still be required for chrome://histogarms.

[Ilya Sherman, 2016/04/20 07:12:14]

FileMetricsProvider appears to use the TakingOwnership version, but not the
non-taking-ownership version.  Unless I'm still overlooking the use, of course
:)

[Ilya Sherman, 2016/04/20 07:12:14]

For the same reason, we wouldn't want to potentially upload what was written out
to disk.  That's why I'm really uncomfortable with PrepareDifference() -- it
seems very easy to misuse it.

[bcwhite, 2016/04/20 16:47:45]

Oh, I see.  You're correct.  Those methods are just wrappers around an internal
method that does the real work so I think they're worth having in order to keep
the API complete.

[bcwhite, 2016/04/20 16:47:45]

I'm not following.  Each metric on disk has both the "active" sample count and
the "logged" sample count side-by-side.  For anything involving logging to UMA,
the difference between the two should be used.  Generally we want to update the
"logged" count at the same time (aka "Delta") but if we can't (say, because the
file is not writable) then something less invasive is necessary (aka
"Difference").  In the latter case, it's assumed that the data will be discarded
thereafter, never to be uploaded again.  Perhaps that should be in the comments.

+  void PrepareDifferenceTakingOwnership(
+      std::unique_ptr<const HistogramBase> histogram);
   void FinishDeltas();
 
  private: