WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
91083
[GTK] REGRESSION (
r122428
) WebKit2APITests/TestWebKitFindController fails "next" test
https://bugs.webkit.org/show_bug.cgi?id=91083
Summary
[GTK] REGRESSION (r122428) WebKit2APITests/TestWebKitFindController fails "ne...
Simon Pena
Reported
2012-07-12 05:52:26 PDT
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.
Attachments
Patch
(3.13 KB, patch)
2012-07-26 02:34 PDT
,
Sergio Villar Senin
cgarcia
: review+
cgarcia
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Simon Pena
Comment 1
2012-07-12 05:53:01 PDT
I'm taking a look at this atm.
Simon Pena
Comment 2
2012-07-12 10:59:45 PDT
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.
Simon Pena
Comment 3
2012-07-12 11:01:02 PDT
Assigning the bug to Sergio, who knows the code better.
Simon Pena
Comment 4
2012-07-17 00:33:59 PDT
Bug #91224
tracks the skipped API tests.
Sergio Villar Senin
Comment 5
2012-07-26 02:34:17 PDT
Created
attachment 154589
[details]
Patch
WebKit Review Bot
Comment 6
2012-07-26 02:36:05 PDT
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
Sergio Villar Senin
Comment 7
2012-07-26 02:37:07 PDT
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.
Martin Robinson
Comment 8
2012-07-26 02:37:45 PDT
Comment on
attachment 154589
[details]
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?
Sergio Villar Senin
Comment 9
2012-07-26 02:47:03 PDT
(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?
Carlos Garcia Campos
Comment 10
2012-08-02 10:57:39 PDT
Comment on
attachment 154589
[details]
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
Carlos Garcia Campos
Comment 11
2012-08-03 02:29:37 PDT
Comment on
attachment 154589
[details]
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.
Sergio Villar Senin
Comment 12
2012-08-14 02:28:37 PDT
Committed
r125533
: <
http://trac.webkit.org/changeset/125533
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug