Bug 1120409 - nsITelemetry.histogramSnapshots will no longer return keyed histograms. r=chutten
authorGregory Moore <olucafont6@yahoo.com>
Fri, 24 Mar 2017 12:38:24 -0700
changeset 398923 c6d5e6b3055d37039d6d6d025b869ba70d6e0dae
parent 398922 a3c28a335237ef6a890d973659996da7b6aecfdf
child 398924 1ef73669e8fccac35b650ff81df1b575a39a0fd5
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1120409
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1120409 - nsITelemetry.histogramSnapshots will no longer return keyed histograms. r=chutten
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -2283,56 +2283,68 @@ TelemetryHistogram::CreateHistogramSnaps
   // depend on histogram enumeration order.
   //
   // Of course, we hope that all of these corruption-statistics
   // histograms are not themselves corrupt...
   internal_IdentifyCorruptHistograms(hs);
 
   // OK, now we can actually reflect things.
   JS::Rooted<JSObject*> hobj(cx);
-  for (auto h : hs) {
-    if (!internal_ShouldReflectHistogram(h) || internal_IsEmpty(h) ||
-        internal_IsExpired(h)) {
+  GeckoProcessType const processTypes[] = { GeckoProcessType_Default, GeckoProcessType_Content, GeckoProcessType_GPU };
+  size_t numProcessTypes = (includeGPUProcess ? 3 : 2);
+  for (size_t i = 0; i < mozilla::Telemetry::HistogramCount; ++i) {
+    if (gHistograms[i].keyed) {
       continue;
     }
 
-    Histogram* original = h;
-#if !defined(MOZ_WIDGET_ANDROID)
-    if (subsession) {
-      h = internal_GetSubsessionHistogram(*h);
-      if (!h) {
+    Histogram* h = nullptr;
+    mozilla::Telemetry::HistogramID id = mozilla::Telemetry::HistogramID(i);
+
+    for (size_t type = 0; type < numProcessTypes; ++type) {
+      nsresult rv = internal_GetHistogramByEnumId(id, &h, processTypes[type]);
+      if (NS_WARN_IF(NS_FAILED(rv)) || !internal_ShouldReflectHistogram(h) ||
+        internal_IsEmpty(h) || internal_IsExpired(h)) {
         continue;
       }
-    }
+
+      Histogram* original = h;
+#if !defined(MOZ_WIDGET_ANDROID)
+      if (subsession) {
+        h = internal_GetSubsessionHistogram(*h);
+        if (!h) {
+          continue;
+        }
+      }
 #endif
 
-    hobj = JS_NewPlainObject(cx);
-    if (!hobj) {
-      return NS_ERROR_FAILURE;
-    }
-    switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
-    case REFLECT_CORRUPT:
-      // We can still hit this case even if ShouldReflectHistograms
-      // returns true.  The histogram lies outside of our control
-      // somehow; just skip it.
-      continue;
-    case REFLECT_FAILURE:
-      return NS_ERROR_FAILURE;
-    case REFLECT_OK:
-      if (!JS_DefineProperty(cx, root_obj, original->histogram_name().c_str(),
-                             hobj, JSPROP_ENUMERATE)) {
+      hobj = JS_NewPlainObject(cx);
+      if (!hobj) {
         return NS_ERROR_FAILURE;
       }
-    }
+      switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
+      case REFLECT_CORRUPT:
+        // We can still hit this case even if ShouldReflectHistograms
+        // returns true.  The histogram lies outside of our control
+        // somehow; just skip it.
+        continue;
+      case REFLECT_FAILURE:
+        return NS_ERROR_FAILURE;
+      case REFLECT_OK:
+        if (!JS_DefineProperty(cx, root_obj, original->histogram_name().c_str(),
+                               hobj, JSPROP_ENUMERATE)) {
+          return NS_ERROR_FAILURE;
+        }
+      }
 
 #if !defined(MOZ_WIDGET_ANDROID)
-    if (subsession && clearSubsession) {
-      h->Clear();
+      if (subsession && clearSubsession) {
+        h->Clear();
+      }
+#endif
     }
-#endif
   }
   return NS_OK;
 }
 
 nsresult
 TelemetryHistogram::RegisteredHistograms(uint32_t aDataset, uint32_t *aCount,
                                          char*** aHistograms)
 {
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
@@ -773,16 +773,24 @@ add_task(function* test_keyed_histogram_
 
   // Restore to disabled
   Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD", false);
   h.add(TEST_KEY, 1);
   Assert.equal(h.snapshot(TEST_KEY).sum, 1,
     "Keyed histogram add should not record when recording is disabled");
 });
 
+add_task(function* test_histogramSnapshots() {
+  let keyed = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_COUNT");
+  keyed.add("a", 1);
+
+  // Check that keyed histograms are not returned
+  Assert.ok(!("TELEMETRY_TEST_KEYED_COUNT#a" in Telemetry.histogramSnapshots));
+});
+
 add_task(function* test_datasets() {
   // Check that datasets work as expected.
 
   const RELEASE_CHANNEL_OPTOUT = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT;
   const RELEASE_CHANNEL_OPTIN  = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN;
 
   // Check that registeredHistogram works properly
   let registered = Telemetry.registeredHistograms(RELEASE_CHANNEL_OPTIN, []);