diff --git a/dom/base/test/browser_use_counters.js b/dom/base/test/browser_use_counters.js index f92b117a5760..3e81bda081e8 100644 --- a/dom/base/test/browser_use_counters.js +++ b/dom/base/test/browser_use_counters.js @@ -3,7 +3,6 @@ requestLongerTimeout(2); var {Promise: promise} = Cu.import("resource://gre/modules/Promise.jsm", {}); -Cu.import("resource://gre/modules/Services.jsm"); const gHttpTestRoot = "http://example.com/browser/dom/base/test/"; @@ -105,18 +104,22 @@ function waitForPageLoad(browser) { }); } -function grabHistogramsFromContent(use_counter_middlefix, page_before = null) { - let telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); - let suffix = Services.appinfo.browserTabsRemoteAutostart ? "#content" : ""; - let gather = () => [ - telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_PAGE" + suffix).snapshot().sum, - telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_DOCUMENT" + suffix).snapshot().sum, - telemetry.getHistogramById("CONTENT_DOCUMENTS_DESTROYED" + suffix).snapshot().sum, - telemetry.getHistogramById("TOP_LEVEL_CONTENT_DOCUMENTS_DESTROYED" + suffix).snapshot().sum, - ]; - return BrowserTestUtils.waitForCondition(() => { - return page_before != telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_PAGE" + suffix).snapshot().sum; - }).then(gather, gather); +function grabHistogramsFromContent(browser, use_counter_middlefix) { + return ContentTask.spawn(browser, { middlefix: use_counter_middlefix }, function* (arg) { + let telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); + function snapshot_histogram(name) { + return telemetry.getHistogramById(name).snapshot(); + } + + let histogram_page_name = "USE_COUNTER2_" + arg.middlefix + "_PAGE"; + let histogram_document_name = "USE_COUNTER2_" + arg.middlefix + "_DOCUMENT"; + let histogram_page = snapshot_histogram(histogram_page_name); + let histogram_document = snapshot_histogram(histogram_document_name); + let histogram_docs = snapshot_histogram("CONTENT_DOCUMENTS_DESTROYED"); + let histogram_toplevel_docs = snapshot_histogram("TOP_LEVEL_CONTENT_DOCUMENTS_DESTROYED"); + return [histogram_page.sum, histogram_document.sum, + histogram_docs.sum, histogram_toplevel_docs.sum]; + }); } var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix, check_documents=true) { @@ -130,7 +133,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + "file_use_counter_outer.html"); yield waitForPageLoad(gBrowser.selectedBrowser); @@ -148,7 +151,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix event.target.removeEventListener("load", listener, true); // We flush the main document first, then the iframe's document to - // ensure any propagation that might happen from content->parent should + // ensure any propagation that might happen from child->parent should // have already happened when counters are reported to telemetry. wu.forceUseCounterFlush(content.document); wu.forceUseCounterFlush(iframe.contentDocument); @@ -171,7 +174,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); is(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); @@ -194,7 +197,7 @@ var check_use_counter_img = Task.async(function* (file, use_counter_middlefix) { // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + "file_use_counter_outer.html"); yield waitForPageLoad(gBrowser.selectedBrowser); @@ -236,7 +239,7 @@ var check_use_counter_img = Task.async(function* (file, use_counter_middlefix) { // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); is(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); is(histogram_document_after, histogram_document_before + 1, @@ -260,7 +263,7 @@ var check_use_counter_direct = Task.async(function* (file, use_counter_middlefix // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + file); yield ContentTask.spawn(gBrowser.selectedBrowser, null, function*() { @@ -289,7 +292,7 @@ var check_use_counter_direct = Task.async(function* (file, use_counter_middlefix // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); (xfail ? todo_is : is)(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); (xfail ? todo_is : is)(histogram_document_after, histogram_document_before + 1, diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index d94edbf8aac3..98c26585e4f8 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5404,19 +5404,3 @@ ContentParent::SendGetFilesResponseAndForget(const nsID& aUUID, Unused << SendGetFilesResponse(aUUID, aResult); } } - -bool -ContentParent::RecvAccumulateChildHistogram( - InfallibleTArray&& aAccumulations) -{ - Telemetry::AccumulateChild(aAccumulations); - return true; -} - -bool -ContentParent::RecvAccumulateChildKeyedHistogram( - InfallibleTArray&& aAccumulations) -{ - Telemetry::AccumulateChildKeyed(aAccumulations); - return true; -} diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index dada52716df6..f32d53f31460 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1134,10 +1134,6 @@ private: virtual bool RecvDeleteGetFilesRequest(const nsID& aID) override; - virtual bool RecvAccumulateChildHistogram( - InfallibleTArray&& aAccumulations) override; - virtual bool RecvAccumulateChildKeyedHistogram( - InfallibleTArray&& aAccumulations) override; public: void SendGetFilesResponseAndForget(const nsID& aID, const GetFilesResponseResult& aResult); diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 03aa98a39f01..b47af2b642cf 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -100,8 +100,6 @@ using mozilla::DataStorageType from "ipc/DataStorageIPCUtils.h"; using mozilla::DocShellOriginAttributes from "mozilla/ipc/BackgroundUtils.h"; using struct mozilla::layers::TextureFactoryIdentifier from "mozilla/layers/CompositorTypes.h"; using struct mozilla::dom::FlyWebPublishOptions from "mozilla/dom/FlyWebPublishOptionsIPCSerializer.h"; -using mozilla::Telemetry::Accumulation from "mozilla/TelemetryComms.h"; -using mozilla::Telemetry::KeyedAccumulation from "mozilla/TelemetryComms.h"; union ChromeRegistryItem { @@ -1218,12 +1216,6 @@ parent: async UnstoreAndBroadcastBlobURLUnregistration(nsCString url); - /** - * Messages for communicating child Telemetry to the parent process - */ - async AccumulateChildHistogram(Accumulation[] accumulations); - async AccumulateChildKeyedHistogram(KeyedAccumulation[] accumulations); - both: async AsyncMessage(nsString aMessage, CpowEntry[] aCpows, Principal aPrincipal, ClonedMessageData aData); diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 1d9a3a0da3e2..f9854c70cd21 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -2359,13 +2359,6 @@ TelemetryImpl::ClearScalars() return NS_OK; } -NS_IMETHODIMP -TelemetryImpl::FlushBatchedChildTelemetry() -{ - TelemetryHistogram::IPCTimerFired(nullptr, nullptr); - return NS_OK; -} - size_t TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) { @@ -2811,18 +2804,6 @@ AccumulateTimeDelta(ID aHistogram, TimeStamp start, TimeStamp end) static_cast((end - start).ToMilliseconds())); } -void -AccumulateChild(const nsTArray& aAccumulations) -{ - TelemetryHistogram::AccumulateChild(aAccumulations); -} - -void -AccumulateChildKeyed(const nsTArray& aAccumulations) -{ - TelemetryHistogram::AccumulateChildKeyed(aAccumulations); -} - void ClearHistogram(ID aId) { diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index 0204036f4b03..6084c209898e 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -33,9 +33,6 @@ namespace HangMonitor { } // namespace HangMonitor namespace Telemetry { -struct Accumulation; -struct KeyedAccumulation; - enum TimerResolution { Millisecond, Microsecond @@ -128,20 +125,6 @@ void AccumulateCategorical(ID id, const nsCString& label); */ void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now()); -/** - * Accumulate child data into child histograms - * - * @param aAccumulations - accumulation actions to perform - */ -void AccumulateChild(const nsTArray& aAccumulations); - -/** - * Accumulate child data into child keyed histograms - * - * @param aAccumulations - accumulation actions to perform - */ -void AccumulateChildKeyed(const nsTArray& aAccumulations); - /** * This clears the data for a histogram in TelemetryHistogramEnums.h. * diff --git a/toolkit/components/telemetry/TelemetryComms.h b/toolkit/components/telemetry/TelemetryComms.h deleted file mode 100644 index 0f2d888e3198..000000000000 --- a/toolkit/components/telemetry/TelemetryComms.h +++ /dev/null @@ -1,84 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2; -*- */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - */ - -#ifndef Telemetry_Comms_h__ -#define Telemetry_Comms_h__ - -#include "ipc/IPCMessageUtils.h" - -namespace mozilla { -namespace Telemetry { - -enum ID : uint32_t; - -struct Accumulation -{ - mozilla::Telemetry::ID mId; - uint32_t mSample; -}; - -struct KeyedAccumulation -{ - mozilla::Telemetry::ID mId; - uint32_t mSample; - nsCString mKey; -}; - -} // namespace Telemetry -} // namespace mozilla - -namespace IPC { - -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::Accumulation paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - aMsg->WriteUInt32(aParam.mId); - WriteParam(aMsg, aParam.mSample); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!aMsg->ReadUInt32(aIter, reinterpret_cast(&(aResult->mId))) || - !ReadParam(aMsg, aIter, &(aResult->mSample))) { - return false; - } - - return true; - } -}; - -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::KeyedAccumulation paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - aMsg->WriteUInt32(aParam.mId); - WriteParam(aMsg, aParam.mSample); - WriteParam(aMsg, aParam.mKey); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!aMsg->ReadUInt32(aIter, reinterpret_cast(&(aResult->mId))) || - !ReadParam(aMsg, aIter, &(aResult->mSample)) || - !ReadParam(aMsg, aIter, &(aResult->mKey))) { - return false; - } - - return true; - } -}; - -} // namespace IPC - -#endif // Telemetry_Comms_h__ diff --git a/toolkit/components/telemetry/TelemetryHistogram.cpp b/toolkit/components/telemetry/TelemetryHistogram.cpp index 969adfaa3d46..c8711bee1243 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.cpp +++ b/toolkit/components/telemetry/TelemetryHistogram.cpp @@ -14,13 +14,9 @@ #include "nsClassHashtable.h" #include "nsITelemetry.h" -#include "mozilla/dom/ContentChild.h" #include "mozilla/dom/ToJSValue.h" -#include "mozilla/Atomics.h" #include "mozilla/StartupTimeline.h" #include "mozilla/StaticMutex.h" -#include "mozilla/StaticPtr.h" -#include "mozilla/Unused.h" #include "TelemetryCommon.h" #include "TelemetryHistogram.h" @@ -35,9 +31,6 @@ using base::FlagHistogram; using base::LinearHistogram; using mozilla::StaticMutex; using mozilla::StaticMutexAutoLock; -using mozilla::StaticAutoPtr; -using mozilla::Telemetry::Accumulation; -using mozilla::Telemetry::KeyedAccumulation; //////////////////////////////////////////////////////////////////////// @@ -99,7 +92,6 @@ using mozilla::Telemetry::KeyedAccumulation; #define EXPIRED_ID "__expired__" #define SUBSESSION_HISTOGRAM_PREFIX "sub#" #define KEYED_HISTOGRAM_NAME_SEPARATOR "#" -#define CHILD_HISTOGRAM_SUFFIX "#content" namespace { @@ -193,13 +185,6 @@ AddonMapType gAddonMap; // The singleton StatisticsRecorder object for this process. base::StatisticsRecorder* gStatisticsRecorder = nullptr; -// For batching and sending child process accumulations to the parent -nsITimer* gIPCTimer = nullptr; -mozilla::Atomic gIPCTimerArmed(false); -mozilla::Atomic gIPCTimerArming(false); -StaticAutoPtr> gAccumulations; -StaticAutoPtr> gKeyedAccumulations; - } // namespace @@ -219,12 +204,6 @@ const mozilla::Telemetry::ID kRecordingInitiallyDisabledIDs[] = { mozilla::Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD }; -// Sending each remote accumulation immediately places undue strain on the -// IPC subsystem. Batch the remote accumulations for a period of time before -// sending them all at once. This value was chosen as a balance between data -// timeliness and performance (see bug 1218576) -const uint32_t kBatchTimeoutMs = 2000; - } // namespace @@ -338,16 +317,6 @@ HistogramInfo::label_id(const char* label, uint32_t* labelId) const return NS_ERROR_FAILURE; } -bool -StringEndsWith(const std::string& name, const std::string& suffix) -{ - if (name.size() < suffix.size()) { - return false; - } - - return name.compare(name.size() - suffix.size(), suffix.size(), suffix) == 0; -} - } // namespace @@ -432,18 +401,6 @@ internal_HistogramGet(const char *name, const char *expiration, return NS_OK; } -CharPtrEntryType* -internal_GetHistogramMapEntry(const char* name) -{ - nsDependentCString histogramName(name); - NS_NAMED_LITERAL_CSTRING(suffix, CHILD_HISTOGRAM_SUFFIX); - if (!StringEndsWith(histogramName, suffix)) { - return gHistogramMap.GetEntry(name); - } - auto root = Substring(histogramName, 0, histogramName.Length() - suffix.Length()); - return gHistogramMap.GetEntry(PromiseFlatCString(root).get()); -} - nsresult internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) { @@ -451,7 +408,7 @@ internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) return NS_ERROR_FAILURE; } - CharPtrEntryType *entry = internal_GetHistogramMapEntry(name); + CharPtrEntryType *entry = gHistogramMap.GetEntry(name); if (!entry) { return NS_ERROR_INVALID_ARG; } @@ -461,12 +418,10 @@ internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) // O(1) histogram lookup by numeric id nsresult -internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, - bool child = false) +internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) { static Histogram* knownHistograms[mozilla::Telemetry::HistogramCount] = {0}; - static Histogram* knownChildHistograms[mozilla::Telemetry::HistogramCount] = {0}; - Histogram *h = child ? knownChildHistograms[id] : knownHistograms[id]; + Histogram *h = knownHistograms[id]; if (h) { *ret = h; return NS_OK; @@ -477,15 +432,8 @@ internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, return NS_ERROR_FAILURE; } - nsCString histogramName; - histogramName.Append(p.id()); - if (child) { - histogramName.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - } - - nsresult rv = internal_HistogramGet(histogramName.get(), p.expiration(), - p.histogramType, p.min, p.max, - p.bucketCount, true, &h); + nsresult rv = internal_HistogramGet(p.id(), p.expiration(), p.histogramType, + p.min, p.max, p.bucketCount, true, &h); if (NS_FAILED(rv)) return rv; @@ -505,11 +453,7 @@ internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, } #endif - if (child) { - *ret = knownChildHistograms[id] = h; - } else { - *ret = knownHistograms[id] = h; - } + *ret = knownHistograms[id] = h; return NS_OK; } @@ -523,9 +467,7 @@ internal_GetHistogramByName(const nsACString &name, Histogram **ret) return rv; } - bool isChild = StringEndsWith(name, - NS_LITERAL_CSTRING(CHILD_HISTOGRAM_SUFFIX)); - rv = internal_GetHistogramByEnumId(id, ret, isChild); + rv = internal_GetHistogramByEnumId(id, ret); if (NS_FAILED(rv)) return rv; @@ -581,7 +523,6 @@ internal_CloneHistogram(const nsACString& newName, } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) - Histogram* internal_GetSubsessionHistogram(Histogram& existing) { @@ -592,14 +533,9 @@ internal_GetSubsessionHistogram(Histogram& existing) return nullptr; } - bool isChild = StringEndsWith(existing.histogram_name(), - CHILD_HISTOGRAM_SUFFIX); - static Histogram* subsession[mozilla::Telemetry::HistogramCount] = {}; - static Histogram* subsessionChild[mozilla::Telemetry::HistogramCount] = {}; - Histogram* cached = isChild ? subsessionChild[id] : subsession[id]; - if (cached) { - return cached; + if (subsession[id]) { + return subsession[id]; } NS_NAMED_LITERAL_CSTRING(prefix, SUBSESSION_HISTOGRAM_PREFIX); @@ -609,15 +545,10 @@ internal_GetSubsessionHistogram(Histogram& existing) } nsCString subsessionName(prefix); - subsessionName.Append(existing.histogram_name().c_str()); + subsessionName.Append(existingName); - Histogram* clone = internal_CloneHistogram(subsessionName, id, existing); - if (isChild) { - subsessionChild[id] = clone; - } else { - subsession[id] = clone; - } - return clone; + subsession[id] = internal_CloneHistogram(subsessionName, id, existing); + return subsession[id]; } #endif @@ -666,13 +597,26 @@ internal_HistogramAdd(Histogram& histogram, int32_t value) return internal_HistogramAdd(histogram, value, dataset); } +nsresult +internal_HistogramAddCategorical(mozilla::Telemetry::ID id, const nsCString& label) +{ + uint32_t labelId = 0; + if (NS_FAILED(gHistograms[id].label_id(label.get(), &labelId))) { + return NS_ERROR_ILLEGAL_VALUE; + } + + Histogram* h = nullptr; + nsresult rv = internal_GetHistogramByEnumId(id, &h); + if (NS_FAILED(rv)) { + return rv; + } + + return internal_HistogramAdd(*h, labelId); +} + void internal_HistogramClear(Histogram& aHistogram, bool onlySubsession) { - MOZ_ASSERT(XRE_IsParentProcess()); - if (!XRE_IsParentProcess()) { - return; - } if (!onlySubsession) { aHistogram.Clear(); } @@ -868,8 +812,6 @@ public: nsresult Add(const nsCString& key, uint32_t aSample); void Clear(bool subsession); - nsresult GetEnumId(mozilla::Telemetry::ID& id); - private: typedef nsBaseHashtableET KeyedHistogramEntry; typedef AutoHashtable KeyedHistogramMapType; @@ -1012,10 +954,6 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample) void KeyedHistogram::Clear(bool onlySubsession) { - MOZ_ASSERT(XRE_IsParentProcess()); - if (!XRE_IsParentProcess()) { - return; - } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) for (auto iter = mSubsessionMap.Iter(); !iter.Done(); iter.Next()) { iter.Get()->mData->Clear(); @@ -1103,12 +1041,6 @@ KeyedHistogram::GetJSSnapshot(JSContext* cx, JS::Handle obj, return NS_OK; } -nsresult -KeyedHistogram::GetEnumId(mozilla::Telemetry::ID& id) -{ - return internal_GetHistogramEnumId(mName.get(), &id); -} - } // namespace @@ -1134,305 +1066,6 @@ internal_GetKeyedHistogramById(const nsACString &name) } // namespace -//////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////// -// -// PRIVATE: functions related to addon histograms - -namespace { - -// Compute the name to pass into Histogram for the addon histogram -// 'name' from the addon 'id'. We can't use 'name' directly because it -// might conflict with other histograms in other addons or even with our -// own. -void -internal_AddonHistogramName(const nsACString &id, const nsACString &name, - nsACString &ret) -{ - ret.Append(id); - ret.Append(':'); - ret.Append(name); -} - -bool -internal_CreateHistogramForAddon(const nsACString &name, - AddonHistogramInfo &info) -{ - Histogram *h; - nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), "never", - info.histogramType, info.min, info.max, - info.bucketCount, true, &h); - if (NS_FAILED(rv)) { - return false; - } - // Don't let this histogram be reported via the normal means - // (e.g. Telemetry.registeredHistograms); we'll make it available in - // other ways. - h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); - info.h = h; - return true; -} - -bool -internal_AddonHistogramReflector(AddonHistogramEntryType *entry, - JSContext *cx, JS::Handle obj) -{ - AddonHistogramInfo &info = entry->mData; - - // Never even accessed the histogram. - if (!info.h) { - // Have to force creation of HISTOGRAM_FLAG histograms. - if (info.histogramType != nsITelemetry::HISTOGRAM_FLAG) - return true; - - if (!internal_CreateHistogramForAddon(entry->GetKey(), info)) { - return false; - } - } - - if (internal_IsEmpty(info.h)) { - return true; - } - - JS::Rooted snapshot(cx, JS_NewPlainObject(cx)); - if (!snapshot) { - // Just consider this to be skippable. - return true; - } - switch (internal_ReflectHistogramSnapshot(cx, snapshot, info.h)) { - case REFLECT_FAILURE: - case REFLECT_CORRUPT: - return false; - case REFLECT_OK: - const nsACString &histogramName = entry->GetKey(); - if (!JS_DefineProperty(cx, obj, PromiseFlatCString(histogramName).get(), - snapshot, JSPROP_ENUMERATE)) { - return false; - } - break; - } - return true; -} - -bool -internal_AddonReflector(AddonEntryType *entry, JSContext *cx, - JS::Handle obj) -{ - const nsACString &addonId = entry->GetKey(); - JS::Rooted subobj(cx, JS_NewPlainObject(cx)); - if (!subobj) { - return false; - } - - AddonHistogramMapType *map = entry->mData; - if (!(map->ReflectIntoJS(internal_AddonHistogramReflector, cx, subobj) - && JS_DefineProperty(cx, obj, PromiseFlatCString(addonId).get(), - subobj, JSPROP_ENUMERATE))) { - return false; - } - return true; -} - -} // namespace - - -//////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////// -// -// PRIVATE: thread-unsafe helpers for the external interface - -// This is a StaticMutex rather than a plain Mutex (1) so that -// it gets initialised in a thread-safe manner the first time -// it is used, and (2) because it is never de-initialised, and -// a normal Mutex would show up as a leak in BloatView. StaticMutex -// also has the "OffTheBooks" property, so it won't show as a leak -// in BloatView. -static StaticMutex gTelemetryHistogramMutex; - -namespace { - -void -internal_SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) -{ - if (!internal_IsHistogramEnumId(aID)) { - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); - return; - } - - if (gHistograms[aID].keyed) { - const nsDependentCString id(gHistograms[aID].id()); - KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); - if (keyed) { - keyed->SetRecordingEnabled(aEnabled); - return; - } - } else { - Histogram *h; - nsresult rv = internal_GetHistogramByEnumId(aID, &h); - if (NS_SUCCEEDED(rv)) { - h->SetRecordingEnabled(aEnabled); - return; - } - } - - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); -} - -void internal_armIPCTimerMainThread() -{ - MOZ_ASSERT(NS_IsMainThread()); - gIPCTimerArming = false; - if (gIPCTimerArmed) { - return; - } - if (!gIPCTimer) { - CallCreateInstance(NS_TIMER_CONTRACTID, &gIPCTimer); - } - if (gIPCTimer) { - gIPCTimer->InitWithFuncCallback(TelemetryHistogram::IPCTimerFired, - nullptr, kBatchTimeoutMs, - nsITimer::TYPE_ONE_SHOT); - gIPCTimerArmed = true; - } -} - -void internal_armIPCTimer() -{ - if (gIPCTimerArmed || gIPCTimerArming) { - return; - } - gIPCTimerArming = true; - if (NS_IsMainThread()) { - internal_armIPCTimerMainThread(); - } else { - NS_DispatchToMainThread(NS_NewRunnableFunction([]() -> void { - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - internal_armIPCTimerMainThread(); - })); - } -} - -bool -internal_RemoteAccumulate(mozilla::Telemetry::ID aId, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - return false; - } - if (!gAccumulations) { - gAccumulations = new nsTArray(); - } - gAccumulations->AppendElement(Accumulation{aId, aSample}); - internal_armIPCTimer(); - return true; -} - -bool -internal_RemoteAccumulate(mozilla::Telemetry::ID aId, - const nsCString& aKey, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - return false; - } - if (!gKeyedAccumulations) { - gKeyedAccumulations = new nsTArray(); - } - gKeyedAccumulations->AppendElement(KeyedAccumulation{aId, aSample, aKey}); - internal_armIPCTimer(); - return true; -} - -void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) -{ - if (!internal_CanRecordBase() || - internal_RemoteAccumulate(aHistogram, aSample)) { - return; - } - Histogram *h; - nsresult rv = internal_GetHistogramByEnumId(aHistogram, &h); - if (NS_SUCCEEDED(rv)) { - internal_HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); - } -} - -void -internal_Accumulate(mozilla::Telemetry::ID aID, - const nsCString& aKey, uint32_t aSample) -{ - if (!gInitDone || !internal_CanRecordBase() || - internal_RemoteAccumulate(aID, aKey, aSample)) { - return; - } - const HistogramInfo& th = gHistograms[aID]; - KeyedHistogram* keyed - = internal_GetKeyedHistogramById(nsDependentCString(th.id())); - MOZ_ASSERT(keyed); - keyed->Add(aKey, aSample); -} - -void -internal_Accumulate(Histogram& aHistogram, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - internal_HistogramAdd(aHistogram, aSample); - return; - } - - mozilla::Telemetry::ID id; - nsresult rv = internal_GetHistogramEnumId(aHistogram.histogram_name().c_str(), &id); - if (NS_SUCCEEDED(rv)) { - internal_RemoteAccumulate(id, aSample); - } -} - -void -internal_Accumulate(KeyedHistogram& aKeyed, - const nsCString& aKey, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - aKeyed.Add(aKey, aSample); - return; - } - - mozilla::Telemetry::ID id; - if (NS_SUCCEEDED(aKeyed.GetEnumId(id))) { - internal_RemoteAccumulate(id, aKey, aSample); - } -} - -void -internal_AccumulateChild(mozilla::Telemetry::ID aId, uint32_t aSample) -{ - if (!internal_CanRecordBase()) { - return; - } - Histogram* h; - nsresult rv = internal_GetHistogramByEnumId(aId, &h, true); - if (NS_SUCCEEDED(rv)) { - internal_HistogramAdd(*h, aSample, gHistograms[aId].dataset); - } else { - NS_WARNING("NS_FAILED GetHistogramByEnumId for CHILD"); - } -} - -void -internal_AccumulateChildKeyed(mozilla::Telemetry::ID aId, - const nsCString& aKey, uint32_t aSample) -{ - if (!gInitDone || !internal_CanRecordBase()) { - return; - } - const HistogramInfo& th = gHistograms[aId]; - nsCString id; - id.Append(th.id()); - id.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); - MOZ_ASSERT(keyed); - keyed->Add(aKey, aSample); -} - -} // namespace - - //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // @@ -1475,47 +1108,52 @@ internal_JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) return true; } - uint32_t value = 0; - mozilla::Telemetry::ID id; + // If we don't have an argument for the count histogram, assume an increment of 1. + // Otherwise, make sure to run some sanity checks on the argument. if ((type == base::CountHistogram::COUNT_HISTOGRAM) && (args.length() == 0)) { - // If we don't have an argument for the count histogram, assume an increment of 1. - // Otherwise, make sure to run some sanity checks on the argument. - value = 1; - } else if (type == base::LinearHistogram::LINEAR_HISTOGRAM && + internal_HistogramAdd(*h, 1); + return true; + } + + // For categorical histograms we allow passing a string argument that specifies the label. + mozilla::Telemetry::ID id; + if (type == base::LinearHistogram::LINEAR_HISTOGRAM && (args.length() > 0) && args[0].isString() && NS_SUCCEEDED(internal_GetHistogramEnumId(h->histogram_name().c_str(), &id)) && gHistograms[id].histogramType == nsITelemetry::HISTOGRAM_CATEGORICAL) { - // For categorical histograms we allow passing a string argument that specifies the label. nsAutoJSString label; if (!label.init(cx, args[0])) { JS_ReportError(cx, "Invalid string parameter"); return false; } - nsresult rv = gHistograms[id].label_id(NS_ConvertUTF16toUTF8(label).get(), &value); + nsresult rv = internal_HistogramAddCategorical(id, NS_ConvertUTF16toUTF8(label)); if (NS_FAILED(rv)) { JS_ReportError(cx, "Unknown label for categorical histogram"); return false; } - } else { - // All other accumulations expect one numerical argument. - if (!args.length()) { - JS_ReportError(cx, "Expected one argument"); - return false; - } - if (!(args[0].isNumber() || args[0].isBoolean())) { - JS_ReportError(cx, "Not a number"); - return false; - } - - if (!JS::ToUint32(cx, args[0], &value)) { - JS_ReportError(cx, "Failed to convert argument"); - return false; - } + return true; } - internal_Accumulate(*h, value); + // All other accumulations expect one numerical argument. + int32_t value = 0; + if (!args.length()) { + JS_ReportError(cx, "Expected one argument"); + return false; + } + + if (!(args[0].isNumber() || args[0].isBoolean())) { + JS_ReportError(cx, "Not a number"); + return false; + } + + if (!JS::ToInt32(cx, args[0], &value)) { + JS_ReportError(cx, "Failed to convert argument"); + return false; + } + + internal_HistogramAdd(*h, value); return true; } @@ -1763,7 +1401,7 @@ internal_JSKeyedHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) } } - internal_Accumulate(*keyed, NS_ConvertUTF16toUTF8(key), value); + keyed->Add(NS_ConvertUTF16toUTF8(key), value); return true; } @@ -1913,11 +1551,184 @@ internal_WrapAndReturnKeyedHistogram(KeyedHistogram *h, JSContext *cx, } // namespace +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// PRIVATE: functions related to addon histograms + +namespace { + +// Compute the name to pass into Histogram for the addon histogram +// 'name' from the addon 'id'. We can't use 'name' directly because it +// might conflict with other histograms in other addons or even with our +// own. +void +internal_AddonHistogramName(const nsACString &id, const nsACString &name, + nsACString &ret) +{ + ret.Append(id); + ret.Append(':'); + ret.Append(name); +} + +bool +internal_CreateHistogramForAddon(const nsACString &name, + AddonHistogramInfo &info) +{ + Histogram *h; + nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), "never", + info.histogramType, info.min, info.max, + info.bucketCount, true, &h); + if (NS_FAILED(rv)) { + return false; + } + // Don't let this histogram be reported via the normal means + // (e.g. Telemetry.registeredHistograms); we'll make it available in + // other ways. + h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); + info.h = h; + return true; +} + +bool +internal_AddonHistogramReflector(AddonHistogramEntryType *entry, + JSContext *cx, JS::Handle obj) +{ + AddonHistogramInfo &info = entry->mData; + + // Never even accessed the histogram. + if (!info.h) { + // Have to force creation of HISTOGRAM_FLAG histograms. + if (info.histogramType != nsITelemetry::HISTOGRAM_FLAG) + return true; + + if (!internal_CreateHistogramForAddon(entry->GetKey(), info)) { + return false; + } + } + + if (internal_IsEmpty(info.h)) { + return true; + } + + JS::Rooted snapshot(cx, JS_NewPlainObject(cx)); + if (!snapshot) { + // Just consider this to be skippable. + return true; + } + switch (internal_ReflectHistogramSnapshot(cx, snapshot, info.h)) { + case REFLECT_FAILURE: + case REFLECT_CORRUPT: + return false; + case REFLECT_OK: + const nsACString &histogramName = entry->GetKey(); + if (!JS_DefineProperty(cx, obj, PromiseFlatCString(histogramName).get(), + snapshot, JSPROP_ENUMERATE)) { + return false; + } + break; + } + return true; +} + +bool +internal_AddonReflector(AddonEntryType *entry, JSContext *cx, + JS::Handle obj) +{ + const nsACString &addonId = entry->GetKey(); + JS::Rooted subobj(cx, JS_NewPlainObject(cx)); + if (!subobj) { + return false; + } + + AddonHistogramMapType *map = entry->mData; + if (!(map->ReflectIntoJS(internal_AddonHistogramReflector, cx, subobj) + && JS_DefineProperty(cx, obj, PromiseFlatCString(addonId).get(), + subobj, JSPROP_ENUMERATE))) { + return false; + } + return true; +} + +} // namespace + + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// PRIVATE: thread-unsafe helpers for the external interface + +namespace { + +void +internal_SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) +{ + if (!internal_IsHistogramEnumId(aID)) { + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); + return; + } + + if (gHistograms[aID].keyed) { + const nsDependentCString id(gHistograms[aID].id()); + KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); + if (keyed) { + keyed->SetRecordingEnabled(aEnabled); + return; + } + } else { + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aID, &h); + if (NS_SUCCEEDED(rv)) { + h->SetRecordingEnabled(aEnabled); + return; + } + } + + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); +} + +void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) +{ + if (!internal_CanRecordBase()) { + return; + } + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aHistogram, &h); + if (NS_SUCCEEDED(rv)) { + internal_HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); + } +} + +void +internal_Accumulate(mozilla::Telemetry::ID aID, + const nsCString& aKey, uint32_t aSample) +{ + if (!gInitDone || !internal_CanRecordBase()) { + return; + } + const HistogramInfo& th = gHistograms[aID]; + KeyedHistogram* keyed + = internal_GetKeyedHistogramById(nsDependentCString(th.id())); + MOZ_ASSERT(keyed); + keyed->Add(aKey, aSample); +} + +} // namespace + + //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // // EXTERNALLY VISIBLE FUNCTIONS in namespace TelemetryHistogram:: +// This is a StaticMutex rather than a plain Mutex (1) so that +// it gets initialised in a thread-safe manner the first time +// it is used, and (2) because it is never de-initialised, and +// a normal Mutex would show up as a leak in BloatView. StaticMutex +// also has the "OffTheBooks" property, so it won't show as a leak +// in BloatView. +static StaticMutex gTelemetryHistogramMutex; + // All of these functions are actually in namespace TelemetryHistogram::, // but the ::TelemetryHistogram prefix is given explicitly. This is // because it is critical to see which calls from these functions are @@ -1981,16 +1792,6 @@ void TelemetryHistogram::InitializeGlobalState(bool canRecordBase, const nsDependentCString expiration(h.expiration()); gKeyedHistograms.Put(id, new KeyedHistogram(id, expiration, h.histogramType, h.min, h.max, h.bucketCount, h.dataset)); - if (XRE_IsParentProcess()) { - // We must create registered child keyed histograms as well or else the - // same code in TelemetrySession.jsm that fails without parent keyed - // histograms will fail without child keyed histograms. - nsCString childId(id); - childId.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - gKeyedHistograms.Put(childId, - new KeyedHistogram(id, expiration, h.histogramType, - h.min, h.max, h.bucketCount, h.dataset)); - } } // Some Telemetry histograms depend on the value of C++ constants and hardcode @@ -2021,11 +1822,6 @@ void TelemetryHistogram::DeInitializeGlobalState() gHistogramMap.Clear(); gKeyedHistograms.Clear(); gAddonMap.Clear(); - gAccumulations = nullptr; - gKeyedAccumulations = nullptr; - if (gIPCTimer) { - NS_RELEASE(gIPCTimer); - } gInitDone = false; } @@ -2131,7 +1927,12 @@ TelemetryHistogram::Accumulate(const char* name, uint32_t sample) if (NS_FAILED(rv)) { return; } - internal_Accumulate(id, sample); + + Histogram *h; + rv = internal_GetHistogramByEnumId(id, &h); + if (NS_SUCCEEDED(rv)) { + internal_HistogramAdd(*h, sample, gHistograms[id].dataset); + } } void @@ -2154,52 +1955,7 @@ TelemetryHistogram::AccumulateCategorical(mozilla::Telemetry::ID aId, const nsCString& label) { StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - uint32_t labelId = 0; - if (NS_FAILED(gHistograms[aId].label_id(label.get(), &labelId))) { - return; - } - internal_Accumulate(aId, labelId); -} - -void -TelemetryHistogram::AccumulateChild(const nsTArray& aAccumulations) -{ - MOZ_ASSERT(XRE_IsParentProcess()); - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - for (uint32_t i = 0; i < aAccumulations.Length(); ++i) { - bool isValid = internal_IsHistogramEnumId(aAccumulations[i].mId); - MOZ_ASSERT(isValid); - if (!isValid) { - continue; - } - internal_AccumulateChild(aAccumulations[i].mId, aAccumulations[i].mSample); - } -} - -void -TelemetryHistogram::AccumulateChildKeyed(const nsTArray& aAccumulations) -{ - MOZ_ASSERT(XRE_IsParentProcess()); - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - for (uint32_t i = 0; i < aAccumulations.Length(); ++i) { - bool isValid = internal_IsHistogramEnumId(aAccumulations[i].mId); - MOZ_ASSERT(isValid); - if (!isValid) { - continue; - } - internal_AccumulateChildKeyed(aAccumulations[i].mId, - aAccumulations[i].mKey, - aAccumulations[i].mSample); - } + internal_HistogramAddCategorical(aId, label); } void @@ -2344,8 +2100,6 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext *cx, mozilla::DebugOnly rv = internal_GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h); MOZ_ASSERT(NS_SUCCEEDED(rv)); - rv = internal_GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h, true); - MOZ_ASSERT(NS_SUCCEEDED(rv)); } } @@ -2609,43 +2363,3 @@ TelemetryHistogram::GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf } return n; } - -// 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) -// -// To ensure we don't loop IPCTimerFired->AccumulateChild->arm timer, we don't -// unset gIPCTimerArmed until the IPC completes -// -// This function must be called on the main thread, otherwise IPC will fail. -void -TelemetryHistogram::IPCTimerFired(nsITimer* aTimer, void* aClosure) -{ - MOZ_ASSERT(NS_IsMainThread()); - nsTArray accumulationsToSend; - nsTArray keyedAccumulationsToSend; - { - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (gAccumulations) { - accumulationsToSend.SwapElements(*gAccumulations); - } - if (gKeyedAccumulations) { - keyedAccumulationsToSend.SwapElements(*gKeyedAccumulations); - } - } - - mozilla::dom::ContentChild* contentChild = mozilla::dom::ContentChild::GetSingleton(); - mozilla::Unused << NS_WARN_IF(!contentChild); - if (contentChild) { - if (accumulationsToSend.Length()) { - mozilla::Unused << - NS_WARN_IF(!contentChild->SendAccumulateChildHistogram(accumulationsToSend)); - } - if (keyedAccumulationsToSend.Length()) { - mozilla::Unused << - NS_WARN_IF(!contentChild->SendAccumulateChildKeyedHistogram(keyedAccumulationsToSend)); - } - } - - gIPCTimerArmed = false; -} diff --git a/toolkit/components/telemetry/TelemetryHistogram.h b/toolkit/components/telemetry/TelemetryHistogram.h index ff19a5efae1d..4bb83dbf5f30 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.h +++ b/toolkit/components/telemetry/TelemetryHistogram.h @@ -8,8 +8,6 @@ #include "mozilla/TelemetryHistogramEnums.h" -#include "mozilla/TelemetryComms.h" - // This module is internal to Telemetry. It encapsulates Telemetry's // histogram accumulation and storage logic. It should only be used by // Telemetry.cpp. These functions should not be used anywhere else. @@ -44,9 +42,6 @@ void Accumulate(const char* name, const nsCString& key, uint32_t sample); void AccumulateCategorical(mozilla::Telemetry::ID aId, const nsCString& aLabel); -void AccumulateChild(const nsTArray& aAccumulations); -void AccumulateChildKeyed(const nsTArray& aAccumulations); - void ClearHistogram(mozilla::Telemetry::ID aId); @@ -107,8 +102,6 @@ GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf); size_t GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf aMallocSizeOf); -void -IPCTimerFired(nsITimer* aTimer, void* aClosure); } // namespace TelemetryHistogram #endif // TelemetryHistogram_h__ diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index c20fdbe22d3a..68d7f0e8c981 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -41,11 +41,6 @@ const REASON_TEST_PING = "test-ping"; const REASON_ENVIRONMENT_CHANGE = "environment-change"; const REASON_SHUTDOWN = "shutdown"; -const HISTOGRAM_SUFFIXES = { - PARENT: "", - CONTENT: "#content", -} - const ENVIRONMENT_CHANGE_LISTENER = "TelemetrySession::onEnvironmentChange"; const MS_IN_ONE_HOUR = 60 * 60 * 1000; @@ -62,6 +57,7 @@ const PREF_UNIFIED = PREF_BRANCH + "unified"; const MESSAGE_TELEMETRY_PAYLOAD = "Telemetry:Payload"; +const MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD = "Telemetry:GetChildPayload"; const MESSAGE_TELEMETRY_THREAD_HANGS = "Telemetry:ChildThreadHangs"; const MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS = "Telemetry:GetChildThreadHangs"; const MESSAGE_TELEMETRY_USS = "Telemetry:USS"; @@ -544,6 +540,13 @@ this.TelemetrySession = Object.freeze({ getPayload: function(reason, clearSubsession = false) { return Impl.getPayload(reason, clearSubsession); }, + /** + * Asks the content processes to send their payloads. + * @returns Object + */ + requestChildPayloads: function() { + return Impl.requestChildPayloads(); + }, /** * Returns a promise that resolves to an array of thread hang stats from content processes, one entry per process. * The structure of each entry is identical to that of "threadHangStats" in nsITelemetry. @@ -881,28 +884,23 @@ var Impl = { }, getHistograms: function getHistograms(subsession, clearSubsession) { - this._log.trace("getHistograms - subsession: " + subsession + - ", clearSubsession: " + clearSubsession); + this._log.trace("getHistograms - subsession: " + subsession + ", clearSubsession: " + clearSubsession); let registered = Telemetry.registeredHistograms(this.getDatasetType(), []); - if (this._testing == false) { - // Omit telemetry test histograms outside of tests. - registered = registered.filter(n => !n.startsWith("TELEMETRY_TEST_")); - } - registered = registered.concat(registered.map(n => "STARTUP_" + n)); - let hls = subsession ? Telemetry.snapshotSubsessionHistograms(clearSubsession) : Telemetry.histogramSnapshots; let ret = {}; for (let name of registered) { - for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) { - if (name + suffix in hls) { - if (!(suffix in ret)) { - ret[suffix] = {}; + for (let n of [name, "STARTUP_" + name]) { + if (n in hls) { + // Omit telemetry test histograms outside of tests. + if (n.startsWith('TELEMETRY_TEST_') && this._testing == false) { + this._log.trace("getHistograms - Skipping test histogram: " + n); + } else { + ret[n] = this.packHistogram(hls[n]); } - ret[suffix][name] = this.packHistogram(hls[name + suffix]); } } } @@ -930,41 +928,36 @@ var Impl = { }, getKeyedHistograms: function(subsession, clearSubsession) { - this._log.trace("getKeyedHistograms - subsession: " + subsession + - ", clearSubsession: " + clearSubsession); + this._log.trace("getKeyedHistograms - subsession: " + subsession + ", clearSubsession: " + clearSubsession); let registered = Telemetry.registeredKeyedHistograms(this.getDatasetType(), []); - if (this._testing == false) { - // Omit telemetry test histograms outside of tests. - registered = registered.filter(id => !id.startsWith("TELEMETRY_TEST_")); - } let ret = {}; for (let id of registered) { - for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) { - let keyed = Telemetry.getKeyedHistogramById(id + suffix); - let snapshot = null; - if (subsession) { - snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear() - : keyed.subsessionSnapshot(); - } else { - snapshot = keyed.snapshot(); - } + // Omit telemetry test histograms outside of tests. + if (id.startsWith('TELEMETRY_TEST_') && this._testing == false) { + this._log.trace("getKeyedHistograms - Skipping test histogram: " + id); + continue; + } + let keyed = Telemetry.getKeyedHistogramById(id); + let snapshot = null; + if (subsession) { + snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear() + : keyed.subsessionSnapshot(); + } else { + snapshot = keyed.snapshot(); + } - let keys = Object.keys(snapshot); - if (keys.length == 0) { - // Skip empty keyed histogram. - continue; - } + let keys = Object.keys(snapshot); + if (keys.length == 0) { + // Skip empty keyed histogram. + continue; + } - if (!(suffix in ret)) { - ret[suffix] = {}; - } - ret[suffix][id] = {}; - for (let key of keys) { - ret[suffix][id][key] = this.packHistogram(snapshot[key]); - } + ret[id] = {}; + for (let key of keys) { + ret[id][key] = this.packHistogram(snapshot[key]); } } @@ -1250,12 +1243,12 @@ var Impl = { // This allows wrapping data retrieval calls in a try-catch block so that // failures don't break the rest of the ping assembly. - const protect = (fn, defaultReturn = null) => { + const protect = (fn) => { try { return fn(); } catch (ex) { this._log.error("assemblePayloadWithMeasurements - caught exception", ex); - return defaultReturn; + return null; } }; @@ -1263,6 +1256,8 @@ var Impl = { let payloadObj = { ver: PAYLOAD_VERSION, simpleMeasurements: simpleMeasurements, + histograms: protect(() => this.getHistograms(isSubsession, clearSubsession)), + keyedHistograms: protect(() => this.getKeyedHistograms(isSubsession, clearSubsession)), }; // Add extended set measurements common to chrome & content processes @@ -1277,21 +1272,14 @@ var Impl = { return payloadObj; } - // Additional payload for chrome process. - let histograms = protect(() => this.getHistograms(isSubsession, clearSubsession), {}); - let keyedHistograms = protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {}); - payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {}; - payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {}; + // Set the scalars for the parent process. payloadObj.processes = { parent: { scalars: protect(() => this.getScalars(isSubsession, clearSubsession)), - }, - content: { - histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT], - keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT], - }, + } }; + // Additional payload for chrome process. payloadObj.info = info; // Add extended set measurements for chrome process. @@ -1533,6 +1521,7 @@ var Impl = { } Services.obs.addObserver(this, "content-child-shutdown", false); + cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD, this); cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS, this); cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_USS, this); @@ -1570,6 +1559,14 @@ var Impl = { let source = message.data.childUUID; delete message.data.childUUID; + for (let child of this._childTelemetry) { + if (child.source === source) { + // Update existing telemetry data. + child.payload = message.data; + return; + } + } + // Did not find existing child in this._childTelemetry. this._childTelemetry.push({ source: source, payload: message.data, @@ -1582,6 +1579,12 @@ var Impl = { break; } + case MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD: + { + // In child process, send the requested Telemetry payload + this.sendContentProcessPing("saved-session"); + break; + } case MESSAGE_TELEMETRY_THREAD_HANGS: { // Accumulate child thread hang stats from this child @@ -1761,6 +1764,11 @@ var Impl = { return this.getSessionPayload(reason, clearSubsession); }, + requestChildPayloads: function() { + this._log.trace("requestChildPayloads"); + ppmm.broadcastAsyncMessage(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD, {}); + }, + getChildThreadHangs: function getChildThreadHangs() { return new Promise((resolve) => { // Return immediately if there are no child processes to get stats from @@ -1838,7 +1846,7 @@ var Impl = { // content-child-shutdown is only registered for content processes. Services.obs.removeObserver(this, "content-child-shutdown"); this.uninstall(); - Telemetry.flushBatchedChildTelemetry(); + this.sendContentProcessPing(REASON_SAVED_SESSION); break; case TOPIC_CYCLE_COLLECTOR_BEGIN: diff --git a/toolkit/components/telemetry/moz.build b/toolkit/components/telemetry/moz.build index 7e38babc2d44..b2cff123823a 100644 --- a/toolkit/components/telemetry/moz.build +++ b/toolkit/components/telemetry/moz.build @@ -19,7 +19,6 @@ EXPORTS.mozilla += [ '!TelemetryScalarEnums.h', 'ProcessedStack.h', 'Telemetry.h', - 'TelemetryComms.h', 'ThreadHangStats.h', ] diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index a07072a3213b..c992b2a8c32d 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -416,10 +416,4 @@ interface nsITelemetry : nsISupports * Resets all the stored scalars. This is intended to be only used in tests. */ void clearScalars(); - - /** - * Immediately sends any Telemetry batched on this process to the parent - * process. This is intended only to be used on process shutdown. - */ - void flushBatchedChildTelemetry(); }; diff --git a/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js b/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js index ad30140d55e0..4233328b600f 100644 --- a/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js +++ b/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js @@ -2,7 +2,6 @@ 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_TELEMETRY_PAYLOAD = "Telemetry:Payload"; const MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD = "Telemetry:GetChildPayload"; @@ -20,9 +19,6 @@ function run_child_test() { let countHist = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT"); countHist.add(); countHist.add(); - let categHist = Telemetry.getHistogramById("TELEMETRY_TEST_CATEGORICAL"); - categHist.add("Label2"); - categHist.add("Label3"); let flagKeyed = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_FLAG"); flagKeyed.add("a", 1); @@ -31,19 +27,20 @@ function run_child_test() { countKeyed.add("a"); countKeyed.add("b"); countKeyed.add("b"); + + // Check payload values. + const payload = TelemetrySession.getPayload("test-ping"); + check_histogram_values(payload); } function check_histogram_values(payload) { const hs = payload.histograms; Assert.ok("TELEMETRY_TEST_COUNT" in hs, "Should have count test histogram."); Assert.ok("TELEMETRY_TEST_FLAG" in hs, "Should have flag test histogram."); - Assert.ok("TELEMETRY_TEST_CATEGORICAL" in hs, "Should have categorical test histogram."); Assert.equal(hs["TELEMETRY_TEST_COUNT"].sum, 2, "Count test histogram should have the right value."); Assert.equal(hs["TELEMETRY_TEST_FLAG"].sum, 1, "Flag test histogram should have the right value."); - Assert.equal(hs["TELEMETRY_TEST_CATEGORICAL"].sum, 3, - "Categorical test histogram should have the right sum."); const kh = payload.keyedHistograms; Assert.ok("TELEMETRY_TEST_KEYED_COUNT" in kh, "Should have keyed count test histogram."); @@ -64,6 +61,8 @@ add_task(function*() { run_child_test(); dump("... done with child test\n"); do_send_remote_message(MESSAGE_CHILD_TEST_DONE); + dump("... waiting for child payload collection\n"); + yield do_await_remote_message(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD); return; } @@ -81,20 +80,20 @@ add_task(function*() { let childPromise = run_test_in_child("test_ChildHistograms.js"); yield do_await_remote_message(MESSAGE_CHILD_TEST_DONE); - yield ContentTaskUtils.waitForCondition(() => { - let payload = TelemetrySession.getPayload("test-ping"); - return payload && - "processes" in payload && - "content" in payload.processes && - "histograms" in payload.processes.content && - "TELEMETRY_TEST_COUNT" in payload.processes.content.histograms; - }); + // Gather payload from child. + dump("... requesting child payloads\n"); + let promiseMessage = do_await_remote_message(MESSAGE_TELEMETRY_PAYLOAD); + TelemetrySession.requestChildPayloads(); + yield promiseMessage; + dump("... received child payload\n"); + + // Check child payload. const payload = TelemetrySession.getPayload("test-ping"); - Assert.ok("processes" in payload, "Should have processes section"); - Assert.ok("content" in payload.processes, "Should have child process section"); - Assert.ok("histograms" in payload.processes.content, "Child process section should have histograms."); - Assert.ok("keyedHistograms" in payload.processes.content, "Child process section should have keyed histograms."); - check_histogram_values(payload.processes.content); + Assert.ok("childPayloads" in payload, "Should have child payloads."); + Assert.equal(payload.childPayloads.length, 1, "Should have received one child payload so far."); + Assert.ok("histograms" in payload.childPayloads[0], "Child payload should have histograms."); + Assert.ok("keyedHistograms" in payload.childPayloads[0], "Child payload should have keyed histograms."); + check_histogram_values(payload.childPayloads[0]); do_test_finished(); }); diff --git a/toolkit/content/aboutTelemetry.css b/toolkit/content/aboutTelemetry.css index 6acf82c201fb..39ade6486080 100644 --- a/toolkit/content/aboutTelemetry.css +++ b/toolkit/content/aboutTelemetry.css @@ -229,14 +229,6 @@ body[dir="rtl"] .copy-node { display: inline; } -.processes-ui { - display: none; -} - -.has-data.expanded .processes-ui { - display: initial; -} - .filter-blocked { display: none; } @@ -265,7 +257,3 @@ body[dir="rtl"] .copy-node { font-size: larger; margin: 5px 0; } - -.process-picker { - margin: 0 0.5em; -} diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js index dd636990d48c..13aa26e382a3 100644 --- a/toolkit/content/aboutTelemetry.js +++ b/toolkit/content/aboutTelemetry.js @@ -306,10 +306,6 @@ var PingPicker = { .addEventListener("click", () => this._movePingIndex(1), false); document.getElementById("choose-payload") .addEventListener("change", () => displayPingData(gPingData), false); - document.getElementById("histograms-processes") - .addEventListener("change", () => displayPingData(gPingData), false); - document.getElementById("keyed-histograms-processes") - .addEventListener("change", () => displayPingData(gPingData), false); }, onPingSourceChanged: function() { @@ -1786,33 +1782,6 @@ function sortStartupMilestones(aSimpleMeasurements) { return result; } -function renderProcessList(ping, selectEl) { - removeAllChildNodes(selectEl); - let option = document.createElement("option"); - option.appendChild(document.createTextNode("parent")); - option.setAttribute("value", ""); - option.selected = true; - selectEl.appendChild(option); - - if (!("processes" in ping.payload)) { - selectEl.disabled = true; - return; - } - selectEl.disabled = false; - - for (let process of Object.keys(ping.payload.processes)) { - // TODO: parent hgrams are on root payload, not in payload.processes.parent - // When/If that gets moved, you'll need to remove this: - if (process === "parent") { - continue; - } - option = document.createElement("option"); - option.appendChild(document.createTextNode(process)); - option.setAttribute("value", process); - selectEl.appendChild(option); - } -} - function renderPayloadList(ping) { // Rebuild the payload select with options: // Parent Payload (selected) @@ -1881,11 +1850,9 @@ function displayPingData(ping, updatePayloadList = false) { const keysHeader = bundle.GetStringFromName("keysHeader"); const valuesHeader = bundle.GetStringFromName("valuesHeader"); - // Update the payload list and process lists + // Update the payload list if (updatePayloadList) { renderPayloadList(ping); - renderProcessList(ping, document.getElementById("histograms-processes")); - renderProcessList(ping, document.getElementById("keyed-histograms-processes")); } // Show general data. @@ -1962,18 +1929,8 @@ function displayPingData(ping, updatePayloadList = false) { removeAllChildNodes(hgramDiv); let histograms = payload.histograms; - - let hgramsSelect = document.getElementById("histograms-processes"); - let hgramsOption = hgramsSelect.selectedOptions.item(0); - let hgramsProcess = hgramsOption.getAttribute("value"); - if (hgramsProcess && - "processes" in ping.payload && - hgramsProcess in ping.payload.processes) { - histograms = ping.payload.processes[hgramsProcess].histograms; - } - hasData = Object.keys(histograms).length > 0; - setHasData("histograms-section", hasData || hgramsSelect.options.length); + setHasData("histograms-section", hasData); if (hasData) { for (let [name, hgram] of Object.entries(histograms)) { @@ -1993,18 +1950,8 @@ function displayPingData(ping, updatePayloadList = false) { let keyedDiv = document.getElementById("keyed-histograms"); removeAllChildNodes(keyedDiv); + setHasData("keyed-histograms-section", false); let keyedHistograms = payload.keyedHistograms; - - let keyedHgramsSelect = document.getElementById("keyed-histograms-processes"); - let keyedHgramsOption = keyedHgramsSelect.selectedOptions.item(0); - let keyedHgramsProcess = keyedHgramsOption.getAttribute("value"); - if (keyedHgramsProcess && - "processes" in ping.payload && - keyedHgramsProcess in ping.payload.processes) { - keyedHistograms = ping.payload.processes[keyedHgramsProcess].keyedHistograms; - } - - setHasData("keyed-histograms-section", keyedHgramsSelect.options.length); if (keyedHistograms) { let hasData = false; for (let [id, keyed] of Object.entries(keyedHistograms)) { @@ -2013,7 +1960,7 @@ function displayPingData(ping, updatePayloadList = false) { KeyedHistogram.render(keyedDiv, id, keyed, {unpacked: true}); } } - setHasData("keyed-histograms-section", hasData || keyedHgramsSelect.options.length); + setHasData("keyed-histograms-section", hasData); } // Show addon histogram data diff --git a/toolkit/content/aboutTelemetry.xhtml b/toolkit/content/aboutTelemetry.xhtml index 8049e3057812..b6d1a80300ad 100644 --- a/toolkit/content/aboutTelemetry.xhtml +++ b/toolkit/content/aboutTelemetry.xhtml @@ -152,9 +152,6 @@ &aboutTelemetry.filterText; -
- -
@@ -164,9 +161,6 @@

&aboutTelemetry.keyedHistogramsSection;

&aboutTelemetry.toggle; &aboutTelemetry.emptySection; -
- -