When laying out a page that uses a downloaded font, preliminary layout is performed using the appropriate fallback font (default serif, sans, or monotype). The download does not start until the first use of the font. When the download completes, CSSFontSelector::fontLoaded() calls Document::scheduleForcedStyleRecalc() to notify of the switch from the fallback to the actual font. Also upon completion of the download, CSSFontFaceSource::fontLoaded() and CSSSegmentedFontFace::fontLoaded() have been calling pruneTable(), which immediately wipes out associated GlyphPageTreeNodes. The problem is that those GlyphPageTreeNodes, the ones associated with the fallback font, may still be in use on the UI thread, frequently by Font::glyphDataForCharacter(), causing a segfault.
Created attachment 82984 [details] proposed patch This patch simply removes the calls to pruneTable() at the moment that the font download is completed, deferring pruning until destruction of the owner. Element::recalcStyle triggers ~Font, which leads to FontCache::removeClient, which leads to ~CSSSegmentedFontFace and ~CSSFontFaceSource, each of which calls pruneTable at a time when it appears safe to do so. This patch is #ifdef'ed for Android, but I believe it to be safe for all platforms.
Could you please attach a crash log, <http://www.webkit.org/quality/crashlogs.html>? I don't understand why this bug says "Platform: Android", but the patch targets every other platform except for Android.
Created attachment 83347 [details] stack trace Attaching stack trace of a typical failure. Here's an excerpt showing the fallout of a freed node pointer: (gdb) frame 1 #1 0x822530a6 in WebCore::Font::glyphDataForCharacter (this=0x78d7f4, c=69, mirror=false, forceSmallCaps=false) at external/webkit/WebCore/platform/graphics/FontFastPath.cpp:76 76 GlyphData data = page->glyphDataForCharacter(c); (gdb) l 71 if (!useSmallCapsFont) { 72 // Fastest loop, for the common case (not small caps). 73 while (true) { 74 page = node->page(); 75 if (page) { 76 GlyphData data = page->glyphDataForCharacter(c); 77 if (data.fontData) { 78 if (data.fontData->platformData().orientation() == Vertical && data.fontData->orientation() == Horizontal && Font::isCJKIdeograph(c)) { 79 const SimpleFontData* ideographFontData = data.fontData->brokenIdeographFontData(); 80 GlyphPageTreeNode* ideographNode = GlyphPageTreeNode::getRootChild(ideographFontData, pageNumber); (gdb) p page $1 = (WebCore::GlyphPage *) 0x9 With the proposed patch, these two calls pruneTable() remain in place for all platforms except android (#if ! PLATFORM(ANDROID)). This may be overly conservative, as I believe the pruning will still take place in a timely fashion when the page is updated in response to Document::scheduleForcedStyleRecalc(), called by CSSFontSelector::fontLoaded(). The more aggressive patch would be to remove these two calls from all platforms.
Comment on attachment 82984 [details] proposed patch This is really really really scary. No changelog = r-.
Created attachment 83678 [details] proposed patch Added missing ChangeLogs
Attachment 83678 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 83684 [details] proposed patch cleaned up rogue tab
Pinging for review
Added oliver (original author) to cc list
Did you mean a different oliver?
Must be, if this code doesn't look familiar. I pulled the name oliver from http://trac.webkit.org/changeset/26484 and didn't catch that it mapped to :olliej. Sorry 'bout that. AFAICT, it looks like :oliver is no longer active.
After some digging it looks like svn history has been borken somehow -- it's definitely referring to me but it's all hyatt's fault.
it == the revision that introduced css fontface. According to the changelog hyatt did it (look changelog being more helpful/correct than revision history!!! hah!)
Thanks for sleuthing that. Haven't connected with hyatt yet via irc; will try smtp.
Underlying issue was traced to skia font matching for custom fonts with bold or italic style. Resolution was found in platform code.
Comment on attachment 83684 [details] proposed patch Cleared review? from attachment 83684 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Is there any update on this? I have recently come accross the same issue when running webcrawler test on motorola devices.