Bug 748907 - don't reflect no-data histograms; r=taras

toolkit/components/telemetry/Telemetry.cpp

@@ -397,6 +397,15 @@ ReflectHistogramSnapshot(JSContext *cx, JSObject *obj, Histogram *h)
   return ReflectHistogramAndSamples(cx, obj, h, ss);
 }
 
+bool
+IsEmpty(const Histogram *h)
+{
+  Histogram::SampleSet ss;
+  h->SnapshotSample(&ss);
+
+  return ss.counts(0) == 0 && ss.sum() == 0;
+}
+
 JSBool
 JSHistogram_Add(JSContext *cx, unsigned argc, jsval *vp)
 {
@@ -860,7 +869,7 @@ TelemetryImpl::GetHistogramSnapshots(JSContext *cx, jsval *ret)
   // OK, now we can actually reflect things.
   for (HistogramIterator it = hs.begin(); it != hs.end(); ++it) {
     Histogram *h = *it;
-    if (!ShouldReflectHistogram(h)) {
+    if (!ShouldReflectHistogram(h) || IsEmpty(h)) {
       continue;
     }
 
@@ -923,6 +932,10 @@ TelemetryImpl::AddonHistogramReflector(AddonHistogramEntryType *entry,
     }
   }
 
+  if (IsEmpty(info.h)) {
+    return true;
+  }
+
   JSObject *snapshot = JS_NewObject(cx, NULL, NULL, NULL);
   if (!snapshot) {
     // Just consider this to be skippable.
@@ -1432,6 +1445,11 @@ TelemetrySessionData::SerializeHistogramData(Pickle &pickle)
     const Histogram *h = *it;
     const char *name = h->histogram_name().c_str();
 
+    // We don't check IsEmpty(h) here.  We discard no-data histograms on
+    // read-in, instead.  It's easier to write out the number of
+    // histograms required that way.  (The pickle interface doesn't make
+    // it easy to go back and overwrite previous data.)
+
     Histogram::SampleSet ss;
     h->SnapshotSample(&ss);
 

toolkit/components/telemetry/TelemetryPing.js

@@ -282,7 +282,8 @@ TelemetryPing.prototype = {
       for (let name in addonHistograms) {
         packedHistograms[name] = this.packHistogram(addonHistograms[name]);
       }
-      ret[addonName] = packedHistograms;
+      if (Object.keys(packedHistograms).length != 0)
+        ret[addonName] = packedHistograms;
     }
 
     return ret;

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

@@ -195,6 +195,9 @@ function test_addons() {
 
   // Check for reflection capabilities.
   var h1 = Telemetry.getAddonHistogram(addon_id, name1);
+  // Verify that although we've created storage for it, we don't reflect it into JS.
+  var snapshots = Telemetry.addonHistogramSnapshots;
+  do_check_false(name1 in snapshots[addon_id]);
   h1.add(1);
   h1.add(3);
   var s1 = h1.snapshot();
@@ -328,6 +331,11 @@ function run_test()
     expect_fail(function () nh("test::bucket_count", min, max, 1, histogram_type));
   }
 
+  // Instantiate the storage for this histogram and make sure it doesn't
+  // get reflected into JS, as it has no interesting data in it.
+  let h = Telemetry.getHistogramById("NEWTAB_PAGE_PINNED_SITES_COUNT");
+  do_check_false("NEWTAB_PAGE_PINNED_SITES_COUNT" in Telemetry.histogramSnapshots);
+
   test_boolean_histogram();
   test_getHistogramById();
   test_histogramFrom();