From 78936f73aa788749af69d9d8a08fbf10cdae0078 Mon Sep 17 00:00:00 2001 From: Andrew Sutherland Date: Thu, 24 Oct 2024 03:02:39 +0000 Subject: [PATCH] Bug 1900706 - Expose equivalent of nsContentUtils::ReportToConsole on nsIGlobalObject. r=smaug A notable change in this stack is to ensure that we populate mOwner on workers. Differential Revision: https://phabricator.services.mozilla.com/D213724 --- dom/base/nsGlobalWindowInner.cpp | 9 ++++ dom/base/nsGlobalWindowInner.h | 6 +++ dom/base/nsIGlobalObject.cpp | 12 +++++ dom/base/nsIGlobalObject.h | 35 ++++++++++++++ dom/performance/PerformanceObserver.cpp | 50 ++++++++------------ dom/performance/PerformanceObserver.h | 5 +- dom/workers/RuntimeService.cpp | 4 +- dom/workers/WorkerPrivate.cpp | 61 +++++++++++++++---------- dom/workers/WorkerPrivate.h | 10 ++-- dom/workers/WorkerScope.cpp | 10 ++++ dom/workers/WorkerScope.h | 6 +++ 11 files changed, 145 insertions(+), 63 deletions(-) diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index 7fb436312edd..e426febdb72b 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -1685,6 +1685,15 @@ mozilla::dom::StorageManager* nsGlobalWindowInner::GetStorageManager() { // * a Window object whose associated Document is fully active bool nsGlobalWindowInner::IsEligibleForMessaging() { return IsFullyActive(); } +void nsGlobalWindowInner::ReportToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) { + nsContentUtils::ReportToConsole(aErrorFlags, aCategory, mDoc, aFile, + aMessageName.get(), aParams, aLocation); +} + nsresult nsGlobalWindowInner::EnsureScriptEnvironment() { // NOTE: We can't use FORWARD_TO_OUTER here because we don't want to fail if // we're called on an inactive inner window. diff --git a/dom/base/nsGlobalWindowInner.h b/dom/base/nsGlobalWindowInner.h index 26e903c18562..6a595470af49 100644 --- a/dom/base/nsGlobalWindowInner.h +++ b/dom/base/nsGlobalWindowInner.h @@ -258,6 +258,12 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget, bool IsEligibleForMessaging() override; + void ReportToConsole(uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, + const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) override; + void TraceGlobalJSObject(JSTracer* aTrc); virtual nsresult EnsureScriptEnvironment() override; diff --git a/dom/base/nsIGlobalObject.cpp b/dom/base/nsIGlobalObject.cpp index 204ac695d673..ac6f63fcba9c 100644 --- a/dom/base/nsIGlobalObject.cpp +++ b/dom/base/nsIGlobalObject.cpp @@ -494,3 +494,15 @@ bool nsIGlobalObject::ShouldResistFingerprinting(CallerType aCallerType, return aCallerType != CallerType::System && ShouldResistFingerprinting(aTarget); } + +void nsIGlobalObject::ReportToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) { + // We pass nullptr for the document because nsGlobalWindowInner handles the + // case where it should be non-null. We also expect the worker impl to + // override. + nsContentUtils::ReportToConsole(aErrorFlags, aCategory, nullptr, aFile, + aMessageName.get(), aParams, aLocation); +} diff --git a/dom/base/nsIGlobalObject.h b/dom/base/nsIGlobalObject.h index 4027dac03e41..4e1d807955ca 100644 --- a/dom/base/nsIGlobalObject.h +++ b/dom/base/nsIGlobalObject.h @@ -315,6 +315,41 @@ class nsIGlobalObject : public nsISupports { virtual bool IsXPCSandbox() { return false; } + /** + * Report a localized error message to the error console. Currently this + * amounts to a wrapper around nsContentUtils::ReportToConsole for window + * globals and a runnable bounced to the main thread to call + * nsContentUtils::ReportToConsole for workers but the intent is to migrate + * towards logging the messages to the `dom::Console` for the global. See + * bug 1900706 for more context. + * + * This method returns void because there is no reasonable action for a caller + * for dynamic failure and we can assert on things like erroneous message + * names. + * + * @param aErrorFlags See nsIScriptError. + * @param aCategory Name of module reporting error. + * @param aFile Properties file containing localized message. + * @param aMessageName Name of localized message. + * @param [aParams=empty-array] (Optional) Parameters to be substituted into + * localized message. + * @param [aURI=nullptr] (Optional) URI of resource containing error; if + * omitted, an attempt will be made to use the URI associated with + * the global (ex: the document URI). + * @param [aSourceLine=u""_ns] (Optional) The text of the line that + * contains the error (may be empty). + * @param [aLineNumber=0] (Optional) Line number within resource + * containing error. + * @param [aColumnNumber=0] (Optional) Column number within resource + * containing error. + */ + virtual void ReportToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams = nsTArray(), + const mozilla::SourceLocation& aLocation = + mozilla::JSCallingLocation::Get()); + protected: virtual ~nsIGlobalObject(); diff --git a/dom/performance/PerformanceObserver.cpp b/dom/performance/PerformanceObserver.cpp index d5d1725c5773..9674af67e5fe 100644 --- a/dom/performance/PerformanceObserver.cpp +++ b/dom/performance/PerformanceObserver.cpp @@ -46,12 +46,9 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(PerformanceObserver) NS_INTERFACE_MAP_ENTRY(nsISupports) NS_INTERFACE_MAP_END -const char UnsupportedEntryTypesIgnoredMsgId[] = "UnsupportedEntryTypesIgnored"; -const char AllEntryTypesIgnoredMsgId[] = "AllEntryTypesIgnored"; - PerformanceObserver::PerformanceObserver(nsPIDOMWindowInner* aOwner, PerformanceObserverCallback& aCb) - : mOwner(aOwner), + : mOwner(aOwner->AsGlobal()), mCallback(&aCb), mObserverType(ObserverTypeUndefined), mConnected(false) { @@ -61,8 +58,11 @@ PerformanceObserver::PerformanceObserver(nsPIDOMWindowInner* aOwner, PerformanceObserver::PerformanceObserver(WorkerPrivate* aWorkerPrivate, PerformanceObserverCallback& aCb) - : mCallback(&aCb), mObserverType(ObserverTypeUndefined), mConnected(false) { - MOZ_ASSERT(aWorkerPrivate); + : mOwner(aWorkerPrivate->GlobalScope()), + mCallback(&aCb), + mObserverType(ObserverTypeUndefined), + mConnected(false) { + MOZ_ASSERT(aWorkerPrivate->GlobalScope()); mPerformance = aWorkerPrivate->GlobalScope()->GetPerformance(); } @@ -137,29 +137,13 @@ static constexpr nsLiteralString kValidTypeNames[5] = { u"mark"_ns, u"measure"_ns, u"navigation"_ns, u"paint"_ns, u"resource"_ns, }; -void PerformanceObserver::ReportUnsupportedTypesErrorToConsole( - bool aIsMainThread, const char* msgId, const nsString& aInvalidTypes) { - if (!aIsMainThread) { - nsTArray params; - params.AppendElement(aInvalidTypes); - WorkerPrivate::ReportErrorToConsole(msgId, params); - } else { - nsCOMPtr ownerWindow = do_QueryInterface(mOwner); - Document* document = ownerWindow->GetExtantDoc(); - AutoTArray params = {aInvalidTypes}; - nsContentUtils::ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, - document, nsContentUtils::eDOM_PROPERTIES, - msgId, params); - } -} - void PerformanceObserver::Observe(const PerformanceObserverInit& aOptions, ErrorResult& aRv) { const Optional>& maybeEntryTypes = aOptions.mEntryTypes; const Optional& maybeType = aOptions.mType; const Optional& maybeBuffered = aOptions.mBuffered; - if (!mPerformance) { + if (!mPerformance || !mOwner) { aRv.Throw(NS_ERROR_FAILURE); return; } @@ -241,16 +225,18 @@ void PerformanceObserver::Observe(const PerformanceObserverInit& aOptions, } if (!invalidTypesJoined.IsEmpty()) { - ReportUnsupportedTypesErrorToConsole(NS_IsMainThread(), - UnsupportedEntryTypesIgnoredMsgId, - invalidTypesJoined); + AutoTArray params = {invalidTypesJoined}; + mOwner->ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, + nsContentUtils::eDOM_PROPERTIES, + "UnsupportedEntryTypesIgnored"_ns, params); + // (we don't return because we're ignoring and we keep going) } /* 3.3.1.5.3 */ if (validEntryTypes.IsEmpty()) { - nsString errorString; - ReportUnsupportedTypesErrorToConsole( - NS_IsMainThread(), AllEntryTypesIgnoredMsgId, errorString); + mOwner->ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, + nsContentUtils::eDOM_PROPERTIES, + "AllEntryTypesIgnored"_ns); return; } @@ -289,8 +275,10 @@ void PerformanceObserver::Observe(const PerformanceObserverInit& aOptions, } if (!typeValid) { - ReportUnsupportedTypesErrorToConsole( - NS_IsMainThread(), UnsupportedEntryTypesIgnoredMsgId, type); + AutoTArray params = {type}; + mOwner->ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, + nsContentUtils::eDOM_PROPERTIES, + "UnsupportedEntryTypesIgnored"_ns, params); return; } diff --git a/dom/performance/PerformanceObserver.h b/dom/performance/PerformanceObserver.h index 7b50eb8b6096..6f7b3a2a2bfb 100644 --- a/dom/performance/PerformanceObserver.h +++ b/dom/performance/PerformanceObserver.h @@ -63,12 +63,9 @@ class PerformanceObserver final : public nsISupports, public nsWrapperCache { bool ObservesTypeOfEntry(PerformanceEntry* aEntry); private: - void ReportUnsupportedTypesErrorToConsole(bool aIsMainThread, - const char* msgId, - const nsString& aInvalidTypes); ~PerformanceObserver(); - nsCOMPtr mOwner; + nsCOMPtr mOwner; RefPtr mCallback; RefPtr mPerformance; nsTArray mEntryTypes; diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp index 4c7bfe156bf8..9ffc693fbade 100644 --- a/dom/workers/RuntimeService.cpp +++ b/dom/workers/RuntimeService.cpp @@ -1154,7 +1154,9 @@ bool RuntimeService::RegisterWorker(WorkerPrivate& aWorkerPrivate) { // Worker spawn gets queued due to hitting max workers per domain // limit so let's log a warning. - WorkerPrivate::ReportErrorToConsole("HittingMaxWorkersPerDomain2"); + WorkerPrivate::ReportErrorToConsole(nsIScriptError::warningFlag, "DOM"_ns, + nsContentUtils::eDOM_PROPERTIES, + "HittingMaxWorkersPerDomain2"_ns); if (isServiceWorker) { Telemetry::Accumulate(Telemetry::SERVICE_WORKER_SPAWN_GETS_QUEUED, 1); diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 65c4e18eded0..fab3b616ec79 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -550,13 +550,14 @@ class PropagateStorageAccessPermissionGrantedRunnable final }; class ReportErrorToConsoleRunnable final : public WorkerParentThreadRunnable { - const char* mMessage; - const nsTArray mParams; - public: // aWorkerPrivate is the worker thread we're on (or the main thread, if null) - static void Report(WorkerPrivate* aWorkerPrivate, const char* aMessage, - const nsTArray& aParams) { + static void Report(WorkerPrivate* aWorkerPrivate, uint32_t aErrorFlags, + const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, + const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) { if (aWorkerPrivate) { aWorkerPrivate->AssertIsOnWorkerThread(); } else { @@ -566,24 +567,32 @@ class ReportErrorToConsoleRunnable final : public WorkerParentThreadRunnable { // Now fire a runnable to do the same on the parent's thread if we can. if (aWorkerPrivate) { RefPtr runnable = - new ReportErrorToConsoleRunnable(aWorkerPrivate, aMessage, aParams); + new ReportErrorToConsoleRunnable(aWorkerPrivate, aErrorFlags, + aCategory, aFile, aMessageName, + aParams, aLocation); runnable->Dispatch(aWorkerPrivate); return; } // Log a warning to the console. - nsContentUtils::ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, - nullptr, nsContentUtils::eDOM_PROPERTIES, - aMessage, aParams); + nsContentUtils::ReportToConsole(aErrorFlags, aCategory, nullptr, aFile, + aMessageName.get(), aParams, aLocation); } private: ReportErrorToConsoleRunnable(WorkerPrivate* aWorkerPrivate, - const char* aMessage, - const nsTArray& aParams) + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, + const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) : WorkerParentThreadRunnable("ReportErrorToConsoleRunnable"), - mMessage(aMessage), - mParams(aParams.Clone()) {} + mErrorFlags(aErrorFlags), + mCategory(aCategory), + mFile(aFile), + mMessageName(aMessageName), + mParams(aParams.Clone()), + mLocation(aLocation) {} virtual void PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override { @@ -597,9 +606,17 @@ class ReportErrorToConsoleRunnable final : public WorkerParentThreadRunnable { WorkerPrivate* aWorkerPrivate) override { WorkerPrivate* parent = aWorkerPrivate->GetParent(); MOZ_ASSERT_IF(!parent, NS_IsMainThread()); - Report(parent, mMessage, mParams); + Report(parent, mErrorFlags, mCategory, mFile, mMessageName, mParams, + mLocation); return true; } + + const uint32_t mErrorFlags; + const nsCString mCategory; + const nsContentUtils::PropertiesFile mFile; + const nsCString mMessageName; + const nsTArray mParams; + const mozilla::SourceLocation mLocation; }; class RunExpiredTimoutsRunnable final : public WorkerThreadRunnable, @@ -5398,20 +5415,18 @@ void WorkerPrivate::ReportError(JSContext* aCx, } // static -void WorkerPrivate::ReportErrorToConsole(const char* aMessage) { - nsTArray emptyParams; - WorkerPrivate::ReportErrorToConsole(aMessage, emptyParams); -} - -// static -void WorkerPrivate::ReportErrorToConsole(const char* aMessage, - const nsTArray& aParams) { +void WorkerPrivate::ReportErrorToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) { WorkerPrivate* wp = nullptr; if (!NS_IsMainThread()) { wp = GetCurrentThreadWorkerPrivate(); } - ReportErrorToConsoleRunnable::Report(wp, aMessage, aParams); + ReportErrorToConsoleRunnable::Report(wp, aErrorFlags, aCategory, aFile, + aMessageName, aParams, aLocation); } int32_t WorkerPrivate::SetTimeout(JSContext* aCx, TimeoutHandler* aHandler, diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 8ca40304e014..5cbd73812b47 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -415,10 +415,12 @@ class WorkerPrivate final void ReportError(JSContext* aCx, JS::ConstUTF8CharsZ aToStringResult, JSErrorReport* aReport); - static void ReportErrorToConsole(const char* aMessage); - - static void ReportErrorToConsole(const char* aMessage, - const nsTArray& aParams); + static void ReportErrorToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams = nsTArray(), + const mozilla::SourceLocation& aLocation = + mozilla::JSCallingLocation::Get()); int32_t SetTimeout(JSContext* aCx, TimeoutHandler* aHandler, int32_t aTimeout, bool aIsInterval, Timeout::Reason aReason, diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index 7f937e047fc8..18f6aa8b8172 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -915,6 +915,16 @@ mozilla::dom::StorageManager* WorkerGlobalScope::GetStorageManager() { bool WorkerGlobalScope::IsEligibleForMessaging() { return mIsEligibleForMessaging; } + +void WorkerGlobalScope::ReportToConsole( + uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) { + WorkerPrivate::ReportErrorToConsole(aErrorFlags, aCategory, aFile, + aMessageName, aParams, aLocation); +} + void WorkerGlobalScope::StorageAccessPermissionGranted() { // Reset the IndexedDB factory. mIndexedDB = nullptr; diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 59ebab3c63a3..57afbf4add66 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -260,6 +260,12 @@ class WorkerGlobalScope : public WorkerGlobalScopeBase { bool IsEligibleForMessaging() final; + void ReportToConsole(uint32_t aErrorFlags, const nsCString& aCategory, + nsContentUtils::PropertiesFile aFile, + const nsCString& aMessageName, + const nsTArray& aParams, + const mozilla::SourceLocation& aLocation) final; + // WorkerGlobalScope WebIDL implementation WorkerGlobalScope* Self() { return this; }