Bug 1956726 - remove the now unused variants of the Telemetry::Accumulate API, r=chutten.

Differential Revision: https://phabricator.services.mozilla.com/D244354

js/xpconnect/src/XPCJSRuntime.cpp

@@ -2586,12 +2586,17 @@ static nsresult JSSizeOfTab(JSObject* objArg, size_t* jsObjectsSize,
 
 }  // namespace xpc
 
+// Temporary workaround until bug 1949494 can land.
+namespace TelemetryHistogram {
+void Accumulate(mozilla::Telemetry::HistogramID aHistogram, uint32_t aSample);
+}
+
 static void AccumulateTelemetryCallback(JSMetric id, uint32_t sample) {
   // clang-format off
   switch (id) {
-#define CASE_ACCUMULATE(NAME, _)                      \
+#define CASE_ACCUMULATE(NAME, _)                                \
-    case JSMetric::NAME:                              \
+    case JSMetric::NAME:                                        \
-      Telemetry::Accumulate(Telemetry::NAME, sample); \
+      TelemetryHistogram::Accumulate(Telemetry::NAME, sample);  \
       break;
 
     FOR_EACH_JS_LEGACY_METRIC(CASE_ACCUMULATE)

toolkit/components/glean/bindings/private/Counter.cpp

@@ -24,9 +24,9 @@ void accumulateToBoolean(HistogramID aId, const nsACString& aLabel,
              "When mirroring to boolean histograms, we only support "
              "accumulating one sample at a time.");
   if (aLabel.EqualsASCII("true")) {
-    mozilla::Telemetry::Accumulate(aId, true);
+    TelemetryHistogram::Accumulate(aId, true);
   } else if (aLabel.EqualsASCII("false")) {
-    mozilla::Telemetry::Accumulate(aId, false);
+    TelemetryHistogram::Accumulate(aId, false);
   } else {
     MOZ_ASSERT_UNREACHABLE(
         "When mirroring to boolean histograms, we only support labels 'true' "
@@ -37,7 +37,7 @@ void accumulateToBoolean(HistogramID aId, const nsACString& aLabel,
 /* static */
 void accumulateToKeyedCount(HistogramID aId, const nsCString& aLabel,
                             int32_t aAmount) {
-  mozilla::Telemetry::Accumulate(aId, aLabel, aAmount);
+  TelemetryHistogram::Accumulate(aId, aLabel, aAmount);
 }
 
 /* static */
@@ -95,7 +95,7 @@ void CounterMetric::Add(int32_t aAmount) const {
     } else {
       auto hgramId = HistogramIdForMetric(mId);
       if (hgramId) {
-        Telemetry::Accumulate(hgramId.extract(), aAmount);
+        TelemetryHistogram::Accumulate(hgramId.extract(), aAmount);
       }
     }
   }

toolkit/components/glean/bindings/private/CustomDistribution.cpp

@@ -15,6 +15,7 @@
 #include "nsPrintfCString.h"
 #include "nsString.h"
 #include "js/PropertyAndElement.h"  // JS_DefineProperty
+#include "GIFFTFwd.h"
 
 namespace mozilla::glean {
 
@@ -28,15 +29,15 @@ void CustomDistributionMetric::AccumulateSamples(
     // N.B.: There is an `Accumulate(nsTArray<T>)`, but `T` is `uint32_t` and
     // we got `uint64_t`s here.
     for (auto sample : aSamples) {
-      Telemetry::Accumulate(id, sample);
+      TelemetryHistogram::Accumulate(id, sample);
     }
   } else if (IsSubmetricId(mId)) {
     GetLabeledDistributionMirrorLock().apply([&](const auto& lock) {
       auto tuple = lock.ref()->MaybeGet(mId);
       if (tuple) {
         for (auto sample : aSamples) {
-          Telemetry::Accumulate(std::get<0>(tuple.ref()),
+          TelemetryHistogram::Accumulate(std::get<0>(tuple.ref()),
-                                std::get<1>(tuple.ref()), sample);
+                                         std::get<1>(tuple.ref()), sample);
         }
       }
     });
@@ -48,13 +49,13 @@ void CustomDistributionMetric::AccumulateSingleSample(uint64_t aSample) const {
   auto hgramId = HistogramIdForMetric(mId);
   if (hgramId) {
     auto id = hgramId.extract();
-    Telemetry::Accumulate(id, aSample);
+    TelemetryHistogram::Accumulate(id, aSample);
   } else if (IsSubmetricId(mId)) {
     GetLabeledDistributionMirrorLock().apply([&](const auto& lock) {
       auto tuple = lock.ref()->MaybeGet(mId);
       if (tuple) {
-        Telemetry::Accumulate(std::get<0>(tuple.ref()),
+        TelemetryHistogram::Accumulate(std::get<0>(tuple.ref()),
-                              std::get<1>(tuple.ref()), aSample);
+                                       std::get<1>(tuple.ref()), aSample);
       }
     });
   }
@@ -69,15 +70,15 @@ void CustomDistributionMetric::AccumulateSamplesSigned(
     // N.B.: There is an `Accumulate(nsTArray<T>)`, but `T` is `uint32_t` and
     // we got `int64_t`s here.
     for (auto sample : aSamples) {
-      Telemetry::Accumulate(id, sample);
+      TelemetryHistogram::Accumulate(id, sample);
     }
   } else if (IsSubmetricId(mId)) {
     GetLabeledDistributionMirrorLock().apply([&](const auto& lock) {
       auto tuple = lock.ref()->MaybeGet(mId);
       if (tuple) {
         for (auto sample : aSamples) {
-          Telemetry::Accumulate(std::get<0>(tuple.ref()),
+          TelemetryHistogram::Accumulate(std::get<0>(tuple.ref()),
-                                std::get<1>(tuple.ref()), sample);
+                                         std::get<1>(tuple.ref()), sample);
         }
       }
     });
@@ -90,13 +91,13 @@ void CustomDistributionMetric::AccumulateSingleSampleSigned(
   auto hgramId = HistogramIdForMetric(mId);
   if (hgramId) {
     auto id = hgramId.extract();
-    Telemetry::Accumulate(id, aSample);
+    TelemetryHistogram::Accumulate(id, aSample);
   } else if (IsSubmetricId(mId)) {
     GetLabeledDistributionMirrorLock().apply([&](const auto& lock) {
       auto tuple = lock.ref()->MaybeGet(mId);
       if (tuple) {
-        Telemetry::Accumulate(std::get<0>(tuple.ref()),
+        TelemetryHistogram::Accumulate(std::get<0>(tuple.ref()),
-                              std::get<1>(tuple.ref()), aSample);
+                                       std::get<1>(tuple.ref()), aSample);
       }
     });
   }

toolkit/components/glean/bindings/private/GIFFTFwd.h

@@ -27,6 +27,11 @@ void Set(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, bool aValue);
 
 namespace TelemetryHistogram {
 uint8_t GetHistogramType(mozilla::Telemetry::HistogramID aId);
+void Accumulate(mozilla::Telemetry::HistogramID aHistogram, uint32_t aSample);
+void Accumulate(mozilla::Telemetry::HistogramID aHistogram,
+                const nsTArray<uint32_t>& aSamples);
+void Accumulate(mozilla::Telemetry::HistogramID aID, const nsCString& aKey,
+                uint32_t aSample);
 void AccumulateCategorical(mozilla::Telemetry::HistogramID aId,
                            const nsCString& aLabel);
 }  // namespace TelemetryHistogram

toolkit/components/glean/bindings/private/MemoryDistribution.cpp

@@ -18,6 +18,7 @@
 #include "nsPrintfCString.h"
 #include "nsString.h"
 #include "js/PropertyAndElement.h"  // JS_DefineProperty
+#include "GIFFTFwd.h"
 
 namespace mozilla::glean {
 
@@ -26,13 +27,13 @@ namespace impl {
 void MemoryDistributionMetric::Accumulate(size_t aSample) const {
   auto hgramId = HistogramIdForMetric(mId);
   if (hgramId) {
-    Telemetry::Accumulate(hgramId.extract(), aSample);
+    TelemetryHistogram::Accumulate(hgramId.extract(), aSample);
   } else if (IsSubmetricId(mId)) {
     GetLabeledDistributionMirrorLock().apply([&](const auto& lock) {
       auto tuple = lock.ref()->MaybeGet(mId);
       if (tuple) {
-        Telemetry::Accumulate(std::get<0>(tuple.ref()),
+        TelemetryHistogram::Accumulate(std::get<0>(tuple.ref()),
-                              std::get<1>(tuple.ref()), aSample);
+                                       std::get<1>(tuple.ref()), aSample);
       }
     });
   }

toolkit/components/glean/bindings/private/TimingDistribution.cpp

@@ -217,25 +217,32 @@ extern "C" NS_EXPORT void GIFFT_TimingDistributionStopAndAccumulate(
         // Values are from Glean's `TimeUnit`
         switch (aUnit) {
           case 0:  // Nanos
-            Accumulate(mirrorId.extract(), duration.ToMicroseconds() * 1000);
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToMicroseconds() * 1000);
             break;
           case 1:  // Micros
-            Accumulate(mirrorId.extract(), duration.ToMicroseconds());
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToMicroseconds());
             break;
           case 2:  // Millis
-            Accumulate(mirrorId.extract(), duration.ToMilliseconds());
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToMilliseconds());
             break;
           case 3:  // Seconds
-            Accumulate(mirrorId.extract(), duration.ToSeconds());
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToSeconds());
             break;
           case 4:  // Minutes
-            Accumulate(mirrorId.extract(), duration.ToSeconds() / 60);
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToSeconds() / 60);
             break;
           case 5:  // Hours
-            Accumulate(mirrorId.extract(), duration.ToSeconds() / 60 / 60);
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToSeconds() / 60 / 60);
             break;
           case 6:  // Days
-            Accumulate(mirrorId.extract(), duration.ToSeconds() / 60 / 60 / 24);
+            TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                           duration.ToSeconds() / 60 / 60 / 24);
             break;
           default:
             MOZ_ASSERT_UNREACHABLE("Invalid/Unsupported time unit");
@@ -251,7 +258,7 @@ extern "C" NS_EXPORT void GIFFT_TimingDistributionAccumulateRawSample(
     uint32_t aMetricId, uint32_t aSample) {
   auto mirrorId = mozilla::glean::HistogramIdForMetric(aMetricId);
   if (mirrorId) {
-    Accumulate(mirrorId.extract(), aSample);
+    TelemetryHistogram::Accumulate(mirrorId.extract(), aSample);
   }
 }
 
@@ -260,7 +267,7 @@ extern "C" NS_EXPORT void GIFFT_TimingDistributionAccumulateRawSamples(
     uint32_t aMetricId, const nsTArray<uint32_t>& aSamples) {
   auto mirrorId = mozilla::glean::HistogramIdForMetric(aMetricId);
   if (mirrorId) {
-    Accumulate(mirrorId.extract(), aSamples);
+    TelemetryHistogram::Accumulate(mirrorId.extract(), aSamples);
   }
 }
 
@@ -303,10 +310,11 @@ extern "C" NS_EXPORT void GIFFT_LabeledTimingDistributionStopAndAccumulate(
       // The timer might not be in the map to be removed if it's already been
       // cancelled or stop_and_accumulate'd.
       if (!NS_WARN_IF(!optStart)) {
-        Accumulate(mirrorId.extract(), PromiseFlatCString(aLabel),
+        TelemetryHistogram::Accumulate(
-                   static_cast<uint32_t>(
+            mirrorId.extract(), PromiseFlatCString(aLabel),
-                       (mozilla::TimeStamp::Now() - optStart.extract())
+            static_cast<uint32_t>(
-                           .ToMilliseconds()));
+                (mozilla::TimeStamp::Now() - optStart.extract())
+                    .ToMilliseconds()));
       }
     });
   }
@@ -317,7 +325,8 @@ extern "C" NS_EXPORT void GIFFT_LabeledTimingDistributionAccumulateRawMillis(
     uint32_t aMetricId, const nsACString& aLabel, uint32_t aMS) {
   auto mirrorId = mozilla::glean::HistogramIdForMetric(aMetricId);
   if (mirrorId) {
-    Accumulate(mirrorId.extract(), PromiseFlatCString(aLabel), aMS);
+    TelemetryHistogram::Accumulate(mirrorId.extract(),
+                                   PromiseFlatCString(aLabel), aMS);
   }
 }
 

toolkit/components/telemetry/core/Stopwatch.cpp

@@ -6,7 +6,6 @@
 
 #include "mozilla/telemetry/Stopwatch.h"
 
-#include "TelemetryHistogram.h"
 #include "TelemetryUserInteraction.h"
 
 #include "js/MapAndSet.h"
@@ -165,18 +164,6 @@ class Timers final : public BackgroundHangAnnotator {
   bool Delete(JSContext* aCx, const nsAString& aHistogram,
               JS::Handle<JSObject*> aObj, const nsAString& aKey);
 
-  int32_t TimeElapsed(JSContext* aCx, const nsAString& aHistogram,
-                      JS::Handle<JSObject*> aObj, const nsAString& aKey,
-                      bool aCanceledOkay = false);
-
-  bool Start(JSContext* aCx, const nsAString& aHistogram,
-             JS::Handle<JSObject*> aObj, const nsAString& aKey,
-             bool aInSeconds = false);
-
-  int32_t Finish(JSContext* aCx, const nsAString& aHistogram,
-                 JS::Handle<JSObject*> aObj, const nsAString& aKey,
-                 bool aCanceledOkay = false);
-
   bool& SuppressErrors() { return mSuppressErrors; }
 
   bool StartUserInteraction(JSContext* aCx, const nsAString& aUserInteraction,
@@ -323,88 +310,6 @@ bool Timers::Delete(JSContext* aCx, const nsAString& aHistogram,
   return false;
 }
 
-int32_t Timers::TimeElapsed(JSContext* aCx, const nsAString& aHistogram,
-                            JS::Handle<JSObject*> aObj, const nsAString& aKey,
-                            bool aCanceledOkay) {
-  RefPtr<Timer> timer = Get(aCx, aHistogram, aObj, aKey, false);
-  if (!timer) {
-    if (!aCanceledOkay && !mSuppressErrors) {
-      LogError(aCx, nsPrintfCString(
-                        "TelemetryStopwatch: requesting elapsed time for "
-                        "nonexisting stopwatch. Histogram: \"%s\", key: \"%s\"",
-                        NS_ConvertUTF16toUTF8(aHistogram).get(),
-                        NS_ConvertUTF16toUTF8(aKey).get()));
-    }
-    return -1;
-  }
-
-  return timer->Elapsed();
-}
-
-bool Timers::Start(JSContext* aCx, const nsAString& aHistogram,
-                   JS::Handle<JSObject*> aObj, const nsAString& aKey,
-                   bool aInSeconds) {
-  if (RefPtr<Timer> timer = Get(aCx, aHistogram, aObj, aKey)) {
-    if (timer->Started()) {
-      if (!mSuppressErrors) {
-        LogError(aCx,
-                 nsPrintfCString(
-                     "TelemetryStopwatch: key \"%s\" was already initialized",
-                     NS_ConvertUTF16toUTF8(aHistogram).get()));
-      }
-      Delete(aCx, aHistogram, aObj, aKey);
-    } else {
-      timer->Start(aInSeconds);
-      return true;
-    }
-  }
-  return false;
-}
-
-int32_t Timers::Finish(JSContext* aCx, const nsAString& aHistogram,
-                       JS::Handle<JSObject*> aObj, const nsAString& aKey,
-                       bool aCanceledOkay) {
-  RefPtr<Timer> timer = GetAndDelete(aCx, aHistogram, aObj, aKey);
-  if (!timer) {
-    if (!aCanceledOkay && !mSuppressErrors) {
-      LogError(aCx, nsPrintfCString(
-                        "TelemetryStopwatch: finishing nonexisting stopwatch. "
-                        "Histogram: \"%s\", key: \"%s\"",
-                        NS_ConvertUTF16toUTF8(aHistogram).get(),
-                        NS_ConvertUTF16toUTF8(aKey).get()));
-    }
-    return -1;
-  }
-
-  int32_t delta = timer->Elapsed();
-  NS_ConvertUTF16toUTF8 histogram(aHistogram);
-  nsresult rv;
-  if (!aKey.IsVoid()) {
-    NS_ConvertUTF16toUTF8 key(aKey);
-    rv = TelemetryHistogram::Accumulate(histogram.get(), key, delta);
-  } else {
-    rv = TelemetryHistogram::Accumulate(histogram.get(), delta);
-  }
-  if (profiler_thread_is_being_profiled_for_markers()) {
-    nsCString markerText = histogram;
-    if (!aKey.IsVoid()) {
-      markerText.AppendLiteral(":");
-      markerText.Append(NS_ConvertUTF16toUTF8(aKey));
-    }
-    PROFILER_MARKER_TEXT("Stopwatch", TELEMETRY,
-                         MarkerTiming::IntervalUntilNowFrom(timer->StartTime()),
-                         markerText);
-  }
-  if (NS_FAILED(rv) && rv != NS_ERROR_NOT_AVAILABLE && !mSuppressErrors) {
-    LogError(aCx, nsPrintfCString(
-                      "TelemetryStopwatch: failed to update the Histogram "
-                      "\"%s\", using key: \"%s\"",
-                      NS_ConvertUTF16toUTF8(aHistogram).get(),
-                      NS_ConvertUTF16toUTF8(aKey).get()));
-  }
-  return NS_SUCCEEDED(rv) ? delta : -1;
-}
-
 bool Timers::StartUserInteraction(JSContext* aCx,
                                   const nsAString& aUserInteraction,
                                   const nsACString& aValue,

toolkit/components/telemetry/core/Telemetry.cpp

@@ -1655,20 +1655,6 @@ TelemetryImpl::GetAllStores(JSContext* aCx,
 
 }  // namespace
 
-////////////////////////////////////////////////////////////////////////
-////////////////////////////////////////////////////////////////////////
-//
-// EXTERNALLY VISIBLE FUNCTIONS in no name space
-// These are NOT listed in Telemetry.h
-
-/**
- * The XRE_TelemetryAdd function is to be used by embedding applications
- * that can't use mozilla::Telemetry::Accumulate() directly.
- */
-void XRE_TelemetryAccumulate(int aID, uint32_t aSample) {
-  mozilla::Telemetry::Accumulate((mozilla::Telemetry::HistogramID)aID, aSample);
-}
-
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
@@ -1759,31 +1745,10 @@ void RecordShutdownEndTimeStamp() {
 
 namespace mozilla::Telemetry {
 
-void Accumulate(HistogramID aHistogram, uint32_t aSample) {
-  TelemetryHistogram::Accumulate(aHistogram, aSample);
-}
-
-void Accumulate(HistogramID aHistogram, const nsTArray<uint32_t>& aSamples) {
-  TelemetryHistogram::Accumulate(aHistogram, aSamples);
-}
-
 void Accumulate(HistogramID aID, const nsCString& aKey, uint32_t aSample) {
   TelemetryHistogram::Accumulate(aID, aKey, aSample);
 }
 
-void Accumulate(HistogramID aID, const nsCString& aKey,
-                const nsTArray<uint32_t>& aSamples) {
-  TelemetryHistogram::Accumulate(aID, aKey, aSamples);
-}
-
-void Accumulate(const char* name, uint32_t sample) {
-  TelemetryHistogram::Accumulate(name, sample);
-}
-
-void Accumulate(const char* name, const nsCString& key, uint32_t sample) {
-  TelemetryHistogram::Accumulate(name, key, sample);
-}
-
 const char* GetHistogramName(HistogramID id) {
   return TelemetryHistogram::GetHistogramName(id);
 }

toolkit/components/telemetry/core/Telemetry.h

@@ -52,62 +52,19 @@ void Init();
 void ShutdownTelemetry();
 
 /**
- * Adds sample to a histogram defined in TelemetryHistogramEnums.h
+ * DEPRECATED:
- *
- * @param id - histogram id
- * @param sample - value to record.
- */
-void Accumulate(HistogramID id, uint32_t sample);
-
-/**
- * Adds an array of samples to a histogram defined in TelemetryHistograms.h
- * @param id - histogram id
- * @param samples - values to record.
- */
-void Accumulate(HistogramID id, const nsTArray<uint32_t>& samples);
-
-/**
  * Adds sample to a keyed histogram defined in TelemetryHistogramEnums.h
+ * The only remaining callers should use keyed boolean or keyed categorical
+ * histograms, that do not have glean equivalents yet (see bug 1657470).
  *
  * @param id - keyed histogram id
  * @param key - the string key
- * @param sample - (optional) value to record, defaults to 1.
+ * @param sample - value to record.
  */
-void Accumulate(HistogramID id, const nsCString& key, uint32_t sample = 1);
+void Accumulate(HistogramID id, const nsCString& key, uint32_t sample);
-
-/**
- * Adds an array of samples to a histogram defined in TelemetryHistograms.h
- * @param id - histogram id
- * @param samples - values to record.
- * @param key - the string key
- */
-void Accumulate(HistogramID id, const nsCString& key,
-                const nsTArray<uint32_t>& samples);
-
-/**
- * Adds a sample to a histogram defined in TelemetryHistogramEnums.h.
- * This function is here to support telemetry measurements from Java,
- * where we have only names and not numeric IDs.  You should almost
- * certainly be using the by-enum-id version instead of this one.
- *
- * @param name - histogram name
- * @param sample - value to record
- */
-void Accumulate(const char* name, uint32_t sample);
-
-/**
- * Adds a sample to a histogram defined in TelemetryHistogramEnums.h.
- * This function is here to support telemetry measurements from Java,
- * where we have only names and not numeric IDs.  You should almost
- * certainly be using the by-enum-id version instead of this one.
- *
- * @param name - histogram name
- * @param key - the string key
- * @param sample - sample - (optional) value to record, defaults to 1.
- */
-void Accumulate(const char* name, const nsCString& key, uint32_t sample = 1);
 
 /**
+ * DEPRECATED:
  * Adds sample to a keyed categorical histogram defined in
  * TelemetryHistogramEnums.h This is the typesafe - and preferred - way to use
  * the keyed categorical histograms by passing values from the corresponding

toolkit/components/telemetry/core/TelemetryHistogram.cpp

@@ -2616,85 +2616,6 @@ void TelemetryHistogram::Accumulate(HistogramID aID, const nsCString& aKey,
   internal_Accumulate(locker, aID, aKey, aSample);
 }
 
-void TelemetryHistogram::Accumulate(HistogramID aID, const nsCString& aKey,
-                                    const nsTArray<uint32_t>& aSamples) {
-  if (NS_WARN_IF(!internal_IsHistogramEnumId(aID))) {
-    MOZ_ASSERT_UNREACHABLE("Histogram usage requires valid ids");
-    return;
-  }
-
-  // Check that this histogram is keyed
-  MOZ_ASSERT(gHistogramInfos[aID].keyed,
-             "Cannot accumulate into a non-keyed histogram using a key.");
-
-  // Check if we're allowed to record in the provided key, for this histogram.
-  if (!gHistogramInfos[aID].allows_key(aKey)) {
-    nsPrintfCString msg("%s - key '%s' not allowed for this keyed histogram",
-                        gHistogramInfos[aID].name(), aKey.get());
-    LogToBrowserConsole(nsIScriptError::errorFlag, NS_ConvertUTF8toUTF16(msg));
-    TelemetryScalar::Add(mozilla::Telemetry::ScalarID::
-                             TELEMETRY_ACCUMULATE_UNKNOWN_HISTOGRAM_KEYS,
-                         NS_ConvertASCIItoUTF16(gHistogramInfos[aID].name()),
-                         1);
-    return;
-  }
-
-  StaticMutexAutoLock locker(gTelemetryHistogramMutex);
-  for (uint32_t sample : aSamples) {
-    internal_Accumulate(locker, aID, aKey, sample);
-  }
-}
-
-nsresult TelemetryHistogram::Accumulate(const char* name, uint32_t sample) {
-  StaticMutexAutoLock locker(gTelemetryHistogramMutex);
-  if (!internal_CanRecordBase()) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-  HistogramID id;
-  nsresult rv =
-      internal_GetHistogramIdByName(locker, nsDependentCString(name), &id);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  internal_Accumulate(locker, id, sample);
-  return NS_OK;
-}
-
-nsresult TelemetryHistogram::Accumulate(const char* name, const nsCString& key,
-                                        uint32_t sample) {
-  bool keyNotAllowed = false;
-
-  {
-    StaticMutexAutoLock locker(gTelemetryHistogramMutex);
-    if (!internal_CanRecordBase()) {
-      return NS_ERROR_NOT_AVAILABLE;
-    }
-    HistogramID id;
-    nsresult rv =
-        internal_GetHistogramIdByName(locker, nsDependentCString(name), &id);
-    if (NS_SUCCEEDED(rv)) {
-      // Check if we're allowed to record in the provided key, for this
-      // histogram.
-      if (gHistogramInfos[id].allows_key(key)) {
-        internal_Accumulate(locker, id, key, sample);
-        return NS_OK;
-      }
-      // We're holding |gTelemetryHistogramMutex|, so we can't print a message
-      // here.
-      keyNotAllowed = true;
-    }
-  }
-
-  if (keyNotAllowed) {
-    LogToBrowserConsole(nsIScriptError::errorFlag,
-                        u"Key not allowed for this keyed histogram"_ns);
-    TelemetryScalar::Add(mozilla::Telemetry::ScalarID::
-                             TELEMETRY_ACCUMULATE_UNKNOWN_HISTOGRAM_KEYS,
-                         NS_ConvertASCIItoUTF16(name), 1);
-  }
-  return NS_ERROR_FAILURE;
-}
-
 void TelemetryHistogram::AccumulateCategorical(HistogramID aId,
                                                const nsCString& label) {
   if (NS_WARN_IF(!internal_IsHistogramEnumId(aId))) {

toolkit/components/telemetry/core/TelemetryHistogram.h

@@ -37,25 +37,6 @@ void Accumulate(mozilla::Telemetry::HistogramID aHistogram,
                 const nsTArray<uint32_t>& aSamples);
 void Accumulate(mozilla::Telemetry::HistogramID aID, const nsCString& aKey,
                 uint32_t aSample);
-void Accumulate(mozilla::Telemetry::HistogramID aID, const nsCString& aKey,
-                const nsTArray<uint32_t>& aSamples);
-/*
- * Accumulate a sample into the named histogram.
- *
- * Returns NS_OK on success.
- * Returns NS_ERROR_NOT_AVAILABLE if recording Telemetry is disabled.
- * Returns NS_ERROR_FAILURE on other errors.
- */
-nsresult Accumulate(const char* name, uint32_t sample);
-
-/*
- * Accumulate a sample into the named keyed histogram by key.
- *
- * Returns NS_OK on success.
- * Returns NS_ERROR_NOT_AVAILABLE if recording Telemetry is disabled.
- * Returns NS_ERROR_FAILURE on other errors.
- */
-nsresult Accumulate(const char* name, const nsCString& key, uint32_t sample);
 
 void AccumulateCategorical(mozilla::Telemetry::HistogramID aId,
                            const nsCString& aLabel);

toolkit/components/telemetry/tests/gtest/TestHistograms.cpp

@@ -28,8 +28,8 @@ TEST_F(TelemetryTestFixture, AccumulateCountHistogram) {
                        false);
 
   // Accumulate in the histogram
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_COUNT, kExpectedValue / 2);
+  TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_COUNT,
-  Telemetry::Accumulate("TELEMETRY_TEST_COUNT", kExpectedValue / 2);
+                                 kExpectedValue);
 
   // Get a snapshot for all the histograms
   JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -101,9 +101,8 @@ TEST_F(TelemetryTestFixture, TestKeyedKeysHistogram) {
   Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "not-allowed"_ns,
                         1);
   Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "testkey"_ns, 0);
-  // Do the same, using the API that accepts the histogram name as a string.
+  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "CommonKey"_ns,
-  Telemetry::Accumulate("TELEMETRY_TEST_KEYED_KEYS", "not-allowed"_ns, 1);
+                        1);
-  Telemetry::Accumulate("TELEMETRY_TEST_KEYED_KEYS", "CommonKey"_ns, 1);
 
   // Get a snapshot for all the histograms
   JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -141,10 +140,9 @@ TEST_F(TelemetryTestFixture, TestKeyedKeysHistogram) {
   ASSERT_TRUE(expectedKeyData.isUndefined())
   << "Unallowed keys must not be recorded in the histogram data";
 
-  // The 'not-allowed' key accumulation for 'TELEMETRY_TESTED_KEYED_KEYS' was
+  // We expect the count of 'telemetry.accumulate_unknown_histogram_keys' to be
-  // attemtped twice, so we expect the count of
+  // 1 for the 'not-allowed' key accumulation.
-  // 'telemetry.accumulate_unknown_histogram_keys' to be 2
+  const uint32_t expectedAccumulateUnknownCount = 1;
-  const uint32_t expectedAccumulateUnknownCount = 2;
   JS::Rooted<JS::Value> scalarsSnapshot(cx.GetJSContext());
   GetScalarsSnapshot(true, cx.GetJSContext(), &scalarsSnapshot);
   CheckKeyedUintScalar("telemetry.accumulate_unknown_histogram_keys",
@@ -278,7 +276,7 @@ TEST_F(TelemetryTestFixture, AccumulateCountHistogram_MultipleSamples) {
                        false);
 
   // Accumulate in histogram
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_COUNT, samples);
+  TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_COUNT, samples);
 
   // Get a snapshot of all the histograms
   JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -310,7 +308,7 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_MultipleSamples) {
                        "TELEMETRY_TEST_LINEAR"_ns, false);
 
   // Accumulate in the histogram
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_LINEAR, samples);
+  TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_LINEAR, samples);
 
   // Get a snapshot of all the histograms
   JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -349,7 +347,7 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_DifferentSamples) {
                        "TELEMETRY_TEST_LINEAR"_ns, false);
 
   // Accumulate in histogram
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_LINEAR, samples);
+  TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_LINEAR, samples);
 
   // Get a snapshot of all histograms
   JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -399,210 +397,6 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_DifferentSamples) {
                        scalarsSnapshot, expectedAccumulateClampedCount);
 }
 
-TEST_F(TelemetryTestFixture, AccumulateKeyedCountHistogram_MultipleSamples) {
-  const nsTArray<uint32_t> samples({5, 10, 15});
-  const uint32_t kExpectedSum = 5 + 10 + 15;
-
-  AutoJSContextWithGlobal cx(mCleanGlobal);
-
-  GetAndClearHistogram(cx.GetJSContext(), mTelemetry,
-                       "TELEMETRY_TEST_KEYED_COUNT"_ns, true);
-
-  // Accumulate data in the provided key within the histogram
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_COUNT, "sample"_ns,
-                        samples);
-
-  // Get a snapshot for all the histograms
-  JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
-  GetSnapshots(cx.GetJSContext(), mTelemetry, "TELEMETRY_TEST_KEYED_COUNT",
-               &snapshot, true);
-
-  // Get the histogram from the snapshot
-  JS::Rooted<JS::Value> histogram(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "TELEMETRY_TEST_KEYED_COUNT", snapshot,
-              &histogram);
-
-  // Get "sample" property from histogram
-  JS::Rooted<JS::Value> expectedKeyData(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "sample", histogram, &expectedKeyData);
-
-  // Get "sum" property from keyed data
-  JS::Rooted<JS::Value> sum(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "sum", expectedKeyData, &sum);
-
-  // Check that the sum stored in the histogram matches with |kExpectedSum|
-  uint32_t uSum = 0;
-  JS::ToUint32(cx.GetJSContext(), sum, &uSum);
-  ASSERT_EQ(uSum, kExpectedSum)
-      << "The histogram is not returning expected sum";
-}
-
-TEST_F(TelemetryTestFixture, TestKeyedLinearHistogram_MultipleSamples) {
-  AutoJSContextWithGlobal cx(mCleanGlobal);
-
-  mTelemetry->ClearScalars();
-  GetAndClearHistogram(cx.GetJSContext(), mTelemetry,
-                       "TELEMETRY_TEST_KEYED_LINEAR"_ns, true);
-
-  const nsTArray<uint32_t> samples({1, 5, 250000, UINT_MAX});
-  // Test the accumulation on the key 'testkey', using
-  // the API that accepts histogram IDs.
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_LINEAR, "testkey"_ns,
-                        samples);
-
-  // Get a snapshot for all the histograms
-  JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
-  GetSnapshots(cx.GetJSContext(), mTelemetry, "TELEMETRY_TEST_KEYED_LINEAR",
-               &snapshot, true);
-
-  // Get the histogram from the snapshot
-  JS::Rooted<JS::Value> histogram(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "TELEMETRY_TEST_KEYED_LINEAR", snapshot,
-              &histogram);
-
-  // Get "testkey" property from histogram.
-  JS::Rooted<JS::Value> expectedKeyData(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "testkey", histogram, &expectedKeyData);
-  ASSERT_TRUE(!expectedKeyData.isUndefined())
-  << "Cannot find the expected key in the histogram data";
-
-  // Get values object from 'testkey' histogram.
-  JS::Rooted<JS::Value> values(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "values", expectedKeyData, &values);
-
-  // Get values in first and last buckets.
-  JS::Rooted<JS::Value> countFirst(cx.GetJSContext());
-  JS::Rooted<JS::Value> countLast(cx.GetJSContext());
-  const uint32_t firstIndex = 1;
-  // Buckets are indexed by their start value
-  const uint32_t lastIndex = 250000;
-  GetElement(cx.GetJSContext(), firstIndex, values, &countFirst);
-  GetElement(cx.GetJSContext(), lastIndex, values, &countLast);
-
-  // Check that the values match.
-  uint32_t uCountFirst = 0;
-  uint32_t uCountLast = 0;
-  JS::ToUint32(cx.GetJSContext(), countFirst, &uCountFirst);
-  JS::ToUint32(cx.GetJSContext(), countLast, &uCountLast);
-
-  const uint32_t kExpectedCountFirst = 2;
-  const uint32_t kExpectedCountLast = 2;
-  ASSERT_EQ(uCountFirst, kExpectedCountFirst)
-      << "The first bucket did not accumulate the correct number of values for "
-         "key 'testkey'";
-  ASSERT_EQ(uCountLast, kExpectedCountLast)
-      << "The last bucket did not accumulate the correct number of values for "
-         "key 'testkey'";
-
-  // We accumulated one keyed values that had to be clamped. We expect the
-  // count in 'telemetry.accumulate_clamped_values' to be 1
-  const uint32_t expectedAccumulateClampedCount = 1;
-  JS::Rooted<JS::Value> scalarsSnapshot(cx.GetJSContext());
-  GetScalarsSnapshot(true, cx.GetJSContext(), &scalarsSnapshot);
-  CheckKeyedUintScalar("telemetry.accumulate_clamped_values",
-                       "TELEMETRY_TEST_KEYED_LINEAR", cx.GetJSContext(),
-                       scalarsSnapshot, expectedAccumulateClampedCount);
-}
-
-TEST_F(TelemetryTestFixture, TestKeyedKeysHistogram_MultipleSamples) {
-  AutoJSContextWithGlobal cx(mCleanGlobal);
-  mTelemetry->ClearScalars();
-  const nsTArray<uint32_t> samples({false, false, true, 32, true});
-
-  GetAndClearHistogram(cx.GetJSContext(), mTelemetry,
-                       "TELEMETRY_TEST_KEYED_KEYS"_ns, true);
-
-  // Test the accumulation on both the allowed and unallowed keys, using
-  // the API that accepts histogram IDs.
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "not-allowed"_ns,
-                        samples);
-  Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "testkey"_ns,
-                        samples);
-
-  // Get a snapshot for all the histograms
-  JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
-  GetSnapshots(cx.GetJSContext(), mTelemetry, "TELEMETRY_TEST_KEYED_KEYS",
-               &snapshot, true);
-
-  // Get the histogram from the snapshot
-  JS::Rooted<JS::Value> histogram(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "TELEMETRY_TEST_KEYED_KEYS", snapshot,
-              &histogram);
-
-  // Get "testkey" property from histogram and check that it stores the correct
-  // data.
-  JS::Rooted<JS::Value> testKeyData(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "testkey", histogram, &testKeyData);
-  ASSERT_TRUE(!testKeyData.isUndefined())
-  << "Cannot find the key 'testkey' in the histogram data";
-
-  JS::Rooted<JS::Value> values(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "values", testKeyData, &values);
-
-  // Get values in buckets 0,1,2
-  const uint32_t falseIndex = 0;
-  const uint32_t trueIndex = 1;
-  const uint32_t otherIndex = 2;
-
-  JS::Rooted<JS::Value> countFalse(cx.GetJSContext());
-  JS::Rooted<JS::Value> countTrue(cx.GetJSContext());
-  JS::Rooted<JS::Value> countOther(cx.GetJSContext());
-
-  GetElement(cx.GetJSContext(), falseIndex, values, &countFalse);
-  GetElement(cx.GetJSContext(), trueIndex, values, &countTrue);
-  GetElement(cx.GetJSContext(), otherIndex, values, &countOther);
-
-  uint32_t uCountFalse = 0;
-  uint32_t uCountTrue = 0;
-  uint32_t uCountOther = 0;
-  JS::ToUint32(cx.GetJSContext(), countFalse, &uCountFalse);
-  JS::ToUint32(cx.GetJSContext(), countTrue, &uCountTrue);
-  JS::ToUint32(cx.GetJSContext(), countOther, &uCountOther);
-
-  const uint32_t kExpectedCountFalse = 2;
-  const uint32_t kExpectedCountTrue = 3;
-  const uint32_t kExpectedCountOther = 0;
-
-  ASSERT_EQ(uCountFalse, kExpectedCountFalse)
-      << "The histogram did not accumulate the correct number of 'false' "
-         "booleans for key 'testkey'";
-  ASSERT_EQ(uCountTrue, kExpectedCountTrue)
-      << "The histogram did not accumulate the correct number of 'true' "
-         "booleans for key 'testkey'";
-  ASSERT_EQ(uCountOther, kExpectedCountOther)
-      << "The histogram did not accumulate the correct number of undefined "
-         "values for key 'testkey'";
-
-  // Here we check that we are not accumulating to a different (but still
-  // 'allowed') key. Get "CommonKey" property from histogram and check that it
-  // has no data. Since we accumulated no data to it, commonKeyData should be
-  // undefined.
-  JS::Rooted<JS::Value> commonKeyData(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "CommonKey", histogram, &commonKeyData);
-  ASSERT_TRUE(commonKeyData.isUndefined())
-  << "Found data in key 'CommonKey' even though we accumulated no data to "
-     "it";
-
-  // Here we check that our function does not allow accumulation into unallowed
-  // keys. Get 'not-allowed' property from histogram and check that this also
-  // has no data. This should contain no data because this key is not allowed.
-  JS::Rooted<JS::Value> notAllowedKeyData(cx.GetJSContext());
-  GetProperty(cx.GetJSContext(), "not-allowed", histogram, &notAllowedKeyData);
-  ASSERT_TRUE(notAllowedKeyData.isUndefined())
-  << "Found data in key 'not-allowed' even though accumuling data to it is "
-     "not allowed";
-
-  // The 'not-allowed' key accumulation for 'TELEMETRY_TESTED_KEYED_KEYS' was
-  // attemtped once, so we expect the count of
-  // 'telemetry.accumulate_unknown_histogram_keys' to be 1
-  const uint32_t expectedAccumulateUnknownCount = 1;
-  JS::Rooted<JS::Value> scalarsSnapshot(cx.GetJSContext());
-  GetScalarsSnapshot(true, cx.GetJSContext(), &scalarsSnapshot);
-  CheckKeyedUintScalar("telemetry.accumulate_unknown_histogram_keys",
-                       "TELEMETRY_TEST_KEYED_KEYS", cx.GetJSContext(),
-                       scalarsSnapshot, expectedAccumulateUnknownCount);
-}
-
 TEST_F(TelemetryTestFixture, GIFFTLabeledCounterToBooleanHgram) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
 

toolkit/xre/Bootstrap.cpp

@@ -36,10 +36,6 @@ class BootstrapImpl final : public Bootstrap {
 
   virtual void NS_LogTerm() override { ::NS_LogTerm(); }
 
-  virtual void XRE_TelemetryAccumulate(int aID, uint32_t aSample) override {
-    ::XRE_TelemetryAccumulate(aID, aSample);
-  }
-
   virtual void XRE_StartupTimelineRecord(int aEvent,
                                          mozilla::TimeStamp aWhen) override {
     ::XRE_StartupTimelineRecord(aEvent, aWhen);

toolkit/xre/Bootstrap.h

@@ -82,8 +82,6 @@ class Bootstrap {
 
   virtual void NS_LogTerm() = 0;
 
-  virtual void XRE_TelemetryAccumulate(int aID, uint32_t aSample) = 0;
-
   virtual void XRE_StartupTimelineRecord(int aEvent,
                                          mozilla::TimeStamp aWhen) = 0;
 

xpcom/build/nsXULAppAPI.h

@@ -350,8 +350,6 @@ bool XRE_ShutdownTestShell();
 void XRE_InstallX11ErrorHandler();
 void XRE_CleanupX11ErrorHandler();
 
-void XRE_TelemetryAccumulate(int aID, uint32_t aSample);
-
 void XRE_StartupTimelineRecord(int aEvent, mozilla::TimeStamp aWhen);
 
 void XRE_InitOmnijar(nsIFile* aGreOmni, nsIFile* aAppOmni);