Bug 1508599 - Return a proper result value on accumulate and surpress error when Telemetry is disabled r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Wed, 28 Nov 2018 19:47:50 +0000
changeset 505132 1062e65758d62ebc00c9a50d06e24d75bb25e863
parent 505131 4ecb983c1655f18460d61dde1e5a06dd2b5d04cb
child 505133 e78bf6b8af12ab2e014bc98011f58dbe199ec1c0
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1508599
milestone65.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 1508599 - Return a proper result value on accumulate and surpress error when Telemetry is disabled r=chutten Differential Revision: https://phabricator.services.mozilla.com/D13244
toolkit/components/telemetry/core/Stopwatch.cpp
toolkit/components/telemetry/core/TelemetryHistogram.cpp
toolkit/components/telemetry/core/TelemetryHistogram.h
--- a/toolkit/components/telemetry/core/Stopwatch.cpp
+++ b/toolkit/components/telemetry/core/Stopwatch.cpp
@@ -401,31 +401,31 @@ Timers::Finish(JSContext* aCx,
                bool aCanceledOkay)
 {
   int32_t delta = TimeElapsed(aCx, aHistogram, aObj, aKey, aCanceledOkay, true);
   if (delta == -1) {
     return delta;
   }
 
   NS_ConvertUTF16toUTF8 histogram(aHistogram);
-  bool ok;
+  nsresult rv;
   if (!aKey.IsVoid()) {
     NS_ConvertUTF16toUTF8 key(aKey);
-    ok = TelemetryHistogram::Accumulate(histogram.get(), key, delta);
+    rv = TelemetryHistogram::Accumulate(histogram.get(), key, delta);
   } else {
-    ok = TelemetryHistogram::Accumulate(histogram.get(), delta);
+    rv = TelemetryHistogram::Accumulate(histogram.get(), delta);
   }
-  if (!ok && !mSuppressErrors) {
+  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 ok ? delta : -1;
+  return NS_SUCCEEDED(rv) ? delta : -1;
 }
 
 
 /* static */ bool
 Stopwatch::Start(const dom::GlobalObject& aGlobal,
                  const nsAString& aHistogram,
                  JS::Handle<JSObject*> aObj,
                  const dom::TelemetryStopwatchOptions& aOptions)
--- a/toolkit/components/telemetry/core/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/core/TelemetryHistogram.cpp
@@ -2621,65 +2621,65 @@ TelemetryHistogram::Accumulate(Histogram
   }
 
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   for(uint32_t sample: aSamples){
     internal_Accumulate(locker, aID, aKey, sample);
   }
 }
 
-bool
+nsresult
 TelemetryHistogram::Accumulate(const char* name, uint32_t sample)
 {
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   if (!internal_CanRecordBase()) {
-    return false;
+    return NS_ERROR_NOT_AVAILABLE;
   }
   HistogramID id;
   nsresult rv = internal_GetHistogramIdByName(locker, nsDependentCString(name), &id);
   if (NS_FAILED(rv)) {
-    return false;
+    return rv;
   }
   internal_Accumulate(locker, id, sample);
-  return true;
+  return NS_OK;
 }
 
-bool
+nsresult
 TelemetryHistogram::Accumulate(const char* name,
                                const nsCString& key, uint32_t sample)
 {
   bool keyNotAllowed = false;
 
   {
     StaticMutexAutoLock locker(gTelemetryHistogramMutex);
     if (!internal_CanRecordBase()) {
-      return false;
+      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 true;
+        return NS_OK;
       }
       // We're holding |gTelemetryHistogramMutex|, so we can't print a message
       // here.
       keyNotAllowed = true;
     }
    }
 
   if (keyNotAllowed) {
     LogToBrowserConsole(nsIScriptError::errorFlag,
                         NS_LITERAL_STRING("Key not allowed for this keyed histogram"));
     TelemetryScalar::Add(
       mozilla::Telemetry::ScalarID::TELEMETRY_ACCUMULATE_UNKNOWN_HISTOGRAM_KEYS,
       NS_ConvertASCIItoUTF16(name), 1);
   }
-  return false;
+  return NS_ERROR_FAILURE;
 }
 
 void
 TelemetryHistogram::AccumulateCategorical(HistogramID aId,
                                           const nsCString& label)
 {
   if (NS_WARN_IF(!internal_IsHistogramEnumId(aId))) {
     MOZ_ASSERT_UNREACHABLE("Histogram usage requires valid ids.");
--- a/toolkit/components/telemetry/core/TelemetryHistogram.h
+++ b/toolkit/components/telemetry/core/TelemetryHistogram.h
@@ -40,18 +40,33 @@ void SetHistogramRecordingEnabled(mozill
 nsresult SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled);
 
 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 Accumulate(mozilla::Telemetry::HistogramID aID, const nsCString& aKey,
                                           const nsTArray<uint32_t>& aSamples);
-bool Accumulate(const char* name, uint32_t sample);
-bool Accumulate(const char* name, const nsCString& key, uint32_t sample);
+/*
+ * 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);
 void AccumulateCategorical(mozilla::Telemetry::HistogramID aId, const nsTArray<nsCString>& aLabels);
 
 void AccumulateChild(mozilla::Telemetry::ProcessID aProcessType,
                      const nsTArray<mozilla::Telemetry::HistogramAccumulation>& aAccumulations);
 void AccumulateChildKeyed(mozilla::Telemetry::ProcessID aProcessType,
                           const nsTArray<mozilla::Telemetry::KeyedHistogramAccumulation>& aAccumulations);