bug 1366294 - Part 4 - Small cleanups to previous patches. r?gfritzsche draft
authorChris H-C <chutten@mozilla.com>
Fri, 23 Jun 2017 16:42:21 -0400
changeset 610612 b146a1d500ed926f29e425adb7a11f0a617e2ad8
parent 610611 e0a34e470ad8943743df571dfa9f6b86f0cd1572
child 610613 b54365078d30e4009da10154aa78ac3889b1b952
push id68956
push userbmo:chutten@mozilla.com
push dateTue, 18 Jul 2017 15:29:20 +0000
reviewersgfritzsche
bugs1366294
milestone56.0a1
bug 1366294 - Part 4 - Small cleanups to previous patches. r?gfritzsche MozReview-Commit-ID: 7PioVNfUUNZ
toolkit/components/telemetry/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -143,17 +143,17 @@ public:
   KeyedHistogram(HistogramID id, const HistogramInfo& info);
   nsresult GetHistogram(const nsCString& name, Histogram** histogram, bool subsession);
   Histogram* GetHistogram(const nsCString& name, bool subsession);
   uint32_t GetHistogramType() const { return mHistogramInfo.histogramType; }
   nsresult GetJSKeys(JSContext* cx, JS::CallArgs& args);
   nsresult GetJSSnapshot(JSContext* cx, JS::Handle<JSObject*> obj,
                          bool subsession, bool clearSubsession);
 
-  nsresult Add(const nsCString& key, uint32_t aSample);
+  nsresult Add(const nsCString& key, uint32_t aSample, ProcessID aProcessType);
   void Clear(bool subsession);
 
   HistogramID GetHistogramID() const { return mId; }
 
 private:
   typedef nsBaseHashtableET<nsCStringHashKey, Histogram*> KeyedHistogramEntry;
   typedef AutoHashtable<KeyedHistogramEntry> KeyedHistogramMapType;
   KeyedHistogramMapType mHistogramMap;
@@ -508,23 +508,27 @@ internal_CreateHistogramInstance(const H
   }
 
   return h;
 }
 
 nsresult
 internal_HistogramAdd(Histogram& histogram,
                       const HistogramID id,
-                      int32_t value)
+                      int32_t value,
+                      ProcessID aProcessType)
 {
   // Check if we are allowed to record the data.
   bool canRecordDataset = CanRecordDataset(gHistogramInfos[id].dataset,
                                            internal_CanRecordBase(),
                                            internal_CanRecordExtended());
-  if (!canRecordDataset || !internal_IsRecordingEnabled(id)) {
+  // If `histogram` is a non-parent-process histogram, then recording-enabled
+  // has been checked in its owner process.
+  if (!canRecordDataset ||
+    (aProcessType == ProcessID::Parent && !internal_IsRecordingEnabled(id))) {
     return NS_OK;
   }
 
   // It is safe to add to the histogram now: the subsession histogram was already
   // cloned from this so we won't add the sample twice.
   histogram.Add(value);
 
   return NS_OK;
@@ -675,22 +679,26 @@ KeyedHistogram::GetHistogram(const nsCSt
   Histogram* h = nullptr;
   if (NS_FAILED(GetHistogram(key, &h, subsession))) {
     return nullptr;
   }
   return h;
 }
 
 nsresult
-KeyedHistogram::Add(const nsCString& key, uint32_t sample)
+KeyedHistogram::Add(const nsCString& key, uint32_t sample,
+                    ProcessID aProcessType)
 {
   bool canRecordDataset = CanRecordDataset(mHistogramInfo.dataset,
                                            internal_CanRecordBase(),
                                            internal_CanRecordExtended());
-  if (!canRecordDataset || !internal_IsRecordingEnabled(mId)) {
+  // If `histogram` is a non-parent-process histogram, then recording-enabled
+  // has been checked in its owner process.
+  if (!canRecordDataset ||
+    (aProcessType == ProcessID::Parent && !internal_IsRecordingEnabled(mId))) {
     return NS_OK;
   }
 
   Histogram* histogram = GetHistogram(key, false);
   MOZ_ASSERT(histogram);
   if (!histogram) {
     return NS_ERROR_FAILURE;
   }
@@ -855,72 +863,72 @@ void internal_Accumulate(HistogramID aId
 {
   if (!internal_CanRecordBase() ||
       internal_RemoteAccumulate(aId, aSample)) {
     return;
   }
 
   Histogram *h = internal_GetHistogramById(aId, ProcessID::Parent, SessionType::Session);
   MOZ_ASSERT(h);
-  internal_HistogramAdd(*h, aId, aSample);
+  internal_HistogramAdd(*h, aId, aSample, ProcessID::Parent);
 
 #if !defined(MOZ_WIDGET_ANDROID)
   h = internal_GetHistogramById(aId, ProcessID::Parent, SessionType::Subsession);
   MOZ_ASSERT(h);
-  internal_HistogramAdd(*h, aId, aSample);
+  internal_HistogramAdd(*h, aId, aSample, ProcessID::Parent);
 #endif
 }
 
 void
 internal_Accumulate(HistogramID aId,
                     const nsCString& aKey, uint32_t aSample)
 {
   if (!gInitDone || !internal_CanRecordBase() ||
       internal_RemoteAccumulate(aId, aKey, aSample)) {
     return;
   }
 
   KeyedHistogram* keyed = internal_GetKeyedHistogramById(aId, ProcessID::Parent);
   MOZ_ASSERT(keyed);
-  keyed->Add(aKey, aSample);
+  keyed->Add(aKey, aSample, ProcessID::Parent);
 }
 
 void
 internal_AccumulateChild(ProcessID aProcessType, HistogramID aId, uint32_t aSample)
 {
   if (!internal_CanRecordBase()) {
     return;
   }
 
   if (Histogram* h = internal_GetHistogramById(aId, aProcessType, SessionType::Session)) {
-    internal_HistogramAdd(*h, aId, aSample);
+    internal_HistogramAdd(*h, aId, aSample, aProcessType);
   } else {
     NS_WARNING("Failed GetHistogramById for CHILD");
   }
 
 #if !defined(MOZ_WIDGET_ANDROID)
   if (Histogram* h = internal_GetHistogramById(aId, aProcessType, SessionType::Subsession)) {
-    internal_HistogramAdd(*h, aId, aSample);
+    internal_HistogramAdd(*h, aId, aSample, aProcessType);
   } else {
     NS_WARNING("Failed GetHistogramById for CHILD");
   }
 #endif
 }
 
 void
 internal_AccumulateChildKeyed(ProcessID aProcessType, HistogramID aId,
                               const nsCString& aKey, uint32_t aSample)
 {
   if (!gInitDone || !internal_CanRecordBase()) {
     return;
   }
 
   KeyedHistogram* keyed = internal_GetKeyedHistogramById(aId, aProcessType);
   MOZ_ASSERT(keyed);
-  keyed->Add(aKey, aSample);
+  keyed->Add(aKey, aSample, aProcessType);
 }
 
 void
 internal_ClearHistogram(HistogramID id, bool onlySubsession)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   if (!XRE_IsParentProcess()) {
     return;
@@ -1175,17 +1183,16 @@ internal_WrapAndReturnHistogram(Histogra
   // by the same thread that runs this function.
   if (!(JS_DefineFunction(cx, obj, "add", internal_JSHistogram_Add, 1, 0)
         && JS_DefineFunction(cx, obj, "snapshot",
                              internal_JSHistogram_Snapshot, 0, 0)
         && JS_DefineFunction(cx, obj, "clear", internal_JSHistogram_Clear, 0, 0))) {
     return NS_ERROR_FAILURE;
   }
 
-  // TODO: delete in finalizer
   JSHistogramData* data = new JSHistogramData{id};
   JS_SetPrivate(obj, data);
   ret.setObject(*obj);
 
   return NS_OK;
 }
 
 void
@@ -1531,17 +1538,16 @@ internal_WrapAndReturnKeyedHistogram(His
 #endif
         && JS_DefineFunction(cx, obj, "keys",
                              internal_JSKeyedHistogram_Keys, 0, 0)
         && JS_DefineFunction(cx, obj, "clear",
                              internal_JSKeyedHistogram_Clear, 0, 0))) {
     return NS_ERROR_FAILURE;
   }
 
-  // TODO: delete in finalizer
   JSHistogramData* data = new JSHistogramData{id};
   JS_SetPrivate(obj, data);
   ret.setObject(*obj);
 
   return NS_OK;
 }
 
 void