37334 2010-04-09 08:37:48 -0700 [Qt] fast/frames/flattening/frameset-flattening-subframesets.html fails intermittently on Qt bot 2010-05-12 01:07:13 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC All CLOSED FIXED Qt P1 Critical --- 33297 35784 1 ossy webkit-qt-unassigned abarth eric hausmann jesus kenneth tonikitoo webkit.review.bot oldest_to_newest 210734 0 ossy 2010-04-09 08:37:48 -0700 http://build.webkit.org/results/Qt%20Linux%20Release/r57338%20%289846%29/fast/frames/flattening/frameset-flattening-subframesets-pretty-diff.html We should fix it to make our bot more stable and green. 210787 1 kenneth 2010-04-09 10:20:00 -0700 That new result is wrong, strange, I dont get this locally. It should be 900 and not 800, or it didnt do the flattening. 210799 2 ossy 2010-04-09 10:38:22 -0700 (In reply to comment #1) > That new result is wrong, strange, I dont get this locally. It should be 900 > and not 800, or it didnt do the flattening. You misinterpreted, it is a flakey test. It passes usually, but sometimes we get this wrong result. 210812 3 kenneth 2010-04-09 10:55:44 -0700 Is that also the case for the mac? 211120 4 ossy 2010-04-10 00:53:28 -0700 (In reply to comment #3) > Is that also the case for the mac? I dont't know, I didn't see this fail on Mac bots before, but is is possible. 212426 5 eric 2010-04-14 10:44:39 -0700 Failed again just now: http://build.webkit.org/results/Qt%20Linux%20Release/r57583%20(10079)/fast/frames/flattening/frameset-flattening-subframesets-diffs.txt 212427 6 eric 2010-04-14 10:45:23 -0700 Actually, failed twice in a row: http://build.webkit.org/results/Qt%20Linux%20Release/r57584%20(10080)/fast/frames/flattening/iframe-flattening-offscreen-diffs.txt Someone probably got a "you broke it" sheriff-bot email as a result. :( 212428 7 eric 2010-04-14 10:45:58 -0700 Nevermind, similar failure, different test. 217666 8 ossy 2010-04-27 05:55:31 -0700 I'm worry because of this falkiness, because it occured 5 times during the last 20 builds. We should fix it asap not to block commit queue or put this test into skipped list. 219536 9 eric 2010-04-30 11:06:32 -0700 It's time to skip this test. Too many false positives blocking the commit-queue and confusing folks looking at the bots. 219538 10 ossy 2010-04-30 11:08:19 -0700 (In reply to comment #9) > It's time to skip this test. Too many false positives blocking the > commit-queue and confusing folks looking at the bots. I agree with you, I'll do it. 219572 11 webkit.review.bot 2010-04-30 12:28:57 -0700 http://trac.webkit.org/changeset/58588 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/58587 http://trac.webkit.org/changeset/58588 219573 12 ossy 2010-04-30 12:33:56 -0700 (In reply to comment #11) > http://trac.webkit.org/changeset/58588 might have broken Tiger Intel Release > The following changes are on the blame list: > http://trac.webkit.org/changeset/58587 > http://trac.webkit.org/changeset/58588 58587 caused it, not me. 219575 13 ossy 2010-04-30 12:35:41 -0700 I forgot to mention, that I skipped the test until fix. http://trac.webkit.org/changeset/58588 220158 14 jesus 2010-05-03 10:19:48 -0700 I've just finish running this test. I did a 100,000 iterations round and then a 10 million one and I got no error at all. Not a single time... I did this on an environment quite similar to the build bot (Debian Lenny, Qt 4.6, the package list provided by Ossy, etc) and I was going to debug it and try to fix, but without fails I can't be of any help. =/ I have no idea of what is causing the random crashes. 220252 15 jesus 2010-05-03 12:56:24 -0700 (In reply to comment #14) > I've just finish running this test. I did a 100,000 iterations round and then a > 10 million one and I got no error at all. Not a single time... > > I did this on an environment quite similar to the build bot (Debian Lenny, Qt > 4.6, the package list provided by Ossy, etc) and I was going to debug it and > try to fix, but without fails I can't be of any help. =/ > > I have no idea of what is causing the random crashes. After talking to Ossy I ran a new round of tests with "run-webkit-tests fast/frames/flattening --iterations 10000 --exit-after-n-failures 1" And I had no failures again. I'll let it running a 1 million iterations round during the night... 220254 16 tonikitoo 2010-05-03 13:01:06 -0700 > After talking to Ossy I ran a new round of tests with "run-webkit-tests > fast/frames/flattening --iterations 10000 --exit-after-n-failures 1" > > And I had no failures again. I'll let it running a 1 million iterations round > during the night... jesus, if it is a sideeffect of any other test, you wont catch it by running this test alone repeatedly. How are you running? Maybe running some tests (folders) that are known to be ran before it, so it can trigger the bad effect(?) 220267 17 ossy 2010-05-03 13:35:56 -0700 Hooray, I catched the bug. :)) layoutTestController.display() in fast/frames/flattening/frameset-flattening-subframesets.html is the culprit. layoutTestController.display() needs communication with the X server, and I think this delay might cause failing sometimes. (If I increased the nice value of X server, fail occured more often.) I commented layoutTestController.display() in fast/frames/flattening/frameset-flattening-subframesets.html, and it works correctly, without any fail. I ran fast/frames/flattening tests with--iterations 10000 option. Do we need display() for this test? Or can we simple remove it from the test case? 220340 18 ossy 2010-05-03 15:47:56 -0700 (In reply to comment #17) > Hooray, I catched the bug. :)) I meant fast/frames/flattening/frameset-flattening-subframe-resize.html is the culprit. fast/frames/flattening/frameset-flattening-subframe-resize.html skipped until fix: http://trac.webkit.org/changeset/58711 222300 19 kenneth 2010-05-07 06:07:01 -0700 (In reply to comment #18) > (In reply to comment #17) > > Hooray, I catched the bug. :)) > I meant fast/frames/flattening/frameset-flattening-subframe-resize.html is the > culprit. > > fast/frames/flattening/frameset-flattening-subframe-resize.html skipped until > fix: http://trac.webkit.org/changeset/58711 I think I raed you fixed this one by removing the call to display()? Can we close if that is right? 222304 20 ossy 2010-05-07 06:10:28 -0700 (In reply to comment #19) > I think I raed you fixed this one by removing the call to display()? Can we > close if that is right? I asked, because I'm not sure if removing the call to display is the correct solution. It works for me locally, but is it correct? If it's OK, we can remove it, and unskip fast/frames/flattening/frameset-flattening-subframe-resize.html and then close the bug. 222311 21 kenneth 2010-05-07 06:18:15 -0700 display was used to force the layout, but it seems that it is not needed, so it should be OK to remove. At least I think it is worth a try. 222328 22 tonikitoo 2010-05-07 06:53:28 -0700 (In reply to comment #21) > display was used to force the layout, but it seems that it is not needed, so it > should be OK to remove. At least I think it is worth a try. there are many other ways to "force a layout" used throughout LayoutTests/* document.body.offsetTop ... is one of the common ones. $ grep -nHR layout LayoutTests/ --include=*.html | grep -i force maybe switching "LayoutTestController.display()" in favor of one of them? 222332 23 kenneth 2010-05-07 06:55:58 -0700 (In reply to comment #22) > (In reply to comment #21) > > display was used to force the layout, but it seems that it is not needed, so it > > should be OK to remove. At least I think it is worth a try. > > there are many other ways to "force a layout" used throughout LayoutTests/* > > document.body.offsetTop > > ... is one of the common ones. > > $ grep -nHR layout LayoutTests/ --include=*.html | grep -i force > > maybe switching "LayoutTestController.display()" in favor of one of them? document.body.offsetTop is already called, that invalidates the layout, but doesn't necessarily to a relayout, which is only done on paint. 222397 24 eric 2010-05-07 08:42:14 -0700 (In reply to comment #23) > document.body.offsetTop is already called, that invalidates the layout, but > doesn't necessarily to a relayout, which is only done on paint. That's not correct. Accessing many javascript properties, including document.body.offsetTop will guarantee that a layout (and thus also a style resolve) has completed. We should not be using .display() for ensuring layout. It's used for ensuring paint for for testing specific paint bugs. That said, if .display() is broken on these bots, then other tests will break. 222462 25 kenneth 2010-05-07 10:18:50 -0700 (In reply to comment #24) > (In reply to comment #23) > > document.body.offsetTop is already called, that invalidates the layout, but > > doesn't necessarily to a relayout, which is only done on paint. > > That's not correct. Accessing many javascript properties, including > document.body.offsetTop will guarantee that a layout (and thus also a style > resolve) has completed. > > We should not be using .display() for ensuring layout. It's used for ensuring > paint for for testing specific paint bugs. > > That said, if .display() is broken on these bots, then other tests will break. Doesn't it just result in a call to setNeedsLayout(), thus marking a layout? Qt calls layoutIfNeeded() during paint. If you are right, that is great, because then we can just remove the display() call, but as you said, that won't fix other tests using display(). 224103 26 55740 tonikitoo 2010-05-11 12:50:40 -0700 Created attachment 55740 patch Committed r59165: <http://trac.webkit.org/changeset/59165> 224104 27 tonikitoo 2010-05-11 12:51:23 -0700 I uploded the patch to make cherry-picking easier 224155 28 webkit.review.bot 2010-05-11 14:24:11 -0700 http://trac.webkit.org/changeset/59165 might have broken Chromium Win Release The following changes are on the blame list: http://trac.webkit.org/changeset/59168 http://trac.webkit.org/changeset/59169 http://trac.webkit.org/changeset/59170 http://trac.webkit.org/changeset/59165 http://trac.webkit.org/changeset/59166 http://trac.webkit.org/changeset/59167 224459 29 hausmann 2010-05-12 00:50:22 -0700 Revision r59165 cherry-picked into qtwebkit-2.0 with commit 79d424c6674d65561b1d926d04b9c8c52fd51384 55740 2010-05-11 12:50:40 -0700 2010-05-11 12:50:40 -0700 patch 0001-Qt-fast-frames-flattening-frameset-flattening-subfra.patch text/plain 4160 tonikitoo RnJvbSBkZjU3NmQ4YWUwYzdkNGM5NDU5Y2NkODZlMDBjYjJhNDJhMWYwNjdmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiB0b25pa2l0b29Ad2Via2l0Lm9yZyA8dG9uaWtpdG9vQHdlYmtp dC5vcmdAMjY4ZjQ1Y2MtY2QwOS0wNDEwLWFiM2MtZDUyNjkxYjRkYmZjPgpEYXRlOiBUdWUsIDEx IE1heSAyMDEwIDE5OjQ4OjM5ICswMDAwClN1YmplY3Q6IFtQQVRDSF0gW1F0XSBmYXN0L2ZyYW1l cy9mbGF0dGVuaW5nL2ZyYW1lc2V0LWZsYXR0ZW5pbmctc3ViZnJhbWVzZXRzLmh0bWwgZmFpbHMg aW50ZXJtaXR0ZW50bHkgb24gUXQgYm90CiBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MzczMzQKClJ1YmJlci1zdGFtcGVkIGJ5IEtlbm5ldGggQ2hyaXN0aWFuc2VuLgpQ YXRjaCBieSBBbnRvbmlvIEdvbWVzIDx0b25pa2l0b29Ad2Via2l0Lm9yZz4KClJlbW92aW5nIHVu bmVlZGVkIGNhbGwgdG8gTGF5b3V0VGVzdENvbnRyb2xsZXIuZGlzcGxheSgpLiBJdCB3YXMgb3Jp Z2luYWxseSBpbnRlbmRpbmcKdG8gZm9yY2UgYSBsYXlvdXQsIGJ1dCBhIHJlLWxheW91dCB3aWxs IGhhcHBlbiByZWdhcmRsZXNzIGJlY2F1c2UgZG9jdW1lbnQuYm9keS5vZmZzZXRUb3AKaXMgY2Fs bGVkIGFmdGVyd2FyZHMKCiogZmFzdC9mcmFtZXMvZmxhdHRlbmluZy9mcmFtZXNldC1mbGF0dGVu aW5nLXN1YmZyYW1lLXJlc2l6ZS5odG1sOgoqIHBsYXRmb3JtL3F0L1NraXBwZWQ6CgpnaXQtc3Zu LWlkOiBodHRwOi8vc3ZuLndlYmtpdC5vcmcvcmVwb3NpdG9yeS93ZWJraXQvdHJ1bmtANTkxNjUg MjY4ZjQ1Y2MtY2QwOS0wNDEwLWFiM2MtZDUyNjkxYjRkYmZjCi0tLQogTGF5b3V0VGVzdHMvQ2hh bmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCAgIDE0ICsrKysrKysrKysrKysr CiAuLi4vZnJhbWVzZXQtZmxhdHRlbmluZy1zdWJmcmFtZS1yZXNpemUuaHRtbCAgICAgICB8ICAg IDEgLQogTGF5b3V0VGVzdHMvcGxhdGZvcm0vcXQvU2tpcHBlZCAgICAgICAgICAgICAgICAgICAg fCAgICA1IC0tLS0tCiAzIGZpbGVzIGNoYW5nZWQsIDE0IGluc2VydGlvbnMoKyksIDYgZGVsZXRp b25zKC0pCgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCmluZGV4IGY0MjQyM2MuLmRlNDk1NTIgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3Rz L0NoYW5nZUxvZworKysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsOCArMSwyMiBAQAor MjAxMC0wNS0xMSAgQW50b25pbyBHb21lcyAgPHRvbmlraXRvb0B3ZWJraXQub3JnPgorCisgICAg ICAgIFJ1YmJlci1zdGFtcGVkIGJ5IEtlbm5ldGggQ2hyaXN0aWFuc2VuLgorCisgICAgICAgIFtR dF0gZmFzdC9mcmFtZXMvZmxhdHRlbmluZy9mcmFtZXNldC1mbGF0dGVuaW5nLXN1YmZyYW1lc2V0 cy5odG1sIGZhaWxzIGludGVybWl0dGVudGx5IG9uIFF0IGJvdAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzczMzQKKworICAgICAgICBSZW1vdmluZyB1 bm5lZWRlZCBjYWxsIHRvIExheW91dFRlc3RDb250cm9sbGVyLmRpc3BsYXkoKS4gSXQgd2FzIG9y aWdpbmFsbHkgaW50ZW5kaW5nCisgICAgICAgIHRvIGZvcmNlIGEgbGF5b3V0LCBidXQgYSByZS1s YXlvdXQgd2lsbCBoYXBwZW4gcmVnYXJkbGVzcyBiZWNhdXNlIGRvY3VtZW50LmJvZHkub2Zmc2V0 VG9wCisgICAgICAgIGlzIGNhbGxlZCBhZnRlcndhcmRzCisKKyAgICAgICAgKiBmYXN0L2ZyYW1l cy9mbGF0dGVuaW5nL2ZyYW1lc2V0LWZsYXR0ZW5pbmctc3ViZnJhbWUtcmVzaXplLmh0bWw6Cisg ICAgICAgICogcGxhdGZvcm0vcXQvU2tpcHBlZDoKKwogMjAxMC0wNS0xMSAgSmlhbiBMaSAgPGpp YW5saUBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgVW5yZXZpZXdlZC4gU2tpcCBmaWxlLXJlYWRl ci5odG1sIHRoYXQgaXMgbm90IGVuYWJsZWQgaW4gY2hyb21pdW0geWV0LgogCiAgICAgICAgICog cGxhdGZvcm0vY2hyb21pdW0vdGVzdF9leHBlY3RhdGlvbnMudHh0OgogCiAyMDEwLTA1LTExICBK aWFuIExpICA8amlhbmxpQGNocm9taXVtLm9yZz4KIApkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMv ZmFzdC9mcmFtZXMvZmxhdHRlbmluZy9mcmFtZXNldC1mbGF0dGVuaW5nLXN1YmZyYW1lLXJlc2l6 ZS5odG1sIGIvTGF5b3V0VGVzdHMvZmFzdC9mcmFtZXMvZmxhdHRlbmluZy9mcmFtZXNldC1mbGF0 dGVuaW5nLXN1YmZyYW1lLXJlc2l6ZS5odG1sCmluZGV4IDUwMGIxZGMuLmZiZTk3NDYgMTAwNjQ0 Ci0tLSBhL0xheW91dFRlc3RzL2Zhc3QvZnJhbWVzL2ZsYXR0ZW5pbmcvZnJhbWVzZXQtZmxhdHRl bmluZy1zdWJmcmFtZS1yZXNpemUuaHRtbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2ZyYW1lcy9m bGF0dGVuaW5nL2ZyYW1lc2V0LWZsYXR0ZW5pbmctc3ViZnJhbWUtcmVzaXplLmh0bWwKQEAgLTE5 LDE3ICsxOSwxNiBAQAogICAgICAgICB7CiAgICAgICAgICAgICB2YXIgZGl2ID0gZnJhbWVzWzBd LmRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCdib3gnKTsKICAgICAgICAgICAgICAgICBkaXYuc3R5 bGUuaGVpZ2h0PSczMDAnOwogICAgICAgICAgICAgICAgIGRpdi5zdHlsZS53aWR0aD0nMzAwJzsK ICAgICAgICAgICAgICAgICBkaXYuc3R5bGUuYmFja2dyb3VuZENvbG9yPSdibHVlJzsKIAogICAg ICAgICAgICAgaWYgKHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikgewogICAgICAgICAgICAg ICAgIGZyYW1lc1swXS5kb2N1bWVudC5ib2R5Lm9mZnNldFRvcDsKLSAgICAgICAgICAgICAgICBs YXlvdXRUZXN0Q29udHJvbGxlci5kaXNwbGF5KCk7CiAgICAgICAgICAgICAgICAgbGF5b3V0VGVz dENvbnRyb2xsZXIubm90aWZ5RG9uZSgpOwogICAgICAgICAgICAgfQogICAgICAgICB9CiAgICAg PC9zY3JpcHQ+CiA8L2hlYWQ+CiA8ZnJhbWVzZXQgYm9yZGVyPTAgY29scz0iKiw3MDAiIG9ubG9h ZD0idGVzdCgpOyI+CiAgICAgPGZyYW1lIHNyYz0icmVzb3VyY2VzL2ZyYW1lc2V0LWZsYXR0ZW5p bmctc3ViZnJhbWUtcmVzaXplLmh0bWwiIG9ubG9hZD0idGVzdDIoKTsiPgogICAgIDxmcmFtZT4K ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQgYi9MYXlvdXRUZXN0 cy9wbGF0Zm9ybS9xdC9Ta2lwcGVkCmluZGV4IDc1MmQ0MTkuLjQ3NDhkNGUgMTAwNjQ0Ci0tLSBh L0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQKKysrIGIvTGF5b3V0VGVzdHMvcGxhdGZv cm0vcXQvU2tpcHBlZApAQCAtMTE2LDIxICsxMTYsMTYgQEAgaHR0cC90ZXN0cy94bWxodHRwcmVx dWVzdC93b3JrZXJzL3NoYXJlZC13b3JrZXItY2xvc2UuaHRtbAogaW5zcGVjdG9yL3RpbWVsaW5l LXBhaW50Lmh0bWwKIAogc3RvcmFnZS9kYXRhYmFzZS1sb2NrLWFmdGVyLXJlbG9hZC5odG1sCiBz dG9yYWdlL2VtcHR5LXN0YXRlbWVudC5odG1sCiBzdG9yYWdlL2V4ZWN1dGUtc3FsLWFyZ3MuaHRt bAogc3RvcmFnZS9oYXNoLWNoYW5nZS13aXRoLXhoci5odG1sCiB0cmFuc2l0aW9ucy9jaGFuZ2Ut dmFsdWVzLWR1cmluZy10cmFuc2l0aW9uLmh0bWwKIAotIyBbUXRdIGZhc3QvZnJhbWVzL2ZsYXR0 ZW5pbmcvZnJhbWVzZXQtZmxhdHRlbmluZy1zdWJmcmFtZXNldHMuaHRtbCBmYWlscyBpbnRlcm1p dHRlbnRseSBvbiBRdCBib3QKLSMgRmFpbHMgY2F1c2VkIGJ5IGxheW91dFRlc3RDb250cm9sbGVy LmRpc3BsYXkoKSBpbiBmYXN0L2ZyYW1lcy9mbGF0dGVuaW5nL2ZyYW1lc2V0LWZsYXR0ZW5pbmct c3ViZnJhbWUtcmVzaXplLmh0bWwKLSMgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTM3MzM0Ci1mYXN0L2ZyYW1lcy9mbGF0dGVuaW5nL2ZyYW1lc2V0LWZsYXR0ZW5pbmct c3ViZnJhbWUtcmVzaXplLmh0bWwKLQogIyBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MzQyODEKIGZhc3Qvd29ya2Vycy9zaGFyZWQtd29ya2VyLWZyYW1lLWxpZmVjeWNs ZS5odG1sCiAKICMgLS0tLS0tLSB3b3JrcyBvbiA2NCBiaXQgYnV0IG5vdCBvbiAzMiBiaXQKIHN2 Zy9jdXN0b20vcGF0aC1nZXRUb3RhbExlbmd0aC5zdmcKIHN2Zy9jdXN0b20vZ2x5cGgtc2V0dGlu Zy1kLWF0dHJpYnV0ZS5zdmcKIAogIyBmYWlsaW5nIG1lZGlhIHRlc3RzCi0tIAoxLjYuMy4zCgo=