Bug 17902 – Acid3 sometimes crashes in WebCore::RenderObject::setNeedsLayout

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Robert Blaut]

In Webkit r31112 on Mac OS X I experience now crash in WebCore::RenderObject::setNeedsLayout.

It's hard to reproduce but it's reproducible. All you should do is reloading and reloading the test. Often stop in the middle of running of test and reload. It's a sort of torture test but always leads to the same crash.

It's related to the bug 17878 which is already fixed.

[Robert Blaut]

Created attachment 19853 [details]
crash log

[Mark Rowe (bdash)]

I was able to reproduce this in the latest nightly build (r31090) after refreshing many times, closing the window, then loading Acid3 once more.

[Mark Rowe (bdash)]

<rdar://problem/5803814>

[Antti Koivisto]

Are you sure this is a regression? I suspect this might be unrelated to the other loader crashes. Based on the crash log renderer() just should be null checked here:

void CSSFontSelector::fontLoaded(CSSSegmentedFontFace*)
{
    if (m_document->inPageCache())
        return;
    m_document->recalcStyle(Document::Force);
    m_document->renderer()->setNeedsLayoutAndPrefWidthsRecalc();
}

[Antti Koivisto]

Removing regression keyword since there is no evidence this is one.

(In reply to comment #4)
> Are you sure this is a regression? I suspect this might be unrelated to the
> other loader crashes. Based on the crash log renderer() just should be null
> checked here:

Why are we reaching this code with a viewless document? Is it in the page cache or is it the document holding the remote font (in which case we should probably be targeting a different document, the one using the font)?

[Antti Koivisto]

I can't reproduce so I only have the crash log to look. CSSFontSelector must have reffed the CachedFont since it is getting the callback. Cache layer is not making any decisions which document to target, it is all subscription based.

Architecturally it is wrong to assume that all documents have render tree. Are there no real scenarios you can think of where we would either have ripped down the render tree or not yet constructed it?

I can't repro either (and I don't think that the bug has to do with your recent loader work). If a document does not have a renderer it should not be asking for fonts. If it had asked for them while it had a renderer and then lost it, it should have cancelled the request.

[Antti Koivisto]

I don't see that that reflected in code. Fonts are requested in CSS parsing time and are in no way tied to rendering as far as I see.

[Eric Seidel (no email)]

Created attachment 19978 [details]
Speculative fix, based on backtrace

 WebCore/css/CSSFontSelector.cpp |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 19978 [details]
Speculative fix, based on backtrace

Suggestions as to how I might make a test for this (i.e. when the document would not have a renderer, yet would be loading fonts) would be most welcome!

[Ismail Donmez]

To reproduce refresh acid3 test 10 times and each time wait for it to finish. Then close Safari and open acid3 test again, it'll crash.

[Eric Seidel (no email)]

*** Bug 17672 has been marked as a duplicate of this bug. ***

[Oliver Hunt]

Comment on attachment 19978 [details]
Speculative fix, based on backtrace

Changelog, and testcase needed 
r=me

[Eric Seidel (no email)]

landed as r31290.  Sadly, w/o test case. :(