RESOLVED FIXED 193428
Implement PerformanceObserver.supportedEntryTypes
https://bugs.webkit.org/show_bug.cgi?id=193428
Summary Implement PerformanceObserver.supportedEntryTypes
cvazac
Reported 2019-01-14 20:18:32 PST
Implement PerformanceObserver.supportedEntryTypes
Attachments
Patch (4.69 KB, patch)
2019-01-14 20:19 PST, cvazac
no flags
Patch (5.06 KB, patch)
2019-01-14 20:38 PST, cvazac
no flags
Patch (5.11 KB, patch)
2019-01-14 20:55 PST, cvazac
no flags
Patch (5.13 KB, patch)
2019-01-14 20:59 PST, cvazac
no flags
Patch (77.71 KB, patch)
2019-01-15 09:37 PST, cvazac
no flags
Patch (88.10 KB, patch)
2019-01-18 08:34 PST, cvazac
no flags
Patch (19.88 KB, patch)
2019-01-18 12:10 PST, cvazac
no flags
Patch (19.34 KB, patch)
2019-01-18 12:13 PST, cvazac
no flags
cvazac
Comment 1 2019-01-14 20:19:25 PST
EWS Watchlist
Comment 2 2019-01-14 20:22:52 PST
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.
cvazac
Comment 3 2019-01-14 20:38:05 PST
Joseph Pecoraro
Comment 4 2019-01-14 20:43:15 PST
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.
Joseph Pecoraro
Comment 5 2019-01-14 20:53:07 PST
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.
cvazac
Comment 6 2019-01-14 20:55:25 PST
cvazac
Comment 7 2019-01-14 20:59:15 PST
Alex Christensen
Comment 8 2019-01-14 20:59:50 PST
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.
Joseph Pecoraro
Comment 9 2019-01-14 21:03:52 PST
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 };
cvazac
Comment 10 2019-01-15 09:37:46 PST
cvazac
Comment 11 2019-01-15 09:38:23 PST
(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.
cvazac
Comment 12 2019-01-18 08:34:08 PST
cvazac
Comment 13 2019-01-18 12:10:10 PST
cvazac
Comment 14 2019-01-18 12:13:10 PST
Joseph Pecoraro
Comment 15 2019-01-18 12:59:38 PST
Comment on attachment 359519 [details] Patch r=me
WebKit Commit Bot
Comment 16 2019-01-24 16:04:58 PST
Comment on attachment 359519 [details] Patch Clearing flags on attachment: 359519 Committed r240454: <https://trac.webkit.org/changeset/240454>
WebKit Commit Bot
Comment 17 2019-01-24 16:05:00 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 18 2019-01-24 16:05:32 PST
Note You need to log in before you can comment on or make changes to this bug.