Fix Resource Timing buffer edge cases for WPT
Created attachment 358530 [details] Patch
Created attachment 358532 [details] Patch
This patch fixes: https://bugs.webkit.org/show_bug.cgi?id=193004 https://bugs.webkit.org/show_bug.cgi?id=193006
Comment on attachment 358532 [details] Patch Attachment 358532 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10663037 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 358540 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 358532 [details] Patch Attachment 358532 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10663138 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 358544 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 358532 [details] Patch Attachment 358532 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10663410 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 358552 [details] Archive of layout-test-results from ews114 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 358532 [details] Patch Attachment 358532 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/10664196 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 358554 [details] Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 358532 [details] Patch Attachment 358532 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10663761 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 358555 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 358532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358532&action=review > Source/WebCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=193213 It seems one WebKit test needs rebasing. > Source/WebCore/ChangeLog:13 > + buffer-full-when-populate-entries.html Should we import them in WebKit (you can do Tools/Scripts/import-w3c-tests -t web-platform-tests/resource-timing for that purpose)? > Source/WebCore/ChangeLog:18 > + (WebCore::Performance::resourceTimingBufferFullTimerFired): Could you describe a little bit the changes?
Created attachment 360247 [details] Patch
Comment on attachment 360247 [details] Patch Attachment 360247 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10904194 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 360249 [details] Archive of layout-test-results from ews100 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 360247 [details] Patch Attachment 360247 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10904210 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 360250 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 360247 [details] Patch Attachment 360247 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/10904292 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 360253 [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 360247 [details] Patch Attachment 360247 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10904244 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 360254 [details] Archive of layout-test-results from ews114 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 360247 [details] Patch Attachment 360247 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10904275 New failing tests: imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 360255 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360264 [details] Patch
Comment on attachment 360264 [details] Patch Attachment 360264 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10907141 New failing tests: imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html
Created attachment 360268 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360344 [details] Patch
Created attachment 360345 [details] Patch
Comment on attachment 360345 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360345&action=review > Source/WebCore/page/Performance.cpp:249 > + if (beforeCount <= afterCount) { If this check is true, it seems we would be loosing some information, how big an issue is it? Should we do a smarter check here? Something like, continue if we ever queue any entry. Or is there a risk with that approach? > LayoutTests/imported/w3c/web-platform-tests/resource-timing/redirects.sub-expected.txt:7 > +FAIL Testing resource entries assert_unreached: http://localhost:8800/common/redirect.py?location=https://www.localhost:9443/resource-timing/resources/blank_page_green.htm?id=xhr is expected to be in the Resource Timing buffer Reached unreachable code We probably could rewrite the test (as a WPT PR) to make www.localhost be 127.0.0.1 in that context. Ditto below.
Comment on attachment 360345 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360345&action=review >> Source/WebCore/page/Performance.cpp:249 >> + if (beforeCount <= afterCount) { > > If this check is true, it seems we would be loosing some information, how big an issue is it? > Should we do a smarter check here? > Something like, continue if we ever queue any entry. Or is there a risk with that approach? In the spec discussions, it was agreed (with rniwa) that this is the desired behaviour. If the event fired, but its handler did not increase the buffer size or cleared the entries, running the event again (because there are entries in the secondary buffer) is likely to result in an infinite loop. So, in this rare case when there's a handler which doesn't make any room in the buffer, we will lose entries. That seems reasonable.
Yeah, in this particular case, keep firing the event is probably no that useful. In particular, we don't want to keep the buffer full in the event the page wasn't listening to the event at all.
Created attachment 362013 [details] Patch
Created attachment 362014 [details] Patch
Comment on attachment 362014 [details] Patch Attachment 362014 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11147120 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 362020 [details] Archive of layout-test-results from ews100 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 362014 [details] Patch Attachment 362014 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11147143 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 362021 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 362014 [details] Patch Attachment 362014 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11147189 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 362024 [details] Archive of layout-test-results from ews113 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 362014 [details] Patch Attachment 362014 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11147464 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 362025 [details] Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Created attachment 362028 [details] Patch
Comment on attachment 362028 [details] Patch Attachment 362028 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11148272 New failing tests: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
Created attachment 362034 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 362039 [details] Patch
Comment on attachment 362039 [details] Patch Attachment 362039 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11150028 New failing tests: imported/w3c/web-platform-tests/resource-timing/buffer-full-add-after-full-event.html
Created attachment 362060 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 362095 [details] Patch
Created attachment 362115 [details] Patch
@youennf can you PTAL? :)
Comment on attachment 362115 [details] Patch Clearing flags on attachment: 362115 Committed r242209: <https://trac.webkit.org/changeset/242209>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48481194>