Bug 76534 – [Qt] fontCache related assertion revealed by svg text layout optimization from r105143

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Gabor Rapcsanyi]

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

[Gabor Rapcsanyi]

Skipped until fix: http://trac.webkit.org/changeset/105269

[Csaba Osztrogonác]

Skipping didn't solve the problem, because the next test started to assert with same error: svg/carto.net/scrollbar.svg

[Csaba Osztrogonác]

I cc-ed SVG and fontcache experts. Have you got any idea?

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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.

[Zoltan Herczeg]

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.

[Nikolas Zimmermann]

Comment on attachment 123304 [details]
Draft patch

So is this safe to land? Or does it need further testing?

[Balazs Kelemen]

(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?

[Balazs Kelemen]

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.

[Balazs Kelemen]

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?

[Csaba Osztrogonác]

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?

[Balazs Kelemen]

(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?

[Csaba Osztrogonác]

(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.

[Balazs Kelemen]

(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.

[Csaba Osztrogonác]

(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.

[Balazs Kelemen]

(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.

[Balazs Kelemen]

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 ***

[Balazs Kelemen]

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.

[Balazs Kelemen]

(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.

[Jocelyn Turcotte]

=== 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.