112075 2013-03-11 16:01:45 -0700 De-flake fast/dom/title-directionality.html 2013-03-11 16:42:45 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jamesr jamesr abarth eric esprehn rniwa tony oldest_to_newest 852821 0 jamesr 2013-03-11 16:01:45 -0700 De-flake fast/dom/title-directionality.html 852824 1 192584 jamesr 2013-03-11 16:03:34 -0700 Created attachment 192584 Patch 852830 2 jamesr 2013-03-11 16:11:10 -0700 This patch does de-flake the test, but I don't fully understand how. Without this patch: Found 1 test; running 1 (500 times each: --repeat-each=100 --iterations=5), skipping 0. Running 1 DumpRenderTree. [17/500] fast/dom/title-directionality.html failed unexpectedly (text diff) ... 478 tests ran as expected, 22 didn't: With this patch: Found 1 test; running 1 (500 times each: --repeat-each=100 --iterations=5), skipping 0. Running 1 DumpRenderTree. All 500 tests ran as expected. I've seen similar tests flake in the same way (differing in a trailing newline) because the HTML parser was at a different point in the main document when the test code ran, usually because of a setTimeout(..., 0) in an inline script. There's no setTimeout here, but the parser is pumped synchronously on an iframe by doc.open()/write()/close(). 852837 3 192584 darin 2013-03-11 16:16:37 -0700 Comment on attachment 192584 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192584&action=review > LayoutTests/ChangeLog:9 > + De-flake fast/dom/title-directionality.html > + https://bugs.webkit.org/show_bug.cgi?id=112075 > + > + Reviewed by NOBODY (OOPS!). > + > + * fast/dom/title-directionality-expected.txt: > + * fast/dom/title-directionality.html: Patch would be much better if this said what the fix was, and why it works. 852840 4 192584 tony 2013-03-11 16:18:22 -0700 Comment on attachment 192584 Patch Seems worth trying. I guess there could be a race between the email iframe load and the first test case. Not sure what type of error that would cause. 852843 5 esprehn 2013-03-11 16:19:26 -0700 This smells like lazyAttach + Node::attach logic and the whitespace fixup stuff in there. eseidel had mentioned that code doesn't look right recently. 852855 6 jamesr 2013-03-11 16:28:28 -0700 (In reply to comment #3) > (From update of attachment 192584 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192584&action=review > > > LayoutTests/ChangeLog:9 > > + De-flake fast/dom/title-directionality.html > > + https://bugs.webkit.org/show_bug.cgi?id=112075 > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + * fast/dom/title-directionality-expected.txt: > > + * fast/dom/title-directionality.html: > > Patch would be much better if this said what the fix was, and why it works. Agreed - I don't fully understand why it works yet, so I can't populate a description. I can put a guess in. 852858 7 jamesr 2013-03-11 16:37:25 -0700 Elliot pointed out that the lazy attachment logic works by considering all nodes 'attached' and then setting the style recalc timer. This style recalc races against the HTML parser hitting the end of the document. If the HTML parser yields after attaching some nodes before the end of the test (i.e. after running the inline script but before hitting the end of the main document), the collapsing logic in Node::attach() behaves in a different way than if the rest of the document is parsed without any interleaving style recalculations. Since this particular test isn't intended to exercise the whitespace collapsing path and we have other (non-flaky) tests for this behavior, this just moves the test logic into an onload handler which fires after the document is fully parsed. I'll land this patch with a description in the ChangeLog based on ^^. 852870 8 jamesr 2013-03-11 16:42:45 -0700 Committed r145429: <http://trac.webkit.org/changeset/145429> 192584 2013-03-11 16:03:34 -0700 2013-03-11 16:18:22 -0700 Patch bug-112075-20130311155934.patch text/plain 4276 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1NDE4CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggM2Q4NGY1N2I4YWQyZjkyMjc4Njcx MWI3YTYwODE4ZWZjNDE0MThlYy4uNTY1ZmJhMmNjY2RiZTlmYWRlNWRlNDg5YzdhMDk0ZGI3N2Zi ZGZiZiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE1IEBACiAyMDEzLTAzLTExICBKYW1lcyBSb2JpbnNvbiAg PGphbWVzckBjaHJvbWl1bS5vcmc+CiAKKyAgICAgICAgRGUtZmxha2UgZmFzdC9kb20vdGl0bGUt ZGlyZWN0aW9uYWxpdHkuaHRtbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTEyMDc1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgKiBmYXN0L2RvbS90aXRsZS1kaXJlY3Rpb25hbGl0eS1leHBlY3RlZC50eHQ6 CisgICAgICAgICogZmFzdC9kb20vdGl0bGUtZGlyZWN0aW9uYWxpdHkuaHRtbDoKKworMjAxMy0w My0xMSAgSmFtZXMgUm9iaW5zb24gIDxqYW1lc3JAY2hyb21pdW0ub3JnPgorCiAgICAgICAgIFVw ZGF0ZSBjaHJvbWl1bSByZW5kZXIgdHJlZSBiYXNlbGluZXMgZm9yIHRyYWNrIGN1ZSBjaGFuZ2Vz IGluIHIxNDUzOTcuCiAKICAgICAgICAgKiBwbGF0Zm9ybS9jaHJvbWl1bS1saW51eC9tZWRpYS90 cmFjay90cmFjay1jdWUtcmVuZGVyaW5nLWhvcml6b250YWwtZXhwZWN0ZWQudHh0OiBBZGRlZC4K ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvZG9tL3RpdGxlLWRpcmVjdGlvbmFsaXR5LWV4 cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL2Zhc3QvZG9tL3RpdGxlLWRpcmVjdGlvbmFsaXR5LWV4 cGVjdGVkLnR4dAppbmRleCBiMjMyNzY4NDBhZGNiN2VjZTg1YzM5MjUzNGM3YzhhZGY5NmFhZDI4 Li43ZGRmOGRkYWM3OGRhODBlNmMyY2JlZDI5YmRlZTRmMWFkMDhmMjQxIDEwMDY0NAotLS0gYS9M YXlvdXRUZXN0cy9mYXN0L2RvbS90aXRsZS1kaXJlY3Rpb25hbGl0eS1leHBlY3RlZC50eHQKKysr IGIvTGF5b3V0VGVzdHMvZmFzdC9kb20vdGl0bGUtZGlyZWN0aW9uYWxpdHktZXhwZWN0ZWQudHh0 CkBAIC0zLDQgKzMsMyBAQAogPGh0bWw+PHRpdGxlIGRpcj1ydGw+Zm9vPC90aXRsZT48L2h0bWw+ IHNob3VsZCBoYXZlIHRpdGxlIGRpcmVjdGlvbiAicnRsIi4gUEFTUwogPGh0bWwgZGlyPXJ0bD48 dGl0bGU+Zm9vPC90aXRsZT48L2h0bWw+IHNob3VsZCBoYXZlIHRpdGxlIGRpcmVjdGlvbiAicnRs Ii4gUEFTUwogPGh0bWwgZGlyPXJ0bD48dGl0bGUgZGlyPWx0cj5mb288L3RpdGxlPjwvaHRtbD4g c2hvdWxkIGhhdmUgdGl0bGUgZGlyZWN0aW9uICJsdHIiLiBQQVNTCi0KZGlmZiAtLWdpdCBhL0xh eW91dFRlc3RzL2Zhc3QvZG9tL3RpdGxlLWRpcmVjdGlvbmFsaXR5Lmh0bWwgYi9MYXlvdXRUZXN0 cy9mYXN0L2RvbS90aXRsZS1kaXJlY3Rpb25hbGl0eS5odG1sCmluZGV4IDI3ZTVhOGYwM2FmOTdl Zjk5MjlmODE1MzhiZTFjZDY4NjgzYmExMDIuLjM3NWE4YzlkZjUzMjY1ZjA5MDEzODNiMGEyMTFl NDhhYmRmODFjNDggMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2Zhc3QvZG9tL3RpdGxlLWRpcmVj dGlvbmFsaXR5Lmh0bWwKKysrIGIvTGF5b3V0VGVzdHMvZmFzdC9kb20vdGl0bGUtZGlyZWN0aW9u YWxpdHkuaHRtbApAQCAtMyw0MyArMyw0NSBAQAogPGhlYWQ+CiA8dGl0bGU+cGFnZSB0aXRsZTwv dGl0bGU+CiA8L2hlYWQ+Ci08Ym9keT4KKzxib2R5IG9ubG9hZD0icnVuVGVzdCgpIj4KIDxpZnJh bWUgaWQ9aWZyYW1lPjwvaWZyYW1lPgogPHNjcmlwdD4KLXZhciBpZnJhbWUgPSBkb2N1bWVudC5n ZXRFbGVtZW50QnlJZCgnaWZyYW1lJyk7CitmdW5jdGlvbiBydW5UZXN0KCkgeworICAgIHZhciBp ZnJhbWUgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgnaWZyYW1lJyk7CiAKLWZ1bmN0aW9uIGFz c2VydERpcmVjdGlvbihsYWJlbCwgZXhwZWN0ZWREaXJlY3Rpb24sIGh0bWwpCi17Ci0gICAgdmFy IGRvYyA9IGlmcmFtZS5jb250ZW50RG9jdW1lbnQ7Ci0gICAgZG9jLm9wZW4oKTsKLSAgICBkb2Mu d3JpdGUoaHRtbCk7Ci0gICAgZG9jLmNsb3NlKCk7CisgICAgZnVuY3Rpb24gYXNzZXJ0RGlyZWN0 aW9uKGxhYmVsLCBleHBlY3RlZERpcmVjdGlvbiwgaHRtbCkKKyAgICB7CisgICAgICAgIHZhciBk b2MgPSBpZnJhbWUuY29udGVudERvY3VtZW50OworICAgICAgICBkb2Mub3BlbigpOworICAgICAg ICBkb2Mud3JpdGUoaHRtbCk7CisgICAgICAgIGRvYy5jbG9zZSgpOwogCi0gICAgdmFyIGRpciA9 IHdpbmRvdy50ZXN0UnVubmVyID8KLSAgICAgICAgdGVzdFJ1bm5lci50aXRsZVRleHREaXJlY3Rp b24KLSAgICAgIDogJ3Rlc3RSdW5uZXIgdW5hdmFpbGFibGUnOwotICAgIHZhciBzdGF0dXMgPSBo dG1sICsgJyBzaG91bGQgaGF2ZSB0aXRsZSBkaXJlY3Rpb24gIicgKyBleHBlY3RlZERpcmVjdGlv biArICciLiAnOwotICAgIGlmIChkaXIgPT0gZXhwZWN0ZWREaXJlY3Rpb24pIHsKLSAgICAgICAg c3RhdHVzICs9ICdQQVNTJzsKLSAgICB9IGVsc2UgewotICAgICAgICBzdGF0dXMgKz0gJ0ZBSUwg KGdvdDogIicgKyBkaXIgKyAnIiknOworICAgICAgICB2YXIgZGlyID0gd2luZG93LnRlc3RSdW5u ZXIgPworICAgICAgICAgICAgdGVzdFJ1bm5lci50aXRsZVRleHREaXJlY3Rpb24KKyAgICAgICAg ICA6ICd0ZXN0UnVubmVyIHVuYXZhaWxhYmxlJzsKKyAgICAgICAgdmFyIHN0YXR1cyA9IGh0bWwg KyAnIHNob3VsZCBoYXZlIHRpdGxlIGRpcmVjdGlvbiAiJyArIGV4cGVjdGVkRGlyZWN0aW9uICsg JyIuICc7CisgICAgICAgIGlmIChkaXIgPT0gZXhwZWN0ZWREaXJlY3Rpb24pIHsKKyAgICAgICAg ICAgIHN0YXR1cyArPSAnUEFTUyc7CisgICAgICAgIH0gZWxzZSB7CisgICAgICAgICAgICBzdGF0 dXMgKz0gJ0ZBSUwgKGdvdDogIicgKyBkaXIgKyAnIiknOworICAgICAgICB9CisgICAgICAgIHZh ciBkaXYgPSBkb2N1bWVudC5jcmVhdGVFbGVtZW50KCdkaXYnKTsKKyAgICAgICAgZGl2LmlubmVy VGV4dCA9IHN0YXR1czsKKyAgICAgICAgZG9jdW1lbnQuYm9keS5hcHBlbmRDaGlsZChkaXYpOwog ICAgIH0KLSAgICB2YXIgZGl2ID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnZGl2Jyk7Ci0gICAg ZGl2LmlubmVyVGV4dCA9IHN0YXR1czsKLSAgICBkb2N1bWVudC5ib2R5LmFwcGVuZENoaWxkKGRp dik7Ci19CiAKLWlmICh3aW5kb3cudGVzdFJ1bm5lcikKLSAgICB0ZXN0UnVubmVyLmR1bXBBc1Rl eHQoKTsKKyAgICBpZiAod2luZG93LnRlc3RSdW5uZXIpCisgICAgICAgIHRlc3RSdW5uZXIuZHVt cEFzVGV4dCgpOwogCi1hc3NlcnREaXJlY3Rpb24oJ25vcm1hbCBkb2MnLCAnbHRyJywKLSAgICAg ICAgICAgICAgICAnPGh0bWw+PHRpdGxlPmZvbzwvdGl0bGU+PC9odG1sPicpOwotYXNzZXJ0RGly ZWN0aW9uKCd0aXRsZSBkaXI9cnRsJywgJ3J0bCcsCi0gICAgICAgICAgICAgICAgJzxodG1sPjx0 aXRsZSBkaXI9cnRsPmZvbzwvdGl0bGU+PC9odG1sPicpOwotYXNzZXJ0RGlyZWN0aW9uKCdodG1s IGRpcj1ydGwnLCAncnRsJywKLSAgICAgICAgICAgICAgICAnPGh0bWwgZGlyPXJ0bD48dGl0bGU+ Zm9vPC90aXRsZT48L2h0bWw+Jyk7Ci1hc3NlcnREaXJlY3Rpb24oJ2h0bWwgZGlyPXJ0bCwgdGl0 bGUgZGlyPWx0cicsICdsdHInLAotICAgICAgICAgICAgICAgICc8aHRtbCBkaXI9cnRsPjx0aXRs ZSBkaXI9bHRyPmZvbzwvdGl0bGU+PC9odG1sPicpOworICAgIGFzc2VydERpcmVjdGlvbignbm9y bWFsIGRvYycsICdsdHInLAorICAgICAgICAgICAgICAgICAgICAnPGh0bWw+PHRpdGxlPmZvbzwv dGl0bGU+PC9odG1sPicpOworICAgIGFzc2VydERpcmVjdGlvbigndGl0bGUgZGlyPXJ0bCcsICdy dGwnLAorICAgICAgICAgICAgICAgICAgICAnPGh0bWw+PHRpdGxlIGRpcj1ydGw+Zm9vPC90aXRs ZT48L2h0bWw+Jyk7CisgICAgYXNzZXJ0RGlyZWN0aW9uKCdodG1sIGRpcj1ydGwnLCAncnRsJywK KyAgICAgICAgICAgICAgICAgICAgJzxodG1sIGRpcj1ydGw+PHRpdGxlPmZvbzwvdGl0bGU+PC9o dG1sPicpOworICAgIGFzc2VydERpcmVjdGlvbignaHRtbCBkaXI9cnRsLCB0aXRsZSBkaXI9bHRy JywgJ2x0cicsCisgICAgICAgICAgICAgICAgICAgICc8aHRtbCBkaXI9cnRsPjx0aXRsZSBkaXI9 bHRyPmZvbzwvdGl0bGU+PC9odG1sPicpOworfQogPC9zY3JpcHQ+CiA8L2JvZHk+CiA8L2h0bWw+ Cg==