Bug 666309 - Histogram.Add should accept boolean, double values r=jorendorff

toolkit/components/telemetry/Telemetry.cpp

@@ -188,15 +188,25 @@ ReflectHistogramSnapshot(JSContext *cx, JSObject *obj, Histogram *h)
 JSBool
 JSHistogram_Add(JSContext *cx, uintN argc, jsval *vp)
 {
-  jsval *argv = JS_ARGV(cx, vp);
-  JSString *str;
-  if (!JS_ConvertArguments(cx, argc, argv, "i", &str))
+  if (!argc) {
+    JS_ReportError(cx, "Expected one argument");
     return JS_FALSE;
-  if (!JSVAL_IS_INT(argv[0]))
+  }
+
+  jsval v = JS_ARGV(cx, vp)[0];
+  int32 value;
+
+  if (!(JSVAL_IS_NUMBER(v) || JSVAL_IS_BOOLEAN(v))) {
+    JS_ReportError(cx, "Not a number");
     return JS_FALSE;
+  }
+
+  if (!JS_ValueToECMAInt32(cx, v, &value)) {
+    return JS_FALSE;
+  }
+
   JSObject *obj = JS_THIS_OBJECT(cx, vp);
   Histogram *h = static_cast<Histogram*>(JS_GetPrivate(cx, obj));
-  PRUint32 value = JSVAL_TO_INT(argv[0]);
   if (h->histogram_type() == Histogram::BOOLEAN_HISTOGRAM)
     h->Add(!!value);
   else

toolkit/components/telemetry/TelemetryPing.js

@@ -284,7 +284,7 @@ TelemetryPing.prototype = {
         success = channel.QueryInterface(Ci.nsIHttpChannel).requestSucceeded;
       } catch(e) {
       }
-      hsuccess.add(success ? 1 : 0);
+      hsuccess.add(success);
       hping.add(new Date() - startTime);
       if (isTestPing)
         Services.obs.notifyObservers(null, "telemetry-test-xhr-complete", null);

toolkit/components/telemetry/tests/unit/test_nsITelemetry.js

@@ -29,6 +29,13 @@ function test_histogram(histogram_type, name, min, max, bucket_count) {
 
   do_check_eq(gh.min, min)
   do_check_eq(gh.max, max)
+ 
+  // Check that booleans work with nonboolean histograms
+  h.add(false);
+  h.add(true);
+  var s = h.snapshot().counts;
+  do_check_eq(s[0], 2)
+  do_check_eq(s[1], 2)
 }
 
 function expect_fail(f) {
@@ -54,11 +61,14 @@ function test_boolean_histogram()
     sum += v;
     h.add(v);
   }
+  h.add(true);
+  h.add(false);
   var s = h.snapshot();
   do_check_eq(s.histogram_type, Telemetry.HISTOGRAM_BOOLEAN);
   // last bucket should always be 0 since .add parameters are normalized to either 0 or 1
-  do_check_eq(s.counts[2],0);
-  do_check_eq(s.sum, 2);
+  do_check_eq(s.counts[2], 0);
+  do_check_eq(s.sum, 3);
+  do_check_eq(s.counts[0], 2);
 }
 
 function test_getHistogramById() {