Summary: | Implement PerformanceObserver.supportedEntryTypes | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | cvazac | ||||||||||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | achristensen, cdumez, commit-queue, esprehn+autocc, ews-watchlist, joepeck, kondapallykalyan, rniwa, webkit-bug-importer, youennf | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Attachments: |
|
Description
cvazac
2019-01-14 20:18:32 PST
Created attachment 359123 [details]
Patch
Attachment 359123 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 359125 [details]
Patch
Comment on attachment 359125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359125&action=review > Source/WebCore/ChangeLog:9 > + PerformanceObserver.supportedEntryTypes should return an array of > + entryTypes that can be observed per specification[1]. > + > + [1] https://w3c.github.io/performance-timeline/#supportedentrytypes-attribute No need for the Markdown syntax. This would read much clearer as a paragraph: PerformanceObserver.supportedEntryTypes should return an array of entryTypes that can be observed per specification: https://w3c.github.io/performance-timeline/#supportedentrytypes-attribute > Source/WebCore/ChangeLog:14 > + This is covered by web-platform-tests[1]. > + [1] LayoutTests/imported/w3c/web-platform-tests/resource-timing/supported_resource_type.*.html Same thing here. > Source/WebCore/page/PerformanceObserver.cpp:119 > + "navigation", Does WebKit actually support navigation entries in PerformanceObserver? I didn't think we do that yet, and if so we probably shouldn't broadcast it as supported. Comment on attachment 359125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359125&action=review >> Source/WebCore/page/PerformanceObserver.cpp:119 >> + "navigation", > > Does WebKit actually support navigation entries in PerformanceObserver? I didn't think we do that yet, and if so we probably shouldn't broadcast it as supported. Yeah, seems like this should be commented out with a FIXME: // FIXME: <https://webkit.org/b/184363> Add support for Navigation Timing Level 2 // "navigation", Because we don't yet have "navigation" entries in our performance timeline. Created attachment 359128 [details]
Patch
Created attachment 359129 [details]
Patch
Comment on attachment 359129 [details]
Patch
It would be nice if there were web platform tests verifying somehow that the entry types claimed to be supported are actually supported.
Comment on attachment 359129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359129&action=review r=me > Source/WebCore/page/PerformanceObserver.cpp:123 > + "resource" Type inference should allow you to drop the explicit type here, and you can more efficiently construct the WTFStrings from literals with the `_s` suffix: return { // FIXME: <https://webkit.org/b/184363> Add support for Navigation Timing Level 2 // "navigation"_s, "mark"_s, "measure"_s, "resource"_s }; Created attachment 359167 [details]
Patch
(In reply to Alex Christensen from comment #8) > Comment on attachment 359129 [details] > Patch > > It would be nice if there were web platform tests verifying somehow that the > entry types claimed to be supported are actually supported. Good idea! Will import this[1] if/when I can get it to land. [1] https://github.com/web-platform-tests/wpt/pull/14867. Created attachment 359492 [details]
Patch
Created attachment 359518 [details]
Patch
Created attachment 359519 [details]
Patch
Comment on attachment 359519 [details]
Patch
r=me
Comment on attachment 359519 [details] Patch Clearing flags on attachment: 359519 Committed r240454: <https://trac.webkit.org/changeset/240454> All reviewed patches have been landed. Closing bug. |