Summary: | Acid3 sometimes crashes in WebCore::RenderObject::setNeedsLayout | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Blaut <webkit> | ||||||
Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Major | CC: | ismail, koivisto, mitz, zimmermann | ||||||
Priority: | P1 | Keywords: | InRadar, NeedsReduction | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
URL: | http://acid3.acidtests.org | ||||||||
Attachments: |
|
Description
Robert Blaut
2008-03-17 15:30:13 PDT
Created attachment 19853 [details]
crash log
I was able to reproduce this in the latest nightly build (r31090) after refreshing many times, closing the window, then loading Acid3 once more. 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(); } 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)? 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. 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. Created attachment 19978 [details]
Speculative fix, based on backtrace
WebCore/css/CSSFontSelector.cpp | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
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!
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. *** Bug 17672 has been marked as a duplicate of this bug. *** Comment on attachment 19978 [details]
Speculative fix, based on backtrace
Changelog, and testcase needed
r=me
landed as r31290. Sadly, w/o test case. :( |