Fix flaky layout tests that rely on setTimeout()s firing after <body> is parsed
Created attachment 193033 [details] Patch
*** Bug 112292 has been marked as a duplicate of this bug. ***
Comment on attachment 193033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193033&action=review LGTM, I suspect you'll see one failure from this. > LayoutTests/fast/block/float/floats-not-cleared-crash.html:9 > +window.onload = crash; > function crash() This doesn't look right. crash isn't defined yet. :)
Look at all these fixable flakes: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fblock%2Ffloat%2Ffloats-not-cleared-crash.html%20fast%2Fblock%2Fline-layout%2Finline-box-wrapper-crash.html%20fast%2Fcss%2Fpositioned-in-relative-position-inline-crash.html%20fast%2Fcss%2Fuser-stylesheet-crash.html%20fast%2Fencoding%2Fscript-in-head.html%20fast%2Fforms%2Ftextarea-placeholder-relayout-assertion.html%20fast%2Finline%2Fupdate-always-create-line-boxes-full-layout-crash.html%20fast%2FinnerHTML%2FinnerHTML-iframe.html%20fast%2Fjs%2Fsame-origin-subframe-about-blank.html%20fast%2Fmulticol%2Fspan%2Fremoval-of-multicol-span-crash.html%20fast%2Ftext%2Finternational%2Fbidi-neutral-in-mixed-direction-run-crash.html%20fast%2Fwriting-mode%2Foverhanging-float-legend-crash.html%20editing%2Finserting%2Finsert-text-into-empty-frameset-crash.html%20editing%2Fstyle%2Fapply-style-crash.html%20fast%2Fblock%2Ffloat%2Ffloat-originating-line-deleted-crash.html%20fast%2Fblock%2Fline-layout%2Finline-box-wrapper-crash.html%20fast%2Fframes%2Fseamless%2Fseamless-form-get.html%20fast%2Fframes%2Fseamless%2Fseamless-form-post-named.html%20fast%2Fframes%2Fseamless%2Fseamless-window-location-href.html%20fast%2Fframes%2Fseamless%2Fseamless-window-location-replace.html
(In reply to comment #3) > (From update of attachment 193033 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193033&action=review > > LGTM, I suspect you'll see one failure from this. > > > LayoutTests/fast/block/float/floats-not-cleared-crash.html:9 > > +window.onload = crash; > > function crash() > > This doesn't look right. crash isn't defined yet. :) You forget this is JavaScript where craziness like this works A-OK :)
Created attachment 193034 [details] Patch for landing
Comment on attachment 193033 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193033&action=review >>> LayoutTests/fast/block/float/floats-not-cleared-crash.html:9 >>> function crash() >> >> This doesn't look right. crash isn't defined yet. :) > > You forget this is JavaScript where craziness like this works A-OK :) THIS IS JAVASCRIPT
Comment on attachment 193034 [details] Patch for landing Rejecting attachment 193034 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 193034, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: fast/innerHTML/innerHTML-iframe.html patching file LayoutTests/fast/js/same-origin-subframe-about-blank.html patching file LayoutTests/fast/multicol/span/removal-of-multicol-span-crash.html patching file LayoutTests/fast/text/international/bidi-neutral-in-mixed-direction-run-crash.html patching file LayoutTests/fast/writing-mode/overhanging-float-legend-crash.html Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://webkit-commit-queue.appspot.com/results/17181210
Comment on attachment 193034 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=193034&action=review Another nice catch! > LayoutTests/fast/encoding/script-in-head.html:8 > - 'setTimeout(function () {' + > + 'window.onload = function () {' + This changes how the test works - previously, the script normally ran after onload, not during it. A less invasive fix would be to start a zero-delay timer in onload. Or if the test is changed as currently proposed, then it doesn't need to be async, waitUntilDone/notifyDone can be removed. Either of these changes probably needs to be done to most of the tests fixed here, I didn't check each one.
(In reply to comment #9) > (From update of attachment 193034 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193034&action=review > > Another nice catch! > > > LayoutTests/fast/encoding/script-in-head.html:8 > > - 'setTimeout(function () {' + > > + 'window.onload = function () {' + > > This changes how the test works - previously, the script normally ran after onload, not during it. A less invasive fix would be to start a zero-delay timer in onload. Yes, it's a change. Reading the test, it won't make a difference in the test coverage. > > Or if the test is changed as currently proposed, then it doesn't need to be async, waitUntilDone/notifyDone can be removed. That's a good catch. I removed the waitUntilDone/notifyDone pairs from most tests, missed this one. Will check the rest. > > Either of these changes probably needs to be done to most of the tests fixed here, I didn't check each one. I did check each one to see if it's sensitive to setTimeout or just needed to run after the rest of the test was parsed. I've left tests alone that are specifically about setTimeout timing (i.e. some security ones that intentionally set up races to make sure we don't crash).
Created attachment 193158 [details] Patch for landing
Comment on attachment 193158 [details] Patch for landing Rejecting attachment 193158 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 193158, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: fast/innerHTML/innerHTML-iframe.html patching file LayoutTests/fast/js/same-origin-subframe-about-blank.html patching file LayoutTests/fast/multicol/span/removal-of-multicol-span-crash.html patching file LayoutTests/fast/text/international/bidi-neutral-in-mixed-direction-run-crash.html patching file LayoutTests/fast/writing-mode/overhanging-float-legend-crash.html Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://webkit-commit-queue.appspot.com/results/17024990
(In reply to comment #12) > (From update of attachment 193158 [details]) > Rejecting attachment 193158 [details] from commit-queue. > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 193158, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue > > Last 500 characters of output: > fast/innerHTML/innerHTML-iframe.html > patching file LayoutTests/fast/js/same-origin-subframe-about-blank.html > patching file LayoutTests/fast/multicol/span/removal-of-multicol-span-crash.html > patching file LayoutTests/fast/text/international/bidi-neutral-in-mixed-direction-run-crash.html > patching file LayoutTests/fast/writing-mode/overhanging-float-legend-crash.html > > Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue > > Full output: http://webkit-commit-queue.appspot.com/results/17024990 Hmm, I think svn-apply may be barfing on the CRLF files.
Committed r145857: <http://trac.webkit.org/changeset/145857>
(In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 193158 [details] [details]) > > Rejecting attachment 193158 [details] [details] from commit-queue. > > >> [...] > > > > Full output: http://webkit-commit-queue.appspot.com/results/17024990 > > Hmm, I think svn-apply may be barfing on the CRLF files. Filed bug 112732 for this issue.