bug 1366294 - Part 5 - Send appropriately-structured Telemetry data to JS r=gfritzsche

Previously we were doing bad string manipulation nonsense. Now when asked for
snapshots C++ can return a properly-formated Object tree.

MozReview-Commit-ID: HAvIbgzUvMU

toolkit/components/telemetry/TelemetryHistogram.cpp

@@ -42,6 +42,7 @@ using mozilla::Telemetry::HistogramCount;
 using mozilla::Telemetry::Common::LogToBrowserConsole;
 using mozilla::Telemetry::Common::RecordedProcessType;
 using mozilla::Telemetry::Common::AutoHashtable;
+using mozilla::Telemetry::Common::GetNameForProcessID;
 using mozilla::Telemetry::Common::IsExpiredVersion;
 using mozilla::Telemetry::Common::CanRecordDataset;
 using mozilla::Telemetry::Common::IsInDataset;
@@ -1943,55 +1944,53 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext *cx,
     }
   }
 
-  // TODO: This won't get us data from different processes.
-  // We'll have to refactor this function to return {"parent": {...}, "content": {...}}.
-
-  // OK, now we can actually reflect things.
-  JS::Rooted<JSObject*> hobj(cx);
-  for (size_t i = 0; i < HistogramCount; ++i) {
-    const HistogramInfo& info = gHistogramInfos[i];
-    if (info.keyed) {
-      continue;
-    }
-
-    HistogramID id = HistogramID(i);
-
-    // TODO: support multiple processes.
-    //for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
-
-    uint32_t process = uint32_t(ProcessID::Parent);
-    if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
-      ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
-      continue;
-    }
-
-    Histogram* h = internal_GetHistogramById(id, ProcessID(process), sessionType, /* instantiate = */ false);
-    if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
-      continue;
-    }
-
-    hobj = JS_NewPlainObject(cx);
-    if (!hobj) {
+  for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
+    JS::Rooted<JSObject*> processObject(cx, JS_NewPlainObject(cx));
+    if (!processObject) {
       return NS_ERROR_FAILURE;
     }
-    switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
-    case REFLECT_FAILURE:
+    if (!JS_DefineProperty(cx, root_obj, GetNameForProcessID(ProcessID(process)),
+                           processObject, JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
-    case REFLECT_OK:
-      if (!JS_DefineProperty(cx, root_obj, gHistogramInfos[id].name(),
-                             hobj, JSPROP_ENUMERATE)) {
+    }
+    for (size_t i = 0; i < HistogramCount; ++i) {
+      const HistogramInfo& info = gHistogramInfos[i];
+      if (info.keyed) {
+        continue;
+      }
+
+      HistogramID id = HistogramID(i);
+
+      if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
+        ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
+        continue;
+      }
+
+      Histogram* h = internal_GetHistogramById(id, ProcessID(process), sessionType, /* instantiate = */ false);
+      if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
+        continue;
+      }
+
+      JS::Rooted<JSObject*> hobj(cx, JS_NewPlainObject(cx));
+      if (!hobj) {
         return NS_ERROR_FAILURE;
       }
-    }
+      switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
+      case REFLECT_FAILURE:
+        return NS_ERROR_FAILURE;
+      case REFLECT_OK:
+        if (!JS_DefineProperty(cx, processObject, gHistogramInfos[id].name(),
+                               hobj, JSPROP_ENUMERATE)) {
+          return NS_ERROR_FAILURE;
+        }
+      }
 
 #if !defined(MOZ_WIDGET_ANDROID)
-    if ((sessionType == SessionType::Subsession) && clearSubsession) {
-      h->Clear();
-    }
+      if ((sessionType == SessionType::Subsession) && clearSubsession) {
+        h->Clear();
+      }
 #endif
-
-    // TODO: support multiple processes.
-    // }
+    }
   }
   return NS_OK;
 }

toolkit/components/telemetry/TelemetrySession.jsm

@@ -936,13 +936,11 @@ var Impl = {
                          : Telemetry.histogramSnapshots;
     let ret = {};
 
-    for (let name of registered) {
-      for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) {
-        if (name + suffix in hls) {
-          if (!(suffix in ret)) {
-            ret[suffix] = {};
-          }
-          ret[suffix][name] = this.packHistogram(hls[name + suffix]);
+    for (let [process, histograms] of Object.entries(hls)) {
+      ret[process] = {};
+      for (let [name, value] of Object.entries(histograms)) {
+        if (registered.includes(name)) {
+          ret[process][name] = this.packHistogram(value);
         }
       }
     }
@@ -1326,8 +1324,8 @@ var Impl = {
     let keyedScalars = protect(() => this.getScalars(isSubsession, clearSubsession, true), {});
     let events = protect(() => this.getEvents(isSubsession, clearSubsession))
 
-    payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {};
-    payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {};
+    payloadObj.histograms = histograms.parent || {};
+    payloadObj.keyedHistograms = keyedHistograms.parent || {};
     payloadObj.processes = {
       parent: {
         scalars: scalars["parent"] || {},
@@ -1337,29 +1335,29 @@ var Impl = {
       content: {
         scalars: scalars["content"],
         keyedScalars: keyedScalars["content"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT],
+        histograms: histograms["content"],
+        keyedHistograms: keyedHistograms["content"],
         events: events["content"] || [],
       },
       extension: {
         scalars: scalars["extension"],
         keyedScalars: keyedScalars["extension"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.EXTENSION],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.EXTENSION],
+        histograms: histograms["extension"],
+        keyedHistograms: keyedHistograms["extension"],
         events: events["extension"] || [],
       },
     };
 
     // Only include the GPU process if we've accumulated data for it.
-    if (HISTOGRAM_SUFFIXES.GPU in histograms ||
-        HISTOGRAM_SUFFIXES.GPU in keyedHistograms ||
+    if ("gpu" in histograms ||
+        "gpu" in keyedHistograms ||
         "gpu" in scalars ||
         "gpu" in keyedScalars) {
       payloadObj.processes.gpu = {
         scalars: scalars["gpu"],
         keyedScalars: keyedScalars["gpu"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.GPU],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.GPU],
+        histograms: histograms["gpu"],
+        keyedHistograms: keyedHistograms["gpu"],
         events: events["gpu"] || [],
       };
     }

toolkit/components/telemetry/nsITelemetry.idl

@@ -52,8 +52,8 @@ interface nsITelemetry : nsISupports
 
 
   /**
-   * An object containing a snapshot from all of the currently registered histograms.
-   * { name1: {data1}, name2:{data2}...}
+   * An object containing a snapshot from all of the currently registered histograms from all processes.
+   * { process1: {name1: {data1}, name2:{data2}...} }
    * where data is consists of the following properties:
    *   min - Minimal bucket size
    *   max - Maximum bucket size