Bug 62498

Summary: Additional FontCache::FontCachePurgePreventer instances needed
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Layout and RenderingAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Major CC: ggaren, mrobinson, msaboff, xan.lopez
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch with additional instances of FontCachePurgePreventer oliver: review+

Description Ryosuke Niwa 2011-06-11 00:16:37 PDT 
Multiple tests are hitting assertion in FontCache::getCachedFontData on GTK:
ASSERTION FAILED: m_purgePreventCount
../../Source/WebCore/platform/graphics/FontCache.cpp(274) : WebCore::SimpleFontData* WebCore::FontCache::getCachedFontData(const WebCore::FontPlatformData*, WebCore::FontCache::ShouldRetain)

Maybe related to http://trac.webkit.org/changeset/88260 ?
Comment 2 Martin Robinson 2011-06-11 08:20:43 PDT 
Michael, any idea why we are hitting this ASSERT? Also what is the purpose of ASSERT_DISABLED in FontCache.cpp. It doesn't appear to be used at all.
Comment 3 Martin Robinson 2011-06-11 08:32:17 PDT 
Here's the backtrace:

#0  0x00002b91548e7037 in WebCore::FontCache::getCachedFontData (this=0xf9b6b0, platformData=0x7ffffe765270, shouldRetain=WebCore::FontCache::DoNotRetain) at ../../Source/WebCore/platform/graphics/FontCache.cpp:274
#1  0x00002b9154d765de in WebCore::FontCache::getFontDataForCharacters (this=0xf9b6b0, font=..., characters=UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-1: ordinal not in range(128)
, length=1) at ../../Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:92
#2  0x00002b91548f7204 in WebCore::Font::glyphDataForCharacter (this=0x21d81e8, c=1513, mirror=false, variant=WebCore::NormalVariant) at ../../Source/WebCore/platform/graphics/FontFastPath.cpp:195
#3  0x00002b915492df04 in WebCore::WidthIterator::advance (this=0x7ffffe765720, offset=28, glyphBuffer=0x0) at ../../Source/WebCore/platform/graphics/WidthIterator.cpp:126
#4  0x00002b91548f8267 in WebCore::Font::floatWidthForSimpleText (this=0x21d81e8, run=..., glyphBuffer=0x0, fallbackFonts=0x0, glyphOverflow=0x0) at ../../Source/WebCore/platform/graphics/FontFastPath.cpp:453
#5  0x00002b91548f3e42 in WebCore::Font::width (this=0x21d81e8, run=..., fallbackFonts=0x0, glyphOverflow=0x0) at ../../Source/WebCore/platform/graphics/Font.cpp:190
#6  0x00002b9154a9914d in WebCore::RenderMenuList::updateOptionsWidth (this=0x21d1c38) at ../../Source/WebCore/rendering/RenderMenuList.cpp:162
#7  0x00002b9154a99235 in WebCore::RenderMenuList::updateFromElement (this=0x21d1c38) at ../../Source/WebCore/rendering/RenderMenuList.cpp:177
#8  0x00002b915462a1b7 in WebCore::updateFromElementCallback (node=0x21d1130) at ../../Source/WebCore/html/HTMLFormControlElement.cpp:265
#9  0x00002b915444288f in WebCore::ContainerNode::dispatchPostAttachCallbacks () at ../../Source/WebCore/dom/ContainerNode.cpp:746
#10 0x00002b91544426ea in WebCore::ContainerNode::resumePostAttachCallbacks (this=0x21c0030) at ../../Source/WebCore/dom/ContainerNode.cpp:713
#11 0x00002b915445b3cd in WebCore::Document::recalcStyle (this=0x21c0030, change=WebCore::Node::NoChange) at ../../Source/WebCore/dom/Document.cpp:1564
#12 0x00002b915445b57d in WebCore::Document::updateStyleIfNeeded (this=0x21c0030) at ../../Source/WebCore/dom/Document.cpp:1586
#13 0x00002b915445b716 in WebCore::Document::updateLayout (this=0x21c0030) at ../../Source/WebCore/dom/Document.cpp:1613
#14 0x00002b915445b868 in WebCore::Document::updateLayoutIgnorePendingStylesheets (this=0x21c0030) at ../../Source/WebCore/dom/Document.cpp:1649
#15 0x00002b91543206ef in WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue (this=0x21d7540, propertyID=1002, updateLayout=WebCore::UpdateLayout) at ../../Source/WebCore/css/CSSComputedStyleDeclaration.cpp:803
#16 0x00002b915431f538 in WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue (this=0x21d7540, propertyID=1002) at ../../Source/WebCore/css/CSSComputedStyleDeclaration.cpp:675
#17 0x00002b91543d19a4 in WebCore::CSSStyleDeclaration::getPropertyCSSValue (this=0x21d7540, propertyName="direction") at ../../Source/WebCore/css/CSSStyleDeclaration.cpp:52
#18 0x00002b915425e207 in WebCore::JSCSSStyleDeclaration::nameGetter (exec=0x2b91a8014038, slotBase=..., propertyName="direction") at ../../Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp:155
#19 0x00002b915336ed69 in JSC::PropertySlot::getValue (this=0x7ffffe767950, exec=0x2b91a8014038, propertyName="direction") at ../../Source/JavaScriptCore/runtime/PropertySlot.h:75
#20 0x00002b91534381e7 in JSC::JSValue::get (this=0x7ffffe7679a0, exec=0x2b91a8014038, propertyName="direction", slot=...) at ../../Source/JavaScriptCore/runtime/JSObject.h:772
#21 0x00002b915342b787 in JSC::cti_op_get_by_id (args=0x7ffffe767a10) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:1592
#22 0x00002b9153429b92 in JSC::JITThunks::tryCacheGetByID (callFrame=0x2b916800b182, codeBlock=0x7ffffe767a10, returnAddress=..., baseValue=..., propertyName=, slot=..., stubInfo=0x156e380) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:943
#23 0x00002b91533fbfe9 in JSC::JITCode::execute (this=0x2b91a8e14180, registerFile=0xf994d8, callFrame=0x2b91a8014038, globalData=0x156e380) at ../../Source/JavaScriptCore/jit/JITCode.h:77
#24 0x00002b91533f83e7 in JSC::Interpreter::execute (this=0xf994c0, program=0x2b91a8e14168, callFrame=0x2b91a84f08f0, scopeChain=0x2b91a876c6e0, thisObj=0x2b91a84e0168) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:778
#25 0x00002b915349130a in JSC::evaluate (exec=0x2b91a84f08f0, scopeChain=0x2b91a876c6e0, source=..., thisValue=...) at ../../Source/JavaScriptCore/runtime/Completion.cpp:64
#26 0x00002b915429ca01 in WebCore::JSMainThreadExecState::evaluate (exec=0x2b91a84f08f0, chain=0x2b91a876c6e0, source=..., thisValue=...) at ../../Source/WebCore/bindings/js/JSMainThreadExecState.h:54
#27 0x00002b91542ce602 in WebCore::ScriptController::evaluateInWorld (this=0xf16cf8, sourceCode=..., world=0x1574840) at ../../Source/WebCore/bindings/js/ScriptController.cpp:143
#28 0x00002b91542ce7c2 in WebCore::ScriptController::evaluate (this=0xf16cf8, sourceCode=...) at ../../Source/WebCore/bindings/js/ScriptController.cpp:166
#29 0x00002b91544f8ebe in WebCore::ScriptElement::executeScript (this=0x21d3fe0, sourceCode=...) at ../../Source/WebCore/dom/ScriptElement.cpp:285
#30 0x00002b91544f8ab8 in WebCore::ScriptElement::prepareScript (this=0x21d3fe0, scriptStartPosition=..., supportLegacyTypes=WebCore::ScriptElement::DisallowLegacyTypeInTypeAttribute) at ../../Source/WebCore/dom/ScriptElement.cpp:241
#31 0x00002b9154693577 in WebCore::HTMLScriptRunner::runScript (this=0x21a4650, script=0x21d3f60, scriptStartPosition=...) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:296
#32 0x00002b9154692be8 in WebCore::HTMLScriptRunner::execute (this=0x21a4650, scriptElement=..., scriptStartPosition=...) at ../../Source/WebCore/html/parser/HTMLScriptRunner.cpp:170
#33 0x00002b9154685f5d in WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder (this=0x216b690) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:205
#34 0x00002b9154686019 in WebCore::HTMLDocumentParser::canTakeNextToken (this=0x216b690, mode=WebCore::HTMLDocumentParser::AllowYield, session=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:223
#35 0x00002b91546863f8 in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x216b690, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:261
#36 0x00002b9154685db2 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x216b690, mode=WebCore::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:175
#37 0x00002b9154686900 in WebCore::HTMLDocumentParser::append (this=0x216b690, source=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:367
#38 0x00002b9154447a85 in WebCore::DecodedDataDocumentParser::appendBytes (this=0x216b690, writer=0x2171320, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473, shouldFlush=false) at ../../Source/WebCore/dom/DecodedDataDocumentParser.cpp:54
#39 0x00002b91547b2711 in WebCore::DocumentWriter::addData (this=0x2171320, str=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., len=2473, flush=false) at ../../Source/WebCore/loader/DocumentWriter.cpp:203
#40 0x00002b91547a6b21 in WebCore::DocumentLoader::commitData (this=0x2171220, bytes=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473) at ../../Source/WebCore/loader/DocumentLoader.cpp:321
#41 0x00002b915413dcb2 in WebKit::FrameLoaderClient::committedLoad (this=0xf70df0, loader=0x2171220, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473) at ../../Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:320
#42 0x00002b91547a69f5 in WebCore::DocumentLoader::commitLoad (this=0x2171220, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473) at ../../Source/WebCore/loader/DocumentLoader.cpp:307
#43 0x00002b91547a6be0 in WebCore::DocumentLoader::receivedData (this=0x2171220, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473) at ../../Source/WebCore/loader/DocumentLoader.cpp:333
#44 0x00002b91547f1231 in WebCore::MainResourceLoader::addData (this=0x21a5380, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473, allAtOnce=false) at ../../Source/WebCore/loader/MainResourceLoader.cpp:168
#45 0x00002b91547fe6c7 in WebCore::ResourceLoader::didReceiveData (this=0x21a5380, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473, encodedDataLength=2473, allAtOnce=false) at ../../Source/WebCore/loader/ResourceLoader.cpp:280
#46 0x00002b91547f2618 in WebCore::MainResourceLoader::didReceiveData (this=0x21a5380, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473, encodedDataLength=2473, allAtOnce=false) at ../../Source/WebCore/loader/MainResourceLoader.cpp:464
#47 0x00002b91547fefca in WebCore::ResourceLoader::didReceiveData (this=0x21a5380, data=0x218fd20 "<!DOCTYPE html>\n<html>\n<head>\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n<style>\n   select { display: block; font-size: 16px; }\n   .reflection { font: 13px \"Lucida Grande\"; pa"..., length=2473, encodedDataLength=2473) at ../../Source/WebCore/loader/ResourceLoader.cpp:443
#48 0x00002b9154d6872b in WebCore::readCallback (source=0x2125640, asyncResult=0x1fea6a0, data=0x0) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:801
#49 0x00002b915830fb59 in async_ready_callback_wrapper (source_object=0x2125640, res=0x1fea6a0, user_data=0x0) at /tmp/buildd/glib2.0-2.28.6/./gio/ginputstream.c:470
#50 0x00002b915831fa68 in complete_in_idle_cb_for_thread (_data=0x21a8f40) at /tmp/buildd/glib2.0-2.28.6/./gio/gsimpleasyncresult.c:812
#51 0x00002b9158e8d4a3 in g_main_dispatch (context=0xefaa40) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:2440
#52 g_main_context_dispatch (context=0xefaa40) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3013
#53 0x00002b9158e8dc80 in g_main_context_iterate (context=0xefaa40, block=1, dispatch=1, self=<value optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3091
#54 0x00002b9158e8e2f2 in g_main_loop_run (loop=0x21684e0) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3299
#55 0x00002b9156dc92b7 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#56 0x000000000042a989 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:705
#57 0x000000000042a026 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:497
#58 0x000000000042c2f4 in main (argc=2, argv=0x7ffffe769408) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1183

Some observations:

* It's trying to render a string with Lucida Grande. We do not have Lucida Grande on the bots, as it's non-free.
* It's trying to get a fallback font when it fails.

Perhaps this RenderMenuList method just needs to have a FontCachePurgePreventer on the stack.
Comment 4 Michael Saboff 2011-06-11 08:58:15 PDT 
You are correct that we need to add FontCachePurgePreventer.  I was evaluating other places in platform code to add this.  I'll post a patch for this hopefully later today.
Comment 5 Michael Saboff 2011-06-13 11:19:36 PDT 
Renamed bug to cover other instances where FontCachePurgePreventer instances are needed.
Comment 6 Michael Saboff 2011-06-13 11:28:48 PDT 
Created attachment 96980 [details]
Patch with additional instances of FontCachePurgePreventer

Used several techniques to find additional locations to add FontCachePurgePreventer instances.
 - The traceback from this bug.
 - A radar <rdar://problem/9588525> with code inspection
 - In FontCache.cpp, set cMaxInactiveFontData and cTargetInactiveFontData to 0 which forces purging inactive fonts after last nested FontCachePurgePreventer is destructed.  Built webkit with this temp change and run tests looking for ASSERT failure.
 - Code inspection of platform code that calls either Font::drawText() or Canvas::drawText().
Comment 7 Michael Saboff 2011-06-13 14:09:06 PDT 
Committed r88691: <http://trac.webkit.org/changeset/88691>