Runtime feature flag for Server-Timing
Created attachment 338263 [details] Patch
Comment on attachment 338263 [details] Patch Since this is off by default, you will need to activate it for WebKitTestRunner/DumpRenderTree.
Comment on attachment 338263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338263&action=review > Source/WTF/wtf/FeatureDefines.h:687 > +#endif These are compile flags. I am not sure we need this one actually. Instead, I would go just with a runtime flag. > Source/WebCore/page/RuntimeEnabledFeatures.h:254 > + bool serverTimingEnabled() const { return m_isServerTimingEnabled; } This is the runtime flag there. To allow STP users to enable it as an experimental feature, you would need to add it to WebPreferences.yaml. You will also need to activate it in WebKitTestRunner and DumpRenderTree by default. To ServiceWorker processes as well probably. See for instance restrictedHTTPResponseAccess above as an example. To validate that it is working, you could for instance add: - some IDL behind the runtime flag: PerformanceServerTiming.idl/.h/.cpp and the change to PerformanceResourceTiming.idl - a test verifying that the property is there on PerformanceResourceTiming.
Comment on attachment 338263 [details] Patch r-'ing based on Youenn's comment.
Created attachment 338990 [details] Patch
Created attachment 338996 [details] Patch
For win, you need to make some changes to Source/WebKitLegacy. You can use https://github.com/WebKit/webkit/commit/ecc55615907980a2ab27df68230081bd5ce07838 as an example. Would be nice to add the IDLs to validate that the flag is working as expected
Created attachment 339381 [details] Patch
Created attachment 339383 [details] Patch
Comment on attachment 339383 [details] Patch Attachment 339383 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7546790 New failing tests: imported/w3c/web-platform-tests/server-timing/resource_timing_idl.html imported/w3c/web-platform-tests/server-timing/navigation_timing_idl.html
Created attachment 339389 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 339383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339383&action=review > Source/WebCore/page/PerformanceServerTiming.h:46 > + double m_duration; We don't use double to store time anymore. Is this monotonically increasing time or a wall time? > Source/WebCore/page/RuntimeEnabledFeatures.h:396 > + bool m_minDeviceWidthEnabled { false }; > + Please remove this.
(In reply to Ryosuke Niwa from comment #12) > Comment on attachment 339383 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339383&action=review > > > Source/WebCore/page/PerformanceServerTiming.h:46 > > + double m_duration; > > We don't use double to store time anymore. > Is this monotonically increasing time or a wall time? it's just a user-defined double: https://w3c.github.io/server-timing/#-dfn-duration-dfn-attribute > > > Source/WebCore/page/RuntimeEnabledFeatures.h:396 > > + bool m_minDeviceWidthEnabled { false }; > > + > > Please remove this. removed :)
Created attachment 339725 [details] Patch
Comment on attachment 339725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339725&action=review > Source/WebCore/page/RuntimeEnabledFeatures.h:395 > + bool m_minDeviceWidthEnabled { false }; Probably do not need that one. > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:132 > + RuntimeEnabledFeatures::sharedFeatures().setServerTimingEnabled(store.getBoolValueForKey(WebPreferencesKey::serverTimingEnabledKey())); It would be nice to have a test for service worker like done for window.
Created attachment 339726 [details] Patch
Created attachment 339732 [details] Patch
Created attachment 339816 [details] Patch
Created attachment 339818 [details] Patch
With regards to WinCairo being red I believe this has to due with an issue for our build not regenerating things properly when IWebPreferencesPrivate.idl is modified. Since this is known we would just need to do a clean build on the bots when landed.
I can't make wincairo happy, although I'm hoping it's an issue with the build process not knowing when to regenerate things. Also, I added a WPT to verify that serverTiming is exposed on PerformanceResourceTiming from inside a Service Worker here: https://github.com/w3c/web-platform-tests/pull/10901 The test passes when I manually copy it into my webkit tree. I will properly import it and add it to this bug when after it's merged into WPT.
r = me. > Also, I added a WPT to verify that serverTiming is exposed on > PerformanceResourceTiming from inside a Service Worker here: > https://github.com/w3c/web-platform-tests/pull/10901 Nice! You could also have included it in this patch. Once it is reviewed here, it is ok to land it directly in WPT. > The test passes when I manually copy it into my webkit tree. I will properly > import it and add it to this bug when after it's merged into WPT.
Created attachment 339866 [details] Patch
Comment on attachment 339866 [details] Patch Attachment 339866 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7611895 New failing tests: imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339880 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 339866 [details] Patch Attachment 339866 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7613065 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html
Created attachment 339890 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 339866 [details] Patch Attachment 339866 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/7613136 New failing tests: imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339891 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 339866 [details] Patch Attachment 339866 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7613423 New failing tests: imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339893 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 339866 [details] Patch Attachment 339866 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7617375 New failing tests: http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339908 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 339965 [details] Patch
Comment on attachment 339965 [details] Patch Attachment 339965 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7624045 New failing tests: imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339970 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 339965 [details] Patch Attachment 339965 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7624078 New failing tests: imported/w3c/web-platform-tests/server-timing/service_worker_idl.html
Created attachment 339975 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6
The builds mac and mac-debug appear to be failing because service-workers aren't enabled. Do you know how to fix that Youenn? Do I have to do something in TestController.cpp?
Created attachment 340025 [details] Patch
(In reply to cvazac from comment #39) > The builds mac and mac-debug appear to be failing because service-workers > aren't enabled. Do you know how to fix that Youenn? Do I have to do > something in TestController.cpp? Any service worker test needs to be skipped for WK1. Either you move the test in a folder that is already skipped or you need to update LayoutTests/platform/mac-wk1/TestExpectations and LayoutTests/platform/win/TestExpectations.
Comment on attachment 340025 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340025&action=review > LayoutTests/platform/mac-wk1/TestExpectations:152 > +imported/w3c/web-platform-tests/server-timing/service_worker_idl.html [ Skip ] You'll probably need to skip it on win as well. Also ios-wk1 for good measure.
(In reply to Chris Dumez from comment #42) > Comment on attachment 340025 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=340025&action=review > > > LayoutTests/platform/mac-wk1/TestExpectations:152 > > +imported/w3c/web-platform-tests/server-timing/service_worker_idl.html [ Skip ] > > You'll probably need to skip it on win as well. Also ios-wk1 for good > measure. Do I need still need to SKIP on win and ios-wk1? The only failures are here (https://webkit-queues.webkit.org/patch/340025/win-ews) and they appear unrelated.
Comment on attachment 340025 [details] Patch r=me. I do not know why the test does not show up as failing in WK1 win bot. Anyway, I think it is nice to help maintainers by skipping it since we know service worker is not supported in WK1.
Created attachment 340095 [details] Patch
Created attachment 340096 [details] Patch
Comment on attachment 340096 [details] Patch Clearing flags on attachment: 340096 Committed r231709: <https://trac.webkit.org/changeset/231709>
All reviewed patches have been landed. Closing bug.
<rdar://problem/40169546>