Serialize/Deserialize support in HistogramBase

content/browser/histogram_synchronizer.cc

Index: content/browser/histogram_synchronizer.cc
diff --git a/content/browser/histogram_synchronizer.cc b/content/browser/histogram_synchronizer.cc
index 2fd3fea2bd66acd433e64496d78230f140a9ca20..38d4c5ac27453fb14c0ab87f1c5abcbf4a57a536 100644
--- a/content/browser/histogram_synchronizer.cc
+++ b/content/browser/histogram_synchronizer.cc
@@ -8,6 +8,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/pickle.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "content/browser/histogram_controller.h"
@@ -272,7 +273,19 @@ void HistogramSynchronizer::OnHistogramDataCollected(
   for (std::vector<std::string>::const_iterator it = pickled_histograms.begin();
        it < pickled_histograms.end();
        ++it) {
-    base::Histogram::DeserializeHistogramInfo(*it);
+    Pickle pickle(it->data(), it->size());
+    PickleIterator iter(pickle);
+    base::HistogramBase* histogram =
+        base::HistogramBase::DeserializeHistogramInfo(&iter);
+    if (histogram) {

[Ilya Sherman, 2012/12/29 00:17:30]

Optional nit: IMO it's tidier to write "if (!histogram) continue" to reduce the
level of indentation.

[kaiwang, 2013/01/08 00:51:40]

Done.

+      if (histogram->flags() &
+          base::HistogramBase::kIPCSerializationSourceFlag) {
+        DVLOG(1) << "Single process mode, histogram observed and not copied: "
+                 << histogram->histogram_name();
+      } else {
+        histogram->AddSamplesFromPickle(&iter);

[Ilya Sherman, 2012/12/29 00:17:30]

Why isn't this done as part of DeserializeHistogramInfo() (which I would suggest
renaming to DeserializeHistogram())?  Would you ever want to deserialize just
the construction parameters, and not the samples?

[kaiwang, 2013/01/08 00:51:40]

IMO
1. histogram is in base/, so it should know nothing about inter process. 
2. We are actually not "deserializing" the histogram with samples, but find the
histogram and then add the diff. So it's a bit odd to put them into a same
function.

[Ilya Sherman, 2013/01/08 22:31:53]

But it already does know about inter-process communication -- the
kIPCSerializationSourceFlag is in the base namespace...
It just seems a fragile to rely on the fact that DeserializeHistogramInfo()
leaves the iterator in just the right place for you to be able to pick it up and
continue reading from it here.  If the two functions did not read from the same
"stream", I would be less concerned about their separation.

+      }
+    }
   }
 
   if (!request)