Bug 76534 - [Qt] fontCache related assertion revealed by svg text layout optimization from r105143
Summary: [Qt] fontCache related assertion revealed by svg text layout optimization fro...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P1 Critical
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 65711 79668
  Show dependency treegraph
 
Reported: 2012-01-18 05:44 PST by Gabor Rapcsanyi
Modified: 2014-02-03 03:19 PST (History)
6 users (show)

See Also:


Attachments
Draft patch (1.14 KB, patch)
2012-01-20 06:18 PST, Zoltan Herczeg
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 2012-01-18 05:44:02 PST
ASSERTION FAILED: fontCache()->generation() == m_generation
../../../../Source/WebCore/platform/graphics/FontFallbackList.cpp(104) : const WebCore::FontData* WebCore::FontFallbackList::fontDataAt(const WebCore::Font*, unsigned int) const
Comment 1 Gabor Rapcsanyi 2012-01-18 06:00:38 PST
Skipped until fix: http://trac.webkit.org/changeset/105269
Comment 2 Csaba Osztrogonác 2012-01-18 08:10:28 PST
Skipping didn't solve the problem, because the next test started to assert with same error: svg/carto.net/scrollbar.svg
Comment 3 Csaba Osztrogonác 2012-01-18 08:12:41 PST
I cc-ed SVG and fontcache experts. Have you got any idea?
Comment 4 Csaba Osztrogonác 2012-01-18 09:38:13 PST
svg/carto.net/combobox.svg is the culprit test.
You can easily reproduce the bug with:
$ Tools/Scripts/old-run-webkit-tests --debug svg/carto.net/combobox.svg RANDOM-SVG-CARTO_NET-TEST
Comment 5 Csaba Osztrogonác 2012-01-18 09:42:44 PST
I skipped a test to paint the bot green: http://trac.webkit.org/changeset/105287

But we really need a proper fix for this assertion.
Comment 6 Zoltan Herczeg 2012-01-20 06:18:07 PST
Created attachment 123304 [details]
Draft patch

This patch fixes this particular issue, but it may have other side effects, so it is early to call this as a proper fix.
Comment 7 Nikolas Zimmermann 2012-01-21 00:35:16 PST
Comment on attachment 123304 [details]
Draft patch

So is this safe to land? Or does it need further testing?
Comment 8 Balazs Kelemen 2012-01-21 14:29:29 PST
(In reply to comment #7)
> (From update of attachment 123304 [details])
> So is this safe to land? Or does it need further testing?

I would rather ask, does this really fix the bug, or just hides it?
Comment 9 Balazs Kelemen 2012-06-18 09:12:42 PDT
Comment on attachment 123304 [details]
Draft patch

Zoltan said to me that it's about where we call clearMemoryCaches, and this reordering untriggered the bug. However, all of this is public API so it should work in any order. I'm going to check if this is reproducible now.
Comment 10 Balazs Kelemen 2012-06-19 06:26:23 PDT
I could not reproduce this bug now with Qt 5. Can we close it? Should we check Qt 4.8 as well, given that it's support will be dropped from trunk soon?
Comment 11 Csaba Osztrogonác 2012-06-19 07:05:25 PDT
It is still valid on Qt 4.8, you can easily reproduce the assert:
$ Tools/Scripts/old-run-webkit-tests --debug svg/carto.net/combobox.svg svg/carto.net/frameless-svg-parse-error.html

Maybe it is valid on Qt 5 too, but I don't have debug Qt 5 build. And unfortunately debug WebKit build isn't possible with release Qt 5.

Could you test it with Qt 5?
Comment 12 Balazs Kelemen 2012-06-19 07:10:34 PDT
(In reply to comment #11)
> It is still valid on Qt 4.8, you can easily reproduce the assert:
> $ Tools/Scripts/old-run-webkit-tests --debug svg/carto.net/combobox.svg svg/carto.net/frameless-svg-parse-error.html
> 
> Maybe it is valid on Qt 5 too, but I don't have debug Qt 5 build. And unfortunately debug WebKit build isn't possible with release Qt 5.
> 
> Could you test it with Qt 5?

I will. The question is: should we care about Qt 4?
Comment 13 Csaba Osztrogonác 2012-06-19 07:17:39 PDT
(In reply to comment #12)
> I will. The question is: should we care about Qt 4?
Yes, we should till Qt 4 is supported officially.
Comment 14 Balazs Kelemen 2012-06-19 07:27:52 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > I will. The question is: should we care about Qt 4?
> Yes, we should till Qt 4 is supported officially.

Qt 4 + trunk support will be dropped in a few weeks (days?). Do you think about that time, or the lifetime of Qt 4? Qt 4 with trunk will be mantained in a branch, so I think Qt 4 QA work will not be our job anymore.
Comment 15 Csaba Osztrogonác 2012-06-19 07:34:41 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > I will. The question is: should we care about Qt 4?
> > Yes, we should till Qt 4 is supported officially.
> 
> Qt 4 + trunk support will be dropped in a few weeks (days?). Do you think about that time, or the lifetime of Qt 4? Qt 4 with trunk will be mantained in a branch, so I think Qt 4 QA work will not be our job anymore.

You're right. But we should care with Qt 4 till the big boss push 
the big red button and remove all Qt 4 related code from trunk.

But it is absolutely unrelated to this topic. The question
is here if the bug is Qt4 related or it is valid on Qt5 too.

If it is Qt4 related, we should move the skipped list entry to qt-4.8/Skipped.
Comment 16 Balazs Kelemen 2012-06-19 07:41:35 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > (In reply to comment #12)
> > > > I will. The question is: should we care about Qt 4?
> > > Yes, we should till Qt 4 is supported officially.
> > 
> > Qt 4 + trunk support will be dropped in a few weeks (days?). Do you think about that time, or the lifetime of Qt 4? Qt 4 with trunk will be mantained in a branch, so I think Qt 4 QA work will not be our job anymore.
> 
> You're right. But we should care with Qt 4 till the big boss push 
> the big red button and remove all Qt 4 related code from trunk.
> 
> But it is absolutely unrelated to this topic. The question
> is here if the bug is Qt4 related or it is valid on Qt5 too.
> 
> If it is Qt4 related, we should move the skipped list entry to qt-4.8/Skipped.

Ok, I just wanted to make this clear :)

Yes, this is still reproducible with Qt 5 WebKit1.
Comment 17 Balazs Kelemen 2012-06-21 06:51:49 PDT
Filed a new bug for it since I'm planning to change cross-platform code.

*** This bug has been marked as a duplicate of bug 89658 ***
Comment 18 Balazs Kelemen 2012-07-03 01:12:24 PDT
Unduplicating since my assumption that the bug is related to the font system was wrong. In fact it is in the svg code from r105143.
Comment 19 Balazs Kelemen 2012-07-03 01:16:19 PDT
(In reply to comment #18)
> Unduplicating since my assumption that the bug is related to the font system was wrong. In fact it is in the svg code from r105143.

See discussion in bug 89658 comment 9.
Comment 20 Jocelyn Turcotte 2014-02-03 03:19:39 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.