31638 2009-11-18 12:35:57 -0800 [Qt] fast/history/back-forward-reset-after-error-handling.html failing due to WorkQueue not being un-frozen 2009-11-22 15:17:40 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED Qt P2 Normal --- 31775 1 tonikitoo tonikitoo abecsi jwieczorek kenneth oldest_to_newest 164667 0 tonikitoo 2009-11-18 12:35:57 -0800 No current QT tests were using the WorkQueue. Now that we fixed its usage (see bug 30953, bug 31158 and bug 31164), we have to un-freeze it after each test execution for it to actually work. patch coming ... 164668 1 43449 tonikitoo 2009-11-18 12:38:06 -0800 Created attachment 43449 (landed in r51299) patch 0.1 164674 2 kenneth 2009-11-18 12:46:20 -0800 Ok, this is going to result in a timeout. In a local patch I have: - // Causes timeout, why? + // The below line is used in other ports, but for us it results in + // a timeout for fast/frames/frame-navigation.html //WorkQueue::shared()->setFrozen(false); Can you investigate that? It only timeouts when running the whole test suite. 164884 3 tonikitoo 2009-11-18 19:56:31 -0800 (In reply to comment #2) > Ok, this is going to result in a timeout. > > In a local patch I have: > > - // Causes timeout, why? > + // The below line is used in other ports, but for us it results in > + // a timeout for fast/frames/frame-navigation.html > //WorkQueue::shared()->setFrozen(false); > > Can you investigate that? It only timeouts when running the whole test suite. thx for looking at the patch. for me no new timeout's happenned. in fact, i saw one test starting to have incorrent layout and one fixed, which was the test this bug fixed. could you please let me know which started to timeout for you ? @kenneth: Regardless that, IMHO this solution is the right thing to do. If it affected other tests, revealing hidden bugs, we should investigate in follow up bugs. 164885 4 tonikitoo 2009-11-18 20:05:38 -0800 (In reply to comment #2) > Ok, this is going to result in a timeout. > > In a local patch I have: > > - // Causes timeout, why? > + // The below line is used in other ports, but for us it results in > + // a timeout for fast/frames/frame-navigation.html > //WorkQueue::shared()->setFrozen(false); > > Can you investigate that? It only timeouts when running the whole test suite. ah, i see in your comment now (frame-navigation.html). sorry about that. for me (linux 32-bit) it did not timeout, but had incorrect layout due to font stuff probably (see diff below): DIFF --- /tmp/layout-test-results/fast/frames/frame-navigation-expected.txt 2009-11-18 21:49:42.000000000 -0400 +++ /tmp/layout-test-results/fast/frames/frame-navigation-actual.txt 2009-11-18 21:49:42.000000000 -0400 @@ -14,8 +14,8 @@ text run at (0,0) width 374: "This tests that navigating to two pages in a subframe," text run at (0,20) width 351: "and then going back does not cause an assertion." RenderBlock {P} at (0,56) size 381x20 - RenderText {#text} at (0,0) size 185x20 - text run at (0,0) width 185: "SUCCESS - Didn't assert!" + RenderText {#text} at (0,0) size 187x20 + text run at (0,0) width 187: "SUCCESS - Didn't assert!" RenderFrame {FRAME} at (403,0) size 397x600 layer at (0,0) size 397x600 RenderView at (0,0) size 397x600 we are maybe free to go here then (?) 164956 5 kenneth 2009-11-19 05:36:55 -0800 (In reply to comment #4) As I said pretty clearly above, it only timeouts when running the whole test suite. If you run it individually it does not timeout. > (In reply to comment #2) > > Ok, this is going to result in a timeout. > > > > In a local patch I have: > > > > - // Causes timeout, why? > > + // The below line is used in other ports, but for us it results in > > + // a timeout for fast/frames/frame-navigation.html > > //WorkQueue::shared()->setFrozen(false); > > > > Can you investigate that? It only timeouts when running the whole test suite. > > ah, i see in your comment now (frame-navigation.html). sorry about that. > > for me (linux 32-bit) it did not timeout, but had incorrect layout due to font > stuff probably (see diff below): > > DIFF > > --- /tmp/layout-test-results/fast/frames/frame-navigation-expected.txt > 2009-11-18 21:49:42.000000000 -0400 > +++ /tmp/layout-test-results/fast/frames/frame-navigation-actual.txt > 2009-11-18 21:49:42.000000000 -0400 > @@ -14,8 +14,8 @@ > text run at (0,0) width 374: "This tests that navigating to > two pages in a subframe," > text run at (0,20) width 351: "and then going back does not > cause an assertion." > RenderBlock {P} at (0,56) size 381x20 > - RenderText {#text} at (0,0) size 185x20 > - text run at (0,0) width 185: "SUCCESS - Didn't assert!" > + RenderText {#text} at (0,0) size 187x20 > + text run at (0,0) width 187: "SUCCESS - Didn't assert!" > RenderFrame {FRAME} at (403,0) size 397x600 > layer at (0,0) size 397x600 > RenderView at (0,0) size 397x600 > > > > > > we are maybe free to go here then (?) 164957 6 tonikitoo 2009-11-19 05:53:24 -0800 (In reply to comment #5) > (In reply to comment #4) > > As I said pretty clearly above, it only timeouts when running the whole test > suite. > > If you run it individually it does not timeout. kenneth, let me be clearer this time too: no timeout to me running the whole test suite. please lets considere get this right fix in , skip revealed hidden bugs, and work on them on followups 164958 7 kenneth 2009-11-19 06:01:59 -0800 I agree, but we have many side-effects and we need to fix these more than the tests. Please get bbandix to test it on the test buildbot. If there are no timeouts, I r+ it. > kenneth, let me be clearer this time too: no timeout to me running the whole > test suite. > > please lets considere get this right fix in , skip revealed hidden bugs, and > work on them on followups 165180 8 tonikitoo 2009-11-19 19:50:16 -0800 andras, could you help us here ? thx 165275 9 abecsi 2009-11-20 04:41:52 -0800 (In reply to comment #8) > andras, could you help us here ? thx Your patch corrects fast/history/back-forward-reset-after-error-handling.html but fast/frames/frame-navigation.html times out on the buildbot. 165276 10 tonikitoo 2009-11-20 04:55:07 -0800 (In reply to comment #8) > andras, could you help us here ? thx kenneth, what do you think ? we fix the timeout first or skip it for now and fix in followup? 165291 11 kenneth 2009-11-20 06:44:57 -0800 I will say, investigate it immediately. Both tests actually seem to be related. If you fix both, please commit. 165661 12 43659 jwieczorek 2009-11-21 15:32:49 -0800 Created attachment 43659 fix the timeout in fast/frames/frame-navigation.html It is interesting that the test in question (fast/frames/frame-navigation.html) is passing at all. It turns out, however, that the expected result, which has been introduced in http://trac.webkit.org/changeset/46634, is wrong. If you compare http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/frames/frame-navigation-expected.txt with http://trac.webkit.org/browser/trunk/LayoutTests/platform/qt/fast/frames/frame-navigation-expected.txt you'll notice that the latter is missing the expected tree in the second child frame. The reason why it is timeouting is that the test performs a load in one of the child frames. The DRT should dump the tree after all tasks in the work queue are proceeded, but it does not, because it waits for the QWebFrame::loadFinished() signal from the main frame, but that obviously does not work with the child frames. The solution is to change the connection to the QWebPage::loadFinished() signal. Here is a patch that fixes the timeout issue. Unfortunately, for some reason I am getting wrong metrics in all tests in my local environment with Qt 4.6. Therefore, I cannot include the valid result for the test, so it would be nice if someone could try to generate the result. Feel free to merge this patch with the first one, if you wish. 165712 13 kenneth 2009-11-22 06:54:18 -0800 Great catch. Maybe a comment in the DRT is deserved? The DRT is complicated and it is easy to break things when fixing other things. For this reason we try to comment why we do what and when, or at least I have been starting to do that. 165713 14 43659 kenneth 2009-11-22 06:55:49 -0800 Comment on attachment 43659 fix the timeout in fast/frames/frame-navigation.html Let's get this in. Later we can add a comment and fix the results. All in all, it is better than how it is today. Great work Jakub 165715 15 43659 commit-queue 2009-11-22 07:16:39 -0800 Comment on attachment 43659 fix the timeout in fast/frames/frame-navigation.html Clearing flags on attachment: 43659 Committed r51293: <http://trac.webkit.org/changeset/51293> 165764 16 tonikitoo 2009-11-22 13:51:56 -0800 i can confirm that this is the fix. i had this patch locally, but could not ask for review earlier because i am on vacation and not able to access internet every day, but jakub came first :) i can confirm that the expected result was wrong and my patch was fixing it too. i will commit the fix for the expected result... (In reply to comment #12) > Created an attachment (id=43659) [details] > fix the timeout in fast/frames/frame-navigation.html > > It is interesting that the test in question (fast/frames/frame-navigation.html) > is passing at all. It turns out, however, that the expected result, which has > been introduced in http://trac.webkit.org/changeset/46634, is wrong. > > If you compare > http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/frames/frame-navigation-expected.txt > > with > http://trac.webkit.org/browser/trunk/LayoutTests/platform/qt/fast/frames/frame-navigation-expected.txt > > you'll notice that the latter is missing the expected tree in the second child > frame. > > The reason why it is timeouting is that the test performs a load in one of the > child frames. The DRT should dump the tree after all tasks in the work queue > are proceeded, but it does not, because it waits for the > QWebFrame::loadFinished() signal from the main frame, but that obviously does > not work with the child frames. The solution is to change the connection to the > QWebPage::loadFinished() signal. > > Here is a patch that fixes the timeout issue. Unfortunately, for some reason I > am getting wrong metrics in all tests in my local environment with Qt 4.6. > Therefore, I cannot include the valid result for the test, so it would be nice > if someone could try to generate the result. > > Feel free to merge this patch with the first one, if you wish. 43449 2009-11-18 12:38:06 -0800 2009-11-22 15:17:00 -0800 (landed in r51299) patch 0.1 0001--Qt-fast-history-back-forward-reset-after-error-han.patch text/plain 1975 tonikitoo RnJvbSBmOTY0YWNhOWE2MTZmNzRmYzc1NDc0ZjE1MWNjZWViODlkNGU4MjE3IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbnRvbmlvIEdvbWVzIDx0b25pa2l0b29Ad2Via2l0Lm9yZz4K RGF0ZTogV2VkLCAxOCBOb3YgMjAwOSAxNjozNzoyMyAtMDQwMApTdWJqZWN0OiBbUEFUQ0hdIFtR dF0gZmFzdC9oaXN0b3J5L2JhY2stZm9yd2FyZC1yZXNldC1hZnRlci1lcnJvci1oYW5kbGluZy5o dG1sIGZhaWxpbmcgZHVlIHRvIFdvcmtRdWV1ZSBub3QgYmVpbmcgdW4tZnJvemVuCiBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzE2MzgKClJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgoKVW5mcmVlemUgV29ya1F1ZXVlIGFmdGVyIGVhY2ggdGVzdCBleGVjdXRpb24u CgoqIER1bXBSZW5kZXJUcmVlL3F0L0R1bXBSZW5kZXJUcmVlLmNwcDoKKFdlYkNvcmU6OkR1bXBS ZW5kZXJUcmVlOjpyZXNldFRvQ29uc2lzdGVudFN0YXRlQmVmb3JlVGVzdGluZyk6Ci0tLQogV2Vi S2l0VG9vbHMvQ2hhbmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxMiArKysr KysrKysrKysKIFdlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL3F0L0R1bXBSZW5kZXJUcmVlLmNw cCB8ICAgIDIgKy0KIDIgZmlsZXMgY2hhbmdlZCwgMTMgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlv bnMoLSkKCmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9DaGFuZ2VMb2cgYi9XZWJLaXRUb29scy9D aGFuZ2VMb2cKaW5kZXggODAxMmMzMy4uZThjZmE3NSAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMv Q2hhbmdlTG9nCisrKyBiL1dlYktpdFRvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisy MDA5LTExLTE4ICBBbnRvbmlvIEdvbWVzICA8dG9uaWtpdG9vQHdlYmtpdC5vcmc+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW1F0XSBmYXN0L2hpc3Rv cnkvYmFjay1mb3J3YXJkLXJlc2V0LWFmdGVyLWVycm9yLWhhbmRsaW5nLmh0bWwgZmFpbGluZyBk dWUgdG8gV29ya1F1ZXVlIG5vdCBiZWluZyB1bi1mcm96ZW4KKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMxNjM4CisKKyAgICAgICAgVW5mcmVlemUgV29y a1F1ZXVlIGFmdGVyIGVhY2ggdGVzdCBleGVjdXRpb24uCisKKyAgICAgICAgKiBEdW1wUmVuZGVy VHJlZS9xdC9EdW1wUmVuZGVyVHJlZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpEdW1wUmVuZGVy VHJlZTo6cmVzZXRUb0NvbnNpc3RlbnRTdGF0ZUJlZm9yZVRlc3RpbmcpOgorCiAyMDA5LTExLTE4 ICBLZW5uZXRoIFJvaGRlIENocmlzdGlhbnNlbiAgPGtlbm5ldGhAd2Via2l0Lm9yZz4KIAogICAg ICAgICBSZXZpZXdlZCBieSBTaW1vbiBIYXVzbWFubi4KZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xz L0R1bXBSZW5kZXJUcmVlL3F0L0R1bXBSZW5kZXJUcmVlLmNwcCBiL1dlYktpdFRvb2xzL0R1bXBS ZW5kZXJUcmVlL3F0L0R1bXBSZW5kZXJUcmVlLmNwcAppbmRleCA3MTdhZjliLi5mNzQ3YWQzIDEw MDY0NAotLS0gYS9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9xdC9EdW1wUmVuZGVyVHJlZS5j cHAKKysrIGIvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvcXQvRHVtcFJlbmRlclRyZWUuY3Bw CkBAIC0zMjgsNyArMzI4LDcgQEAgdm9pZCBEdW1wUmVuZGVyVHJlZTo6cmVzZXRUb0NvbnNpc3Rl bnRTdGF0ZUJlZm9yZVRlc3RpbmcoKQogCiAgICAgV29ya1F1ZXVlOjpzaGFyZWQoKS0+Y2xlYXIo KTsKICAgICAvLyBDYXVzZXMgdGltZW91dCwgd2h5PwotICAgIC8vV29ya1F1ZXVlOjpzaGFyZWQo KS0+c2V0RnJvemVuKGZhbHNlKTsKKyAgICBXb3JrUXVldWU6OnNoYXJlZCgpLT5zZXRGcm96ZW4o ZmFsc2UpOwogCiAgICAgbV9jb250cm9sbGVyLT5yZXNldCgpOwogICAgIHF0X2RydF9yZXNldE9y aWdpbkFjY2Vzc1doaXRlTGlzdHMoKTsKLS0gCjEuNi4wLjQKCg== 43659 2009-11-21 15:32:49 -0800 2009-11-22 07:16:39 -0800 fix the timeout in fast/frames/frame-navigation.html 0001-Qt-Fix-the-timeout-of-fast-frames-frame-navigation.h.patch text/plain 2320 jwieczorek RnJvbSA3MmQ0ZDkxMWMyNWI5MDAwYzQ5Y2Q4MDMyYTdmNTljNGQyMjdkODlmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKYWt1YiBXaWVjem9yZWsgPGZhdzIxN0BnbWFpbC5jb20+CkRh dGU6IFN1biwgMjIgTm92IDIwMDkgMDA6MDE6NTYgKzAwMDAKU3ViamVjdDogW1BBVENIXSBbUXRd IEZpeCB0aGUgdGltZW91dCBvZiBmYXN0L2ZyYW1lcy9mcmFtZS1uYXZpZ2F0aW9uLmh0bWwKIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMTYzOAoKVGhlIHRlc3QgaXMg dGltZW91dGluZywgYmVjYXVzZSBpdCB1c2VzIHRoZSBXb3JrUXVldWUgdG8gbG9hZCBhIGRvY3Vt ZW50IGluIG9uZQpvZiB0aGUgY2hpbGQgZnJhbWVzIGFuZCBvbmNlIHRoZSBsb2FkaW5nIGlzIGZp bmlzaGVkLCB0aGUgRFJUIGRvZXMgbm90IGR1bXAgdGhlCnRyZWUuIFRoaXMgaXMgYmVjYXVzZSBp dCB3YWl0cyBmb3IgdGhlIFFXZWJGcmFtZTo6bG9hZEZpbmlzaGVkKCkgc2lnbmFsIGZyb20KdGhl IG1haW4gZnJhbWUsIHdoaWxlIGl0IHNob3VsZCBjb25uZWN0IHRvIFFXZWJQYWdlOjpsb2FkRmlu aXNoZWQoKS4KLS0tCiBXZWJLaXRUb29scy9DaGFuZ2VMb2cgICAgICAgICAgICAgICAgICAgICAg ICAgICAgfCAgIDE1ICsrKysrKysrKysrKysrKwogV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUv cXQvRHVtcFJlbmRlclRyZWUuY3BwIHwgICAgMiArLQogMiBmaWxlcyBjaGFuZ2VkLCAxNiBpbnNl cnRpb25zKCspLCAxIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5n ZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxvZwppbmRleCA2MDliODg2Li5kZmYyOWRkIDEwMDY0 NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9n CkBAIC0xLDMgKzEsMTggQEAKKzIwMDktMTEtMjEgIEpha3ViIFdpZWN6b3JlayAgPGZhdzIxN0Bn bWFpbC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgW1F0XSBGaXggdGhlIHRpbWVvdXQgb2YgZmFzdC9mcmFtZXMvZnJhbWUtbmF2aWdhdGlvbi5o dG1sCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMTYz OAorCisgICAgICAgIFRoZSB0ZXN0IGlzIHRpbWVvdXRpbmcsIGJlY2F1c2UgaXQgdXNlcyB0aGUg V29ya1F1ZXVlIHRvIGxvYWQgYSBkb2N1bWVudCBpbiBvbmUKKyAgICAgICAgb2YgdGhlIGNoaWxk IGZyYW1lcyBhbmQgb25jZSB0aGUgbG9hZGluZyBpcyBmaW5pc2hlZCwgdGhlIERSVCBkb2VzIG5v dCBkdW1wIHRoZQorICAgICAgICB0cmVlLiBUaGlzIGlzIGJlY2F1c2UgaXQgd2FpdHMgZm9yIHRo ZSBRV2ViRnJhbWU6OmxvYWRGaW5pc2hlZCgpIHNpZ25hbCBmcm9tCisgICAgICAgIHRoZSBtYWlu IGZyYW1lLCB3aGlsZSBpdCBzaG91bGQgY29ubmVjdCB0byBRV2ViUGFnZTo6bG9hZEZpbmlzaGVk KCkuCisKKyAgICAgICAgKiBEdW1wUmVuZGVyVHJlZS9xdC9EdW1wUmVuZGVyVHJlZS5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpEdW1wUmVuZGVyVHJlZTo6RHVtcFJlbmRlclRyZWUpOgorCiAyMDA5 LTExLTIwICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgRXJpYyBTZWlkZWwuCmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJl ZS9xdC9EdW1wUmVuZGVyVHJlZS5jcHAgYi9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9xdC9E dW1wUmVuZGVyVHJlZS5jcHAKaW5kZXggYjNiNTJiNy4uZWUxNWM3MCAxMDA2NDQKLS0tIGEvV2Vi S2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvcXQvRHVtcFJlbmRlclRyZWUuY3BwCisrKyBiL1dlYktp dFRvb2xzL0R1bXBSZW5kZXJUcmVlL3F0L0R1bXBSZW5kZXJUcmVlLmNwcApAQCAtMjY2LDcgKzI2 Niw3IEBAIER1bXBSZW5kZXJUcmVlOjpEdW1wUmVuZGVyVHJlZSgpCiAgICAgICAgICAgICB0aGlz LCBTTE9UKGNvbm5lY3RGcmFtZShRV2ViRnJhbWUgKikpKTsKICAgICBjb25uZWN0RnJhbWUobV9w YWdlLT5tYWluRnJhbWUoKSk7CiAKLSAgICBjb25uZWN0KG1fcGFnZS0+bWFpbkZyYW1lKCksIFNJ R05BTChsb2FkRmluaXNoZWQoYm9vbCkpLAorICAgIGNvbm5lY3QobV9wYWdlLCBTSUdOQUwobG9h ZEZpbmlzaGVkKGJvb2wpKSwKICAgICAgICAgICAgIG1fY29udHJvbGxlciwgU0xPVChtYXliZUR1 bXAoYm9vbCkpKTsKIAogICAgIGNvbm5lY3QobV9wYWdlLT5tYWluRnJhbWUoKSwgU0lHTkFMKHRp dGxlQ2hhbmdlZChjb25zdCBRU3RyaW5nJikpLAotLSAKMS42LjUKCg==