Consolidate serialization code in base::HistogramDeltasSerializer.

content/child/child_histogram_message_filter.cc

Index: content/child/child_histogram_message_filter.cc
diff --git a/content/child/child_histogram_message_filter.cc b/content/child/child_histogram_message_filter.cc
index 427dd0d7d26e3952bd3b2bc568d3d4969632c958..4c7ccb8bb5e742cd7c0b938f88f7834828d504c8 100644
--- a/content/child/child_histogram_message_filter.cc
+++ b/content/child/child_histogram_message_filter.cc
@@ -8,8 +8,7 @@
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
-#include "base/metrics/statistics_recorder.h"
-#include "base/pickle.h"
+#include "base/metrics/histogram_delta_serializer.h"
 #include "content/child/child_process.h"
 #include "content/child/child_thread.h"
 #include "content/common/child_process_messages.h"
@@ -18,8 +17,7 @@ namespace content {
 
 ChildHistogramMessageFilter::ChildHistogramMessageFilter()
     : channel_(NULL),
-      io_message_loop_(ChildProcess::current()->io_message_loop_proxy()),
-      histogram_snapshot_manager_(this) {
+      io_message_loop_(ChildProcess::current()->io_message_loop_proxy()) {
 }
 
 ChildHistogramMessageFilter::~ChildHistogramMessageFilter() {
@@ -54,53 +52,17 @@ void ChildHistogramMessageFilter::OnGetChildHistogramData(int sequence_number) {
 }
 
 void ChildHistogramMessageFilter::UploadAllHistograms(int sequence_number) {
-  DCHECK_EQ(0u, pickled_histograms_.size());
-
-  // Push snapshots into our pickled_histograms_ vector.

[Ilya Sherman, 2013/10/17 04:37:42]

nit: What happened to this line of the comment?

[Vitaly Buka (NO REVIEWS), 2013/10/17 10:15:47]

I guess with current implementation we don't need to know here that it's
"pickled_histograms_ vector".

[Ilya Sherman, 2013/10/17 22:41:46]

Well, the vector is renamed, but IMO it's still useful to document that calling
PrepareDeltas() has a side-effect of calling into RecordDelta() which updates
the serialized_deltas_ vector.

[Vitaly Buka (NO REVIEWS), 2013/10/18 05:33:34]

What if just change interface to make this more clear.

On 2013/10/17 22:41:46, Ilya Sherman wrote:

-  // Note: Before serializing, we set the kIPCSerializationSourceFlag for all
-  // the histograms, so that the receiving process can distinguish them from the
-  // local histograms.
-  histogram_snapshot_manager_.PrepareDeltas(
-      base::Histogram::kIPCSerializationSourceFlag, false);
+  if (!histogram_delta_serializer_) {
+    histogram_delta_serializer_.reset(
+        new base::HistogramDeltasSerializer("ChildProcess"));
+  }

[Ilya Sherman, 2013/10/17 04:37:42]

Why lazy-initialize this rather than including it as a direct class member
variable?

[Vitaly Buka (NO REVIEWS), 2013/10/17 10:15:47]

Here, in addition to includes optimization, we can avoid initialization if
metrics server is not enabled.

 
   channel_->Send(new ChildProcessHostMsg_ChildHistogramData(
-      sequence_number, pickled_histograms_));
+      sequence_number, histogram_delta_serializer_->GetNewDeltas()));
 
-  pickled_histograms_.clear();
   static int count = 0;
   count++;
   DHISTOGRAM_COUNTS("Histogram.ChildProcessHistogramSentCount", count);
 }
 
-void ChildHistogramMessageFilter::RecordDelta(
-    const base::HistogramBase& histogram,
-    const base::HistogramSamples& snapshot) {
-  DCHECK_NE(0, snapshot.TotalCount());
-
-  Pickle pickle;
-  histogram.SerializeInfo(&pickle);
-  snapshot.Serialize(&pickle);
-
-  pickled_histograms_.push_back(
-      std::string(static_cast<const char*>(pickle.data()), pickle.size()));
-}
-
-void ChildHistogramMessageFilter::InconsistencyDetected(
-    base::HistogramBase::Inconsistency problem) {
-  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcess",
-                            problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
-}
-
-void ChildHistogramMessageFilter::UniqueInconsistencyDetected(
-    base::HistogramBase::Inconsistency problem) {
-  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcessUnique",
-                            problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
-}
-
-void ChildHistogramMessageFilter::InconsistencyDetectedInLoggedCount(
-    int amount) {
-  UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotChildProcess",
-                       std::abs(amount));
-}
-
 }  // namespace content