Fix ResourceTiming XHR tests flakiness
Created attachment 279638 [details] Patch
This should fix the test flakiness observed in https://bugs.webkit.org/show_bug.cgi?id=157816 It also removes the XHR-specific treatment that I added in https://bugs.webkit.org/show_bug.cgi?id=157669 as it is not required. It seems like the ResourceTiming entries are added in CachedResourceLoader::loadDone only after the onload event for the XHR is running. I've opened a related spec issue to see what the correct behavior here should be: https://github.com/w3c/resource-timing/issues/55
Comment on attachment 279638 [details] Patch Attachment 279638 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1373989 New failing tests: http/tests/performance/performance-resource-timing-xhr-single-entry.html
Created attachment 279640 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 279638 [details] Patch Attachment 279638 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1374049 New failing tests: http/tests/performance/performance-resource-timing-xhr-single-entry.html
Created attachment 279641 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 279638 [details] Patch Attachment 279638 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1374057 New failing tests: http/tests/performance/performance-resource-timing-xhr-single-entry.html
Created attachment 279642 [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.11.4
Comment on attachment 279638 [details] Patch Attachment 279638 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1374054 New failing tests: http/tests/performance/performance-resource-timing-xhr-single-entry.html
Created attachment 279643 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 279645 [details] Patch
Comment on attachment 279645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279645&action=review > LayoutTests/http/tests/performance/performance-resource-timing-cached-entries.html:35 > + setTimeout(runTest, 10); A timeout of 10 ms is never right in regression tests. A process can be easily suspended for much longer. It's especially expected on GuardMalloc and ASan tests, but even release build tests have a lot of competition for resources.
(In reply to comment #12) > Comment on attachment 279645 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279645&action=review > > > LayoutTests/http/tests/performance/performance-resource-timing-cached-entries.html:35 > > + setTimeout(runTest, 10); > > A timeout of 10 ms is never right in regression tests. A process can be > easily suspended for much longer. It's especially expected on GuardMalloc > and ASan tests, but even release build tests have a lot of competition for > resources. Any timeout would do here, it's just that I need for the test to run after the XHR's load event is over. So if the process will get suspended for a longer period, that's not an issue as long as the test's timeout is not exceeded. Let me know if a different timeout value (or a different method to run the test after the onload is over) is preferred.
Comment on attachment 279645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279645&action=review Would it work with a timeout of 0? I think a better approach would be to create the xhr after you're done with the first runTest call to make sure it will never start before the first one. > LayoutTests/http/tests/performance/performance-resource-timing-xhr-single-entry.html:35 > + setTimeout(runTest, 10); Why do you call runTest twice but only expect it to be run once?
(In reply to comment #14) > Comment on attachment 279645 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279645&action=review > > Would it work with a timeout of 0? I believe it would. > I think a better approach would be to > create the xhr after you're done with the first runTest call to make sure it > will never start before the first one. > > > LayoutTests/http/tests/performance/performance-resource-timing-xhr-single-entry.html:35 > > + setTimeout(runTest, 10); > > Why do you call runTest twice but only expect it to be run once? I copied this mechanism from another test where the XHR call is racing with window's onload event (as XHR is not blocking onload). So, I wanted the test to run once at the later one of those events, but not inside XHR's onload event, as the entries are not yet added at this point. It's true that for the xhr-single-entry I could just eliminate the window.onload part altogether.
Created attachment 279666 [details] Patch
Created attachment 279707 [details] Patch
Latest patch adds back the tests that got rolled out in http://trac.webkit.org/changeset/201343, as this fixes their flakiness. Let me know if it's preferred that I'll split this patch into two separate ones (as it treats two different flakiness issues).
Could you describe the difference in your latest patch?
Created attachment 279818 [details] Patch
I removed the parts related to initiator tests flakiness, and only left in the parts related to XHR flakiness, in order to make the patch clearer. I'll submit the initiator parts as a separate patch.
Initiator changes are at https://bugs.webkit.org/show_bug.cgi?id=158094
Comment on attachment 279818 [details] Patch Clearing flags on attachment: 279818 Committed r201414: <http://trac.webkit.org/changeset/201414>
All reviewed patches have been landed. Closing bug.