From 8f236832ffd951ee17a86ce360d1138ba34b4fbb Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Wed, 29 Jun 2022 15:01:51 +0000 Subject: [PATCH] Bug 1758115 - Part 2: Streamline locking, initialization and shutdown for TimelineConsumers, r=smaug The current implementation of TimelineConsumers contains some unnecessary complexity due to how it is initialized as a singleton, and the need for it to be initialized and used in a threadsafe way. This patch attempts to simplify it by making all members static, and removing the need to explicitly observe shutdown for cleanup. In addition, this approach avoids the risk of the type being accessed from off-main-thread during initialization or shutdown. Depends on D150442 Differential Revision: https://phabricator.services.mozilla.com/D150443 --- docshell/base/nsDocShell.cpp | 46 +++---- .../timeline/AutoGlobalTimelineMarker.cpp | 14 +-- .../timeline/AutoRestyleTimelineMarker.cpp | 10 +- docshell/base/timeline/AutoTimelineMarker.cpp | 12 +- docshell/base/timeline/TimelineConsumers.cpp | 117 ++++-------------- docshell/base/timeline/TimelineConsumers.h | 64 ++++------ dom/base/Document.cpp | 5 +- dom/console/Console.cpp | 9 +- dom/events/EventListenerManager.cpp | 9 +- dom/ipc/BrowserChild.cpp | 11 +- dom/messagechannel/MessagePort.cpp | 14 +-- dom/workers/MessageEventRunnable.cpp | 7 +- dom/workers/Worker.cpp | 7 +- dom/workers/WorkerPrivate.cpp | 7 +- layout/base/PresShell.cpp | 10 +- layout/base/nsDocumentViewer.cpp | 6 +- layout/base/nsRefreshDriver.cpp | 19 ++- layout/build/nsLayoutStatics.cpp | 2 - view/nsView.cpp | 7 +- xpcom/base/CycleCollectedJSRuntime.cpp | 5 +- 20 files changed, 127 insertions(+), 254 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index b875c6930639..06c2f7fb1e65 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -2304,20 +2304,15 @@ nsDocShell::SetRecordProfileTimelineMarkers(bool aValue) { return NS_OK; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines) { - return NS_OK; - } - if (aValue) { - MOZ_ASSERT(!timelines->HasConsumer(this)); - timelines->AddConsumer(this); - MOZ_ASSERT(timelines->HasConsumer(this)); + MOZ_ASSERT(!TimelineConsumers::HasConsumer(this)); + TimelineConsumers::AddConsumer(this); + MOZ_ASSERT(TimelineConsumers::HasConsumer(this)); UseEntryScriptProfiling(); } else { - MOZ_ASSERT(timelines->HasConsumer(this)); - timelines->RemoveConsumer(this); - MOZ_ASSERT(!timelines->HasConsumer(this)); + MOZ_ASSERT(TimelineConsumers::HasConsumer(this)); + TimelineConsumers::RemoveConsumer(this); + MOZ_ASSERT(!TimelineConsumers::HasConsumer(this)); UnuseEntryScriptProfiling(); } @@ -2332,15 +2327,10 @@ nsDocShell::GetRecordProfileTimelineMarkers(bool* aValue) { nsresult nsDocShell::PopProfileTimelineMarkers( JSContext* aCx, JS::MutableHandle aOut) { - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines) { - return NS_OK; - } - nsTArray store; SequenceRooter rooter(aCx, &store); - timelines->PopMarkers(this, aCx, store); + TimelineConsumers::PopMarkers(this, aCx, store); if (!ToJSValue(aCx, store, aOut)) { JS_ClearPendingException(aCx); @@ -13434,14 +13424,11 @@ void nsDocShell::NotifyJSRunToCompletionStart(const char* aReason, JS::Handle aAsyncStack, const char* aAsyncCause) { // If first start, mark interval start. - if (mJSRunToCompletionDepth == 0) { - RefPtr timelines = TimelineConsumers::Get(); - if (timelines && timelines->HasConsumer(this)) { - timelines->AddMarkerForDocShell( - this, mozilla::MakeUnique( - aReason, aFunctionName, aFilename, aLineNumber, - MarkerTracingType::START, aAsyncStack, aAsyncCause)); - } + if (mJSRunToCompletionDepth == 0 && TimelineConsumers::HasConsumer(this)) { + TimelineConsumers::AddMarkerForDocShell( + this, mozilla::MakeUnique( + aReason, aFunctionName, aFilename, aLineNumber, + MarkerTracingType::START, aAsyncStack, aAsyncCause)); } mJSRunToCompletionDepth++; @@ -13451,12 +13438,9 @@ void nsDocShell::NotifyJSRunToCompletionStop() { mJSRunToCompletionDepth--; // If last stop, mark interval end. - if (mJSRunToCompletionDepth == 0) { - RefPtr timelines = TimelineConsumers::Get(); - if (timelines && timelines->HasConsumer(this)) { - timelines->AddMarkerForDocShell(this, "Javascript", - MarkerTracingType::END); - } + if (mJSRunToCompletionDepth == 0 && TimelineConsumers::HasConsumer(this)) { + TimelineConsumers::AddMarkerForDocShell(this, "Javascript", + MarkerTracingType::END); } } diff --git a/docshell/base/timeline/AutoGlobalTimelineMarker.cpp b/docshell/base/timeline/AutoGlobalTimelineMarker.cpp index d8814a960d54..bf5a19cbab33 100644 --- a/docshell/base/timeline/AutoGlobalTimelineMarker.cpp +++ b/docshell/base/timeline/AutoGlobalTimelineMarker.cpp @@ -17,25 +17,23 @@ AutoGlobalTimelineMarker::AutoGlobalTimelineMarker( : mName(aName), mStackRequest(aStackRequest) { MOZ_ASSERT(NS_IsMainThread()); - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || timelines->IsEmpty()) { + if (TimelineConsumers::IsEmpty()) { return; } - timelines->AddMarkerForAllObservedDocShells(mName, MarkerTracingType::START, - mStackRequest); + TimelineConsumers::AddMarkerForAllObservedDocShells( + mName, MarkerTracingType::START, mStackRequest); } AutoGlobalTimelineMarker::~AutoGlobalTimelineMarker() { MOZ_ASSERT(NS_IsMainThread()); - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || timelines->IsEmpty()) { + if (TimelineConsumers::IsEmpty()) { return; } - timelines->AddMarkerForAllObservedDocShells(mName, MarkerTracingType::END, - mStackRequest); + TimelineConsumers::AddMarkerForAllObservedDocShells( + mName, MarkerTracingType::END, mStackRequest); } } // namespace mozilla diff --git a/docshell/base/timeline/AutoRestyleTimelineMarker.cpp b/docshell/base/timeline/AutoRestyleTimelineMarker.cpp index c012dfbc1756..d9992bc33f3c 100644 --- a/docshell/base/timeline/AutoRestyleTimelineMarker.cpp +++ b/docshell/base/timeline/AutoRestyleTimelineMarker.cpp @@ -22,13 +22,12 @@ AutoRestyleTimelineMarker::AutoRestyleTimelineMarker(nsIDocShell* aDocShell, return; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || !timelines->HasConsumer(aDocShell)) { + if (!TimelineConsumers::HasConsumer(aDocShell)) { return; } mDocShell = aDocShell; - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( mDocShell, MakeUnique(mIsAnimationOnly, MarkerTracingType::START)); } @@ -40,12 +39,11 @@ AutoRestyleTimelineMarker::~AutoRestyleTimelineMarker() { return; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || !timelines->HasConsumer(mDocShell)) { + if (!TimelineConsumers::HasConsumer(mDocShell)) { return; } - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( mDocShell, MakeUnique(mIsAnimationOnly, MarkerTracingType::END)); } diff --git a/docshell/base/timeline/AutoTimelineMarker.cpp b/docshell/base/timeline/AutoTimelineMarker.cpp index 714b818f0a4b..a826706d8e2d 100644 --- a/docshell/base/timeline/AutoTimelineMarker.cpp +++ b/docshell/base/timeline/AutoTimelineMarker.cpp @@ -21,13 +21,13 @@ AutoTimelineMarker::AutoTimelineMarker(nsIDocShell* aDocShell, return; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || !timelines->HasConsumer(aDocShell)) { + if (!TimelineConsumers::HasConsumer(aDocShell)) { return; } mDocShell = aDocShell; - timelines->AddMarkerForDocShell(mDocShell, mName, MarkerTracingType::START); + TimelineConsumers::AddMarkerForDocShell(mDocShell, mName, + MarkerTracingType::START); } AutoTimelineMarker::~AutoTimelineMarker() { @@ -37,12 +37,12 @@ AutoTimelineMarker::~AutoTimelineMarker() { return; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || !timelines->HasConsumer(mDocShell)) { + if (!TimelineConsumers::HasConsumer(mDocShell)) { return; } - timelines->AddMarkerForDocShell(mDocShell, mName, MarkerTracingType::END); + TimelineConsumers::AddMarkerForDocShell(mDocShell, mName, + MarkerTracingType::END); } } // namespace mozilla diff --git a/docshell/base/timeline/TimelineConsumers.cpp b/docshell/base/timeline/TimelineConsumers.cpp index 502b53ac83f4..e475b74ee166 100644 --- a/docshell/base/timeline/TimelineConsumers.cpp +++ b/docshell/base/timeline/TimelineConsumers.cpp @@ -6,134 +6,65 @@ #include "TimelineConsumers.h" -#include "mozilla/ClearOnShutdown.h" #include "mozilla/ObservedDocShell.h" -#include "mozilla/Services.h" #include "mozilla/TimelineMarker.h" #include "jsapi.h" #include "nsAppRunner.h" // for XRE_IsContentProcess, XRE_IsParentProcess #include "nsCRT.h" #include "nsDocShell.h" -#include "nsIObserverService.h" namespace mozilla { -NS_IMPL_ISUPPORTS(TimelineConsumers, nsIObserver); - StaticMutex TimelineConsumers::sMutex; -// Manually manage this singleton's lifetime and destroy it before shutdown. -// This avoids the leakchecker detecting false-positive memory leaks when -// using automatic memory management (i.e. statically instantiating this -// singleton inside the `Get` method), which would automatically destroy it on -// application shutdown, but too late for the leakchecker. Sigh... -StaticRefPtr TimelineConsumers::sInstance; +uint32_t TimelineConsumers::sActiveConsumers = 0; -// This flag makes sure the singleton never gets instantiated while a shutdown -// is in progress. This can actually happen, and `ClearOnShutdown` doesn't work -// in these cases. -Atomic TimelineConsumers::sInShutdown{false}; +StaticAutoPtr> TimelineConsumers::sMarkersStores; -already_AddRefed TimelineConsumers::Get() { - // Using this class is not supported yet for other processes other than - // parent or content. To avoid accidental checks to methods like `IsEmpty`, - // which would probably always be true in those cases, assert here. - // Remember, there will be different singletons available to each process. - - // TODO: we have to avoid calling this function in socket process. - MOZ_ASSERT(XRE_IsContentProcess() || XRE_IsParentProcess()); - - // If we are shutting down, don't bother doing anything. Note: we can only - // know whether or not we're in shutdown if we're instantiated. - if (sInShutdown) { - return nullptr; +LinkedList& TimelineConsumers::MarkersStores() { + if (!sMarkersStores) { + sMarkersStores = new LinkedList; } - - RefPtr copy = sInstance.get(); - return copy.forget(); + return *sMarkersStores; } -/* static */ -void TimelineConsumers::Init() { - MOZ_ASSERT(!sInstance); - RefPtr instance = new TimelineConsumers(); - - nsCOMPtr obs = services::GetObserverService(); - if (!obs) { - return; - } - if (NS_WARN_IF(NS_FAILED( - obs->AddObserver(instance, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false)))) { - return; - } - - sInstance = instance; - ClearOnShutdown(&sInstance); -} - -bool TimelineConsumers::RemoveObservers() { - nsCOMPtr obs = services::GetObserverService(); - if (!obs) { - return false; - } - if (NS_WARN_IF(NS_FAILED( - obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID)))) { - return false; - } - return true; -} - -nsresult TimelineConsumers::Observe(nsISupports* aSubject, const char* aTopic, - const char16_t* aData) { - if (!nsCRT::strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) { - sInShutdown = true; - RemoveObservers(); - return NS_OK; - } - - MOZ_ASSERT(false, "TimelineConsumers got unexpected topic!"); - return NS_ERROR_UNEXPECTED; -} - -TimelineConsumers::TimelineConsumers() : mActiveConsumers(0) {} - void TimelineConsumers::AddConsumer(nsDocShell* aDocShell) { MOZ_ASSERT(NS_IsMainThread()); StaticMutexAutoLock lock( - sMutex); // for `mActiveConsumers` and `mMarkersStores`. + sMutex); // for `sActiveConsumers` and `sMarkersStores`. UniquePtr& observed = aDocShell->mObserved; MOZ_ASSERT(!observed); - if (mActiveConsumers == 0) { + if (sActiveConsumers == 0) { JS::SetProfileTimelineRecordingEnabled(true); } - mActiveConsumers++; + sActiveConsumers++; ObservedDocShell* obsDocShell = new ObservedDocShell(aDocShell); MarkersStorage* storage = static_cast(obsDocShell); observed.reset(obsDocShell); - mMarkersStores.insertFront(storage); + MarkersStores().insertFront(storage); } void TimelineConsumers::RemoveConsumer(nsDocShell* aDocShell) { MOZ_ASSERT(NS_IsMainThread()); StaticMutexAutoLock lock( - sMutex); // for `mActiveConsumers` and `mMarkersStores`. + sMutex); // for `sActiveConsumers` and `sMarkersStores`. UniquePtr& observed = aDocShell->mObserved; MOZ_ASSERT(observed); - mActiveConsumers--; - if (mActiveConsumers == 0) { + sActiveConsumers--; + if (sActiveConsumers == 0) { JS::SetProfileTimelineRecordingEnabled(false); } // Clear all markers from the `mTimelineMarkers` store. - observed.get()->ClearMarkers(); - // Remove self from the `mMarkersStores` store. - observed.get()->remove(); + observed->ClearMarkers(); + // Remove self from the `sMarkersStores` store. + observed->remove(); // Prepare for becoming a consumer later. observed.reset(nullptr); } @@ -144,8 +75,8 @@ bool TimelineConsumers::HasConsumer(nsIDocShell* aDocShell) { } bool TimelineConsumers::IsEmpty() { - StaticMutexAutoLock lock(sMutex); // for `mActiveConsumers`. - return mActiveConsumers == 0; + StaticMutexAutoLock lock(sMutex); // for `sActiveConsumers`. + return sActiveConsumers == 0; } void TimelineConsumers::AddMarkerForDocShell(nsDocShell* aDocShell, @@ -208,9 +139,9 @@ void TimelineConsumers::AddMarkerForAllObservedDocShells( const char* aName, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest /* = STACK */) { bool isMainThread = NS_IsMainThread(); - StaticMutexAutoLock lock(sMutex); // for `mMarkersStores`. + StaticMutexAutoLock lock(sMutex); // for `sMarkersStores`. - for (MarkersStorage* storage = mMarkersStores.getFirst(); storage != nullptr; + for (MarkersStorage* storage = MarkersStores().getFirst(); storage != nullptr; storage = storage->getNext()) { UniquePtr marker = MakeUnique(aName, aTracingType, aStackRequest); @@ -226,9 +157,9 @@ void TimelineConsumers::AddMarkerForAllObservedDocShells( const char* aName, const TimeStamp& aTime, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest /* = STACK */) { bool isMainThread = NS_IsMainThread(); - StaticMutexAutoLock lock(sMutex); // for `mMarkersStores`. + StaticMutexAutoLock lock(sMutex); // for `sMarkersStores`. - for (MarkersStorage* storage = mMarkersStores.getFirst(); storage != nullptr; + for (MarkersStorage* storage = MarkersStores().getFirst(); storage != nullptr; storage = storage->getNext()) { UniquePtr marker = MakeUnique(aName, aTime, aTracingType, aStackRequest); @@ -243,9 +174,9 @@ void TimelineConsumers::AddMarkerForAllObservedDocShells( void TimelineConsumers::AddMarkerForAllObservedDocShells( UniquePtr& aMarker) { bool isMainThread = NS_IsMainThread(); - StaticMutexAutoLock lock(sMutex); // for `mMarkersStores`. + StaticMutexAutoLock lock(sMutex); // for `sMarkersStores`. - for (MarkersStorage* storage = mMarkersStores.getFirst(); storage != nullptr; + for (MarkersStorage* storage = MarkersStores().getFirst(); storage != nullptr; storage = storage->getNext()) { UniquePtr clone = aMarker->Clone(); if (isMainThread) { diff --git a/docshell/base/timeline/TimelineConsumers.h b/docshell/base/timeline/TimelineConsumers.h index 7b2b6669c244..ee67e233f8ac 100644 --- a/docshell/base/timeline/TimelineConsumers.h +++ b/docshell/base/timeline/TimelineConsumers.h @@ -28,36 +28,21 @@ namespace dom { struct ProfileTimelineMarker; } -class TimelineConsumers : public nsIObserver { +class TimelineConsumers { public: - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_NSIOBSERVER - - private: - TimelineConsumers(); - TimelineConsumers(const TimelineConsumers& aOther) = delete; - void operator=(const TimelineConsumers& aOther) = delete; - virtual ~TimelineConsumers() = default; - - bool RemoveObservers(); - - public: - static void Init(); - static already_AddRefed Get(); - // Methods for registering interested consumers (i.e. "devtools toolboxes"). // Each consumer should be directly focused on a particular docshell, but // timeline markers don't necessarily have to be tied to that docshell. // See the public `AddMarker*` methods below. // Main thread only. - void AddConsumer(nsDocShell* aDocShell); - void RemoveConsumer(nsDocShell* aDocShell); + static void AddConsumer(nsDocShell* aDocShell); + static void RemoveConsumer(nsDocShell* aDocShell); - bool HasConsumer(nsIDocShell* aDocShell); + static bool HasConsumer(nsIDocShell* aDocShell); // Checks if there's any existing interested consumer. // May be called from any thread. - bool IsEmpty(); + static bool IsEmpty(); // Methods for adding markers relevant for particular docshells, or generic // (meaning that they either can't be tied to a particular docshell, or one @@ -70,18 +55,18 @@ class TimelineConsumers : public nsIObserver { // These methods create a basic TimelineMarker from a name and some metadata, // relevant for a specific docshell. // Main thread only. - void AddMarkerForDocShell( + static void AddMarkerForDocShell( nsDocShell* aDocShell, const char* aName, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); - void AddMarkerForDocShell( + static void AddMarkerForDocShell( nsIDocShell* aDocShell, const char* aName, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); - void AddMarkerForDocShell( + static void AddMarkerForDocShell( nsDocShell* aDocShell, const char* aName, const TimeStamp& aTime, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); - void AddMarkerForDocShell( + static void AddMarkerForDocShell( nsIDocShell* aDocShell, const char* aName, const TimeStamp& aTime, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); @@ -89,41 +74,38 @@ class TimelineConsumers : public nsIObserver { // These methods register and receive ownership of an already created marker, // relevant for a specific docshell. // Main thread only. - void AddMarkerForDocShell(nsDocShell* aDocShell, - UniquePtr&& aMarker); - void AddMarkerForDocShell(nsIDocShell* aDocShell, - UniquePtr&& aMarker); + static void AddMarkerForDocShell(nsDocShell* aDocShell, + UniquePtr&& aMarker); + static void AddMarkerForDocShell(nsIDocShell* aDocShell, + UniquePtr&& aMarker); // These methods create a basic marker from a name and some metadata, // which doesn't have to be relevant to a specific docshell. // May be called from any thread. - void AddMarkerForAllObservedDocShells( + static void AddMarkerForAllObservedDocShells( const char* aName, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); - void AddMarkerForAllObservedDocShells( + static void AddMarkerForAllObservedDocShells( const char* aName, const TimeStamp& aTime, MarkerTracingType aTracingType, MarkerStackRequest aStackRequest = MarkerStackRequest::STACK); // This method clones and registers an already instantiated marker, // which doesn't have to be relevant to a specific docshell. // May be called from any thread. - void AddMarkerForAllObservedDocShells( + static void AddMarkerForAllObservedDocShells( UniquePtr& aMarker); - void PopMarkers(nsDocShell* aDocShell, JSContext* aCx, - nsTArray& aStore); + static void PopMarkers(nsDocShell* aDocShell, JSContext* aCx, + nsTArray& aStore); private: - static StaticRefPtr sInstance; - static Atomic sInShutdown; + static StaticMutex sMutex; - // Counter for how many timelines are currently interested in markers, - // and a list of the MarkersStorage interfaces representing them. - unsigned long mActiveConsumers; - LinkedList mMarkersStores; + static LinkedList& MarkersStores() REQUIRES(sMutex); - // Protects this class's data structures. - static StaticMutex sMutex MOZ_UNANNOTATED; + static uint32_t sActiveConsumers GUARDED_BY(sMutex); + static StaticAutoPtr> sMarkersStores + GUARDED_BY(sMutex); }; } // namespace mozilla diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index c0dbef9e1fb6..a210d1077c40 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -8041,11 +8041,10 @@ void Document::DispatchContentLoadedEvents() { MaybeResolveReadyForIdle(); } - RefPtr timelines = TimelineConsumers::Get(); nsIDocShell* docShell = this->GetDocShell(); - if (timelines && timelines->HasConsumer(docShell)) { - timelines->AddMarkerForDocShell( + if (TimelineConsumers::HasConsumer(docShell)) { + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique("document::DOMContentLoaded")); } diff --git a/dom/console/Console.cpp b/dom/console/Console.cpp index 56b260f7f0d5..c0d0f6fea1b5 100644 --- a/dom/console/Console.cpp +++ b/dom/console/Console.cpp @@ -2599,8 +2599,7 @@ bool Console::MonotonicTimer(JSContext* aCx, MethodName aMethodName, *aTimeStamp = performance->Now(); nsDocShell* docShell = static_cast(win->GetDocShell()); - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && timelines->HasConsumer(docShell); + bool isTimelineRecording = TimelineConsumers::HasConsumer(docShell); // The 'timeStamp' recordings do not need an argument; use empty string // if no arguments passed in. @@ -2617,8 +2616,8 @@ bool Console::MonotonicTimer(JSContext* aCx, MethodName aMethodName, return false; } - timelines->AddMarkerForDocShell(docShell, - MakeUnique(key)); + TimelineConsumers::AddMarkerForDocShell( + docShell, MakeUnique(key)); } // For `console.time(foo)` and `console.timeEnd(foo)`. else if (isTimelineRecording && aData.Length() == 1) { @@ -2633,7 +2632,7 @@ bool Console::MonotonicTimer(JSContext* aCx, MethodName aMethodName, return false; } - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique(key, aMethodName == MethodTime ? MarkerTracingType::START diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 879c833aade7..f5da89d3e424 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -1466,19 +1466,18 @@ void EventListenerManager::HandleEventInternal(nsPresContext* aPresContext, // Maybe add a marker to the docshell's timeline, but only // bother with all the logic if some docshell is recording. nsCOMPtr docShell; - RefPtr timelines = TimelineConsumers::Get(); bool needsEndEventMarker = false; if (mIsMainThreadELM && listener->mListenerType != Listener::eNativeListener) { docShell = nsContentUtils::GetDocShellForEventTarget(mTarget); if (docShell) { - if (timelines && timelines->HasConsumer(docShell)) { + if (TimelineConsumers::HasConsumer(docShell)) { needsEndEventMarker = true; nsAutoString typeStr; (*aDOMEvent)->GetType(typeStr); uint16_t phase = (*aDOMEvent)->EventPhase(); - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique( typeStr, phase, MarkerTracingType::START)); } @@ -1516,8 +1515,8 @@ void EventListenerManager::HandleEventInternal(nsPresContext* aPresContext, aEvent->mFlags.mInPassiveListener = false; if (needsEndEventMarker) { - timelines->AddMarkerForDocShell(docShell, "DOMEvent", - MarkerTracingType::END); + TimelineConsumers::AddMarkerForDocShell(docShell, "DOMEvent", + MarkerTracingType::END); } } } diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 7aa758b12397..8f73bd0041fe 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -3084,19 +3084,18 @@ void BrowserChild::DidRequestComposite(const TimeStamp& aCompositeReqStart, } nsDocShell* docShell = static_cast(docShellComPtr.get()); - RefPtr timelines = TimelineConsumers::Get(); - if (timelines && timelines->HasConsumer(docShell)) { + if (TimelineConsumers::HasConsumer(docShell)) { // Since we're assuming that it's impossible for content JS to directly // trigger a synchronous paint, we can avoid capturing a stack trace here, // which means we won't run into JS engine reentrancy issues like bug // 1310014. - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( docShell, "CompositeForwardTransaction", aCompositeReqStart, MarkerTracingType::START, MarkerStackRequest::NO_STACK); - timelines->AddMarkerForDocShell(docShell, "CompositeForwardTransaction", - aCompositeReqEnd, MarkerTracingType::END, - MarkerStackRequest::NO_STACK); + TimelineConsumers::AddMarkerForDocShell( + docShell, "CompositeForwardTransaction", aCompositeReqEnd, + MarkerTracingType::END, MarkerStackRequest::NO_STACK); } } diff --git a/dom/messagechannel/MessagePort.cpp b/dom/messagechannel/MessagePort.cpp index 3acac4f4ba8f..07cf42bd1f9a 100644 --- a/dom/messagechannel/MessagePort.cpp +++ b/dom/messagechannel/MessagePort.cpp @@ -115,8 +115,7 @@ class PostMessageRunnable final : public CancelableRunnable { UniquePtr start; UniquePtr end; - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && !timelines->IsEmpty(); + bool isTimelineRecording = !TimelineConsumers::IsEmpty(); if (isTimelineRecording) { start = MakeUnique( @@ -131,8 +130,8 @@ class PostMessageRunnable final : public CancelableRunnable { end = MakeUnique( ProfileTimelineMessagePortOperationType::DeserializeData, MarkerTracingType::END); - timelines->AddMarkerForAllObservedDocShells(start); - timelines->AddMarkerForAllObservedDocShells(end); + TimelineConsumers::AddMarkerForAllObservedDocShells(start); + TimelineConsumers::AddMarkerForAllObservedDocShells(end); } if (NS_WARN_IF(rv.Failed())) { @@ -341,8 +340,7 @@ void MessagePort::PostMessage(JSContext* aCx, JS::Handle aMessage, UniquePtr start; UniquePtr end; - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && !timelines->IsEmpty(); + bool isTimelineRecording = !TimelineConsumers::IsEmpty(); if (isTimelineRecording) { start = MakeUnique( @@ -357,8 +355,8 @@ void MessagePort::PostMessage(JSContext* aCx, JS::Handle aMessage, end = MakeUnique( ProfileTimelineMessagePortOperationType::SerializeData, MarkerTracingType::END); - timelines->AddMarkerForAllObservedDocShells(start); - timelines->AddMarkerForAllObservedDocShells(end); + TimelineConsumers::AddMarkerForAllObservedDocShells(start); + TimelineConsumers::AddMarkerForAllObservedDocShells(end); } if (NS_WARN_IF(aRv.Failed())) { diff --git a/dom/workers/MessageEventRunnable.cpp b/dom/workers/MessageEventRunnable.cpp index a0739078e6f9..9ead17b4707f 100644 --- a/dom/workers/MessageEventRunnable.cpp +++ b/dom/workers/MessageEventRunnable.cpp @@ -49,8 +49,7 @@ bool MessageEventRunnable::DispatchDOMEvent(JSContext* aCx, UniquePtr start; UniquePtr end; - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && !timelines->IsEmpty(); + bool isTimelineRecording = !TimelineConsumers::IsEmpty(); if (isTimelineRecording) { start = MakeUnique( @@ -80,8 +79,8 @@ bool MessageEventRunnable::DispatchDOMEvent(JSContext* aCx, ? ProfileTimelineWorkerOperationType::DeserializeDataOnMainThread : ProfileTimelineWorkerOperationType::DeserializeDataOffMainThread, MarkerTracingType::END); - timelines->AddMarkerForAllObservedDocShells(start); - timelines->AddMarkerForAllObservedDocShells(end); + TimelineConsumers::AddMarkerForAllObservedDocShells(start); + TimelineConsumers::AddMarkerForAllObservedDocShells(end); } if (NS_WARN_IF(rv.Failed())) { diff --git a/dom/workers/Worker.cpp b/dom/workers/Worker.cpp index 17d9ce4edec1..2caef7880dd8 100644 --- a/dom/workers/Worker.cpp +++ b/dom/workers/Worker.cpp @@ -111,8 +111,7 @@ void Worker::PostMessage(JSContext* aCx, JS::Handle aMessage, UniquePtr start; UniquePtr end; - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && !timelines->IsEmpty(); + bool isTimelineRecording = !TimelineConsumers::IsEmpty(); if (isTimelineRecording) { start = MakeUnique( @@ -146,8 +145,8 @@ void Worker::PostMessage(JSContext* aCx, JS::Handle aMessage, ? ProfileTimelineWorkerOperationType::SerializeDataOnMainThread : ProfileTimelineWorkerOperationType::SerializeDataOffMainThread, MarkerTracingType::END); - timelines->AddMarkerForAllObservedDocShells(start); - timelines->AddMarkerForAllObservedDocShells(end); + TimelineConsumers::AddMarkerForAllObservedDocShells(start); + TimelineConsumers::AddMarkerForAllObservedDocShells(end); } if (NS_WARN_IF(aRv.Failed())) { diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 49f5b79b2c32..b5388893961c 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -4416,8 +4416,7 @@ void WorkerPrivate::PostMessageToParent( UniquePtr start; UniquePtr end; - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && !timelines->IsEmpty(); + bool isTimelineRecording = !TimelineConsumers::IsEmpty(); if (isTimelineRecording) { start = MakeUnique( @@ -4444,8 +4443,8 @@ void WorkerPrivate::PostMessageToParent( ? ProfileTimelineWorkerOperationType::SerializeDataOnMainThread : ProfileTimelineWorkerOperationType::SerializeDataOffMainThread, MarkerTracingType::END); - timelines->AddMarkerForAllObservedDocShells(start); - timelines->AddMarkerForAllObservedDocShells(end); + TimelineConsumers::AddMarkerForAllObservedDocShells(start); + TimelineConsumers::AddMarkerForAllObservedDocShells(end); } if (NS_WARN_IF(aRv.Failed())) { diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 8cd1cee03620..6edbafe61ef9 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -9512,12 +9512,11 @@ bool PresShell::DoReflow(nsIFrame* target, bool aInterruptible, nsDocShell* docShell = static_cast(GetPresContext()->GetDocShell()); - RefPtr timelines = TimelineConsumers::Get(); - bool isTimelineRecording = timelines && timelines->HasConsumer(docShell); + bool isTimelineRecording = TimelineConsumers::HasConsumer(docShell); if (isTimelineRecording) { - timelines->AddMarkerForDocShell(docShell, "Reflow", - MarkerTracingType::START); + TimelineConsumers::AddMarkerForDocShell(docShell, "Reflow", + MarkerTracingType::START); } Maybe innerWindowID; @@ -9720,7 +9719,8 @@ bool PresShell::DoReflow(nsIFrame* target, bool aInterruptible, } if (isTimelineRecording) { - timelines->AddMarkerForDocShell(docShell, "Reflow", MarkerTracingType::END); + TimelineConsumers::AddMarkerForDocShell(docShell, "Reflow", + MarkerTracingType::END); } return !interrupted; diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 0cc4c87cb3db..a6787cd323c4 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -1023,10 +1023,8 @@ nsDocumentViewer::LoadComplete(nsresult aStatus) { } // Notify any devtools about the load. - RefPtr timelines = TimelineConsumers::Get(); - - if (timelines && timelines->HasConsumer(docShell)) { - timelines->AddMarkerForDocShell( + if (TimelineConsumers::HasConsumer(docShell)) { + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique("document::Load")); } diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index 704a57d43c28..cebb46d63072 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -2086,8 +2086,7 @@ static void GetProfileTimelineSubDocShells(nsDocShell* aRootDocShell, return; } - RefPtr timelines = TimelineConsumers::Get(); - if (!timelines || timelines->IsEmpty()) { + if (TimelineConsumers::IsEmpty()) { return; } @@ -2698,18 +2697,15 @@ void nsRefreshDriver::Tick(VsyncId aId, TimeStamp aNowTime, mCompositionPayloads.Clear(); } - RefPtr timelines = TimelineConsumers::Get(); - nsTArray profilingDocShells; GetProfileTimelineSubDocShells(GetDocShell(mPresContext), profilingDocShells); for (nsDocShell* docShell : profilingDocShells) { // For the sake of the profile timeline's simplicity, this is flagged as // paint even if it includes creating display lists - MOZ_ASSERT(timelines); - MOZ_ASSERT(timelines->HasConsumer(docShell)); - timelines->AddMarkerForDocShell(docShell, "Paint", - MarkerTracingType::START); + MOZ_ASSERT(TimelineConsumers::HasConsumer(docShell)); + TimelineConsumers::AddMarkerForDocShell(docShell, "Paint", + MarkerTracingType::START); } #ifdef MOZ_DUMP_PAINTING @@ -2736,10 +2732,9 @@ void nsRefreshDriver::Tick(VsyncId aId, TimeStamp aNowTime, #endif for (nsDocShell* docShell : profilingDocShells) { - MOZ_ASSERT(timelines); - MOZ_ASSERT(timelines->HasConsumer(docShell)); - timelines->AddMarkerForDocShell(docShell, "Paint", - MarkerTracingType::END); + MOZ_ASSERT(TimelineConsumers::HasConsumer(docShell)); + TimelineConsumers::AddMarkerForDocShell(docShell, "Paint", + MarkerTracingType::END); } dispatchTasksAfterTick = true; diff --git a/layout/build/nsLayoutStatics.cpp b/layout/build/nsLayoutStatics.cpp index c7337bfda78f..3dfadc2cef7a 100644 --- a/layout/build/nsLayoutStatics.cpp +++ b/layout/build/nsLayoutStatics.cpp @@ -234,8 +234,6 @@ nsresult nsLayoutStatics::Initialize() { PointerEventHandler::InitializeStatics(); TouchManager::InitializeStatics(); - TimelineConsumers::Init(); - nsWindowMemoryReporter::Init(); SVGElementFactory::Init(); diff --git a/view/nsView.cpp b/view/nsView.cpp index 5e40bf2bdf18..a1cdea2b6740 100644 --- a/view/nsView.cpp +++ b/view/nsView.cpp @@ -1083,13 +1083,12 @@ void nsView::DidCompositeWindow(mozilla::layers::TransactionId aTransactionId, } nsIDocShell* docShell = context->GetDocShell(); - RefPtr timelines = TimelineConsumers::Get(); - if (timelines && timelines->HasConsumer(docShell)) { - timelines->AddMarkerForDocShell( + if (TimelineConsumers::HasConsumer(docShell)) { + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique( aCompositeStart, MarkerTracingType::START)); - timelines->AddMarkerForDocShell( + TimelineConsumers::AddMarkerForDocShell( docShell, MakeUnique(aCompositeEnd, MarkerTracingType::END)); } diff --git a/xpcom/base/CycleCollectedJSRuntime.cpp b/xpcom/base/CycleCollectedJSRuntime.cpp index 011a44ab01d3..c515336017f3 100644 --- a/xpcom/base/CycleCollectedJSRuntime.cpp +++ b/xpcom/base/CycleCollectedJSRuntime.cpp @@ -1175,11 +1175,10 @@ void CycleCollectedJSRuntime::GCNurseryCollectionCallback( MOZ_ASSERT(CycleCollectedJSContext::Get()->Context() == aContext); MOZ_ASSERT(NS_IsMainThread()); - RefPtr timelines = TimelineConsumers::Get(); - if (timelines && !timelines->IsEmpty()) { + if (!TimelineConsumers::IsEmpty()) { UniquePtr abstractMarker( MakeUnique(aProgress, aReason)); - timelines->AddMarkerForAllObservedDocShells(abstractMarker); + TimelineConsumers::AddMarkerForAllObservedDocShells(abstractMarker); } if (aProgress == JS::GCNurseryProgress::GC_NURSERY_COLLECTION_START) {