91083 2012-07-12 05:52:26 -0700 [GTK] REGRESSION (r122428) WebKit2APITests/TestWebKitFindController fails "next" test 2012-08-14 02:28:37 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 spena svillar cgarcia gustavo mrobinson svillar webkit.review.bot zan oldest_to_newest 667683 0 spena 2012-07-12 05:52:26 -0700 ERROR:../../Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:168:void testFindControllerNext(FindControllerTest*, gconstpointer): assertion failed: (test->m_matchCount == 2) TEST: /home/slave/webkitgtk/gtk-linux-64-release/build/Tools/gtk/../Scripts/../../WebKitBuild/Release/Programs/WebKit2APITests/TestWebKitFindController... (pid=20393) /webkit2/WebKitFindController/getters: OK /webkit2/WebKitFindController/instance: OK /webkit2/WebKitFindController/text-found: OK /webkit2/WebKitFindController/text-not-found: OK /webkit2/WebKitFindController/match-count: OK /webkit2/WebKitFindController/max-match-count: OK /webkit2/WebKitFindController/next: FAIL After r122428 - <http://trac.webkit.org/changeset/122428>, this test fails. 667684 1 spena 2012-07-12 05:53:01 -0700 I'm taking a look at this atm. 667990 2 spena 2012-07-12 10:59:45 -0700 Several tests ("next", "previous" and "hide") were failing after the changeset. Next and Previous now report just one result in their search_next and search_previous methods. I'm not sure if this how it should be (so the tests need to be updated) or if it is an actual regression. Once that the "next" and "previous" tests are fixed, "hide" fails, identifying the "originalPixbuf" and "unhighlightedPixbuf" as different. Adding a delay with .wait seems to fix it (so this could be a consequence of the different behaviour regarding markAllTextMatches and unmarkAllTextMatches), but again, I'm not sure about this being the proper fix. 667993 3 spena 2012-07-12 11:01:02 -0700 Assigning the bug to Sergio, who knows the code better. 671055 4 spena 2012-07-17 00:33:59 -0700 Bug #91224 tracks the skipped API tests. 679086 5 154589 svillar 2012-07-26 02:34:17 -0700 Created attachment 154589 Patch 679088 6 webkit.review.bot 2012-07-26 02:36:05 -0700 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 679089 7 svillar 2012-07-26 02:37:07 -0700 So yeah, Simon's analysis is basically correct. After r122428 the FindController returns just 1 match for search_next()/search_prev() if the test is found, so we should update the expected behaviour in the test. Apart from that the hide() unit test was flaky because it isn't enough to wait for the "draw" signal once, we have to wait also until all the glib events are processed. 679090 8 154589 mrobinson 2012-07-26 02:37:45 -0700 Comment on attachment 154589 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154589&action=review > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:69 > g_main_loop_run(m_mainLoop); > + while (g_main_context_iteration(g_main_context_default(), FALSE)) { } I think this means that you should rename the method here, waitUntilWebViewDrawSignal is no longer accurate. What events are you waiting for in particular? 679093 9 svillar 2012-07-26 02:47:03 -0700 (In reply to comment #8) > (From update of attachment 154589 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154589&action=review > > > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:69 > > g_main_loop_run(m_mainLoop); > > + while (g_main_context_iteration(g_main_context_default(), FALSE)) { } > > I think this means that you should rename the method here, waitUntilWebViewDrawSignal is no longer accurate. What events are you waiting for in particular? Well thing is that waiting for "draw" does not ensure that the actual contents are displayed, when I asked the gtk folks they told me that was the safest approach. What about waitUnitWebViewContentsDrawn? 685582 10 154589 cgarcia 2012-08-02 10:57:39 -0700 Comment on attachment 154589 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154589&action=review >>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:69 >>> + while (g_main_context_iteration(g_main_context_default(), FALSE)) { } >> >> I think this means that you should rename the method here, waitUntilWebViewDrawSignal is no longer accurate. What events are you waiting for in particular? > > Well thing is that waiting for "draw" does not ensure that the actual contents are displayed, when I asked the gtk folks they told me that was the safest approach. What about waitUnitWebViewContentsDrawn? Maybe you could use gdk_events_pending() to check whether there are gdk events to be processed 686479 11 154589 cgarcia 2012-08-03 02:29:37 -0700 Comment on attachment 154589 Patch This patch fixes two different issues actually, one is the previous/next issue (you should also unskip the tests cases, btw) which looks good to me, and the other is the hide test case that is flaky. Could you please file a different bug report for the hide issue? Feel free to commit the prev/next part, unskipping also the test cases. 694718 12 svillar 2012-08-14 02:28:37 -0700 Committed r125533: <http://trac.webkit.org/changeset/125533> 154589 2012-07-26 02:34:17 -0700 2012-08-03 02:29:37 -0700 Patch bug-91083-20120726113405.patch text/plain 3209 svillar U3VidmVyc2lvbiBSZXZpc2lvbjogMTIzMzEyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZTE4NTVhMzNjZDEwYmE5 ZGQ0N2M0NmEyMDk5NjdiOTBlNzJiMDQ1ZS4uZTg0YzFkNWRlNzY5MTdjMGY2MzMwOTQ5NTJhZWFi ZjJjZjhlZWRlYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA3LTI2ICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCisgICAgICAgIFtHVEtdIFJF R1JFU1NJT04gKHIxMjI0MjgpIFdlYktpdDJBUElUZXN0cy9UZXN0V2ViS2l0RmluZENvbnRyb2xs ZXIgZmFpbHMgIm5leHQiIHRlc3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTkxMDgzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgQWZ0ZXIgcjEyMjQyOCBzZWFyY2hfbmV4dCgpL3NlYXJjaF9wcmV2KCkgc2hv dWxkIHJldHVybiBqdXN0IDEgbWF0Y2gKKyAgICAgICAgaWYgdGhlIHRleHQgaXMgZm91bmQuIElt cHJvdmVkIGFsc28gdGhlIHJlbGlhYmlsaXR5IG9mIHRoZSB0ZXN0LCB3ZQorICAgICAgICBub3cg d2FpdCBmb3IgdGhlICJkcmF3IiBzaWduYWwgcGx1cyBhbGwgdGhlIHJlbWFpbmluZyBldmVudHMg dG8gdGFrZQorICAgICAgICB0aGUgc2NyZWVuc2hvdHMuCisKKyAgICAgICAgKiBVSVByb2Nlc3Mv QVBJL2d0ay90ZXN0cy9UZXN0V2ViS2l0RmluZENvbnRyb2xsZXIuY3BwOgorICAgICAgICAodGVz dEZpbmRDb250cm9sbGVyTmV4dCk6CisgICAgICAgICh0ZXN0RmluZENvbnRyb2xsZXJQcmV2aW91 cyk6CisKIDIwMTItMDctMjMgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0ub3JnPgogCiAg ICAgICAgIFJlbmFtZSBFTkFCTEVfTUVURVJfVEFHIGFuZCBFTkFCTEVfUFJPR1JFU1NfVEFHIHRv IEVOQUJMRV9NRVRFUl9FTEVNRU5UIGFuZCBFTkFCTEVfUFJPR1JFU1NfRUxFTUVOVCByZXNwZWN0 aXZlbHkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL3Rlc3Rz L1Rlc3RXZWJLaXRGaW5kQ29udHJvbGxlci5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv QVBJL2d0ay90ZXN0cy9UZXN0V2ViS2l0RmluZENvbnRyb2xsZXIuY3BwCmluZGV4IGExODVkYjhh NzcxZWNkMDdjZTY2NzI1MzNiMTFkMTZkZjRmY2Q2NTUuLmJlNDc5YzZlNWY5Y2E2MGZjMzNjNTM2 MzgwN2IwZDdjOWVmOTZhYjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9B UEkvZ3RrL3Rlc3RzL1Rlc3RXZWJLaXRGaW5kQ29udHJvbGxlci5jcHAKKysrIGIvU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9ndGsvdGVzdHMvVGVzdFdlYktpdEZpbmRDb250cm9sbGVyLmNw cApAQCAtNjYsNiArNjYsNyBAQCBwdWJsaWM6CiAgICAgewogICAgICAgICBnX3NpZ25hbF9jb25u ZWN0X2FmdGVyKG1fd2ViVmlldywgImRyYXciLCBHX0NBTExCQUNLKHdlYlZpZXdEcmF3KSwgdGhp cyk7CiAgICAgICAgIGdfbWFpbl9sb29wX3J1bihtX21haW5Mb29wKTsKKyAgICAgICAgd2hpbGUg KGdfbWFpbl9jb250ZXh0X2l0ZXJhdGlvbihnX21haW5fY29udGV4dF9kZWZhdWx0KCksIEZBTFNF KSkgeyB9CiAgICAgfQogCiAgICAgR1JlZlB0cjxXZWJLaXRGaW5kQ29udHJvbGxlcj4gbV9maW5k Q29udHJvbGxlcjsKQEAgLTE2NSwxNCArMTY2LDE0IEBAIHN0YXRpYyB2b2lkIHRlc3RGaW5kQ29u dHJvbGxlck5leHQoRmluZENvbnRyb2xsZXJUZXN0KiB0ZXN0LCBnY29uc3Rwb2ludGVyKQogICAg IHRlc3QtPndhaXRVbnRpbEZpbmRGaW5pc2hlZCgpOwogCiAgICAgZ19hc3NlcnQodGVzdC0+bV90 ZXh0Rm91bmQpOwotICAgIGdfYXNzZXJ0KHRlc3QtPm1fbWF0Y2hDb3VudCA9PSAyKTsKKyAgICBn X2Fzc2VydCh0ZXN0LT5tX21hdGNoQ291bnQgPT0gMSk7CiAgICAgZ19hc3NlcnQoISh3ZWJraXRf ZmluZF9jb250cm9sbGVyX2dldF9vcHRpb25zKHRlc3QtPm1fZmluZENvbnRyb2xsZXIuZ2V0KCkp ICYgV0VCS0lUX0ZJTkRfT1BUSU9OU19CQUNLV0FSRFMpKTsKIAogICAgIHdlYmtpdF9maW5kX2Nv bnRyb2xsZXJfc2VhcmNoX25leHQodGVzdC0+bV9maW5kQ29udHJvbGxlci5nZXQoKSk7CiAgICAg dGVzdC0+d2FpdFVudGlsRmluZEZpbmlzaGVkKCk7CiAKICAgICBnX2Fzc2VydCghdGVzdC0+bV90 ZXh0Rm91bmQpOwotICAgIGdfYXNzZXJ0KHRlc3QtPm1fbWF0Y2hDb3VudCA9PSAyKTsKKyAgICBn X2Fzc2VydCh0ZXN0LT5tX21hdGNoQ291bnQgPT0gMSk7CiAgICAgZ19hc3NlcnQoISh3ZWJraXRf ZmluZF9jb250cm9sbGVyX2dldF9vcHRpb25zKHRlc3QtPm1fZmluZENvbnRyb2xsZXIuZ2V0KCkp ICYgV0VCS0lUX0ZJTkRfT1BUSU9OU19CQUNLV0FSRFMpKTsKIH0KIApAQCAtMTkxLDE0ICsxOTIs MTQgQEAgc3RhdGljIHZvaWQgdGVzdEZpbmRDb250cm9sbGVyUHJldmlvdXMoRmluZENvbnRyb2xs ZXJUZXN0KiB0ZXN0LCBnY29uc3Rwb2ludGVyKQogICAgIHRlc3QtPndhaXRVbnRpbEZpbmRGaW5p c2hlZCgpOwogCiAgICAgZ19hc3NlcnQodGVzdC0+bV90ZXh0Rm91bmQpOwotICAgIGdfYXNzZXJ0 KHRlc3QtPm1fbWF0Y2hDb3VudCA9PSAyKTsKKyAgICBnX2Fzc2VydCh0ZXN0LT5tX21hdGNoQ291 bnQgPT0gMSk7CiAgICAgZ19hc3NlcnQoISh3ZWJraXRfZmluZF9jb250cm9sbGVyX2dldF9vcHRp b25zKHRlc3QtPm1fZmluZENvbnRyb2xsZXIuZ2V0KCkpICYgV0VCS0lUX0ZJTkRfT1BUSU9OU19C QUNLV0FSRFMpKTsKIAogICAgIHdlYmtpdF9maW5kX2NvbnRyb2xsZXJfc2VhcmNoX3ByZXZpb3Vz KHRlc3QtPm1fZmluZENvbnRyb2xsZXIuZ2V0KCkpOwogICAgIHRlc3QtPndhaXRVbnRpbEZpbmRG aW5pc2hlZCgpOwogCiAgICAgZ19hc3NlcnQodGVzdC0+bV90ZXh0Rm91bmQpOwotICAgIGdfYXNz ZXJ0KHRlc3QtPm1fbWF0Y2hDb3VudCA9PSAyKTsKKyAgICBnX2Fzc2VydCh0ZXN0LT5tX21hdGNo Q291bnQgPT0gMSk7CiAgICAgZ19hc3NlcnQod2Via2l0X2ZpbmRfY29udHJvbGxlcl9nZXRfb3B0 aW9ucyh0ZXN0LT5tX2ZpbmRDb250cm9sbGVyLmdldCgpKSAmIFdFQktJVF9GSU5EX09QVElPTlNf QkFDS1dBUkRTKTsKIH0KIAo=