Return a NULL histogram pointer on construction error

base/metrics/histogram.h

Index: base/metrics/histogram.h
diff --git a/base/metrics/histogram.h b/base/metrics/histogram.h
index 9845362d98aa31cad7557383e2fc1e039ad5f67e..090fb57c51a0d0a70a1b0ee9159f98a74334a937 100644
--- a/base/metrics/histogram.h
+++ b/base/metrics/histogram.h
@@ -385,16 +385,20 @@ class BASE_EXPORT Histogram : public HistogramBase {
   // buckets <= (maximum - minimum + 2) - this is to ensure that we don't have
   // more buckets than the range of numbers; having more buckets than 1 per
   // value in the range would be nonsensical.
-  static HistogramBase* FactoryGet(const std::string& name,
-                                   Sample minimum,
-                                   Sample maximum,
-                                   size_t bucket_count,
-                                   int32 flags);
-  static HistogramBase* FactoryTimeGet(const std::string& name,
-                                       base::TimeDelta minimum,
-                                       base::TimeDelta maximum,
-                                       size_t bucket_count,
-                                       int32 flags);
+  static HistogramBase* FactoryGet(
+      const std::string& name,
+      Sample minimum,
+      Sample maximum,
+      size_t bucket_count,
+      int32 flags,
+      int32 construction_behavior = kDoNotAllowBadConstruction);

[jar (doing other things), 2014/01/24 16:15:09]

Sadly, this is a violation of the style guide.

Perhaps instead of using a CHECK() in the method, we can return a NULL pointer
when there is a violation.  That would then effectively CHECK (crash) if the
caller was not prepared for "bad construction".  If the caller could tolerate
bad construction, and wanted the flow to continue, then they could replace the
NULL pointer with the planned "dummy_histogram".

[elijahtaylor1, 2014/01/24 20:37:22]

If allowing NULL dereference is ok for what we're considering bad code in
Chrome, this approach makes everything much simpler.  Both the extension and
Pepper API do a FactoryGet then immediately Add, they don't keep the object
around, so we can just guard against NULL dereference there and everything
should be good.

+  static HistogramBase* FactoryTimeGet(
+      const std::string& name,
+      base::TimeDelta minimum,
+      base::TimeDelta maximum,
+      size_t bucket_count,
+      int32 flags,
+      int32 construction_behavior = kDoNotAllowBadConstruction);
 
   // Time call for use with DHISTOGRAM*.
   // Returns TimeTicks::Now() in debug and TimeTicks() in release build.
@@ -546,16 +550,19 @@ class BASE_EXPORT LinearHistogram : public Histogram {
 
   /* minimum should start from 1. 0 is as minimum is invalid. 0 is an implicit
      default underflow bucket. */
-  static HistogramBase* FactoryGet(const std::string& name,
-                                   Sample minimum,
-                                   Sample maximum,
-                                   size_t bucket_count,
-                                   int32 flags);
+  static HistogramBase* FactoryGet(
+      const std::string& name,
+      Sample minimum,
+      Sample maximum,
+      size_t bucket_count,
+      int32 flags,
+      int32 construction_behavior = kDoNotAllowBadConstruction);
   static HistogramBase* FactoryTimeGet(const std::string& name,
-                                       TimeDelta minimum,
-                                       TimeDelta maximum,
-                                       size_t bucket_count,
-                                       int32 flags);
+      TimeDelta minimum,
+      TimeDelta maximum,
+      size_t bucket_count,
+      int32 flags,
+      int32 construction_behavior = kDoNotAllowBadConstruction);
 
   struct DescriptionPair {
     Sample sample;
@@ -573,7 +580,8 @@ class BASE_EXPORT LinearHistogram : public Histogram {
       Sample maximum,
       size_t bucket_count,
       int32 flags,
-      const DescriptionPair descriptions[]);
+      const DescriptionPair descriptions[],
+      int32 construction_behavior = kDoNotAllowBadConstruction);
 
   static void InitializeBucketRanges(Sample minimum,
                                      Sample maximum,