Create a histogram serialization mechanism in components/metrics

base/metrics/metric_sample_chromeos.h

+// Copyright (c) 2014 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.
+
+#ifndef BASE_METRICS_METRIC_SAMPLE_CHROMEOS_H_
+#define BASE_METRICS_METRIC_SAMPLE_CHROMEOS_H_
+
+#include <string>
+
+#include "base/base_export.h"
+
+using std::string;

[achaulk, 2014/04/17 16:45:18]

I think the general rule is no using declarations in header files

+
+namespace base {
+// Abstract class representing a metric sample (single measures).
+// This class and its subclasses are used by libmetrics (ChromeOS) to serialize
+// and deserialize measurements to send them to a metrics sending service.
+// It is meant to be a simple container with serialization functions.
+class BASE_EXPORT MetricSample {
+ public:
+  // Types of metric sample used.
+  enum SampleType {
+    CRASH,
+    HISTOGRAM,
+    LINEAR_HISTOGRAM,
+    SPARSE_HISTOGRAM,
+    USER_ACTION
+  };
+  explicit MetricSample(SampleType t, string metric_name);

[Alexei Svitkine (slow), 2014/04/17 16:33:21]

Use a better name than |t|. Make |metric_name| a const ref.

+  virtual ~MetricSample();
+
+  // Returns true if the sample is valid (can be serialized without ambiguity).
+  //
+  // This function should be used to filter bad samples before serializing them
+  // to disk.
+  virtual bool IsValid() const;
+  SampleType type() const;

[Alexei Svitkine (slow), 2014/04/17 16:33:21]

For trivial successors, you can inline them in the header.

[Alexei Svitkine (slow), 2014/04/17 16:39:40]

*accessors

+  string name() const;
+
+  // Returns a serialized version of the sample.
+  //
+  // This function should be overriden by subclasses to define their own
+  // serialization format.
+  // The general serialized message is:
+  // |sampletype|,\0,<sample type specific serialized data>,\0
+  //
+  // The format is akward but it is implemented independently in both chrome os
+  // and chrome. We will keep it for now.
+  virtual string ToString() const;
+
+ protected:

[Alexei Svitkine (slow), 2014/04/17 16:33:21]

Do these need to be protected or can they be private? (Since the ctor takes the
initial values and there's accessors to get them.) Can they also be const?

Can you add a DISALLOW_COPY_AND_ASSIGN macro to private section for all of these
classes?

+  SampleType type_;
+  string name_;
+};
+}  // namespace base
+#endif  // BASE_METRICS_METRIC_SAMPLE_CHROMEOS_H_