Bug 37322 - [Qt] tst_QWebHistory::serialize_2() fails
Summary: [Qt] tst_QWebHistory::serialize_2() fails
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P1 Blocker
Assignee: Jędrzej Nowacki
URL:
Keywords: Qt
: 38679 (view as bug list)
Depends on:
Blocks: 38654
  Show dependency treegraph
 
Reported: 2010-04-09 02:59 PDT by Simon Hausmann
Modified: 2010-05-18 01:56 PDT (History)
6 users (show)

See Also:


Attachments
solution (842 bytes, patch)
2010-04-09 09:08 PDT, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff
Fix v1 (1.79 KB, patch)
2010-04-19 12:59 PDT, Jędrzej Nowacki
no flags Details | Formatted Diff | Diff
Fix v2 (1.35 KB, patch)
2010-04-19 16:58 PDT, Jędrzej Nowacki
hausmann: review+
jedrzej.nowacki: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2010-04-09 02:59:40 PDT
On Linux/X11:

ASSERTION FAILED: item->documentSequenceNumber() == history()->currentItem()->documentSequenceNumber()
(../../../WebCore/loader/FrameLoader.cpp:3771 void WebCore::FrameLoader::navigateWithinDocument(WebCore::HistoryItem*))
QFATAL : tst_QWebHistory::serialize_2() Received signal 11
FAIL!  : tst_QWebHistory::serialize_2() Received a fatal error.
   Loc: [Unknown file(0)]
Comment 1 Jędrzej Nowacki 2010-04-09 04:27:39 PDT
I can reproduce it on debian(32)
Comment 2 Jędrzej Nowacki 2010-04-09 05:58:05 PDT
The crash is caused by https://bugs.webkit.org/show_bug.cgi?id=33224
Comment 3 Jędrzej Nowacki 2010-04-09 09:08:25 PDT
Created attachment 52958 [details]
solution

The diff solve the problem but is a bit strange that we have to set nullitem document sequence number. I'm posting the diff here as a backup, I will investigate.
Comment 4 Jędrzej Nowacki 2010-04-19 12:59:24 PDT
Created attachment 53708 [details]
Fix v1

The solution shoiuld work, I want to do some manual testing before r? flag, but for now it pass all autotests.
Comment 5 Jędrzej Nowacki 2010-04-19 16:58:25 PDT
Created attachment 53740 [details]
Fix v2

Ok, one line solution. Fixes symptoms. After the patch the serialization behave in the same manner as before 33224 bug.

Autotests pass. I tried manual testing and it seems to work well.
Comment 6 Jędrzej Nowacki 2010-04-19 17:08:06 PDT
(In reply to comment #5)
> Created an attachment (id=53740) [details]
> Fix v2
> 
> Ok, one line solution. Fixes symptoms. After the patch the serialization behave
> in the same manner as before 33224 bug.
> 
> Autotests pass. I tried manual testing and it seems to work well.

Patch assumes that if a HistoryItems are in the same group, we can separate them without any consequences. If the assumption is bad then we have to bump streaming protocol version and save on each serialization document sequence number.
Comment 7 Diego Gonzalez 2010-05-04 11:50:05 PDT
I did no get this test failing anymore. Does anyone is getting this test still failing?
Comment 8 Jędrzej Nowacki 2010-05-05 02:18:10 PDT
(In reply to comment #7)
> I did no get this test failing anymore. Does anyone is getting this test still
> failing?
Hi, yes problem is still valid. It is an assert so it is visible only in debug, do you use debug build? If you use debug build then provide us more information about your platform. Thank you :-)

The bug was reproduced with trunk on Linux(Debian - testing 32).
Comment 9 Diego Gonzalez 2010-05-05 10:32:50 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > I did no get this test failing anymore. Does anyone is getting this test still
> > failing?
> Hi, yes problem is still valid. It is an assert so it is visible only in debug,
> do you use debug build? If you use debug build then provide us more information
> about your platform. Thank you :-)
> 
> The bug was reproduced with trunk on Linux(Debian - testing 32).

Yes sure, We did not test in debug mode. Got the fail now :)
Comment 10 Jesus Sanchez-Palencia 2010-05-06 13:42:54 PDT
*** Bug 38679 has been marked as a duplicate of this bug. ***
Comment 11 Simon Hausmann 2010-05-18 01:43:05 PDT
Committed r59660: <http://trac.webkit.org/changeset/59660>
Comment 12 Simon Hausmann 2010-05-18 01:46:40 PDT
Revision r59660 cherry-picked into qtwebkit-2.0 with commit 71e6346a3e2bf9d137d03be4d6dcff79ea0c8358
Comment 13 WebKit Review Bot 2010-05-18 01:56:36 PDT
http://trac.webkit.org/changeset/59660 might have broken Chromium Win Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/59659
http://trac.webkit.org/changeset/59660