From 5a17556f02183d70e230a7b1c0f288aaee79aee7 Mon Sep 17 00:00:00 2001 From: "arthur.iakab" Date: Tue, 8 May 2018 17:38:29 +0300 Subject: [PATCH] Backed out 6 changesets (bug 1320052) on request by Dexter for requently failing test verify dom/base/test/browser_use_counters.js Backed out changeset 07db52945b1f (bug 1320052) Backed out changeset 7a64a23bf183 (bug 1320052) Backed out changeset c95a4b0c6642 (bug 1320052) Backed out changeset 4749633bd02f (bug 1320052) Backed out changeset c7b8f6d55a0b (bug 1320052) Backed out changeset b0396db0229c (bug 1320052) --- dom/base/test/browser_use_counters.js | 4 +- .../test/xpcshell/head_telemetry.js | 3 + .../osfile/tests/xpcshell/test_telemetry.js | 4 + toolkit/components/telemetry/Scalars.yaml | 2 + toolkit/components/telemetry/Telemetry.cpp | 16 +- .../telemetry/TelemetryHistogram.cpp | 271 ++++++++++++++---- .../components/telemetry/TelemetryHistogram.h | 4 +- .../components/telemetry/TelemetrySession.jsm | 12 +- toolkit/components/telemetry/nsITelemetry.idl | 10 +- .../tests/gtest/TelemetryTestHelpers.cpp | 4 +- .../telemetry/tests/gtest/TestHistograms.cpp | 4 +- .../tests/unit/test_TelemetryHistograms.js | 221 +++++++++++--- .../tests/unit/test_TelemetrySession.js | 242 ++++++++++++++++ .../tests/xpcshell/test_terminator_reload.js | 2 + 14 files changed, 691 insertions(+), 108 deletions(-) diff --git a/dom/base/test/browser_use_counters.js b/dom/base/test/browser_use_counters.js index a3066140b5e1..a1f73b441eaf 100644 --- a/dom/base/test/browser_use_counters.js +++ b/dom/base/test/browser_use_counters.js @@ -111,9 +111,9 @@ function grabHistogramsFromContent(use_counter_middlefix, page_before = null) { let gather = () => { let snapshots; if (Services.appinfo.browserTabsRemoteAutostart) { - snapshots = telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false).content; + snapshots = telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false, false).content; } else { - snapshots = telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false).parent; + snapshots = telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false, false).parent; } let checkGet = (probe) => { return snapshots[probe] ? snapshots[probe].sum : 0; diff --git a/toolkit/components/extensions/test/xpcshell/head_telemetry.js b/toolkit/components/extensions/test/xpcshell/head_telemetry.js index 4a09db58f3aa..a5a26dd55f1b 100644 --- a/toolkit/components/extensions/test/xpcshell/head_telemetry.js +++ b/toolkit/components/extensions/test/xpcshell/head_telemetry.js @@ -15,11 +15,13 @@ function arraySum(arr) { function clearHistograms() { Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, true /* clear */); } function getSnapshots(process) { return Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, false /* clear */)[process]; } @@ -29,6 +31,7 @@ function getSnapshots(process) { function promiseTelemetryRecorded(id, process, expectedCount) { let condition = () => { let snapshot = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, false /* clear */)[process][id]; return snapshot && arraySum(snapshot.counts) >= expectedCount; }; diff --git a/toolkit/components/osfile/tests/xpcshell/test_telemetry.js b/toolkit/components/osfile/tests/xpcshell/test_telemetry.js index ab41171977e6..0acad36636ca 100644 --- a/toolkit/components/osfile/tests/xpcshell/test_telemetry.js +++ b/toolkit/components/osfile/tests/xpcshell/test_telemetry.js @@ -28,12 +28,14 @@ add_task(async function test_startup() { let READY = "OSFILE_WORKER_READY_MS"; let before = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false, false).parent; // Launch the OS.File worker await File.getCurrentDirectory(); let after = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false, false).parent; @@ -50,6 +52,7 @@ add_task(async function test_writeAtomic() { let LABEL = "OSFILE_WRITEATOMIC_JANK_MS"; let before = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false, false).parent; // Perform a write. @@ -57,6 +60,7 @@ add_task(async function test_writeAtomic() { await File.writeAtomic(path, LABEL, { tmpPath: path + ".tmp" } ); let after = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false, false).parent; Assert.equal(getCount(after[LABEL]), getCount(before[LABEL]) + 1); diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index 3db97896fe53..46f3d2dcdb3b 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -1181,6 +1181,8 @@ telemetry: description: > The count of accumulations that had to be clamped down to not overflow, keyed to the histogram name of the overflowing accumulation. + This is doubled for non-keyed desktop Firefox Histograms because of + saved-session pings. expires: never kind: uint keyed: true diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 795ddfafa826..0ab48e982868 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -577,20 +577,32 @@ TelemetryImpl::SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled) } NS_IMETHODIMP -TelemetryImpl::SnapshotHistograms(unsigned int aDataset, +TelemetryImpl::SnapshotHistograms(unsigned int aDataset, bool aSubsession, bool aClearHistograms, JSContext* aCx, JS::MutableHandleValue aResult) { +#if defined(MOZ_WIDGET_ANDROID) + if (aSubsession) { + return NS_OK; + } +#endif return TelemetryHistogram::CreateHistogramSnapshots(aCx, aResult, aDataset, + aSubsession, aClearHistograms); } NS_IMETHODIMP -TelemetryImpl::SnapshotKeyedHistograms(unsigned int aDataset, +TelemetryImpl::SnapshotKeyedHistograms(unsigned int aDataset, bool aSubsession, bool aClearHistograms, JSContext* aCx, JS::MutableHandleValue aResult) { +#if defined(MOZ_WIDGET_ANDROID) + if (aSubsession) { + return NS_OK; + } +#endif return TelemetryHistogram::GetKeyedHistogramSnapshots(aCx, aResult, aDataset, + aSubsession, aClearHistograms); } diff --git a/toolkit/components/telemetry/TelemetryHistogram.cpp b/toolkit/components/telemetry/TelemetryHistogram.cpp index 1b78f754a03e..5ee835b08a57 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.cpp +++ b/toolkit/components/telemetry/TelemetryHistogram.cpp @@ -144,19 +144,25 @@ enum reflectStatus { REFLECT_FAILURE }; +enum class SessionType { + Session = 0, + Subsession = 1, + Count, +}; + class KeyedHistogram { public: KeyedHistogram(HistogramID id, const HistogramInfo& info); ~KeyedHistogram(); - nsresult GetHistogram(const nsCString& name, Histogram** histogram); - Histogram* GetHistogram(const nsCString& name); + 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 obj, - bool clearSubsession); + bool subsession, bool clearSubsession); nsresult Add(const nsCString& key, uint32_t aSample, ProcessID aProcessType); - void Clear(); + void Clear(bool subsession); HistogramID GetHistogramID() const { return mId; } @@ -164,6 +170,9 @@ private: typedef nsBaseHashtableET KeyedHistogramEntry; typedef AutoHashtable KeyedHistogramMapType; KeyedHistogramMapType mHistogramMap; +#if !defined(MOZ_WIDGET_ANDROID) + KeyedHistogramMapType mSubsessionMap; +#endif static bool ReflectKeyedHistogram(KeyedHistogramEntry* entry, JSContext* cx, @@ -195,6 +204,7 @@ bool gCanRecordExtended = false; // We use separate ones for plain and keyed histograms. Histogram** gHistogramStorage; // Keyed histograms internally map string keys to individual Histogram instances. +// KeyedHistogram keeps track of session & subsession histograms internally. KeyedHistogram** gKeyedHistogramStorage; // Cache of histogram name to a histogram id. @@ -246,31 +256,37 @@ size_t internal_KeyedHistogramStorageIndex(HistogramID aHistogramId, } size_t internal_HistogramStorageIndex(HistogramID aHistogramId, - ProcessID aProcessId) + ProcessID aProcessId, + SessionType aSessionType) { static_assert( HistogramCount < - std::numeric_limits::max() / size_t(ProcessID::Count), - "Too many histograms and processes to store in a 1D array."); + std::numeric_limits::max() / size_t(ProcessID::Count) / size_t(SessionType::Count), + "Too many histograms, processes, and session types to store in a 1D " + "array."); - return aHistogramId * size_t(ProcessID::Count) + size_t(aProcessId); + return aHistogramId * size_t(ProcessID::Count) * size_t(SessionType::Count) + + size_t(aProcessId) * size_t(SessionType::Count) + + size_t(aSessionType); } Histogram* internal_GetHistogramFromStorage(HistogramID aHistogramId, - ProcessID aProcessId) + ProcessID aProcessId, + SessionType aSessionType) { - size_t index = internal_HistogramStorageIndex(aHistogramId, aProcessId); + size_t index = internal_HistogramStorageIndex(aHistogramId, aProcessId, aSessionType); return gHistogramStorage[index]; } void internal_SetHistogramInStorage(HistogramID aHistogramId, ProcessID aProcessId, + SessionType aSessionType, Histogram* aHistogram) { MOZ_ASSERT(XRE_IsParentProcess(), "Histograms are stored only in the parent process."); - size_t index = internal_HistogramStorageIndex(aHistogramId, aProcessId); + size_t index = internal_HistogramStorageIndex(aHistogramId, aProcessId, aSessionType); MOZ_ASSERT(!gHistogramStorage[index], "Mustn't overwrite storage without clearing it first."); gHistogramStorage[index] = aHistogram; @@ -309,13 +325,17 @@ internal_IsHistogramEnumId(HistogramID aID) // Look up a plain histogram by id. Histogram* -internal_GetHistogramById(HistogramID histogramId, ProcessID processId, bool instantiate = true) +internal_GetHistogramById(HistogramID histogramId, ProcessID processId, SessionType sessionType, + bool instantiate = true) { MOZ_ASSERT(internal_IsHistogramEnumId(histogramId)); MOZ_ASSERT(!gHistogramInfos[histogramId].keyed); MOZ_ASSERT(processId < ProcessID::Count); + MOZ_ASSERT(sessionType < SessionType::Count); - Histogram* h = internal_GetHistogramFromStorage(histogramId, processId); + Histogram* h = internal_GetHistogramFromStorage(histogramId, + processId, + sessionType); if (h || !instantiate) { return h; } @@ -324,7 +344,7 @@ internal_GetHistogramById(HistogramID histogramId, ProcessID processId, bool ins const int bucketsOffset = gHistogramBucketLowerBoundIndex[histogramId]; h = internal_CreateHistogramInstance(info, bucketsOffset); MOZ_ASSERT(h); - internal_SetHistogramInStorage(histogramId, processId, h); + internal_SetHistogramInStorage(histogramId, processId, sessionType, h); return h; } @@ -364,9 +384,9 @@ internal_GetHistogramIdByName(const nsACString& name, HistogramID* id) // Clear a histogram from storage. void -internal_ClearHistogramById(HistogramID histogramId, ProcessID processId) +internal_ClearHistogramById(HistogramID histogramId, ProcessID processId, SessionType sessionType) { - size_t index = internal_HistogramStorageIndex(histogramId, processId); + size_t index = internal_HistogramStorageIndex(histogramId, processId, sessionType); if (gHistogramStorage[index] == gExpiredHistogram) { // We keep gExpiredHistogram until TelemetryHistogram::DeInitializeGlobalState return; @@ -608,6 +628,8 @@ internal_HistogramAdd(Histogram& histogram, value = INT_MAX; } + // 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; @@ -712,6 +734,9 @@ namespace { KeyedHistogram::KeyedHistogram(HistogramID id, const HistogramInfo& info) : mHistogramMap() +#if !defined(MOZ_WIDGET_ANDROID) + , mSubsessionMap() +#endif , mId(id) , mHistogramInfo(info) { @@ -727,12 +752,29 @@ KeyedHistogram::~KeyedHistogram() delete h; } mHistogramMap.Clear(); + +#if !defined(MOZ_WIDGET_ANDROID) + for (auto iter = mSubsessionMap.Iter(); !iter.Done(); iter.Next()) { + Histogram* h = iter.Get()->mData; + if (h == gExpiredHistogram) { + continue; + } + delete h; + } + mSubsessionMap.Clear(); +#endif } nsresult -KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram) +KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram, + bool subsession) { - KeyedHistogramEntry* entry = mHistogramMap.GetEntry(key); +#if !defined(MOZ_WIDGET_ANDROID) + KeyedHistogramMapType& map = subsession ? mSubsessionMap : mHistogramMap; +#else + KeyedHistogramMapType& map = mHistogramMap; +#endif + KeyedHistogramEntry* entry = map.GetEntry(key); if (entry) { *histogram = entry->mData; return NS_OK; @@ -747,7 +789,7 @@ KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram) h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); *histogram = h; - entry = mHistogramMap.PutEntry(key); + entry = map.PutEntry(key); if (MOZ_UNLIKELY(!entry)) { return NS_ERROR_OUT_OF_MEMORY; } @@ -757,10 +799,10 @@ KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram) } Histogram* -KeyedHistogram::GetHistogram(const nsCString& key) +KeyedHistogram::GetHistogram(const nsCString& key, bool subsession) { Histogram* h = nullptr; - if (NS_FAILED(GetHistogram(key, &h))) { + if (NS_FAILED(GetHistogram(key, &h, subsession))) { return nullptr; } return h; @@ -785,11 +827,18 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample, return NS_OK; } - Histogram* histogram = GetHistogram(key); + Histogram* histogram = GetHistogram(key, false); MOZ_ASSERT(histogram); if (!histogram) { return NS_ERROR_FAILURE; } +#if !defined(MOZ_WIDGET_ANDROID) + Histogram* subsession = GetHistogram(key, true); + MOZ_ASSERT(subsession); + if (!subsession) { + return NS_ERROR_FAILURE; + } +#endif // The internal representation of a base::Histogram's buckets uses `int`. // Clamp large values of `sample` to be INT_MAX so they continue to be treated @@ -802,16 +851,32 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample, } histogram->Add(sample); +#if !defined(MOZ_WIDGET_ANDROID) + subsession->Add(sample); +#endif return NS_OK; } void -KeyedHistogram::Clear() +KeyedHistogram::Clear(bool onlySubsession) { MOZ_ASSERT(XRE_IsParentProcess()); if (!XRE_IsParentProcess()) { return; } +#if !defined(MOZ_WIDGET_ANDROID) + for (auto iter = mSubsessionMap.Iter(); !iter.Done(); iter.Next()) { + Histogram* h = iter.Get()->mData; + if (h == gExpiredHistogram) { + continue; + } + delete h; + } + mSubsessionMap.Clear(); + if (onlySubsession) { + return; + } +#endif for (auto iter = mHistogramMap.Iter(); !iter.Done(); iter.Next()) { Histogram* h = iter.Get()->mData; @@ -872,15 +937,23 @@ KeyedHistogram::ReflectKeyedHistogram(KeyedHistogramEntry* entry, } nsresult -KeyedHistogram::GetJSSnapshot(JSContext* cx, JS::Handle obj, bool clearSubsession) +KeyedHistogram::GetJSSnapshot(JSContext* cx, JS::Handle obj, + bool subsession, bool clearSubsession) { - if (!mHistogramMap.ReflectIntoJS(&KeyedHistogram::ReflectKeyedHistogram, cx, obj)) { +#if !defined(MOZ_WIDGET_ANDROID) + KeyedHistogramMapType& map = subsession ? mSubsessionMap : mHistogramMap; +#else + KeyedHistogramMapType& map = mHistogramMap; +#endif + if (!map.ReflectIntoJS(&KeyedHistogram::ReflectKeyedHistogram, cx, obj)) { return NS_ERROR_FAILURE; } - if (clearSubsession) { - Clear(); +#if !defined(MOZ_WIDGET_ANDROID) + if (subsession && clearSubsession) { + Clear(true); } +#endif return NS_OK; } @@ -940,9 +1013,15 @@ void internal_Accumulate(HistogramID aId, uint32_t aSample) return; } - Histogram *h = internal_GetHistogramById(aId, ProcessID::Parent); + Histogram *h = internal_GetHistogramById(aId, ProcessID::Parent, SessionType::Session); MOZ_ASSERT(h); 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, ProcessID::Parent); +#endif } void @@ -966,11 +1045,19 @@ internal_AccumulateChild(ProcessID aProcessType, HistogramID aId, uint32_t aSamp return; } - if (Histogram* h = internal_GetHistogramById(aId, aProcessType)) { + if (Histogram* h = internal_GetHistogramById(aId, aProcessType, SessionType::Session)) { 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, aProcessType); + } else { + NS_WARNING("Failed GetHistogramById for CHILD"); + } +#endif } void @@ -987,7 +1074,7 @@ internal_AccumulateChildKeyed(ProcessID aProcessType, HistogramID aId, } void -internal_ClearHistogram(HistogramID id) +internal_ClearHistogram(HistogramID id, bool onlySubsession) { MOZ_ASSERT(XRE_IsParentProcess()); if (!XRE_IsParentProcess()) { @@ -999,14 +1086,28 @@ internal_ClearHistogram(HistogramID id) for (uint32_t process = 0; process < static_cast(ProcessID::Count); ++process) { KeyedHistogram* kh = internal_GetKeyedHistogramById(id, static_cast(process), /* instantiate = */ false); if (kh) { - kh->Clear(); + kh->Clear(onlySubsession); } } } + // Handle plain histograms. + // Define the session types we want to clear. + nsTArray sessionTypes; + if (!onlySubsession) { + sessionTypes.AppendElement(SessionType::Session); + } +#if !defined(MOZ_WIDGET_ANDROID) + sessionTypes.AppendElement(SessionType::Subsession); +#endif + // Now reset the histograms instances for all processes. - for (uint32_t process = 0; process < static_cast(ProcessID::Count); ++process) { - internal_ClearHistogramById(id, static_cast(process)); + for (SessionType sessionType : sessionTypes) { + for (uint32_t process = 0; process < static_cast(ProcessID::Count); ++process) { + internal_ClearHistogramById(id, + static_cast(process), + sessionType); + } } } @@ -1235,9 +1336,9 @@ internal_JSHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) MOZ_ASSERT(internal_IsHistogramEnumId(id)); // This is not good standard behavior given that we have histogram instances - // covering multiple processes. + // covering multiple processes and two session types. // However, changing this requires some broader changes to callers. - h = internal_GetHistogramById(id, ProcessID::Parent); + h = internal_GetHistogramById(id, ProcessID::Parent, SessionType::Session); // Take a snapshot of Histogram::SampleSet here, protected by the lock, // and then, outside of the lock protection, mirror it to a JS structure MOZ_ASSERT(h); @@ -1279,16 +1380,28 @@ internal_JSHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) JSHistogramData* data = static_cast(JS_GetPrivate(obj)); MOZ_ASSERT(data); + bool onlySubsession = false; // This function should always return |undefined| and never fail but // rather report failures using the console. args.rval().setUndefined(); +#if !defined(MOZ_WIDGET_ANDROID) + if (args.length() >= 1) { + if (!args[0].isBoolean()) { + JS_ReportErrorASCII(cx, "Not a boolean"); + return false; + } + + onlySubsession = JS::ToBoolean(args[0]); + } +#endif + HistogramID id = data->histogramId; { StaticMutexAutoLock locker(gTelemetryHistogramMutex); MOZ_ASSERT(internal_IsHistogramEnumId(id)); - internal_ClearHistogram(id); + internal_ClearHistogram(id, onlySubsession); } return true; @@ -1349,6 +1462,8 @@ internal_JSHistogram_finalize(JSFreeOp*, JSObject* obj) // internal_JSKeyedHistogram_Add // internal_JSKeyedHistogram_Keys // internal_JSKeyedHistogram_Snapshot +// internal_JSKeyedHistogram_SubsessionSnapshot +// internal_JSKeyedHistogram_SnapshotSubsessionAndClear // internal_JSKeyedHistogram_Clear // internal_WrapAndReturnKeyedHistogram // @@ -1378,7 +1493,7 @@ static const JSClass sJSKeyedHistogramClass = { bool internal_KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, JS::Value *vp, - bool clearSubsession) + bool subsession, bool clearSubsession) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); @@ -1399,7 +1514,7 @@ internal_KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, args.rval().setUndefined(); // This is not good standard behavior given that we have histogram instances - // covering multiple processes. + // covering multiple processes and two session types. // However, changing this requires some broader changes to callers. KeyedHistogram* keyed = internal_GetKeyedHistogramById(id, ProcessID::Parent, /* instantiate = */ true); if (!keyed) { @@ -1414,7 +1529,7 @@ internal_KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, return false; } - if (!NS_SUCCEEDED(keyed->GetJSSnapshot(cx, snapshot, clearSubsession))) { + if (!NS_SUCCEEDED(keyed->GetJSSnapshot(cx, snapshot, subsession, clearSubsession))) { JS_ReportErrorASCII(cx, "Failed to reflect keyed histograms"); return false; } @@ -1430,7 +1545,7 @@ internal_KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, } Histogram* h = nullptr; - nsresult rv = keyed->GetHistogram(NS_ConvertUTF16toUTF8(key), &h); + nsresult rv = keyed->GetHistogram(NS_ConvertUTF16toUTF8(key), &h, subsession); if (NS_FAILED(rv)) { JS_ReportErrorASCII(cx, "Failed to get histogram"); return false; @@ -1538,7 +1653,7 @@ internal_JSKeyedHistogram_Keys(JSContext *cx, unsigned argc, JS::Value *vp) MOZ_ASSERT(internal_IsHistogramEnumId(id)); // This is not good standard behavior given that we have histogram instances - // covering multiple processes. + // covering multiple processes and two session types. // However, changing this requires some broader changes to callers. keyed = internal_GetKeyedHistogramById(id, ProcessID::Parent); } @@ -1554,9 +1669,33 @@ internal_JSKeyedHistogram_Keys(JSContext *cx, unsigned argc, JS::Value *vp) bool internal_JSKeyedHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) { - return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, false); + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, false, false); } +#if !defined(MOZ_WIDGET_ANDROID) +bool +internal_JSKeyedHistogram_SubsessionSnapshot(JSContext *cx, + unsigned argc, JS::Value *vp) +{ + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, true, false); +} +#endif + +#if !defined(MOZ_WIDGET_ANDROID) +bool +internal_JSKeyedHistogram_SnapshotSubsessionAndClear(JSContext *cx, + unsigned argc, + JS::Value *vp) +{ + JS::CallArgs args = JS::CallArgsFromVp(argc, vp); + if (args.length() != 0) { + JS_ReportErrorASCII(cx, "No key arguments supported for snapshotSubsessionAndClear"); + } + + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, true, true); +} +#endif + bool internal_JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) { @@ -1577,13 +1716,24 @@ internal_JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) // rather report failures using the console. args.rval().setUndefined(); + bool onlySubsession = false; + #if !defined(MOZ_WIDGET_ANDROID) + if (args.length() >= 1) { + if (!(args[0].isNumber() || args[0].isBoolean())) { + JS_ReportErrorASCII(cx, "Not a boolean"); + return false; + } + onlySubsession = JS::ToBoolean(args[0]); + } + #endif + KeyedHistogram* keyed = nullptr; { MOZ_ASSERT(internal_IsHistogramEnumId(id)); StaticMutexAutoLock locker(gTelemetryHistogramMutex); // This is not good standard behavior given that we have histogram instances - // covering multiple processes. + // covering multiple processes and two session types. // However, changing this requires some broader changes to callers. keyed = internal_GetKeyedHistogramById(id, ProcessID::Parent, /* instantiate = */ false); @@ -1591,7 +1741,7 @@ internal_JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) return true; } - keyed->Clear(); + keyed->Clear(onlySubsession); } return true; @@ -1611,6 +1761,12 @@ internal_WrapAndReturnKeyedHistogram(HistogramID id, JSContext *cx, if (!(JS_DefineFunction(cx, obj, "add", internal_JSKeyedHistogram_Add, 2, 0) && JS_DefineFunction(cx, obj, "snapshot", internal_JSKeyedHistogram_Snapshot, 1, 0) +#if !defined(MOZ_WIDGET_ANDROID) + && JS_DefineFunction(cx, obj, "subsessionSnapshot", + internal_JSKeyedHistogram_SubsessionSnapshot, 1, 0) + && JS_DefineFunction(cx, obj, "snapshotSubsessionAndClear", + internal_JSKeyedHistogram_SnapshotSubsessionAndClear, 0, 0) +#endif && JS_DefineFunction(cx, obj, "keys", internal_JSKeyedHistogram_Keys, 0, 0) && JS_DefineFunction(cx, obj, "clear", @@ -1667,7 +1823,7 @@ void TelemetryHistogram::InitializeGlobalState(bool canRecordBase, if (XRE_IsParentProcess()) { gHistogramStorage = - new Histogram*[HistogramCount * size_t(ProcessID::Count)] {}; + new Histogram*[HistogramCount * size_t(ProcessID::Count) * size_t(SessionType::Count)] {}; gKeyedHistogramStorage = new KeyedHistogram*[HistogramCount * size_t(ProcessID::Count)] {}; } @@ -1729,7 +1885,7 @@ void TelemetryHistogram::DeInitializeGlobalState() // FactoryGet `new`s Histograms for us, but requires us to manually delete. if (XRE_IsParentProcess()) { - for (size_t i = 0; i < HistogramCount * size_t(ProcessID::Count); ++i) { + for (size_t i = 0; i < HistogramCount * size_t(ProcessID::Count) * size_t(SessionType::Count); ++i) { if (i < HistogramCount * size_t(ProcessID::Count)) { delete gKeyedHistogramStorage[i]; } @@ -2122,8 +2278,15 @@ nsresult TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx, JS::MutableHandleValue aResult, unsigned int aDataset, + bool aSubsession, bool aClearSubsession) { +#if defined(MOZ_WIDGET_ANDROID) + if (aSubsession) { + return NS_OK; + } +#endif + // Runs without protection from |gTelemetryHistogramMutex| JS::Rooted root_obj(aCx, JS_NewPlainObject(aCx)); if (!root_obj) { @@ -2138,6 +2301,8 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx, includeGPUProcess = gpm->AttemptedGPUProcess(); } + SessionType sessionType = SessionType(aSubsession); + // Struct used to keep information about the histograms for which a // snapshot should be created struct MOZ_NON_MEMMOVABLE HistogramProcessInfo { @@ -2177,6 +2342,7 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx, bool shouldInstantiate = info.histogramType == nsITelemetry::HISTOGRAM_FLAG; Histogram* h = internal_GetHistogramById(id, ProcessID(process), + sessionType, shouldInstantiate); if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) { continue; @@ -2188,9 +2354,11 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx, return NS_ERROR_OUT_OF_MEMORY; } - if (aClearSubsession) { +#if !defined(MOZ_WIDGET_ANDROID) + if ((sessionType == SessionType::Subsession) && aClearSubsession) { h->Clear(); } +#endif } } } @@ -2240,8 +2408,14 @@ nsresult TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext* aCx, JS::MutableHandleValue aResult, unsigned int aDataset, + bool aSubsession, bool aClearSubsession) { +#if defined(MOZ_WIDGET_ANDROID) + if (aSubsession) { + return NS_OK; + } +#endif // Runs without protection from |gTelemetryHistogramMutex| JS::Rooted obj(aCx, JS_NewPlainObject(aCx)); if (!obj) { @@ -2292,7 +2466,8 @@ TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext* aCx, return NS_ERROR_FAILURE; } - if (!NS_SUCCEEDED(keyed->GetJSSnapshot(aCx, snapshot, aClearSubsession))) { + if (!NS_SUCCEEDED(keyed->GetJSSnapshot(aCx, snapshot, aSubsession, + aClearSubsession))) { return NS_ERROR_FAILURE; } diff --git a/toolkit/components/telemetry/TelemetryHistogram.h b/toolkit/components/telemetry/TelemetryHistogram.h index b02892fd39de..240d8ca14a54 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.h +++ b/toolkit/components/telemetry/TelemetryHistogram.h @@ -65,11 +65,11 @@ GetHistogramName(mozilla::Telemetry::HistogramID id); nsresult CreateHistogramSnapshots(JSContext* aCx, JS::MutableHandleValue aResult, unsigned int aDataset, - bool aClearSubsession); + bool aSubsession, bool aClearSubsession); nsresult GetKeyedHistogramSnapshots(JSContext *aCx, JS::MutableHandleValue aResult, unsigned int aDataset, - bool aClearSubsession); + bool aSubsession, bool aClearSubsession); size_t GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf); diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 345b15b6d5f4..4c1b7ae94727 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -871,8 +871,8 @@ var Impl = { : Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT; }, - getHistograms: function getHistograms(clearSubsession) { - let hls = Telemetry.snapshotHistograms(this.getDatasetType(), clearSubsession); + getHistograms: function getHistograms(subsession, clearSubsession) { + let hls = Telemetry.snapshotHistograms(this.getDatasetType(), subsession, clearSubsession); let ret = {}; for (let [process, histograms] of Object.entries(hls)) { @@ -887,8 +887,8 @@ var Impl = { return ret; }, - getKeyedHistograms(clearSubsession) { - let khs = Telemetry.snapshotKeyedHistograms(this.getDatasetType(), clearSubsession); + getKeyedHistograms(subsession, clearSubsession) { + let khs = Telemetry.snapshotKeyedHistograms(this.getDatasetType(), subsession, clearSubsession); let ret = {}; for (let [process, histograms] of Object.entries(khs)) { @@ -1221,8 +1221,8 @@ var Impl = { // Additional payload for chrome process. let measurements = { - histograms: protect(() => this.getHistograms(clearSubsession), {}), - keyedHistograms: protect(() => this.getKeyedHistograms(clearSubsession), {}), + histograms: protect(() => this.getHistograms(isSubsession, clearSubsession), {}), + keyedHistograms: protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {}), scalars: protect(() => this.getScalars(isSubsession, clearSubsession), {}), keyedScalars: protect(() => this.getScalars(isSubsession, clearSubsession, true), {}), events: protect(() => this.getEvents(isSubsession, clearSubsession)), diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index 12e45965cafd..794b4d11af81 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -66,10 +66,11 @@ interface nsITelemetry : nsISupports * sum - sum of the bucket contents * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. - * @param aClear Whether to clear out the histograms after snapshotting + * @param aSubsession Whether to return the internally-duplicated subsession histograms + * @param aClear Whether to clear out the subsession histograms after snapshotting (only works if aSubsession is true) */ [implicit_jscontext] - jsval snapshotHistograms(in uint32_t aDataset, in boolean aClear); + jsval snapshotHistograms(in uint32_t aDataset, in boolean aSubsession, in boolean aClear); /** * The amount of time, in milliseconds, that the last session took @@ -232,10 +233,11 @@ interface nsITelemetry : nsISupports * { process1: {name1: {histogramData1}, name2:{histogramData2}...}} * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. - * @param aClear Whether to clear out the keyed histograms after snapshotting + * @param aSubsession Whether to return the internally-duplicated subsession keyed histograms + * @param aClear Whether to clear out the subsession keyed histograms after snapshotting (only works if aSubsession is true) */ [implicit_jscontext] - jsval snapshotKeyedHistograms(in uint32_t aDataset, in boolean aClear); + jsval snapshotKeyedHistograms(in uint32_t aDataset, in boolean aSubsession, in boolean aClear); /** * Create and return a histogram registered in TelemetryHistograms.h. diff --git a/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp b/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp index da4fe71c9775..fde9d325825e 100644 --- a/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp +++ b/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp @@ -176,8 +176,8 @@ GetSnapshots(JSContext* cx, nsCOMPtr mTelemetry, const char* name, JS::MutableHandleValue valueOut, bool is_keyed) { JS::RootedValue snapshots(cx); - nsresult rv = is_keyed ? mTelemetry->SnapshotKeyedHistograms(nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN, false, cx, &snapshots) - : mTelemetry->SnapshotHistograms(nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN, false, cx, &snapshots); + nsresult rv = is_keyed ? mTelemetry->SnapshotKeyedHistograms(nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN, false, false, cx, &snapshots) + : mTelemetry->SnapshotHistograms(nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN, false, false, cx, &snapshots); JS::RootedValue snapshot(cx); GetProperty(cx, "parent", snapshots, &snapshot); diff --git a/toolkit/components/telemetry/tests/gtest/TestHistograms.cpp b/toolkit/components/telemetry/tests/gtest/TestHistograms.cpp index 32f069da73e1..8c6581deb037 100644 --- a/toolkit/components/telemetry/tests/gtest/TestHistograms.cpp +++ b/toolkit/components/telemetry/tests/gtest/TestHistograms.cpp @@ -356,8 +356,8 @@ TEST_F(TelemetryTestFixture, AccumulateLinearHistogram_DifferentSamples) ASSERT_EQ(uCountLast, kExpectedCountLast) << "The last bucket did not accumulate the correct number of values"; // We accumulated two values that had to be clamped. We expect the count in - // 'telemetry.accumulate_clamped_values' to be 2 (only one storage). - const uint32_t expectedAccumulateClampedCount = 2; + // 'telemetry.accumulate_clamped_values' to be 4 + const uint32_t expectedAccumulateClampedCount = 4; JS::RootedValue scalarsSnapshot(cx.GetJSContext()); GetScalarsSnapshot(true, cx.GetJSContext(),&scalarsSnapshot); CheckKeyedUintScalar("telemetry.accumulate_clamped_values", diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js b/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js index 418eb4d53e6d..eb9a5f2c2352 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js @@ -73,6 +73,7 @@ function check_histogram(histogram_type, name, min, max, bucket_count) { Assert.equal(i, 1); } var hgrams = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false, false).parent; let gh = hgrams[name]; Assert.equal(gh.histogram_type, histogram_type); @@ -116,6 +117,7 @@ function test_instantiate() { h.add(1); let snapshot = h.snapshot(); let subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, false /* clear */).parent; Assert.ok(ID in subsession); Assert.equal(snapshot.sum, subsession[ID].sum, @@ -175,6 +177,7 @@ add_task(async function test_noSerialization() { // get reflected into JS, as it has no interesting data in it. Telemetry.getHistogramById("NEWTAB_PAGE_PINNED_SITES_COUNT"); let histograms = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.equal(false, "NEWTAB_PAGE_PINNED_SITES_COUNT" in histograms); }); @@ -443,6 +446,7 @@ add_task(async function test_expired_histogram() { for (let process of ["main", "content", "gpu", "extension"]) { let histograms = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */); if (!(process in histograms)) { info("Nothing present for process " + process); @@ -451,6 +455,7 @@ add_task(async function test_expired_histogram() { Assert.equal(histograms[process].__expired__, undefined); } let parentHgrams = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.equal(parentHgrams[test_expired_id], undefined); }); @@ -509,6 +514,7 @@ add_task(async function test_keyed_boolean_histogram() { Assert.deepEqual(h.snapshot(), testSnapShot); let parentHgrams = Telemetry.snapshotKeyedHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.deepEqual(parentHgrams[KEYED_ID], testSnapShot); @@ -566,6 +572,7 @@ add_task(async function test_keyed_count_histogram() { Assert.deepEqual(h.snapshot(), testSnapShot); let parentHgrams = Telemetry.snapshotKeyedHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.deepEqual(parentHgrams[KEYED_ID], testSnapShot); @@ -640,6 +647,7 @@ add_task(async function test_keyed_flag_histogram() { Assert.deepEqual(h.snapshot(), testSnapshot); let parentHgrams = Telemetry.snapshotKeyedHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.deepEqual(parentHgrams[KEYED_ID], testSnapshot); @@ -791,6 +799,7 @@ add_task(async function test_histogramSnapshots() { // Check that keyed histograms are not returned let parentHgrams = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.ok(!("TELEMETRY_TEST_KEYED_COUNT" in parentHgrams)); }); @@ -803,12 +812,14 @@ add_task(async function test_datasets() { // Check that registeredHistogram works properly let registered = Telemetry.snapshotHistograms(RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */); registered = new Set(Object.keys(registered.parent)); Assert.ok(registered.has("TELEMETRY_TEST_FLAG")); Assert.ok(registered.has("TELEMETRY_TEST_RELEASE_OPTIN")); Assert.ok(registered.has("TELEMETRY_TEST_RELEASE_OPTOUT")); registered = Telemetry.snapshotHistograms(RELEASE_CHANNEL_OPTOUT, + false /* subsession */, false /* clear */); registered = new Set(Object.keys(registered.parent)); Assert.ok(!registered.has("TELEMETRY_TEST_FLAG")); @@ -817,17 +828,185 @@ add_task(async function test_datasets() { // Check that registeredKeyedHistograms works properly registered = Telemetry.snapshotKeyedHistograms(RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */); registered = new Set(Object.keys(registered.parent)); Assert.ok(registered.has("TELEMETRY_TEST_KEYED_FLAG")); Assert.ok(registered.has("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT")); registered = Telemetry.snapshotKeyedHistograms(RELEASE_CHANNEL_OPTOUT, + false /* subsession */, false /* clear */); registered = new Set(Object.keys(registered.parent)); Assert.ok(!registered.has("TELEMETRY_TEST_KEYED_FLAG")); Assert.ok(registered.has("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT")); }); +add_task({ + skip_if: () => gIsAndroid +}, +function test_subsession() { + const COUNT = "TELEMETRY_TEST_COUNT"; + const FLAG = "TELEMETRY_TEST_FLAG"; + let h = Telemetry.getHistogramById(COUNT); + let flag = Telemetry.getHistogramById(FLAG); + + // Both original and duplicate should start out the same. + h.clear(); + let snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + let subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(!(COUNT in snapshot)); + Assert.ok(!(COUNT in subsession)); + + // They should instantiate and pick-up the count. + h.add(1); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(COUNT in snapshot); + Assert.ok(COUNT in subsession); + Assert.equal(snapshot[COUNT].sum, 1); + Assert.equal(subsession[COUNT].sum, 1); + + // They should still reset properly. + h.clear(); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(!(COUNT in snapshot)); + Assert.ok(!(COUNT in subsession)); + + // Both should instantiate and pick-up the count. + h.add(1); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(COUNT in snapshot); + Assert.ok(COUNT in subsession); + Assert.equal(snapshot[COUNT].sum, 1); + Assert.equal(subsession[COUNT].sum, 1); + + // Check that we are able to only reset the duplicate histogram. + h.clear(true); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(COUNT in snapshot); + Assert.ok(!(COUNT in subsession)); + Assert.equal(snapshot[COUNT].sum, 1); + + // Both should register the next count. + h.add(1); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.equal(snapshot[COUNT].sum, 2); + Assert.equal(subsession[COUNT].sum, 1); + + // Retrieve a subsession snapshot and pass the flag to + // clear subsession histograms too. + h.clear(); + flag.clear(); + h.add(1); + flag.add(1); + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + true /* clear */).parent; + Assert.ok(COUNT in snapshot); + Assert.ok(COUNT in subsession); + Assert.ok(FLAG in snapshot); + Assert.ok(FLAG in subsession); + Assert.equal(snapshot[COUNT].sum, 1); + Assert.equal(subsession[COUNT].sum, 1); + Assert.equal(snapshot[FLAG].sum, 1); + Assert.equal(subsession[FLAG].sum, 1); + + // The next subsesssion snapshot should show the histograms + // got reset. + snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, + false /* clear */).parent; + subsession = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + true /* subsession */, + false /* clear */).parent; + Assert.ok(COUNT in snapshot); + Assert.ok(!(COUNT in subsession)); + Assert.ok(FLAG in snapshot); + Assert.ok(FLAG in subsession); + Assert.equal(snapshot[COUNT].sum, 1); + Assert.equal(snapshot[FLAG].sum, 1); + Assert.equal(subsession[FLAG].sum, 0); +}); + +add_task({ + skip_if: () => gIsAndroid +}, +function test_keyed_subsession() { + let h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_FLAG"); + const KEY = "foo"; + + // Both original and subsession should start out the same. + h.clear(); + Assert.ok(!(KEY in h.snapshot())); + Assert.ok(!(KEY in h.subsessionSnapshot())); + Assert.equal(h.snapshot(KEY).sum, 0); + Assert.equal(h.subsessionSnapshot(KEY).sum, 0); + + // Both should register the flag. + h.add(KEY, 1); + Assert.ok(KEY in h.snapshot()); + Assert.ok(KEY in h.subsessionSnapshot()); + Assert.equal(h.snapshot(KEY).sum, 1); + Assert.equal(h.subsessionSnapshot(KEY).sum, 1); + + // Check that we are able to only reset the subsession histogram. + h.clear(true); + Assert.ok(KEY in h.snapshot()); + Assert.ok(!(KEY in h.subsessionSnapshot())); + Assert.equal(h.snapshot(KEY).sum, 1); + Assert.equal(h.subsessionSnapshot(KEY).sum, 0); + + // Setting the flag again should make both match again. + h.add(KEY, 1); + Assert.ok(KEY in h.snapshot()); + Assert.ok(KEY in h.subsessionSnapshot()); + Assert.equal(h.snapshot(KEY).sum, 1); + Assert.equal(h.subsessionSnapshot(KEY).sum, 1); + + // Check that "snapshot and clear" works properly. + let snapshot = h.snapshot(); + let subsession = h.snapshotSubsessionAndClear(); + Assert.ok(KEY in snapshot); + Assert.ok(KEY in subsession); + Assert.equal(snapshot[KEY].sum, 1); + Assert.equal(subsession[KEY].sum, 1); + + subsession = h.subsessionSnapshot(); + Assert.ok(!(KEY in subsession)); + Assert.equal(h.subsessionSnapshot(KEY).sum, 0); +}); + add_task(async function test_keyed_keys() { let h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_KEYS"); h.clear(); @@ -1099,6 +1278,7 @@ async function test_productSpecificHistograms() { mobile_histo.add(42); let histograms = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.ok(DEFAULT_PRODUCTS_HISTOGRAM in histograms, "Should have recorded default products histogram"); @@ -1132,6 +1312,7 @@ async function test_mobileSpecificHistograms() { mobile_histo.add(1); let histograms = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; Assert.ok(DEFAULT_PRODUCTS_HISTOGRAM in histograms, "Should have recorded default products histogram"); @@ -1140,43 +1321,3 @@ async function test_mobileSpecificHistograms() { Assert.ok(!(DESKTOP_ONLY_HISTOGRAM in histograms), "Should not have recorded desktop-only histogram"); }); - -add_task({ - skip_if: () => gIsAndroid -}, -async function test_clearHistogramsOnSnapshot() { - const COUNT = "TELEMETRY_TEST_COUNT"; - let h = Telemetry.getHistogramById(COUNT); - h.clear(); - let snapshot; - - // The first snapshot should be empty, nothing recorded. - snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, - false /* clear */).parent; - Assert.ok(!(COUNT in snapshot)); - - // After recording into a histogram, the data should be in the snapshot. Don't delete it. - h.add(1); - - Assert.equal(h.snapshot().sum, 1); - snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, - false /* clear */).parent; - Assert.ok(COUNT in snapshot); - Assert.equal(snapshot[COUNT].sum, 1); - - // After recording into a histogram again, the data should be updated and in the snapshot. - // Clean up after. - h.add(41); - - Assert.equal(h.snapshot().sum, 42); - snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, - true /* clear */).parent; - Assert.ok(COUNT in snapshot); - Assert.equal(snapshot[COUNT].sum, 42); - - // Finally, no data should be in the snapshot. - Assert.equal(h.snapshot().sum, 0); - snapshot = Telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, - false /* clear */).parent; - Assert.ok(!(COUNT in snapshot)); -}); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 46df9ec2db97..0ea36bcf473c 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -752,6 +752,248 @@ add_task(async function test_checkSubsessionEvents() { await TelemetryController.testShutdown(); }); +add_task(async function test_checkSubsessionHistograms() { + if (gIsAndroid) { + // We don't support subsessions yet on Android. + return; + } + + let now = new Date(2020, 1, 1, 12, 0, 0); + let expectedDate = new Date(2020, 1, 1, 12, 0, 0); + fakeNow(now); + await TelemetryController.testReset(); + + const COUNT_ID = "TELEMETRY_TEST_COUNT"; + const KEYED_ID = "TELEMETRY_TEST_KEYED_COUNT"; + const count = Telemetry.getHistogramById(COUNT_ID); + const keyed = Telemetry.getKeyedHistogramById(KEYED_ID); + + const stableHistograms = new Set([ + "TELEMETRY_TEST_FLAG", + "TELEMETRY_TEST_COUNT", + "TELEMETRY_TEST_RELEASE_OPTOUT", + "TELEMETRY_TEST_RELEASE_OPTIN", + "STARTUP_CRASH_DETECTED", + ]); + + const stableKeyedHistograms = new Set([ + "TELEMETRY_TEST_KEYED_FLAG", + "TELEMETRY_TEST_KEYED_COUNT", + "TELEMETRY_TEST_KEYED_RELEASE_OPTIN", + "TELEMETRY_TEST_KEYED_RELEASE_OPTOUT", + ]); + + // List of prefixes of histograms that could anomalously be present in + // the subsession snapshot but not the session snapshot. + // If you add something to this list, please reference a bug# + const possibleAnomalyPrefixes = [ + "CYCLE_COLLECTOR_WORKER", // non-MT CC can happen between payload gathering - bug 1398431 + ]; + + // Compare the two sets of histograms. + // The "subsession" histograms should match the registered + // "classic" histograms. However, histograms can change + // between us collecting the different payloads, so we only + // check for deep equality on known stable histograms. + let checkHistograms = (classic, subsession, message) => { + for (let id of Object.keys(subsession)) { + if (possibleAnomalyPrefixes.some(prefix => id.startsWith(prefix))) { + continue; + } + Assert.ok(id in classic, message + ` (${id})`); + if (stableHistograms.has(id)) { + Assert.deepEqual(classic[id], + subsession[id], message); + } else { + Assert.equal(classic[id].histogram_type, + subsession[id].histogram_type, message); + } + } + }; + + // Same as above, except for keyed histograms. + let checkKeyedHistograms = (classic, subsession, message) => { + for (let id of Object.keys(subsession)) { + Assert.ok(id in classic, message); + if (stableKeyedHistograms.has(id)) { + Assert.deepEqual(classic[id], + subsession[id], message); + } + } + }; + + // Both classic and subsession payload histograms should start the same. + // The payloads should be identical for now except for the reason. + count.clear(); + keyed.clear(); + let classic = TelemetrySession.getPayload(); + let subsession = TelemetrySession.getPayload("environment-change"); + + Assert.equal(classic.info.reason, "gather-payload"); + Assert.equal(subsession.info.reason, "environment-change"); + Assert.ok(!(COUNT_ID in classic.histograms)); + Assert.ok(!(COUNT_ID in subsession.histograms)); + Assert.ok(!(KEYED_ID in classic.keyedHistograms)); + Assert.ok(!(KEYED_ID in subsession.keyedHistograms)); + + checkHistograms(classic.histograms, subsession.histograms, "Should start the same"); + checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms, "Keyed should start the same"); + + // Adding values should get picked up in both. + count.add(1); + keyed.add("a", 1); + keyed.add("b", 1); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + + Assert.ok(COUNT_ID in classic.histograms); + Assert.ok(COUNT_ID in subsession.histograms); + Assert.ok(KEYED_ID in classic.keyedHistograms); + Assert.ok(KEYED_ID in subsession.keyedHistograms); + Assert.equal(classic.histograms[COUNT_ID].sum, 1); + Assert.equal(classic.keyedHistograms[KEYED_ID].a.sum, 1); + Assert.equal(classic.keyedHistograms[KEYED_ID].b.sum, 1); + + checkHistograms(classic.histograms, subsession.histograms, "Added values should be picked up"); + checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms, "Added values should be picked up by keyed"); + + // Values should still reset properly. + count.clear(); + keyed.clear(); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + + Assert.ok(!(COUNT_ID in classic.histograms)); + Assert.ok(!(COUNT_ID in subsession.histograms)); + Assert.ok(!(KEYED_ID in classic.keyedHistograms)); + Assert.ok(!(KEYED_ID in subsession.keyedHistograms)); + + checkHistograms(classic.histograms, subsession.histograms, "Values should reset"); + checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms, "Keyed values should reset"); + + // Adding values should get picked up in both. + count.add(1); + keyed.add("a", 1); + keyed.add("b", 1); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + + Assert.ok(COUNT_ID in classic.histograms); + Assert.ok(COUNT_ID in subsession.histograms); + Assert.ok(KEYED_ID in classic.keyedHistograms); + Assert.ok(KEYED_ID in subsession.keyedHistograms); + Assert.equal(classic.histograms[COUNT_ID].sum, 1); + Assert.equal(classic.keyedHistograms[KEYED_ID].a.sum, 1); + Assert.equal(classic.keyedHistograms[KEYED_ID].b.sum, 1); + + checkHistograms(classic.histograms, subsession.histograms, "Adding values should be picked up again"); + checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms, "Adding values should be picked up by keyed again"); + + // We should be able to reset only the subsession histograms. + // First check that "snapshot and clear" still returns the old state... + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change", true); + + let subsessionStartDate = new Date(classic.info.subsessionStartDate); + Assert.equal(subsessionStartDate.toISOString(), expectedDate.toISOString()); + subsessionStartDate = new Date(subsession.info.subsessionStartDate); + Assert.equal(subsessionStartDate.toISOString(), expectedDate.toISOString()); + checkHistograms(classic.histograms, subsession.histograms, "Should be able to reset subsession"); + checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms, "Should be able to reset subsession keyed"); + + // ... then check that the next snapshot shows the subsession + // histograms got reset. + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + + Assert.ok(COUNT_ID in classic.histograms); + Assert.ok(!(COUNT_ID in subsession.histograms)); + Assert.equal(classic.histograms[COUNT_ID].sum, 1); + + Assert.ok(KEYED_ID in classic.keyedHistograms); + Assert.ok(!(KEYED_ID in subsession.keyedHistograms)); + Assert.equal(classic.keyedHistograms[KEYED_ID].a.sum, 1); + Assert.equal(classic.keyedHistograms[KEYED_ID].b.sum, 1); + + // Adding values should get picked up in both again. + count.add(1); + keyed.add("a", 1); + keyed.add("b", 1); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + + Assert.ok(COUNT_ID in classic.histograms); + Assert.ok(COUNT_ID in subsession.histograms); + Assert.equal(classic.histograms[COUNT_ID].sum, 2); + Assert.equal(subsession.histograms[COUNT_ID].sum, 1); + + Assert.ok(KEYED_ID in classic.keyedHistograms); + Assert.ok(KEYED_ID in subsession.keyedHistograms); + Assert.equal(classic.keyedHistograms[KEYED_ID].a.sum, 2); + Assert.equal(classic.keyedHistograms[KEYED_ID].b.sum, 2); + Assert.equal(subsession.keyedHistograms[KEYED_ID].a.sum, 1); + Assert.equal(subsession.keyedHistograms[KEYED_ID].b.sum, 1); + + await TelemetryController.testShutdown(); +}); + +add_task(async function test_checkSubsessionData() { + if (gIsAndroid) { + // We don't support subsessions yet on Android. + return; + } + + // Keep track of the active ticks count if the session recorder is available. + let getActiveTicks = () => TelemetrySession.getPayload().simpleMeasurements.activeTicks; + let activeTicksAtSubsessionStart = getActiveTicks(); + let expectedActiveTicks = activeTicksAtSubsessionStart; + + let incrementActiveTicks = () => { + TelemetrySession.observe(null, "user-interaction-active"); + ++expectedActiveTicks; + }; + + await TelemetryController.testReset(); + + // Both classic and subsession payload data should be the same on the first subsession. + incrementActiveTicks(); + let classic = TelemetrySession.getPayload(); + let subsession = TelemetrySession.getPayload("environment-change"); + Assert.equal(classic.simpleMeasurements.activeTicks, expectedActiveTicks, + "Classic pings must count active ticks (in simpleMeasurements) since the beginning of the session."); + Assert.equal(subsession.simpleMeasurements.activeTicks, expectedActiveTicks, + "Subsessions must count active ticks (in simpleMeasurements) as classic pings on the first subsession."); + Assert.equal(subsession.processes.parent.scalars["browser.engagement.active_ticks"], expectedActiveTicks, + "Subsessions must count active ticks (in scalars) as classic pings on the first subsession."); + + // Start a new subsession and check that the active ticks are correctly reported. + incrementActiveTicks(); + activeTicksAtSubsessionStart = getActiveTicks(); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change", true); + Assert.equal(classic.simpleMeasurements.activeTicks, expectedActiveTicks, + "Classic pings must count active ticks (in simpleMeasurements) since the beginning of the session."); + Assert.equal(subsession.simpleMeasurements.activeTicks, expectedActiveTicks, + "Pings must not lose the tick count when starting a new subsession."); + Assert.equal(subsession.processes.parent.scalars["browser.engagement.active_ticks"], expectedActiveTicks, + "Active ticks (in scalars) must not lose count for a previous subsession when starting a new subsession."); + + // Get a new subsession payload without clearing the subsession. + incrementActiveTicks(); + classic = TelemetrySession.getPayload(); + subsession = TelemetrySession.getPayload("environment-change"); + Assert.equal(classic.simpleMeasurements.activeTicks, expectedActiveTicks, + "Classic pings must count active ticks (in simpleMeasurements) since the beginning of the session."); + Assert.equal(subsession.simpleMeasurements.activeTicks, + expectedActiveTicks - activeTicksAtSubsessionStart, + "Subsessions must count active ticks (in simpleMeasurements) since the last new subsession."); + Assert.equal(subsession.processes.parent.scalars["browser.engagement.active_ticks"], + expectedActiveTicks - activeTicksAtSubsessionStart, + "Subsessions must count active ticks (in scalars) since the last new subsession."); + + await TelemetryController.testShutdown(); +}); + add_task(async function test_dailyCollection() { if (gIsAndroid) { // We don't do daily collections yet on Android. diff --git a/toolkit/components/terminator/tests/xpcshell/test_terminator_reload.js b/toolkit/components/terminator/tests/xpcshell/test_terminator_reload.js index 6c4e2c8f1ff3..55dde64f5af5 100644 --- a/toolkit/components/terminator/tests/xpcshell/test_terminator_reload.js +++ b/toolkit/components/terminator/tests/xpcshell/test_terminator_reload.js @@ -30,6 +30,7 @@ add_task(async function test_reload() { info("Forging data"); let data = {}; let telemetrySnapshots = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; let i = 0; for (let k of Object.keys(HISTOGRAMS)) { @@ -62,6 +63,7 @@ add_task(async function test_reload() { await wait; telemetrySnapshots = Services.telemetry.snapshotHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, + false /* subsession */, false /* clear */).parent; for (let k of Object.keys(HISTOGRAMS)) { let id = HISTOGRAMS[k];