Consolidate serialization code in base::HistogramDeltasSerializer.

base/metrics/histogram_delta_serializer_unittest.cc

Index: base/metrics/histogram_delta_serializer_unittest.cc
diff --git a/base/metrics/histogram_delta_serializer_unittest.cc b/base/metrics/histogram_delta_serializer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7dbf1b6661cfac87b69b239b052bf567bf1b1466
--- /dev/null
+++ b/base/metrics/histogram_delta_serializer_unittest.cc
@@ -0,0 +1,54 @@
+// 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 <vector>
+
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/statistics_recorder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+TEST(HistogramDeltasSerializerTest, DeserializeHistogramAndAddSamples) {
+  StatisticsRecorder statistic_recorder;
+
+  HistogramDeltasSerializer serializer("HistogramDeltasSerializerTest");
+  // Store current state for future deltas calculation.
+  serializer.GetNewDeltas();

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

I don't follow what the purpose of this line is...

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

Done. It's unnecessary.

+
+  // Nothing was changed yet.
+  EXPECT_TRUE(serializer.GetNewDeltas().empty());
+
+  HistogramBase* histogram = Histogram::FactoryGet(
+      "TestHistogram", 1, 1000, 10, HistogramBase::kIPCSerializationSourceFlag);
+  histogram->Add(1);
+  histogram->Add(10);
+  histogram->Add(100);
+  histogram->Add(1000);
+
+  std::vector<std::string> deltas(serializer.GetNewDeltas());
+  EXPECT_FALSE(deltas.empty());
+
+  // The histogram has kIPCSerializationSourceFlag. So samples will be ignored.

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

Did you mean to deserialize the histograms at some point before this part of the
test?

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

No. I just ported this test from prev implementation.
But I believe here author just checks values the where set in lines 28-31
We will de-serialize later and check at lines 38-41

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

[Vitaly Buka (NO REVIEWS), 2013/10/17 10:22:33]

Let me fix my comment.
I guess author just checks here that histogram is the same as it was set few
lines above.
Later we de-serialize and check that counters has double value.


On 2013/10/17 10:15:47, Vitaly Buka wrote:

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

I think you're overlooking line 77 of the original test.

On 2013/10/17 10:22:33, Vitaly Buka wrote:

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

Thanks. My understanding of test was incorrect. 


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

+  scoped_ptr<HistogramSamples> snapshot(histogram->SnapshotSamples());
+  EXPECT_EQ(1, snapshot->GetCount(1));
+  EXPECT_EQ(1, snapshot->GetCount(10));
+  EXPECT_EQ(1, snapshot->GetCount(100));
+  EXPECT_EQ(1, snapshot->GetCount(1000));
+
+  // Clear kIPCSerializationSourceFlag to emulate multi-process usage.
+  histogram->ClearFlags(HistogramBase::kIPCSerializationSourceFlag);
+  HistogramDeltasSerializer::DeserializeAndAddSamples(deltas);
+
+  scoped_ptr<HistogramSamples> snapshot2(histogram->SnapshotSamples());
+  EXPECT_EQ(2, snapshot2->GetCount(1));
+  EXPECT_EQ(2, snapshot2->GetCount(10));
+  EXPECT_EQ(2, snapshot2->GetCount(100));
+  EXPECT_EQ(2, snapshot2->GetCount(1000));
+}
+
+}  // namespace base