diff --git a/browser/modules/test/browser_UsageTelemetry_content.js b/browser/modules/test/browser_UsageTelemetry_content.js index f3c0fb8fd5cd..264fedfc3ff3 100644 --- a/browser/modules/test/browser_UsageTelemetry_content.js +++ b/browser/modules/test/browser_UsageTelemetry_content.js @@ -81,8 +81,7 @@ add_task(function* test_context_menu() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "contextmenu", null, {engine: "other-MozSearch"}]]); contextMenu.hidePopup(); @@ -118,8 +117,7 @@ add_task(function* test_about_newtab() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "about_newtab", "enter", {engine: "other-MozSearch"}]]); yield BrowserTestUtils.removeTab(tab); diff --git a/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js b/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js index ca976b2a3732..6612c623b66c 100644 --- a/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js +++ b/browser/modules/test/browser_UsageTelemetry_content_aboutHome.js @@ -81,8 +81,7 @@ add_task(function* test_abouthome_simpleQuery() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "about_home", "enter", {engine: "other-MozSearch"}]]); yield BrowserTestUtils.removeTab(tab); diff --git a/browser/modules/test/browser_UsageTelemetry_searchbar.js b/browser/modules/test/browser_UsageTelemetry_searchbar.js index 6feda3604626..319807fb9255 100644 --- a/browser/modules/test/browser_UsageTelemetry_searchbar.js +++ b/browser/modules/test/browser_UsageTelemetry_searchbar.js @@ -107,8 +107,7 @@ add_task(function* test_plainQuery() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "searchbar", "enter", {engine: "other-MozSearch"}]]); yield BrowserTestUtils.removeTab(tab); @@ -142,8 +141,7 @@ add_task(function* test_oneOff() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "searchbar", "oneoff", {engine: "other-MozSearch2"}]]); yield BrowserTestUtils.removeTab(tab); @@ -189,8 +187,7 @@ add_task(function* test_suggestion() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "searchbar", "suggestion", {engine: searchEngineId}]]); Services.search.currentEngine = previousEngine; diff --git a/browser/modules/test/browser_UsageTelemetry_urlbar.js b/browser/modules/test/browser_UsageTelemetry_urlbar.js index 11a2b054d16f..5dfd0eb69f36 100644 --- a/browser/modules/test/browser_UsageTelemetry_urlbar.js +++ b/browser/modules/test/browser_UsageTelemetry_urlbar.js @@ -126,8 +126,7 @@ add_task(function* test_simpleQuery() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "urlbar", "enter", {engine: "other-MozSearch"}]]); // Check the histograms as well. @@ -172,8 +171,7 @@ add_task(function* test_searchAlias() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "urlbar", "alias", {engine: "other-MozSearch"}]]); // Check the histograms as well. @@ -221,8 +219,7 @@ add_task(function* test_oneOff() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "urlbar", "oneoff", {engine: "other-MozSearch"}]]); // Check the histograms as well. @@ -282,8 +279,7 @@ add_task(function* test_suggestion() { // Also check events. let events = Services.telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - Assert.ok("default" in events, "We should have recorded events in the parent process."); - events = events.default.filter(e => e[1] == "navigation" && e[2] == "search"); + events = events.filter(e => e[1] == "navigation" && e[2] == "search"); checkEvents(events, [["navigation", "search", "urlbar", "suggestion", {engine: searchEngineId}]]); // Check the histograms as well. diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 200e6d0ef580..3d102c7ae8ba 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5078,13 +5078,6 @@ ContentParent::RecvUpdateChildKeyedScalars( return IPC_OK(); } -mozilla::ipc::IPCResult -ContentParent::RecvRecordChildEvents(nsTArray&& aEvents) -{ - TelemetryIPC::RecordChildEvents(GeckoProcessType_Content, aEvents); - return IPC_OK(); -} - PURLClassifierParent* ContentParent::AllocPURLClassifierParent(const Principal& aPrincipal, const bool& aUseTrackingProtection, diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 54ffa3cddb0b..115937034000 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1122,8 +1122,6 @@ private: InfallibleTArray&& aScalarActions) override; virtual mozilla::ipc::IPCResult RecvUpdateChildKeyedScalars( InfallibleTArray&& aScalarActions) override; - virtual mozilla::ipc::IPCResult RecvRecordChildEvents( - nsTArray&& events) override; public: void SendGetFilesResponseAndForget(const nsID& aID, const GetFilesResponseResult& aResult); diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 956c49b7e4cd..a80382d1034a 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -90,7 +90,6 @@ using mozilla::Telemetry::Accumulation from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::KeyedAccumulation from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::ScalarAction from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::KeyedScalarAction from "mozilla/TelemetryComms.h"; -using mozilla::Telemetry::ChildEventData from "mozilla/TelemetryComms.h"; union ChromeRegistryItem { @@ -1199,7 +1198,6 @@ parent: async AccumulateChildKeyedHistograms(KeyedAccumulation[] accumulations); async UpdateChildScalars(ScalarAction[] updates); async UpdateChildKeyedScalars(KeyedScalarAction[] updates); - async RecordChildEvents(ChildEventData[] events); sync GetA11yContentId() returns (uint32_t aContentId); diff --git a/gfx/ipc/GPUChild.cpp b/gfx/ipc/GPUChild.cpp index d8512e460f10..2e33cd9608d9 100644 --- a/gfx/ipc/GPUChild.cpp +++ b/gfx/ipc/GPUChild.cpp @@ -172,13 +172,6 @@ GPUChild::RecvUpdateChildKeyedScalars(InfallibleTArray&& aSca return IPC_OK(); } -mozilla::ipc::IPCResult -GPUChild::RecvRecordChildEvents(nsTArray&& aEvents) -{ - TelemetryIPC::RecordChildEvents(GeckoProcessType_GPU, aEvents); - return IPC_OK(); -} - mozilla::ipc::IPCResult GPUChild::RecvNotifyDeviceReset() { diff --git a/gfx/ipc/GPUChild.h b/gfx/ipc/GPUChild.h index f0df1e4d174a..42ed051711ac 100644 --- a/gfx/ipc/GPUChild.h +++ b/gfx/ipc/GPUChild.h @@ -43,13 +43,10 @@ public: mozilla::ipc::IPCResult RecvInitComplete(const GPUDeviceData& aData) override; mozilla::ipc::IPCResult RecvReportCheckerboard(const uint32_t& aSeverity, const nsCString& aLog) override; mozilla::ipc::IPCResult RecvInitCrashReporter(Shmem&& shmem, const NativeThreadId& aThreadId) override; - mozilla::ipc::IPCResult RecvAccumulateChildHistograms(InfallibleTArray&& aAccumulations) override; mozilla::ipc::IPCResult RecvAccumulateChildKeyedHistograms(InfallibleTArray&& aAccumulations) override; mozilla::ipc::IPCResult RecvUpdateChildScalars(InfallibleTArray&& aScalarActions) override; mozilla::ipc::IPCResult RecvUpdateChildKeyedScalars(InfallibleTArray&& aScalarActions) override; - mozilla::ipc::IPCResult RecvRecordChildEvents(nsTArray&& events) override; - void ActorDestroy(ActorDestroyReason aWhy) override; mozilla::ipc::IPCResult RecvGraphicsError(const nsCString& aError) override; mozilla::ipc::IPCResult RecvNotifyUiObservers(const nsCString& aTopic) override; diff --git a/gfx/ipc/PGPU.ipdl b/gfx/ipc/PGPU.ipdl index a9bd7044a8d4..fec1fc1fd22a 100644 --- a/gfx/ipc/PGPU.ipdl +++ b/gfx/ipc/PGPU.ipdl @@ -22,7 +22,6 @@ using mozilla::Telemetry::Accumulation from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::KeyedAccumulation from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::ScalarAction from "mozilla/TelemetryComms.h"; using mozilla::Telemetry::KeyedScalarAction from "mozilla/TelemetryComms.h"; -using mozilla::Telemetry::ChildEventData from "mozilla/TelemetryComms.h"; namespace mozilla { namespace gfx { @@ -115,7 +114,6 @@ child: async AccumulateChildKeyedHistograms(KeyedAccumulation[] accumulations); async UpdateChildScalars(ScalarAction[] actions); async UpdateChildKeyedScalars(KeyedScalarAction[] actions); - async RecordChildEvents(ChildEventData[] events); async NotifyDeviceReset(); diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index 5cafa9935054..89964499636a 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -38,7 +38,6 @@ struct Accumulation; struct KeyedAccumulation; struct ScalarAction; struct KeyedScalarAction; -struct ChildEventData; enum TimerResolution { Millisecond, diff --git a/toolkit/components/telemetry/TelemetryEvent.cpp b/toolkit/components/telemetry/TelemetryEvent.cpp index 9dd655c5c76a..5971cc6c4379 100644 --- a/toolkit/components/telemetry/TelemetryEvent.cpp +++ b/toolkit/components/telemetry/TelemetryEvent.cpp @@ -14,7 +14,6 @@ #include "mozilla/Unused.h" #include "mozilla/Maybe.h" #include "mozilla/StaticPtr.h" -#include "mozilla/Pair.h" #include "jsapi.h" #include "nsJSUtils.h" #include "nsXULAppAPI.h" @@ -23,13 +22,13 @@ #include "TelemetryCommon.h" #include "TelemetryEvent.h" #include "TelemetryEventData.h" -#include "ipc/TelemetryIPCAccumulator.h" using mozilla::StaticMutex; using mozilla::StaticMutexAutoLock; using mozilla::ArrayLength; using mozilla::Maybe; using mozilla::Nothing; +using mozilla::Pair; using mozilla::StaticAutoPtr; using mozilla::Telemetry::Common::AutoHashtable; using mozilla::Telemetry::Common::IsExpiredVersion; @@ -37,10 +36,6 @@ using mozilla::Telemetry::Common::CanRecordDataset; using mozilla::Telemetry::Common::IsInDataset; using mozilla::Telemetry::Common::MsSinceProcessStart; using mozilla::Telemetry::Common::LogToBrowserConsole; -using mozilla::Telemetry::EventExtraEntry; -using mozilla::Telemetry::ChildEventData; - -namespace TelemetryIPCAccumulator = mozilla::TelemetryIPCAccumulator; //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// @@ -108,7 +103,12 @@ enum class RecordEventResult { StorageLimitReached, }; -typedef nsTArray ExtraArray; +struct ExtraEntry { + const nsCString key; + const nsCString value; +}; + +typedef nsTArray ExtraArray; class EventRecord { public: @@ -258,17 +258,15 @@ StringUintMap gCategoryNameIDMap; // This tracks the IDs of the categories for which recording is enabled. nsTHashtable gEnabledCategories; -// The main event storage. Events are inserted here, keyed by process id and -// in recording order. -typedef nsTArray EventRecordArray; -nsClassHashtable gEventRecords; +// The main event storage. Events are inserted here in recording order. +StaticAutoPtr> gEventRecords; } // namespace //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // -// PRIVATE: thread-safe helpers for event recording. +// PRIVATE: thread-unsafe helpers for event recording. namespace { @@ -286,27 +284,14 @@ CanRecordEvent(const StaticMutexAutoLock& lock, const CommonEventInfo& info) return gEnabledCategories.GetEntry(info.category_offset); } -EventRecordArray* -GetEventRecordsForProcess(const StaticMutexAutoLock& lock, GeckoProcessType processType) -{ - EventRecordArray* eventRecords = nullptr; - if (!gEventRecords.Get(processType, &eventRecords)) { - eventRecords = new EventRecordArray(); - gEventRecords.Put(processType, eventRecords); - } - return eventRecords; -} - RecordEventResult -RecordEvent(const StaticMutexAutoLock& lock, GeckoProcessType processType, - double timestamp, const nsACString& category, - const nsACString& method, const nsACString& object, - const Maybe& value, const ExtraArray& extra) +RecordEvent(const StaticMutexAutoLock& lock, double timestamp, + const nsACString& category, const nsACString& method, + const nsACString& object, const Maybe& value, + const ExtraArray& extra) { - EventRecordArray* eventRecords = GetEventRecordsForProcess(lock, processType); - // Apply hard limit on event count in storage. - if (eventRecords->Length() >= kMaxEventRecords) { + if (gEventRecords->Length() >= kMaxEventRecords) { return RecordEventResult::StorageLimitReached; } @@ -344,113 +329,12 @@ RecordEvent(const StaticMutexAutoLock& lock, GeckoProcessType processType, } // Add event record. - eventRecords->AppendElement(EventRecord(timestamp, eventId, value, extra)); + gEventRecords->AppendElement(EventRecord(timestamp, eventId, value, extra)); return RecordEventResult::Ok; } } // anonymous namespace -//////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////// -// -// PRIVATE: thread-unsafe helpers for event handling. - -namespace { - -nsresult -SerializeEventsArray(const EventRecordArray& events, - JSContext* cx, - JS::MutableHandleObject result) -{ - // We serialize the events to a JS array. - JS::RootedObject eventsArray(cx, JS_NewArrayObject(cx, events.Length())); - if (!eventsArray) { - return NS_ERROR_FAILURE; - } - - for (uint32_t i = 0; i < events.Length(); ++i) { - const EventRecord& record = events[i]; - const EventInfo& info = gEventInfo[record.EventId()]; - - // Each entry is an array of one of the forms: - // [timestamp, category, method, object, value] - // [timestamp, category, method, object, null, extra] - // [timestamp, category, method, object, value, extra] - JS::AutoValueVector items(cx); - - // Add timestamp. - JS::Rooted val(cx); - if (!items.append(JS::NumberValue(floor(record.Timestamp())))) { - return NS_ERROR_FAILURE; - } - - // Add category, method, object. - const char* strings[] = { - info.common_info.category(), - info.method(), - info.object(), - }; - for (const char* s : strings) { - const NS_ConvertUTF8toUTF16 wide(s); - if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) { - return NS_ERROR_FAILURE; - } - } - - // Add the optional string value only when needed. - // When the value field is empty and extra is not set, we can save a little space that way. - // We still need to submit a null value if extra is set, to match the form: - // [ts, category, method, object, null, extra] - if (record.Value()) { - const NS_ConvertUTF8toUTF16 wide(record.Value().value()); - if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) { - return NS_ERROR_FAILURE; - } - } else if (!record.Extra().IsEmpty()) { - if (!items.append(JS::NullValue())) { - return NS_ERROR_FAILURE; - } - } - - // Add the optional extra dictionary. - // To save a little space, only add it when it is not empty. - if (!record.Extra().IsEmpty()) { - JS::RootedObject obj(cx, JS_NewPlainObject(cx)); - if (!obj) { - return NS_ERROR_FAILURE; - } - - // Add extra key & value entries. - const ExtraArray& extra = record.Extra(); - for (uint32_t i = 0; i < extra.Length(); ++i) { - const NS_ConvertUTF8toUTF16 wide(extra[i].value); - JS::Rooted value(cx); - value.setString(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())); - - if (!JS_DefineProperty(cx, obj, extra[i].key.get(), value, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - } - val.setObject(*obj); - - if (!items.append(val)) { - return NS_ERROR_FAILURE; - } - } - - // Add the record to the events array. - JS::RootedObject itemsArray(cx, JS_NewArrayObject(cx, items)); - if (!JS_DefineElement(cx, eventsArray, i, itemsArray, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - } - - result.set(eventsArray); - return NS_OK; -} - -} // anonymous namespace - //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // @@ -478,6 +362,8 @@ TelemetryEvent::InitializeGlobalState(bool aCanRecordBase, bool aCanRecordExtend gCanRecordBase = aCanRecordBase; gCanRecordExtended = aCanRecordExtended; + gEventRecords = new nsTArray(); + // Populate the static event name->id cache. Note that the event names are // statically allocated and come from the automatically generated TelemetryEventData.h. const uint32_t eventCount = static_cast(mozilla::Telemetry::EventID::EventCount); @@ -518,7 +404,8 @@ TelemetryEvent::DeInitializeGlobalState() gEventNameIDMap.Clear(); gCategoryNameIDMap.Clear(); gEnabledCategories.Clear(); - gEventRecords.Clear(); + gEventRecords->Clear(); + gEventRecords = nullptr; gInitDone = false; } @@ -536,25 +423,17 @@ TelemetryEvent::SetCanRecordExtended(bool b) { gCanRecordExtended = b; } -nsresult -TelemetryEvent::RecordChildEvents(GeckoProcessType aProcessType, - const nsTArray& aEvents) -{ - MOZ_ASSERT(XRE_IsParentProcess()); - StaticMutexAutoLock locker(gTelemetryEventsMutex); - for (uint32_t i = 0; i < aEvents.Length(); ++i) { - const mozilla::Telemetry::ChildEventData e = aEvents[i]; - ::RecordEvent(locker, aProcessType, e.timestamp, e.category, e.method, e.object, e.value, e.extra); - } - return NS_OK; -} - nsresult TelemetryEvent::RecordEvent(const nsACString& aCategory, const nsACString& aMethod, const nsACString& aObject, JS::HandleValue aValue, JS::HandleValue aExtra, JSContext* cx, uint8_t optional_argc) { + // Currently only recording in the parent process is supported. + if (!XRE_IsParentProcess()) { + return NS_OK; + } + // Get the current time. double timestamp = -1; nsresult rv = MsSinceProcessStart(×tamp); @@ -637,15 +516,10 @@ TelemetryEvent::RecordEvent(const nsACString& aCategory, const nsACString& aMeth TruncateToByteLength(str, kMaxExtraValueByteLength); } - extra.AppendElement(EventExtraEntry{NS_ConvertUTF16toUTF8(key), str}); + extra.AppendElement(ExtraEntry{NS_ConvertUTF16toUTF8(key), str}); } } - if (!XRE_IsParentProcess()) { - TelemetryIPCAccumulator::RecordChildEvent(timestamp, aCategory, aMethod, aObject, value, extra); - return NS_OK; - } - // Lock for accessing internal data. // While the lock is being held, no complex calls like JS calls can be made, // as all of these could record Telemetry, which would result in deadlock. @@ -657,7 +531,7 @@ TelemetryEvent::RecordEvent(const nsACString& aCategory, const nsACString& aMeth return NS_ERROR_FAILURE; } - res = ::RecordEvent(lock, GeckoProcessType_Default, timestamp, aCategory, aMethod, aObject, value, extra); + res = ::RecordEvent(lock, timestamp, aCategory, aMethod, aObject, value, extra); } // Trigger warnings or errors where needed. @@ -686,18 +560,8 @@ nsresult TelemetryEvent::CreateSnapshots(uint32_t aDataset, bool aClear, JSContext* cx, uint8_t optional_argc, JS::MutableHandleValue aResult) { - if (!XRE_IsParentProcess()) { - return NS_ERROR_FAILURE; - } - - // Creating a JS snapshot of the events is a two-step process: - // (1) Lock the storage and copy the events into function-local storage. - // (2) Serialize the events into JS. - // We can't hold a lock for (2) because we will run into deadlocks otherwise - // from JS recording Telemetry. - - // (1) Extract the events from storage with a lock held. - nsTArray> processEvents; + // Extract the events from storage. + nsTArray events; { StaticMutexAutoLock locker(gTelemetryEventsMutex); @@ -705,51 +569,103 @@ TelemetryEvent::CreateSnapshots(uint32_t aDataset, bool aClear, JSContext* cx, return NS_ERROR_FAILURE; } - for (auto iter = gEventRecords.Iter(); !iter.Done(); iter.Next()) { - const EventRecordArray* eventStorage = static_cast(iter.Data()); - EventRecordArray events; + uint32_t len = gEventRecords->Length(); + for (uint32_t i = 0; i < len; ++i) { + const EventRecord& record = (*gEventRecords)[i]; + const EventInfo& info = gEventInfo[record.EventId()]; - const uint32_t len = eventStorage->Length(); - for (uint32_t i = 0; i < len; ++i) { - const EventRecord& record = (*eventStorage)[i]; - const EventInfo& info = gEventInfo[record.EventId()]; - - if (IsInDataset(info.common_info.dataset, aDataset)) { - events.AppendElement(record); - } - } - - if (events.Length()) { - const char* processName = XRE_ChildProcessTypeToString(GeckoProcessType(iter.Key())); - processEvents.AppendElement(mozilla::MakePair(processName, events)); + if (IsInDataset(info.common_info.dataset, aDataset)) { + events.AppendElement(record); } } if (aClear) { - gEventRecords.Clear(); + gEventRecords->Clear(); } } - // (2) Serialize the events to a JS object. - JS::RootedObject rootObj(cx, JS_NewPlainObject(cx)); - if (!rootObj) { + // We serialize the events to a JS array. + JS::RootedObject eventsArray(cx, JS_NewArrayObject(cx, events.Length())); + if (!eventsArray) { return NS_ERROR_FAILURE; } - const uint32_t processLength = processEvents.Length(); - for (uint32_t i = 0; i < processLength; ++i) - { - JS::RootedObject eventsArray(cx); - if (NS_FAILED(SerializeEventsArray(processEvents[i].second(), cx, &eventsArray))) { + for (uint32_t i = 0; i < events.Length(); ++i) { + const EventRecord& record = events[i]; + const EventInfo& info = gEventInfo[record.EventId()]; + + // Each entry is an array of one of the forms: + // [timestamp, category, method, object, value] + // [timestamp, category, method, object, null, extra] + // [timestamp, category, method, object, value, extra] + JS::AutoValueVector items(cx); + + // Add timestamp. + JS::Rooted val(cx); + if (!items.append(JS::NumberValue(floor(record.Timestamp())))) { return NS_ERROR_FAILURE; } - if (!JS_DefineProperty(cx, rootObj, processEvents[i].first(), eventsArray, JSPROP_ENUMERATE)) { + // Add category, method, object. + const char* strings[] = { + info.common_info.category(), + info.method(), + info.object(), + }; + for (const char* s : strings) { + const NS_ConvertUTF8toUTF16 wide(s); + if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) { + return NS_ERROR_FAILURE; + } + } + + // Add the optional string value only when needed. + // When extra is empty and this has no value, we can save a little space. + if (record.Value()) { + const NS_ConvertUTF8toUTF16 wide(record.Value().value()); + if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) { + return NS_ERROR_FAILURE; + } + } else if (!record.Extra().IsEmpty()) { + if (!items.append(JS::NullValue())) { + return NS_ERROR_FAILURE; + } + } + + // Add the optional extra dictionary. + // To save a little space, only add it when it is not empty. + if (!record.Extra().IsEmpty()) { + JS::RootedObject obj(cx, JS_NewPlainObject(cx)); + if (!obj) { + return NS_ERROR_FAILURE; + } + + // Add extra key & value entries. + const ExtraArray& extra = record.Extra(); + for (uint32_t i = 0; i < extra.Length(); ++i) { + const NS_ConvertUTF8toUTF16 wide(extra[i].value); + JS::Rooted value(cx); + value.setString(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())); + + if (!JS_DefineProperty(cx, obj, extra[i].key.get(), value, JSPROP_ENUMERATE)) { + return NS_ERROR_FAILURE; + } + } + val.setObject(*obj); + + if (!items.append(val)) { + return NS_ERROR_FAILURE; + } + } + + // Add the record to the events array. + JS::RootedObject itemsArray(cx, JS_NewArrayObject(cx, items)); + if (!JS_DefineElement(cx, eventsArray, i, itemsArray, JSPROP_ENUMERATE)) { return NS_ERROR_FAILURE; } } - aResult.setObject(*rootObj); + aResult.setObject(*eventsArray); return NS_OK; } @@ -765,7 +681,7 @@ TelemetryEvent::ClearEvents() return; } - gEventRecords.Clear(); + gEventRecords->Clear(); } void @@ -793,16 +709,9 @@ TelemetryEvent::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) StaticMutexAutoLock locker(gTelemetryEventsMutex); size_t n = 0; - - n += gEventRecords.ShallowSizeOfExcludingThis(aMallocSizeOf); - for (auto iter = gEventRecords.Iter(); !iter.Done(); iter.Next()) { - EventRecordArray* eventRecords = static_cast(iter.Data()); - n += eventRecords->ShallowSizeOfIncludingThis(aMallocSizeOf); - - const uint32_t len = eventRecords->Length(); - for (uint32_t i = 0; i < len; ++i) { - n += (*eventRecords)[i].SizeOfExcludingThis(aMallocSizeOf); - } + n += gEventRecords->ShallowSizeOfIncludingThis(aMallocSizeOf); + for (uint32_t i = 0; i < gEventRecords->Length(); ++i) { + n += (*gEventRecords)[i].SizeOfExcludingThis(aMallocSizeOf); } n += gEventNameIDMap.ShallowSizeOfExcludingThis(aMallocSizeOf); diff --git a/toolkit/components/telemetry/TelemetryEvent.h b/toolkit/components/telemetry/TelemetryEvent.h index 7fefcb81c592..3fca94bc4a71 100644 --- a/toolkit/components/telemetry/TelemetryEvent.h +++ b/toolkit/components/telemetry/TelemetryEvent.h @@ -8,12 +8,6 @@ #include "mozilla/TelemetryEventEnums.h" -namespace mozilla { -namespace Telemetry { - struct ChildEventData; -} -} - // This module is internal to Telemetry. It encapsulates Telemetry's // event recording and storage logic. It should only be used by // Telemetry.cpp. These functions should not be used anywhere else. @@ -36,10 +30,6 @@ void SetEventRecordingEnabled(const nsACString& aCategory, bool aEnabled); nsresult CreateSnapshots(uint32_t aDataset, bool aClear, JSContext* aCx, uint8_t optional_argc, JS::MutableHandleValue aResult); -// Record events from child processes. -nsresult RecordChildEvents(GeckoProcessType aProcessType, - const nsTArray& aEvents); - // Only to be used for testing. void ClearEvents(); diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index d27481f16469..41d0513937c9 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1028,17 +1028,15 @@ var Impl = { return []; } - let snapshot = Telemetry.snapshotBuiltinEvents(this.getDatasetType(), - clearSubsession); + let events = Telemetry.snapshotBuiltinEvents(this.getDatasetType(), + clearSubsession); // Don't return the test events outside of test environments. if (!this._testing) { - for (let proc of Object.keys(snapshot)) { - snapshot[proc] = snapshot[proc].filter(e => !e[1].startsWith("telemetry.test")); - } + events = events.filter(e => !e[1].startsWith("telemetry.test")); } - return snapshot; + return events; }, getThreadHangStats: function getThreadHangStats(stats) { @@ -1298,7 +1296,6 @@ var Impl = { let keyedHistograms = protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {}); let scalars = protect(() => this.getScalars(isSubsession, clearSubsession), {}); let keyedScalars = protect(() => this.getScalars(isSubsession, clearSubsession, true), {}); - let events = protect(() => this.getEvents(isSubsession, clearSubsession)) payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {}; payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {}; @@ -1306,14 +1303,13 @@ var Impl = { parent: { scalars: scalars[INTERNAL_PROCESSES_NAMES.PARENT] || {}, keyedScalars: keyedScalars[INTERNAL_PROCESSES_NAMES.PARENT] || {}, - events: events[INTERNAL_PROCESSES_NAMES.PARENT] || [], + events: protect(() => this.getEvents(isSubsession, clearSubsession)), }, content: { scalars: scalars[INTERNAL_PROCESSES_NAMES.CONTENT], keyedScalars: keyedScalars[INTERNAL_PROCESSES_NAMES.CONTENT], histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT], keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT], - events: events[INTERNAL_PROCESSES_NAMES.CONTENT] || [], }, }; @@ -1327,7 +1323,6 @@ var Impl = { keyedScalars: keyedScalars[INTERNAL_PROCESSES_NAMES.GPU], histograms: histograms[HISTOGRAM_SUFFIXES.GPU], keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.GPU], - events: events[INTERNAL_PROCESSES_NAMES.GPU] || [], }; } diff --git a/toolkit/components/telemetry/ipc/TelemetryComms.h b/toolkit/components/telemetry/ipc/TelemetryComms.h index b3063592af94..52f67c9b9cc5 100644 --- a/toolkit/components/telemetry/ipc/TelemetryComms.h +++ b/toolkit/components/telemetry/ipc/TelemetryComms.h @@ -59,20 +59,6 @@ struct KeyedScalarAction Maybe mData; }; -struct EventExtraEntry { - nsCString key; - nsCString value; -}; - -struct ChildEventData { - double timestamp; - nsCString category; - nsCString method; - nsCString object; - mozilla::Maybe value; - nsTArray extra; -}; - } // namespace Telemetry } // namespace mozilla @@ -302,60 +288,6 @@ ParamTraits } }; -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::ChildEventData paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - WriteParam(aMsg, aParam.timestamp); - WriteParam(aMsg, aParam.category); - WriteParam(aMsg, aParam.method); - WriteParam(aMsg, aParam.object); - WriteParam(aMsg, aParam.value); - WriteParam(aMsg, aParam.extra); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!ReadParam(aMsg, aIter, &(aResult->timestamp)) || - !ReadParam(aMsg, aIter, &(aResult->category)) || - !ReadParam(aMsg, aIter, &(aResult->method)) || - !ReadParam(aMsg, aIter, &(aResult->object)) || - !ReadParam(aMsg, aIter, &(aResult->value)) || - !ReadParam(aMsg, aIter, &(aResult->extra))) { - return false; - } - - return true; - } -}; - -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::EventExtraEntry paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - WriteParam(aMsg, aParam.key); - WriteParam(aMsg, aParam.value); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!ReadParam(aMsg, aIter, &(aResult->key)) || - !ReadParam(aMsg, aIter, &(aResult->value))) { - return false; - } - - return true; - } -}; - } // namespace IPC #endif // Telemetry_Comms_h__ diff --git a/toolkit/components/telemetry/ipc/TelemetryIPC.cpp b/toolkit/components/telemetry/ipc/TelemetryIPC.cpp index c1cb1f4c4da1..adc4c093a2de 100644 --- a/toolkit/components/telemetry/ipc/TelemetryIPC.cpp +++ b/toolkit/components/telemetry/ipc/TelemetryIPC.cpp @@ -7,7 +7,6 @@ #include "TelemetryIPC.h" #include "../TelemetryScalar.h" #include "../TelemetryHistogram.h" -#include "../TelemetryEvent.h" namespace mozilla { @@ -39,10 +38,4 @@ TelemetryIPC::UpdateChildKeyedScalars(GeckoProcessType aProcessType, TelemetryScalar::UpdateChildKeyedData(aProcessType, aScalarActions); } -void -TelemetryIPC::RecordChildEvents(GeckoProcessType aProcessType, const nsTArray& aEvents) -{ - TelemetryEvent::RecordChildEvents(aProcessType, aEvents); -} - } diff --git a/toolkit/components/telemetry/ipc/TelemetryIPC.h b/toolkit/components/telemetry/ipc/TelemetryIPC.h index b800e1f1910d..ee0ccdfd6ff8 100644 --- a/toolkit/components/telemetry/ipc/TelemetryIPC.h +++ b/toolkit/components/telemetry/ipc/TelemetryIPC.h @@ -20,7 +20,6 @@ struct Accumulation; struct KeyedAccumulation; struct ScalarAction; struct KeyedScalarAction; -struct ChildEventData; } @@ -29,7 +28,6 @@ namespace TelemetryIPC { /** * Accumulate child process data into histograms for the given process type. * - * @param aProcessType - the process type to accumulate the histograms for * @param aAccumulations - accumulation actions to perform */ void AccumulateChildHistograms(GeckoProcessType aProcessType, const nsTArray& aAccumulations); @@ -37,7 +35,6 @@ void AccumulateChildHistograms(GeckoProcessType aProcessType, const nsTArray& aAccumulations); @@ -45,27 +42,17 @@ void AccumulateChildKeyedHistograms(GeckoProcessType aProcessType, const nsTArra /** * Update scalars for the given process type with the data coming from child process. * - * @param aProcessType - the process type to process the scalar actions for * @param aScalarActions - actions to update the scalar data */ void UpdateChildScalars(GeckoProcessType aProcessType, const nsTArray& aScalarActions); /** - * Update keyed scalars for the given process type with the data coming from child process. + * Update keyed scalars for the given process type with the data coming from child process. * - * @param aProcessType - the process type to process the keyed scalar actions for * @param aScalarActions - actions to update the keyed scalar data */ void UpdateChildKeyedScalars(GeckoProcessType aProcessType, const nsTArray& aScalarActions); -/** - * Record events for the given process type with the data coming from child process. - * - * @param aProcessType - the process type to record the events for - * @param aEvents - events to record - */ -void RecordChildEvents(GeckoProcessType aProcessType, const nsTArray& aEvents); - } } diff --git a/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.cpp b/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.cpp index db502f18a4d7..374e54fe4512 100644 --- a/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.cpp +++ b/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.cpp @@ -27,7 +27,6 @@ using mozilla::Telemetry::ScalarActionType; using mozilla::Telemetry::ScalarAction; using mozilla::Telemetry::KeyedScalarAction; using mozilla::Telemetry::ScalarVariant; -using mozilla::Telemetry::ChildEventData; namespace TelemetryIPCAccumulator = mozilla::TelemetryIPCAccumulator; @@ -42,17 +41,16 @@ const uint32_t kBatchTimeoutMs = 2000; // manage to reach this high water mark of elements. const size_t kHistogramAccumulationsArrayHighWaterMark = 5 * 1024; -// This timer is used for batching and sending child process accumulations to the parent. +// For batching and sending child process accumulations to the parent nsITimer* gIPCTimer = nullptr; mozilla::Atomic gIPCTimerArmed(false); mozilla::Atomic gIPCTimerArming(false); -// This batches child process accumulations that should be sent to the parent. +// For batching and sending child process accumulations to the parent StaticAutoPtr> gHistogramAccumulations; StaticAutoPtr> gKeyedHistogramAccumulations; StaticAutoPtr> gChildScalarsActions; StaticAutoPtr> gChildKeyedScalarsActions; -StaticAutoPtr> gChildEvents; // This is a StaticMutex rather than a plain Mutex so that (1) // it gets initialised in a thread-safe manner the first time @@ -64,8 +62,7 @@ static StaticMutex gTelemetryIPCAccumulatorMutex; namespace { -void -DoArmIPCTimerMainThread(const StaticMutexAutoLock& lock) +void DoArmIPCTimerMainThread(const StaticMutexAutoLock& lock) { MOZ_ASSERT(NS_IsMainThread()); gIPCTimerArming = false; @@ -84,8 +81,7 @@ DoArmIPCTimerMainThread(const StaticMutexAutoLock& lock) } } -void -ArmIPCTimer(const StaticMutexAutoLock& lock) +void ArmIPCTimer(const StaticMutexAutoLock& lock) { if (gIPCTimerArmed || gIPCTimerArming) { return; @@ -175,26 +171,6 @@ TelemetryIPCAccumulator::RecordChildKeyedScalarAction(mozilla::Telemetry::Scalar ArmIPCTimer(locker); } -void -TelemetryIPCAccumulator::RecordChildEvent(double timestamp, - const nsACString& category, - const nsACString& method, - const nsACString& object, - const mozilla::Maybe& value, - const nsTArray& extra) -{ - StaticMutexAutoLock locker(gTelemetryIPCAccumulatorMutex); - if (!gChildEvents) { - gChildEvents = new nsTArray(); - } - // Store the action. - gChildEvents->AppendElement(ChildEventData{timestamp, nsCString(category), - nsCString(method), nsCString(object), - value, - nsTArray(extra)}); - ArmIPCTimer(locker); -} - // This method takes the lock only to double-buffer the batched telemetry. // It releases the lock before calling out to IPC code which can (and does) // Accumulate (which would deadlock) @@ -207,8 +183,6 @@ SendAccumulatedData(TActor* ipcActor) nsTArray keyedAccumulationsToSend; nsTArray scalarsToSend; nsTArray keyedScalarsToSend; - nsTArray eventsToSend; - { StaticMutexAutoLock locker(gTelemetryIPCAccumulatorMutex); if (gHistogramAccumulations) { @@ -224,9 +198,6 @@ SendAccumulatedData(TActor* ipcActor) if (gChildKeyedScalarsActions) { keyedScalarsToSend.SwapElements(*gChildKeyedScalarsActions); } - if (gChildEvents) { - eventsToSend.SwapElements(*gChildEvents); - } } // Send the accumulated data to the parent process. @@ -247,10 +218,6 @@ SendAccumulatedData(TActor* ipcActor) mozilla::Unused << NS_WARN_IF(!ipcActor->SendUpdateChildKeyedScalars(keyedScalarsToSend)); } - if (eventsToSend.Length()) { - mozilla::Unused << - NS_WARN_IF(!ipcActor->SendRecordChildEvents(eventsToSend)); - } } @@ -293,7 +260,6 @@ TelemetryIPCAccumulator::DeInitializeGlobalState() gKeyedHistogramAccumulations = nullptr; gChildScalarsActions = nullptr; gChildKeyedScalarsActions = nullptr; - gChildEvents = nullptr; } void diff --git a/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.h b/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.h index df04abf1598f..40c65b1df097 100644 --- a/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.h +++ b/toolkit/components/telemetry/ipc/TelemetryIPCAccumulator.h @@ -7,7 +7,6 @@ #define TelemetryIPCAccumulator_h__ #include "mozilla/AlreadyAddRefed.h" -#include "mozilla/Maybe.h" #include "TelemetryComms.h" class nsIRunnable; @@ -32,13 +31,6 @@ void RecordChildKeyedScalarAction(mozilla::Telemetry::ScalarID aId, const nsAStr mozilla::Telemetry::ScalarActionType aAction, const mozilla::Telemetry::ScalarVariant& aValue); -void RecordChildEvent(double timestamp, - const nsACString& category, - const nsACString& method, - const nsACString& object, - const mozilla::Maybe& value, - const nsTArray& extra); - void IPCTimerFired(nsITimer* aTimer, void* aClosure); void DeInitializeGlobalState(); diff --git a/toolkit/components/telemetry/tests/unit/test_ChildEvents.js b/toolkit/components/telemetry/tests/unit/test_ChildEvents.js deleted file mode 100644 index ab095fdc6bca..000000000000 --- a/toolkit/components/telemetry/tests/unit/test_ChildEvents.js +++ /dev/null @@ -1,90 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ -*/ - -Cu.import("resource://gre/modules/Services.jsm", this); -Cu.import("resource://gre/modules/TelemetryController.jsm", this); -Cu.import("resource://gre/modules/TelemetrySession.jsm", this); -Cu.import("resource://gre/modules/PromiseUtils.jsm", this); -Cu.import("resource://testing-common/ContentTaskUtils.jsm", this); - -const MESSAGE_CHILD_TEST_DONE = "ChildTest:Done"; - -const PLATFORM_VERSION = "1.9.2"; -const APP_VERSION = "1"; -const APP_ID = "xpcshell@tests.mozilla.org"; -const APP_NAME = "XPCShell"; - -const RECORDED_CONTENT_EVENTS = [ - ["telemetry.test", "test1", "object1"], - ["telemetry.test", "test1", "object1", null, {"key1": "x", "key2": "y"}], - ["telemetry.test", "test1", "object1", "foo", {"key1": "x", "key2": "y"}], -]; - -function run_child_test() { - // Record some events in the "content" process. - RECORDED_CONTENT_EVENTS.forEach(e => Telemetry.recordEvent(...e)); -} - -/** - * This function waits until content events are reported into the - * events snapshot. - */ -function* waitForContentEvents() { - yield ContentTaskUtils.waitForCondition(() => { - const snapshot = - Telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false); - return Object.keys(snapshot).includes("tab"); - }); -} - -add_task(function*() { - if (!runningInParent) { - TelemetryController.testSetupContent(); - run_child_test(); - do_send_remote_message(MESSAGE_CHILD_TEST_DONE); - return; - } - - // Setup. - do_get_profile(true); - loadAddonManager(APP_ID, APP_NAME, APP_VERSION, PLATFORM_VERSION); - Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true); - yield TelemetryController.testSetup(); - // Make sure we don't generate unexpected pings due to pref changes. - yield setEmptyPrefWatchlist(); - // Enable recording for the test event category. - Telemetry.setEventRecordingEnabled("telemetry.test", true); - - // Run test in child, don't wait for it to finish: just wait for the - // MESSAGE_CHILD_TEST_DONE. - run_test_in_child("test_ChildEvents.js"); - yield do_await_remote_message(MESSAGE_CHILD_TEST_DONE); - - // Once events are set by the content process, they don't immediately get - // sent to the parent process. Wait for the Telemetry IPC Timer to trigger - // and batch send the data back to the parent process. - yield waitForContentEvents(); - - // Get an "environment-changed" ping rather than a "test-ping", as - // event measurements are only supported in subsession pings. - const payload = TelemetrySession.getPayload("environment-change"); - - // Validate the event data. - Assert.ok("processes" in payload, "Should have processes section"); - Assert.ok("parent" in payload.processes, "Should have main process section"); - Assert.ok("events" in payload.processes.parent, "Main process section should have events."); - Assert.ok("content" in payload.processes, "Should have child process section"); - Assert.ok("events" in payload.processes.content, "Child process section should have events."); - - let events = payload.processes.content.events.map(e => e.slice(1)); - Assert.deepEqual(events, RECORDED_CONTENT_EVENTS, "Should have recorded content events."); - - // Make sure all events are cleared from storage properly. - let snapshot = - Telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 1, "Should have events from one process."); - snapshot = - Telemetry.snapshotBuiltinEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should have cleared all events from storage."); -}); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js index 994938ededb2..04ba5c68f6f3 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js @@ -45,32 +45,29 @@ add_task(function* test_recording_state() { // Both test categories should be off by default. events.forEach(e => Telemetry.recordEvent(...e)); let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not have recorded any events."); + Assert.deepEqual(snapshot, [], "Should not have recorded any events."); // Enable one test category and see that we record correctly. Telemetry.setEventRecordingEnabled("telemetry.test", true); events.forEach(e => Telemetry.recordEvent(...e)); snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - Assert.equal(snapshot.default.length, 1, "Should have recorded one event."); - Assert.equal(snapshot.default[0][1], "telemetry.test", "Should have recorded one event in telemetry.test"); + Assert.equal(snapshot.length, 1, "Should have recorded one event."); + Assert.equal(snapshot[0][1], "telemetry.test", "Should have recorded one event in telemetry.test"); // Also enable the other test category and see that we record correctly. Telemetry.setEventRecordingEnabled("telemetry.test.second", true); events.forEach(e => Telemetry.recordEvent(...e)); snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - Assert.equal(snapshot.default.length, 2, "Should have recorded two events."); - Assert.equal(snapshot.default[0][1], "telemetry.test", "Should have recorded one event in telemetry.test"); - Assert.equal(snapshot.default[1][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second"); + Assert.equal(snapshot.length, 2, "Should have recorded two events."); + Assert.equal(snapshot[0][1], "telemetry.test", "Should have recorded one event in telemetry.test"); + Assert.equal(snapshot[1][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second"); // Now turn of one category again and check that this works as expected. Telemetry.setEventRecordingEnabled("telemetry.test", false); events.forEach(e => Telemetry.recordEvent(...e)); snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - Assert.equal(snapshot.default.length, 1, "Should have recorded one event."); - Assert.equal(snapshot.default[0][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second"); + Assert.equal(snapshot.length, 1, "Should have recorded one event."); + Assert.equal(snapshot[0][1], "telemetry.test.second", "Should have recorded one event in telemetry.test.second"); }); add_task(function* recording_setup() { @@ -145,15 +142,13 @@ add_task(function* test_recording() { }; // Check that the expected events were recorded. - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, false); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - checkEvents(snapshot.default, expected); + let events = Telemetry.snapshotBuiltinEvents(OPTIN, false); + checkEvents(events, expected); // Check serializing only opt-out events. - snapshot = Telemetry.snapshotBuiltinEvents(OPTOUT, false); - Assert.ok(("default" in snapshot), "Should have entry for main process."); + events = Telemetry.snapshotBuiltinEvents(OPTOUT, false); let filtered = expected.filter(e => e.optout == true); - checkEvents(snapshot.default, filtered); + checkEvents(events, filtered); }); add_task(function* test_clear() { @@ -167,13 +162,12 @@ add_task(function* test_clear() { // Check that events were recorded. // The events are cleared by passing the respective flag. - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - Assert.equal(snapshot.default.length, 2 * COUNT, `Should have recorded ${2 * COUNT} events.`); + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 2 * COUNT, `Should have recorded ${2 * COUNT} events.`); // Now the events should be cleared. - snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, false); - Assert.equal(Object.keys(snapshot).length, 0, `Should have cleared the events.`); + events = Telemetry.snapshotBuiltinEvents(OPTIN, false); + Assert.equal(events.length, 0, `Should have cleared the events.`); }); add_task(function* test_expiry() { @@ -181,19 +175,18 @@ add_task(function* test_expiry() { // Recording call with event that is expired by version. Telemetry.recordEvent("telemetry.test", "expired_version", "object1"); - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event with expired version."); + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event with expired version."); // Recording call with event that is expired by date. Telemetry.recordEvent("telemetry.test", "expired_date", "object1"); - snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event with expired date."); + events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event with expired date."); // Recording call with event that has expiry_version and expiry_date in the future. Telemetry.recordEvent("telemetry.test", "not_expired_optout", "object1"); - snapshot = Telemetry.snapshotBuiltinEvents(OPTOUT, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - Assert.equal(snapshot.default.length, 1, "Should record event when date and version are not expired."); + events = Telemetry.snapshotBuiltinEvents(OPTOUT, true); + Assert.equal(events.length, 1, "Should record event when date and version are not expired."); }); add_task(function* test_invalidParams() { @@ -201,23 +194,23 @@ add_task(function* test_invalidParams() { // Recording call with wrong type for value argument. Telemetry.recordEvent("telemetry.test", "test1", "object1", 1); - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event when value argument with invalid type is passed."); + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event when value argument with invalid type is passed."); // Recording call with wrong type for extra argument. Telemetry.recordEvent("telemetry.test", "test1", "object1", null, "invalid"); - snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event when extra argument with invalid type is passed."); + events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event when extra argument with invalid type is passed."); // Recording call with unknown extra key. Telemetry.recordEvent("telemetry.test", "test1", "object1", null, {"key3": "x"}); - snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event when extra argument with invalid key is passed."); + events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event when extra argument with invalid key is passed."); // Recording call with invalid value type. Telemetry.recordEvent("telemetry.test", "test1", "object1", null, {"key3": 1}); - snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.equal(Object.keys(snapshot).length, 0, "Should not record event when extra argument with invalid value type is passed."); + events = Telemetry.snapshotBuiltinEvents(OPTIN, true); + Assert.equal(events.length, 0, "Should not record event when extra argument with invalid value type is passed."); }); add_task(function* test_storageLimit() { @@ -231,9 +224,7 @@ add_task(function* test_storageLimit() { } // Check that the right events were recorded. - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - let events = snapshot.default; + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); Assert.equal(events.length, LIMIT, `Should have only recorded ${LIMIT} events`); Assert.ok(events.every((e, idx) => e[4] === String(idx)), "Should have recorded all events from before hitting the limit."); @@ -274,9 +265,7 @@ add_task(function* test_valueLimits() { } // Check that the right events were recorded. - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - let events = snapshot.default; + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); Assert.equal(events.length, expected.length, "Should have recorded the expected number of events"); for (let i = 0; i < expected.length; ++i) { @@ -294,9 +283,7 @@ add_task(function* test_unicodeValues() { Telemetry.recordEvent("telemetry.test", "test1", "object1", null, {"key1": value}); // Check that the values were correctly recorded. - let snapshot = Telemetry.snapshotBuiltinEvents(OPTIN, true); - Assert.ok(("default" in snapshot), "Should have entry for main process."); - let events = snapshot.default; + let events = Telemetry.snapshotBuiltinEvents(OPTIN, true); Assert.equal(events.length, 2, "Should have recorded 2 events."); Assert.equal(events[0][4], value, "Should have recorded the right value."); Assert.equal(events[1][5]["key1"], value, "Should have recorded the right extra value."); diff --git a/toolkit/components/telemetry/tests/unit/xpcshell.ini b/toolkit/components/telemetry/tests/unit/xpcshell.ini index 79346a168fdf..544d0fc89fe2 100644 --- a/toolkit/components/telemetry/tests/unit/xpcshell.ini +++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini @@ -56,7 +56,7 @@ tags = addons run-sequentially = Bug 1046307, test can fail intermittently when CPU load is high [test_TelemetrySend.js] [test_ChildHistograms.js] -skip-if = os == "android" # Disabled due to crashes (see bug 1331366) +skip-if = os == "android" tags = addons [test_ChildScalars.js] skip-if = os == "android" # Disabled due to crashes (see bug 1331366) @@ -67,8 +67,6 @@ tags = addons skip-if = toolkit == 'android' [test_TelemetryCaptureStack.js] [test_TelemetryEvents.js] -[test_ChildEvents.js] -skip-if = os == "android" # Disabled due to crashes (see bug 1331366) [test_TelemetryModules.js] [test_PingSender.js] skip-if = (os == "android") || (os == "linux" && bits == 32)