** Steps to reproduce: ** 1. Navigate to example.com (or any web page) in Safari and run the following code in the console: ``` new PerformanceObserver((list) => { for (const entry of list.getEntries()) { console.log(entry); } }).observe({type: 'resource'}); ``` 2. Notice that the following error is thrown: TypeError: Member PerformanceObserverInit.entryTypes is required and must be an instance of sequence ** What should happen: ** No error should be thrown if the `type` options is used instead of `entryTypes`. ** Justification: ** The WebIDL for PerformanceObseverInit [1] shows that it accepts either an `entryTypes` option OR a `type` option: ``` dictionary PerformanceObserverInit { sequence<DOMString> entryTypes; DOMString type; boolean buffered; }; ``` [1] https://w3c.github.io/performance-timeline/#dom-performanceobserverinit
<rdar://problem/60611562>
Created attachment 405287 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Created attachment 405289 [details] Patch
Created attachment 405301 [details] Patch
Comment on attachment 405301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405301&action=review > Source/WebCore/page/PerformanceObserver.cpp:68 > + for (const String& entryType : *init.entryTypes) { const auto& > Source/WebCore/page/PerformanceObserver.cpp:-74 > - You could replace the two above if by: if (filter.isEmpty()) return { }; That would allow to remove validEntryTypes for instance. > LayoutTests/imported/w3c/web-platform-tests/resource-timing/buffered-flag.any.worker-expected.txt:2 > +Harness Error (TIMEOUT), message = null Do you know why all these tests are timing out? Since they are timing out, we should probably skip them in TestExpectations.
Comment on attachment 405301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405301&action=review >> Source/WebCore/page/PerformanceObserver.cpp:68 >> + for (const String& entryType : *init.entryTypes) { > > const auto& Iād go further and suggest just auto&
Created attachment 405375 [details] Patch
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
Created attachment 405388 [details] Patch
Committed r265001: <https://trac.webkit.org/changeset/265001> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405388 [details].
Comment on attachment 405388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405388&action=review > Source/WebCore/page/PerformanceObserver.cpp:69 > + filter.add(*type); Could remove a little bit of churn with WTFMove. > Source/WebCore/page/PerformanceObserver.cpp:77 > filter.add(*type); Could remove a little bit of churn with WTFMove.
(In reply to Darin Adler from comment #12) > Comment on attachment 405388 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=405388&action=review > > > Source/WebCore/page/PerformanceObserver.cpp:69 > > + filter.add(*type); > > Could remove a little bit of churn with WTFMove. > > > Source/WebCore/page/PerformanceObserver.cpp:77 > > filter.add(*type); > > Could remove a little bit of churn with WTFMove. Probably not, type is an Optional of an enum and filter is an OptionSet.
Comment on attachment 405388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405388&action=review >>> Source/WebCore/page/PerformanceObserver.cpp:69 >>> + filter.add(*type); >> >> Could remove a little bit of churn with WTFMove. > > Probably not, type is an Optional of an enum and filter is an OptionSet. Got it. I thought it was a string, not a "parsed string".