Consolidate serialization code in base::HistogramDeltasSerializer.

base/metrics/histogram_delta_serializer.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/metrics/histogram_delta_serializer.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_snapshot_manager.h"
+#include "base/pickle.h"
+#include "base/strings/stringprintf.h"
+#include "base/values.h"
+
+namespace base {
+
+namespace {
+
+void DeserializeHistogramAndAddSamples(PickleIterator* iter) {

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

nit: Please add documentation for this function.

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

Done. Recovered old doc string.

+  HistogramBase* histogram = DeserializeHistogramInfo(iter);
+  if (!histogram)
+    return;
+
+  if (histogram->flags() & HistogramBase::kIPCSerializationSourceFlag) {
+    DVLOG(1) << "Single process mode, histogram observed and not copied: "
+             << histogram->histogram_name();
+    return;
+  }
+  histogram->AddSamplesFromPickle(iter);
+}
+
+}
+
+HistogramDeltasSerializer::HistogramDeltasSerializer(
+    const std::string& caller_name)
+    : caller_name_(caller_name),
+      histogram_snapshot_manager_(new HistogramSnapshotManager(this)) {

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

nit: Why heap-allocate this, rather than including it as a direct member
variable?

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

Recently I was asked often to do so. I had impression we are encouraged to do so
to avoid includes. But after re-reading
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts I see that
it's not true.
 
Done.

On 2013/10/17 04:37:42, Ilya Sherman wrote:

+}
+
+HistogramDeltasSerializer::~HistogramDeltasSerializer() {
+}
+
+const HistogramDeltasSerializer::SerializedDeltas&
+    HistogramDeltasSerializer::GetNewDeltas() {
+  histograms_.clear();
+  // 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(
+      Histogram::kIPCSerializationSourceFlag, false);
+  return histograms_;
+}
+
+// static
+void HistogramDeltasSerializer::DeserializeAndAddSamples(
+    const SerializedDeltas& deltas) {
+  for (SerializedDeltas::const_iterator it = deltas.begin();
+       it != deltas.end(); ++it) {
+    Pickle pickle(it->data(), it->size());
+    PickleIterator iter(pickle);
+    DeserializeHistogramAndAddSamples(&iter);
+  }
+}
+
+void HistogramDeltasSerializer::RecordDelta(const HistogramBase& histogram,
+                                            const HistogramSamples& snapshot) {
+    DCHECK_NE(0, snapshot.TotalCount());
+
+    Pickle pickle;
+    histogram.SerializeInfo(&pickle);
+    snapshot.Serialize(&pickle);
+
+    histograms_.push_back(
+        std::string(static_cast<const char*>(pickle.data()), pickle.size()));
+  }
+
+void HistogramDeltasSerializer::InconsistencyDetected(
+    HistogramBase::Inconsistency problem) {
+  UMA_HISTOGRAM_ENUMERATION(StringPrintf("Histogram.Inconsistencies%s",
+                                         caller_name_.c_str()),

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

Why StringPrintf rather than just using the concatenation operator, i.e. """
"Histogram.Inconsistencies" + caller_name_ """?

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

UMA_HISTOGRAM_ENUMERATION requires the histogram name to be a runtime constant. 
You'll need to expand out the macro if the name can vary, as it seems to here.

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

Done.

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

Done.

+                            problem, HistogramBase::NEVER_EXCEEDED_VALUE);
+}
+
+void HistogramDeltasSerializer::UniqueInconsistencyDetected(
+    HistogramBase::Inconsistency problem) {
+  UMA_HISTOGRAM_ENUMERATION(StringPrintf("Histogram.Inconsistencies%sUnique",
+                                         caller_name_.c_str()),
+                            problem, HistogramBase::NEVER_EXCEEDED_VALUE);
+}
+
+void HistogramDeltasSerializer::InconsistencyDetectedInLoggedCount(int amount) {
+  UMA_HISTOGRAM_COUNTS(StringPrintf("Histogram.InconsistentSnapshot%s",
+                                    caller_name_.c_str()),
+                            std::abs(amount));
+}
+
+}  // namespace base