From e748187360f8c987b487dd90c5dea1b489498d43 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 2 Aug 2023 02:41:40 -0500 Subject: Issue #2282 - - Properly implement Performance Timeline Level 2 w3c spec. https://bugzilla.mozilla.org/show_bug.cgi?id=1539006 Do not throw from PerformanceObserver.observe when none of the entryTypes are known. https://bugzilla.mozilla.org/show_bug.cgi?id=1403027 Implement PerformanceObserver::takeRecords(). https://bugzilla.mozilla.org/show_bug.cgi?id=1436692 "server" is not a valid PerformanceEntry type. https://bugzilla.mozilla.org/show_bug.cgi?id=1463065 Fix a null ptr crash in PerformanceObserver::Observe. https://bugzilla.mozilla.org/show_bug.cgi?id=1631346 --- dom/locales/en-US/chrome/dom/dom.properties | 2 + dom/performance/Performance.cpp | 18 +- dom/performance/PerformanceObserver.cpp | 230 ++++++++++++++++++--- dom/performance/PerformanceObserver.h | 18 ++ dom/performance/tests/test_performance_observer.js | 14 +- dom/webidl/PerformanceObserver.webidl | 10 +- dom/workers/WorkerPrivate.cpp | 38 +++- dom/workers/WorkerPrivate.h | 3 + 8 files changed, 284 insertions(+), 49 deletions(-) diff --git a/dom/locales/en-US/chrome/dom/dom.properties b/dom/locales/en-US/chrome/dom/dom.properties index 27b4eebf12..f0a6363af0 100644 --- a/dom/locales/en-US/chrome/dom/dom.properties +++ b/dom/locales/en-US/chrome/dom/dom.properties @@ -321,3 +321,5 @@ PushStateFloodingPrevented=Call to pushState or replaceState ignored due to exce # LOCALIZATION NOTE: Do not translate "Reload" ReloadFloodingPrevented=Call to Reload ignored due to excessive calls within a short timeframe. DOMQuadBoundsAttrWarning=DOMQuad.bounds is deprecated in favor of DOMQuad.getBounds() +UnsupportedEntryTypesIgnored=Ignoring unsupported entryTypes: %S. +AllEntryTypesIgnored=No valid entryTypes; aborting registration. diff --git a/dom/performance/Performance.cpp b/dom/performance/Performance.cpp index 217faa5afe..e8a46409ef 100755 --- a/dom/performance/Performance.cpp +++ b/dom/performance/Performance.cpp @@ -712,9 +712,21 @@ Performance::QueueEntry(PerformanceEntry* aEntry) if (mObservers.IsEmpty()) { return; } - NS_OBSERVER_ARRAY_NOTIFY_XPCOM_OBSERVERS(mObservers, - PerformanceObserver, - QueueEntry, (aEntry)); + nsTObserverArray interestedObservers; + nsTObserverArray::ForwardIterator observerIt( + mObservers); + while (observerIt.HasMore()) { + PerformanceObserver* observer = observerIt.GetNext(); + if (observer->ObservesTypeOfEntry(aEntry)) { + interestedObservers.AppendElement(observer); + } + } + + if (interestedObservers.IsEmpty()) { + return; + } + + NS_OBSERVER_ARRAY_NOTIFY_XPCOM_OBSERVERS(interestedObservers, PerformanceObserver, QueueEntry, (aEntry)); if (!mPendingNotificationObserversTask) { RunNotificationObserversTask(); diff --git a/dom/performance/PerformanceObserver.cpp b/dom/performance/PerformanceObserver.cpp index 26f93e8fc1..5482c1f2a6 100644 --- a/dom/performance/PerformanceObserver.cpp +++ b/dom/performance/PerformanceObserver.cpp @@ -9,6 +9,7 @@ #include "mozilla/dom/PerformanceBinding.h" #include "mozilla/dom/PerformanceEntryBinding.h" #include "mozilla/dom/PerformanceObserverBinding.h" +#include "nsIScriptError.h" #include "nsPIDOMWindow.h" #include "nsQueryObject.h" #include "nsString.h" @@ -27,12 +28,14 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(PerformanceObserver) NS_IMPL_CYCLE_COLLECTION_UNLINK(mCallback) NS_IMPL_CYCLE_COLLECTION_UNLINK(mPerformance) NS_IMPL_CYCLE_COLLECTION_UNLINK(mOwner) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mQueuedEntries) NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(PerformanceObserver) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCallback) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPerformance) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOwner) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mQueuedEntries) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(PerformanceObserver) @@ -43,10 +46,14 @@ 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) , mCallback(&aCb) + , mObserverType(ObserverTypeUndefined) , mConnected(false) { MOZ_ASSERT(mOwner); @@ -56,6 +63,7 @@ PerformanceObserver::PerformanceObserver(nsPIDOMWindowInner* aOwner, PerformanceObserver::PerformanceObserver(WorkerPrivate* aWorkerPrivate, PerformanceObserverCallback& aCb) : mCallback(&aCb) + , mObserverType(ObserverTypeUndefined) , mConnected(false) { MOZ_ASSERT(aWorkerPrivate); @@ -115,7 +123,8 @@ PerformanceObserver::Notify() mQueuedEntries.Clear(); ErrorResult rv; - mCallback->Call(this, *list, *this, rv); + RefPtr callback(mCallback); + callback->Call(this, *list, *this, rv); if (NS_WARN_IF(rv.Failed())) { rv.SuppressException(); } @@ -126,63 +135,227 @@ PerformanceObserver::QueueEntry(PerformanceEntry* aEntry) { MOZ_ASSERT(aEntry); - nsAutoString entryType; - aEntry->GetEntryType(entryType); - if (!mEntryTypes.Contains(entryType)) { + if (!ObservesTypeOfEntry(aEntry)) { return; } mQueuedEntries.AppendElement(aEntry); } +/* + * Keep this list in alphabetical order. + * https://w3c.github.io/performance-timeline/#supportedentrytypes-attribute + */ static const char16_t *const sValidTypeNames[4] = { u"mark", u"measure", - u"resource", - u"server" + u"navigation", + u"resource" }; void -PerformanceObserver::Observe(const PerformanceObserverInit& aOptions, - ErrorResult& aRv) -{ - if (aOptions.mEntryTypes.IsEmpty()) { - aRv.Throw(NS_ERROR_DOM_TYPE_ERR); +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); + nsIDocument* document = ownerWindow->GetExtantDoc(); + const char16_t* params[] = {aInvalidTypes.get()}; + nsContentUtils::ReportToConsole( + nsIScriptError::warningFlag, NS_LITERAL_CSTRING("DOM"), document, + nsContentUtils::eDOM_PROPERTIES, msgId, params, 1); + } + return; +} + +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) { + aRv.Throw(NS_ERROR_FAILURE); return; } - nsTArray validEntryTypes; + if (!maybeEntryTypes.WasPassed() && !maybeType.WasPassed()) { + /* Per spec (3.3.1.2), this should be a syntax error. */ + aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); + return; + } - for (const char16_t* name : sValidTypeNames) { - nsDependentString validTypeName(name); - if (aOptions.mEntryTypes.Contains(validTypeName) && - !validEntryTypes.Contains(validTypeName)) { - validEntryTypes.AppendElement(validTypeName); + if (maybeEntryTypes.WasPassed() && + (maybeType.WasPassed() || maybeBuffered.WasPassed())) { + /* Per spec (3.3.1.3), this, too, should be a syntax error. */ + aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); + return; + } + + /* 3.3.1.4.1 */ + if (mObserverType == ObserverTypeUndefined) { + if (maybeEntryTypes.WasPassed()) { + mObserverType = ObserverTypeMultiple; + } else { + mObserverType = ObserverTypeSingle; } } - if (validEntryTypes.IsEmpty()) { - aRv.Throw(NS_ERROR_DOM_TYPE_ERR); + /* 3.3.1.4.2 */ + if (mObserverType == ObserverTypeSingle && maybeEntryTypes.WasPassed()) { + aRv.Throw(NS_ERROR_DOM_INVALID_MODIFICATION_ERR); + return; + } + /* 3.3.1.4.3 */ + if (mObserverType == ObserverTypeMultiple && maybeType.WasPassed()) { + aRv.Throw(NS_ERROR_DOM_INVALID_MODIFICATION_ERR); return; } - mEntryTypes.SwapElements(validEntryTypes); + /* 3.3.1.5 */ + if (mObserverType == ObserverTypeMultiple) { + const Sequence& entryTypes = maybeEntryTypes.Value(); - mPerformance->AddObserver(this); + if (entryTypes.IsEmpty()) { + return; + } + + /* 3.3.1.5.2 */ + nsTArray validEntryTypes; + for (const char16_t* name : sValidTypeNames) { + nsDependentString validTypeName(name); + if (entryTypes.Contains(validTypeName) && + !validEntryTypes.Contains(validTypeName)) { + validEntryTypes.AppendElement(validTypeName); + } + } + + nsAutoString invalidTypesJoined; + bool addComma = false; + for (const auto& type : entryTypes) { + if (!validEntryTypes.Contains(type)) { + if (addComma) { + invalidTypesJoined.AppendLiteral(", "); + } + addComma = true; + invalidTypesJoined.Append(type); + } + } + + if (!invalidTypesJoined.IsEmpty()) { + ReportUnsupportedTypesErrorToConsole(NS_IsMainThread(), + UnsupportedEntryTypesIgnoredMsgId, + invalidTypesJoined); + } + + /* 3.3.1.5.3 */ + if (validEntryTypes.IsEmpty()) { + nsString errorString; + ReportUnsupportedTypesErrorToConsole( + NS_IsMainThread(), AllEntryTypesIgnoredMsgId, errorString); + return; + } + + /* + * Registered or not, we clear out the list of options, and start fresh + * with the one that we are using here. (3.3.1.5.4,5) + */ + mOptions.Clear(); + mOptions.AppendElement(aOptions); + + } else { + MOZ_ASSERT(mObserverType == ObserverTypeSingle); + bool typeValid = false; + nsString type = maybeType.Value(); + + /* 3.3.1.6.2 */ + for (const char16_t* name : sValidTypeNames) { + nsDependentString validTypeName(name); + if (type == validTypeName) { + typeValid = true; + break; + } + } + + if (!typeValid) { + ReportUnsupportedTypesErrorToConsole( + NS_IsMainThread(), UnsupportedEntryTypesIgnoredMsgId, type); + return; + } + + /* 3.3.1.6.4, 3.3.1.6.4 */ + bool didUpdateOptionsList = false; + nsTArray updatedOptionsList; + for (auto& option : mOptions) { + if (option.mType.WasPassed() && option.mType.Value() == type) { + updatedOptionsList.AppendElement(aOptions); + didUpdateOptionsList = true; + } else { + updatedOptionsList.AppendElement(option); + } + } + if (!didUpdateOptionsList) { + updatedOptionsList.AppendElement(aOptions); + } + mOptions.SwapElements(updatedOptionsList); - if (aOptions.mBuffered) { - for (auto entryType : mEntryTypes) { + /* 3.3.1.6.5 */ + if (maybeBuffered.WasPassed() && maybeBuffered.Value()) { nsTArray> existingEntries; - mPerformance->GetEntriesByType(entryType, existingEntries); + mPerformance->GetEntriesByType(type, existingEntries); if (!existingEntries.IsEmpty()) { mQueuedEntries.AppendElements(existingEntries); } } } - + /* Add ourselves to the list of registered performance + * observers, if necessary. (3.3.1.5.4,5; 3.3.1.6.4) + */ + mPerformance->AddObserver(this); mConnected = true; } +void +PerformanceObserver::GetSupportedEntryTypes(const GlobalObject& aGlobal, JS::MutableHandle aObject) +{ + nsTArray validTypes; + JS::Rooted val(aGlobal.Context()); + + for (const char16_t* name : sValidTypeNames) { + nsString validTypeName(name); + validTypes.AppendElement(validTypeName); + } + + if (!ToJSValue(aGlobal.Context(), validTypes, &val)) { + /* + * If this conversion fails, we don't set a result. + * The spec does not allow us to throw an exception. + */ + return; + } + aObject.set(&val.toObject()); +} + +bool +PerformanceObserver::ObservesTypeOfEntry(PerformanceEntry* aEntry) +{ + for (auto& option : mOptions) { + if (option.mType.WasPassed()) { + if (option.mType.Value() == aEntry->GetEntryType()) { + return true; + } + } else { + if (option.mEntryTypes.Value().Contains(aEntry->GetEntryType())) { + return true; + } + } + } + return false; +} + void PerformanceObserver::Disconnect() { @@ -192,3 +365,10 @@ PerformanceObserver::Disconnect() mConnected = false; } } + +void +PerformanceObserver::TakeRecords(nsTArray>& aRetval) +{ + MOZ_ASSERT(aRetval.IsEmpty()); + aRetval.SwapElements(mQueuedEntries); +} diff --git a/dom/performance/PerformanceObserver.h b/dom/performance/PerformanceObserver.h index 283000d581..25d5f98f6a 100644 --- a/dom/performance/PerformanceObserver.h +++ b/dom/performance/PerformanceObserver.h @@ -55,19 +55,37 @@ public: void Observe(const PerformanceObserverInit& aOptions, mozilla::ErrorResult& aRv); + static void GetSupportedEntryTypes(const GlobalObject& aGlobal, + JS::MutableHandle aObject); void Disconnect(); + void TakeRecords(nsTArray>& aRetval); + void Notify(); void QueueEntry(PerformanceEntry* aEntry); + bool ObservesTypeOfEntry(PerformanceEntry* aEntry); + private: + void ReportUnsupportedTypesErrorToConsole(bool aIsMainThread, + const char* msgId, + const nsString& aInvalidTypes); ~PerformanceObserver(); nsCOMPtr mOwner; RefPtr mCallback; RefPtr mPerformance; nsTArray mEntryTypes; + nsTArray mOptions; + enum { + ObserverTypeUndefined, + ObserverTypeSingle, + ObserverTypeMultiple, + } mObserverType; + /* + * This is also known as registered, in the spec. + */ bool mConnected; nsTArray> mQueuedEntries; }; diff --git a/dom/performance/tests/test_performance_observer.js b/dom/performance/tests/test_performance_observer.js index 9716570e29..767240a99b 100644 --- a/dom/performance/tests/test_performance_observer.js +++ b/dom/performance/tests/test_performance_observer.js @@ -14,25 +14,25 @@ test(t => { var observer = new PerformanceObserver(() => { }); - assert_throws({name: "TypeError"}, function() { + assert_throws({name: "SyntaxError"}, function() { observer.observe(); - }, "observe() should throw TypeError exception if no option specified."); + }, "observe() should throw SyntaxError exception if no option specified."); - assert_throws({name: "TypeError"}, function() { + assert_throws({name: "SyntaxError"}, function() { observer.observe({ unsupportedAttribute: "unsupported" }); - }, "obsrve() should throw TypeError exception if the option has no 'entryTypes' attribute."); + }, "observe() should throw SyntaxError exception if the option has no 'entryTypes' attribute."); assert_throws({name: "TypeError"}, function() { observer.observe({ entryTypes: [] }); - }, "obsrve() should throw TypeError exception if 'entryTypes' attribute is an empty sequence."); + }, "observe() should throw TypeError exception if 'entryTypes' attribute is an empty sequence."); assert_throws({name: "TypeError"}, function() { observer.observe({ entryTypes: null }); - }, "obsrve() should throw TypeError exception if 'entryTypes' attribute is null."); + }, "observe() should throw TypeError exception if 'entryTypes' attribute is null."); assert_throws({name: "TypeError"}, function() { observer.observe({ entryTypes: ["invalid"]}); - }, "obsrve() should throw TypeError exception if 'entryTypes' attribute value is invalid."); + }, "observe() should throw TypeError exception if 'entryTypes' attribute value is invalid."); }, "Test that PerformanceObserver.observe throws exception"); function promiseObserve(test, options) { diff --git a/dom/webidl/PerformanceObserver.webidl b/dom/webidl/PerformanceObserver.webidl index 4cebecbeba..4e08adb34b 100644 --- a/dom/webidl/PerformanceObserver.webidl +++ b/dom/webidl/PerformanceObserver.webidl @@ -8,8 +8,9 @@ */ dictionary PerformanceObserverInit { - required sequence entryTypes; - boolean buffered = false; + sequence entryTypes; + DOMString type; + boolean buffered; }; callback PerformanceObserverCallback = void (PerformanceObserverEntryList entries, PerformanceObserver observer); @@ -18,7 +19,8 @@ callback PerformanceObserverCallback = void (PerformanceObserverEntryList entrie Constructor(PerformanceObserverCallback callback), Exposed=(Window,Worker)] interface PerformanceObserver { - [Throws] - void observe(PerformanceObserverInit options); + [Throws] void observe(optional PerformanceObserverInit options); void disconnect(); + PerformanceEntryList takeRecords(); + static readonly attribute object supportedEntryTypes; }; diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index a7a4929f3b..2f2fa78d42 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1000,11 +1000,13 @@ private: class ReportErrorToConsoleRunnable final : public WorkerRunnable { 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) + Report(WorkerPrivate* aWorkerPrivate, const char* aMessage, + const nsTArray& aParams) { if (aWorkerPrivate) { aWorkerPrivate->AssertIsOnWorkerThread(); @@ -1015,23 +1017,30 @@ public: // Now fire a runnable to do the same on the parent's thread if we can. if (aWorkerPrivate) { RefPtr runnable = - new ReportErrorToConsoleRunnable(aWorkerPrivate, aMessage); + new ReportErrorToConsoleRunnable(aWorkerPrivate, aMessage, aParams); runnable->Dispatch(); return; } + uint16_t paramCount = aParams.Length(); + const char16_t** params = new const char16_t*[paramCount]; + for (uint16_t i=0; i& aParams) : WorkerRunnable(aWorkerPrivate, ParentThreadUnchangedBusyCount), - mMessage(aMessage) + mMessage(aMessage), mParams(aParams) { } virtual void @@ -1048,7 +1057,7 @@ private: { WorkerPrivate* parent = aWorkerPrivate->GetParent(); MOZ_ASSERT_IF(!parent, NS_IsMainThread()); - Report(parent, mMessage); + Report(parent, mMessage, mParams); return true; } }; @@ -6022,13 +6031,22 @@ WorkerPrivate::ReportError(JSContext* aCx, JS::ConstUTF8CharsZ aToStringResult, // static void WorkerPrivate::ReportErrorToConsole(const char* aMessage) +{ + nsTArray emptyParams; + WorkerPrivate::ReportErrorToConsole(aMessage, emptyParams); +} + +// static +void +WorkerPrivate::ReportErrorToConsole(const char* aMessage, + const nsTArray& aParams) { WorkerPrivate* wp = nullptr; if (!NS_IsMainThread()) { wp = GetCurrentThreadWorkerPrivate(); } - ReportErrorToConsoleRunnable::Report(wp, aMessage); + ReportErrorToConsoleRunnable::Report(wp, aMessage, aParams); } int32_t diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 0a71420047..351f5458f1 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -1206,6 +1206,9 @@ public: static void ReportErrorToConsole(const char* aMessage); + static void + ReportErrorToConsole(const char* aMessage, const nsTArray& aParams); + int32_t SetTimeout(JSContext* aCx, nsIScriptTimeoutHandler* aHandler, int32_t aTimeout, bool aIsInterval, -- cgit v1.2.3