Bug 40672 - [Qt] REGRESSION(r61207): qwebhistory unit test hangs
Summary: [Qt] REGRESSION(r61207): qwebhistory unit test hangs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Jędrzej Nowacki
URL:
Keywords: Qt, QtTriaged, Regression
Depends on:
Blocks: 31625
  Show dependency treegraph
 
Reported: 2010-06-16 05:03 PDT by Gabor Rapcsanyi
Modified: 2011-04-06 09:00 PDT (History)
11 users (show)

See Also:


Attachments
Fix v1 (3.28 KB, patch)
2010-06-18 06:32 PDT, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff
Fix (3.03 KB, patch)
2010-07-01 01:07 PDT, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gabor Rapcsanyi 2010-06-16 05:03:25 PDT
The qwebhistory Qt unit test falls into infinite loop.
After this revision the tst_qwebhistory stuck with this output:

********* Start testing of tst_QWebHistory *********
Config: Using QTest library 4.6.2, Qt 4.6.2
PASS   : tst_QWebHistory::initTestCase()
PASS   : tst_QWebHistory::title()
PASS   : tst_QWebHistory::count()
Comment 1 Darin Fisher (:fishd, Google) 2010-06-16 13:00:00 PDT
Perhaps this is related to the addition of the HistoryItem::itemSequenceNumber property.  http://trac.webkit.org/changeset/61207

I don't know enough about the Qt port, but if you have any questions about that patch, I'd more than happy to help answer them.
Comment 2 Jędrzej Nowacki 2010-06-17 02:17:15 PDT
I reprodced it, It is not an infite loop it simply hangs.

I don't think that it is related to HistoryItem::itemSequenceNumber. It seems that QWebPage doesn't emit signal loadFinished() after call to QWebHistory::back().
Comment 3 Csaba Osztrogonác 2010-06-17 03:47:40 PDT
FYI: I disabled running tst_qwebhistory test on
http://webkit.sed.hu/buildbot/waterfall site
temporarily (until fix) to make bot happier.
Comment 4 Jędrzej Nowacki 2010-06-17 05:48:02 PDT
(In reply to comment #2)
> I don't think that it is related to HistoryItem::itemSequenceNumber. 
I was wrong; reverting the http://trac.webkit.org/changeset/61207 helps.
Comment 5 Jędrzej Nowacki 2010-06-18 06:32:28 PDT
Created attachment 59103 [details]
Fix v1

The patch slightly change behavior of the functions goToItem, back, forward. Now loadFinished signal can be emitted immediately. I think that it should be ok, as we didn't explicitly define it.
Comment 6 Simon Hausmann 2010-06-20 04:30:31 PDT
Comment on attachment 59103 [details]
Fix v1

WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp:87
 +      connect(page, SIGNAL(loadFinished(bool)), &waitForLoadFinished, SLOT(quit()), Qt::QueuedConnection);
Why the queued connection?
Comment 7 WebKit Commit Bot 2010-06-20 04:46:29 PDT
Comment on attachment 59103 [details]
Fix v1

Clearing flags on attachment: 59103

Committed r61504: <http://trac.webkit.org/changeset/61504>
Comment 8 WebKit Commit Bot 2010-06-20 04:46:34 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Jędrzej Nowacki 2010-06-21 09:27:00 PDT
(In reply to comment #6)
> (From update of attachment 59103 [details])
> WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp:87
>  +      connect(page, SIGNAL(loadFinished(bool)), &waitForLoadFinished, SLOT(quit()), Qt::QueuedConnection);
> Why the queued connection?

As I wrote in comment 5 the patch slightly change behavior. I think it is because  load function isn't called while traversing through history.
Comment 10 Csaba Osztrogonác 2010-06-24 07:45:30 PDT
It works with Qt-4.7:
********* Start testing of tst_QWebHistory *********
Config: Using QTest library 4.7.0, Qt 4.7.0
PASS   : tst_QWebHistory::initTestCase()
PASS   : tst_QWebHistory::title()
PASS   : tst_QWebHistory::count()
PASS   : tst_QWebHistory::back()
PASS   : tst_QWebHistory::forward()
PASS   : tst_QWebHistory::itemAt()
PASS   : tst_QWebHistory::goToItem()
PASS   : tst_QWebHistory::items()
PASS   : tst_QWebHistory::serialize_1()
PASS   : tst_QWebHistory::serialize_2()
PASS   : tst_QWebHistory::serialize_3()
PASS   : tst_QWebHistory::saveAndRestore_crash_1()
PASS   : tst_QWebHistory::saveAndRestore_crash_2()
PASS   : tst_QWebHistory::saveAndRestore_crash_3()
PASS   : tst_QWebHistory::popPushState()
PASS   : tst_QWebHistory::clear()
PASS   : tst_QWebHistory::cleanupTestCase()
Totals: 17 passed, 0 failed, 0 skipped
********* Finished testing of tst_QWebHistory *********


But unfortunately it still hangs with Qt 4.6.2:
********* Start testing of tst_QWebHistory *********
Config: Using QTest library 4.6.2, Qt 4.6.2
PASS   : tst_QWebHistory::initTestCase()
PASS   : tst_QWebHistory::title()
PASS   : tst_QWebHistory::count()
PASS   : tst_QWebHistory::back()
PASS   : tst_QWebHistory::forward()
PASS   : tst_QWebHistory::itemAt()
PASS   : tst_QWebHistory::goToItem()
PASS   : tst_QWebHistory::items()
PASS   : tst_QWebHistory::serialize_1()
^CQFATAL : tst_QWebHistory::serialize_2() Received signal 2 --> I stopped here.
Comment 11 Jędrzej Nowacki 2010-06-29 06:45:21 PDT
(In reply to comment #10)
> It works with Qt-4.7:
(...)
> But unfortunately it still hangs with Qt 4.6.2:
I can confirm, it doesn't work with 4.6.2, but I tried 4.6.3 (default on my debian machine) and it is ok.
Comment 12 Jędrzej Nowacki 2010-06-29 07:26:00 PDT
It is really odd, It seems that there are a dependency between tests even if an operation are applied to different instances of the QWebPage. Each of the test pass separetelly. But:

./tst_qwebhistory serialize_1 serialize_1 

********* Start testing of tst_QWebHistory *********
Config: Using QTest library 4.6.2, Qt 4.6.2
PASS   : tst_QWebHistory::initTestCase()
PASS   : tst_QWebHistory::serialize_1()
FAIL!  : tst_QWebHistory::serialize_1() Compared values are not the same
   Actual (hist->count()): 0
   Expected (histsize): 5
   Loc: [/home/nierob/dev/WebKit/WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp(210)]
PASS   : tst_QWebHistory::cleanupTestCase()

Problem is trigered by serialization, only...
Comment 13 Jędrzej Nowacki 2010-07-01 01:07:36 PDT
Created attachment 60212 [details]
Fix

I removed a bug workaround. It seems that it is not needed for the Qt 4.6 (checked with 4.6.1, 4.6.2, 4.6.3). 

I couldn't find a Qt change that was neccessery for the workaround as the only link to it was via an expired pasbin's element.
Comment 14 Jędrzej Nowacki 2010-07-01 01:35:07 PDT
Comment on attachment 60212 [details]
Fix

It doesn't work correctly.
Comment 15 Benjamin Poulain 2011-04-06 09:00:16 PDT
We can close this, we do not support 4.6 anymore.