Bug 18310

Summary: Acid3 gets segmentation fault with pango
Product: WebKit Reporter: Dirk Schulze <krit>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ahya365, alp, blinxwang, cbergstrom, diegoe
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Proposed patch
none
Revised patch zecke: review+

Description Dirk Schulze 2008-04-04 03:35:42 PDT
Acid3-test gets a segmentation fault after 77% with pango enabled.

This message came after 77% of Acid3 in GtkLauncher:

(lt-GtkLauncher:3899): Pango-CRITICAL **: pango_font_describe_with_absolute_size: assertion `font != NULL' failed

(lt-GtkLauncher:3899): Pango-CRITICAL **: pango_font_description_get_family: assertion `desc != NULL' failed
Segmentation fault (core dumped)
Comment 1 nousername 2008-05-02 16:22:20 PDT
Same here, but it fails at 69% for me using the last 3 nightlies (32778, 32777, 32698). Also, I don't see a segfault message, just the Pango-CRITICAL warnings.

Rebuilding webkitgtk with the freetype backend lets Acid3 finish at 100%. The tradeoff is that font rendering doesn't look as nice.
Comment 2 Adam Williamson 2008-05-15 09:11:20 PDT
Confirmed again here, same result as Josh (though I get bug-buddy popping up). Build 33029.
Comment 3 ./C 2008-07-10 01:43:26 PDT
Full stack with debugging to help..

gdb /usr/bin/midori 
GNU gdb 6.7.1
Copyright (C) 2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu"...
(no debugging symbols found)
Using host libthread_db library "/lib64/libthread_db.so.1".
(gdb) run
Starting program: /usr/bin/midori 
(no debugging symbols found)
warning: Lowest section in /usr/lib64/libicudata.so.38 is .hash at 0000000000000190
[Thread debugging using libthread_db enabled]
[New Thread 0x2b39aa754cc0 (LWP 26686)]

(midori:26686): Gtk-WARNING **: Unable to locate theme engine in module_path: "murrine",
UNIMPLEMENTED: 
(WebCore/platform/gtk/PasteboardGtk.cpp:90 WebCore::Pasteboard::Pasteboard())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:630 virtual void WebKit::FrameLoaderClient::provisionalLoadStarted())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:401 virtual bool WebKit::FrameLoaderClient::hasWebView() const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:456 virtual void WebKit::FrameLoaderClient::setCopiesOnScroll())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:637 virtual void WebKit::FrameLoaderClient::prepareForDataSourceReplacement())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:407 virtual bool WebKit::FrameLoaderClient::hasFrameView() const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:217 virtual void WebKit::EditorClient::clearUndoRedoOperations())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:356 virtual WebCore::String WebKit::FrameLoaderClient::overrideMediaType() const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:418 virtual void WebKit::FrameLoaderClient::frameLoadCompleted())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:451 virtual void WebKit::FrameLoaderClient::forceLayoutForNonHTML())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:264 virtual void WebKit::ChromeClient::addToDirtyRegion(const WebCore::IntRect&))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:569 virtual void WebKit::FrameLoaderClient::cancelPolicyCheck())
UNIMPLEMENTED: 
(WebKit/gtk/webkit/webkitwebview.cpp:569 WebKitNavigationResponse webkit_web_view_real_navigation_requested(WebKitWebView*, WebKitWebFrame*, WebKitNetworkRequest*))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:594 virtual bool WebKit::FrameLoaderClient::canHandleRequest(const WebCore::ResourceRequest&) const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:210 virtual void WebKit::FrameLoaderClient::assignIdentifierToInitialRequest(long unsigned int, WebCore::DocumentLoader*, const WebCore::ResourceRequest&))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:205 virtual void WebKit::FrameLoaderClient::dispatchWillSendRequest(WebCore::DocumentLoader*, long unsigned int, WebCore::ResourceRequest&, const WebCore::ResourceResponse&))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:606 virtual bool WebKit::FrameLoaderClient::representationExistsForURLScheme(const WebCore::String&) const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:559 virtual void WebKit::FrameLoaderClient::dispatchDidFirstLayout())
[New Thread 0x40804950 (LWP 26689)]
[Thread 0x40804950 (LWP 26689) exited]
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:600 virtual bool WebKit::FrameLoaderClient::canShowMIMEType(const WebCore::String&) const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:513 virtual void WebKit::FrameLoaderClient::dispatchWillClose())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:441 virtual void WebKit::FrameLoaderClient::makeRepresentation(WebCore::DocumentLoader*))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:756 virtual void WebKit::FrameLoaderClient::updateGlobalHistory(const WebCore::KURL&))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:584 virtual void WebKit::FrameLoaderClient::willChangeTitle(WebCore::DocumentLoader*))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:648 virtual void WebKit::FrameLoaderClient::dispatchDidReceiveContentLength(WebCore::DocumentLoader*, long unsigned int, int))
UNIMPLEMENTED: 
(WebKit/gtk/webkit/webkitwebview.cpp:575 void webkit_web_view_real_window_object_cleared(WebKitWebView*, WebKitWebFrame*, OpaqueJSContext*, OpaqueJSValue*))
UNIMPLEMENTED: 
(WebCore/platform/gtk/RenderThemeGtk.cpp:415 virtual void WebCore::RenderThemeGtk::systemFont(int, WebCore::FontDescription&) const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:554 virtual void WebKit::FrameLoaderClient::dispatchDidFinishDocumentLoad())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:574 virtual void WebKit::FrameLoaderClient::dispatchDidLoadMainResource(WebCore::DocumentLoader*))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:653 virtual void WebKit::FrameLoaderClient::dispatchDidFinishLoading(WebCore::DocumentLoader*, long unsigned int))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:103 virtual bool WebKit::EditorClient::isContinuousSpellCheckingEnabled())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:488 virtual void WebKit::FrameLoaderClient::dispatchDidHandleOnloadEvents())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:139 virtual bool WebKit::EditorClient::shouldChangeSelectedRange(WebCore::Range*, WebCore::Range*, WebCore::EAffinity, bool))
UNIMPLEMENTED: 
(WebCore/page/gtk/EventHandlerGtk.cpp:90 bool WebCore::EventHandler::eventActivatedView(const WebCore::PlatformMouseEvent&) const)
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:133 virtual bool WebKit::EditorClient::shouldInsertText(const WebCore::String&, WebCore::Range*, WebCore::EditorInsertAction))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:207 virtual void WebKit::EditorClient::registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand>))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:162 virtual void WebKit::EditorClient::respondToChangedContents())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:423 virtual void WebKit::FrameLoaderClient::saveViewStateToItem(WebCore::HistoryItem*))
UNIMPLEMENTED: 
(WebCore/page/gtk/FrameGtk.cpp:53 void WebCore::Frame::clearPlatformScriptObjects())
[New Thread 0x40804950 (LWP 26691)]
[Thread 0x40804950 (LWP 26691) exited]
[New Thread 0x40804950 (LWP 26692)]
[Thread 0x40804950 (LWP 26692) exited]
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:44 virtual WebCore::DragSourceAction WebKit::DragClient::dragSourceActionMaskForPoint(const WebCore::IntPoint&))
[New Thread 0x40804950 (LWP 26709)]
[Thread 0x40804950 (LWP 26709) exited]
UNIMPLEMENTED: 
(WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:595 void WebCore::GraphicsContext::setPlatformShadow(const WebCore::IntSize&, int, const WebCore::Color&))
UNIMPLEMENTED: 
(WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:600 void WebCore::GraphicsContext::clearPlatformShadow())
NPP_GetJavaClass()
NPP_Initialize()
NPP_GetValue()
NPP_GetValue()
NPP_GetJavaClass()
NPP_Initialize()
NPP_GetValue()
NPP_GetValue()
UNIMPLEMENTED: 
(WebCore/platform/gtk/LocalizedStringsGtk.cpp:326 WebCore::String WebCore::imageTitle(const WebCore::String&, const WebCore::IntSize&))
UNIMPLEMENTED: 
(WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:594 static void WebCore::MediaPlayerPrivate::getSupportedTypes(WTF::HashSet<WebCore::String, WebCore::StringHash, WTF::HashTraits<WebCore::String> >&))
UNIMPLEMENTED: 
(WebCore/platform/gtk/PlatformScreenGtk.cpp:49 int WebCore::screenDepthPerComponent(WebCore::Widget*))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:503 virtual void WebKit::FrameLoaderClient::dispatchWillPerformClientRedirect(const WebCore::KURL&, double, double))
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:498 virtual void WebKit::FrameLoaderClient::dispatchDidCancelClientRedirect())
UNIMPLEMENTED: 
(WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:663 virtual bool WebKit::FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache(WebCore::DocumentLoader*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, int))

(midori:26686): Pango-CRITICAL **: pango_font_describe_with_absolute_size: assertion `font != NULL' failed

(midori:26686): Pango-CRITICAL **: pango_font_description_get_family: assertion `desc != NULL' failed

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x2b39aa754cc0 (LWP 26686)]
0x00002b39a7b05115 in g_str_hash () from /usr/lib/libglib-2.0.so.0
(gdb) bt
#0  0x00002b39a7b05115 in g_str_hash () from /usr/lib/libglib-2.0.so.0
#1  0x00002b39a7ad3dad in g_hash_table_lookup () from /usr/lib/libglib-2.0.so.0
#2  0x00002b399cb34f40 in WebCore::FontPlatformData::isFixedPitch (this=0x0) at WebCore/platform/graphics/gtk/FontPlatformDataPango.cpp:194
#3  0x00002b399cb35ddd in WebCore::SimpleFontData::determinePitch (this=0x152baa0) at WebCore/platform/graphics/gtk/SimpleFontDataPango.cpp:121
#4  0x00002b399c62bde7 in SimpleFontData (this=0x152baa0, f=@0x7fff0f35fb90, customFont=224, loading=255, svgFontData=0x13a17a0) at WebCore/platform/graphics/SimpleFontData.cpp:85
#5  0x00002b399c0fbbba in WebCore::CSSFontFaceSource::getFontData (this=0x13f95f0, fontDescription=@0x152baa0, syntheticBold=false, syntheticItalic=false, fontSelector=0xb666e0)
    at ./WebCore/platform/graphics/FontDescription.h:75
#6  0x00002b399c0eb753 in WebCore::CSSFontFace::getFontData (this=0x13a7a20, fontDescription=@0x12a8500, syntheticBold=false, syntheticItalic=false) at ./JavaScriptCore/wtf/Vector.h:446
#7  0x00002b399c154146 in WebCore::CSSSegmentedFontFace::getFontData (this=0x13b2690, fontDescription=@0x12a8500) at ./JavaScriptCore/wtf/RefPtr.h:446
#8  0x00002b399c0f253d in WebCore::CSSFontSelector::getFontData (this=0x142be40, fontDescription=@0x12a8500, familyName=@0x1) at ./JavaScriptCore/wtf/RefPtr.h:61
#9  0x00002b399c6067c8 in WebCore::FontCache::getFontData (font=@0x12a8500, familyIndex=@0x12a85b0, fontSelector=0x142be40) at WebCore/platform/graphics/Font.h:163
#10 0x00002b399c619221 in WebCore::FontFallbackList::fontDataAt (this=0x12a8580, font=0x12a8500, realizedFontIndex=2828708576) at ./JavaScriptCore/wtf/RefPtr.h:55
#11 0x00002b399c619404 in WebCore::FontFallbackList::determinePitch (this=0x12a8580, font=0x2b39a89aaae0) at WebCore/platform/graphics/FontFallbackList.h:60
#12 0x00002b399c60fae2 in WebCore::Font::isFixedPitch (this=0x12a8500) at WebCore/platform/graphics/FontFallbackList.h:49
#13 0x00002b399c786930 in WebCore::RenderText::calcPrefWidths (this=0x12ab728, leadWidth=0) at WebCore/rendering/RenderText.cpp:368
#14 0x00002b399c7846b1 in WebCore::RenderText::maxPrefWidth (this=0x12ab728) at WebCore/rendering/RenderText.cpp:500
#15 0x00002b399c7858cc in WebCore::RenderText::width (this=0x12ab728, from=0, len=24, f=@0x12a8500, xPos=0) at WebCore/rendering/RenderText.cpp:1016
#16 0x00002b399c6a7b1c in WebCore::RenderBlock::findNextLineBreak (this=0xd23ee8, resolver=@0x7fff0f3605e0, clear=0x7fff0f360534) at WebCore/rendering/bidi.cpp:1997
#17 0x00002b399c6a9462 in WebCore::RenderBlock::layoutInlineChildren (this=0xd23ee8, relayoutChildren=true, repaintTop=@0x7fff0f360790, repaintBottom=@0x7fff0f360794)
    at WebCore/rendering/bidi.cpp:944
#18 0x00002b399c6d7c03 in WebCore::RenderBlock::layoutBlock (this=0xd23ee8, relayoutChildren=true) at WebCore/rendering/RenderBlock.cpp:624
#19 0x00002b399c6c4185 in WebCore::RenderBlock::layout (this=0xd23ee8) at WebCore/rendering/RenderBlock.cpp:535
#20 0x00002b399cadaa8c in WebCore::RenderSVGText::layout (this=0xd23ee8) at WebCore/rendering/RenderSVGText.cpp:103
#21 0x00002b399cad8edb in WebCore::RenderSVGRoot::layout (this=0x13da088) at WebCore/rendering/RenderObject.h:507
#22 0x00002b399c6d52c8 in WebCore::RenderBlock::layoutBlockChildren (this=0x12b6708, relayoutChildren=false, maxFloatBottom=@0x7fff0f360acc) at WebCore/rendering/RenderBlock.cpp:1280
#23 0x00002b399c6d80ad in WebCore::RenderBlock::layoutBlock (this=0x12b6708, relayoutChildren=false) at WebCore/rendering/RenderBlock.cpp:626
#24 0x00002b399c6c4185 in WebCore::RenderBlock::layout (this=0x12b6708) at WebCore/rendering/RenderBlock.cpp:535
#25 0x00002b399c794c3a in WebCore::RenderView::layout (this=0x12b6708) at WebCore/rendering/RenderView.cpp:113
#26 0x00002b399c5a4779 in WebCore::FrameView::layout (this=0x13441c0, allowSubtree=44) at WebCore/page/FrameView.cpp:483
#27 0x00002b399c1c88a5 in WebCore::Document::updateLayout (this=0x1500b00) at WebCore/dom/Document.cpp:1197
#28 0x00002b399c1ea98b in WebCore::Document::updateLayoutIgnorePendingStylesheets (this=0x1500b00) at WebCore/dom/Document.cpp:1228
#29 0x00002b399ca5fa11 in WebCore::SVGTextContentElement::getNumberOfChars (this=0x132f120) at ./WebCore/dom/Node.h:296
#30 0x00002b399cd90003 in WebCore::jsSVGTextContentElementPrototypeFunctionGetNumberOfChars (exec=0x7fff0f3619b0, thisValue=0x2aaab3b24e80, args=@0xffffffff)
    at ./JavaScriptCore/kjs/JSNumberCell.h:121
#31 0x00002b399cdf5dd1 in KJS::Machine::privateExecute (this=0x94f1b0, flag=2828708576, exec=0x7fff0f3619b0, registerFile=0x94f1c8, r=0x2aaab23a5178, scopeChain=0x13ca320, 
    codeBlock=0x13da2a0, exception=0xfceba0) at JavaScriptCore/VM/Machine.cpp:2285
#32 0x00002b399cdfb27e in KJS::Machine::execute (this=0x94f1b0, functionBodyNode=0x1430080, exec=0xfceb90, function=0x2aaab25bdb80, thisObj=0x13a17a0, args=@0x10, scopeChain=0xc43300, 
    exception=0xfceba0) at JavaScriptCore/VM/Machine.cpp:764
#33 0x00002b399ceeb9b6 in KJS::JSFunction::call (this=0x2aaab25bdb80, exec=0xfceb90, thisValue=0x2b39a89aaae0, args=@0x7fff0f361c00) at JavaScriptCore/kjs/ExecState.h:82
#34 0x00002b399ceedd88 in KJS::call (exec=0xfceb90, functionObject=0x2aaab25bdb80, callType=2828708576, callData=@0x6512d0, thisValue=0x2aaab25bfa80, args=@0x7fff0f361c00)
    at JavaScriptCore/kjs/JSValue.cpp:111
#35 0x00002b399c0b88f0 in WebCore::ScheduledAction::execute (this=0x13a1010, windowShell=0x2aaab25bfa80) at ./JavaScriptCore/kjs/protect.h:68
#36 0x00002b399c068815 in WebCore::JSDOMWindowBase::timerFired (this=0x2aaab25bfa40, timer=0x1016310) at WebCore/bindings/js/JSDOMWindowBase.cpp:1266
#37 0x00002b399c068b6d in WebCore::DOMWindowTimer::fired (this=0x2b39a89aaae0) at WebCore/bindings/js/JSDOMWindowBase.cpp:1300
#38 0x00002b399c667ad7 in WebCore::TimerBase::fireTimers (fireTime=1215699071.059026, firingTimers=@0x7fff0f361e80) at WebCore/platform/Timer.cpp:347
#39 0x00002b399c667c28 in WebCore::TimerBase::sharedTimerFired () at WebCore/platform/Timer.cpp:368
#40 0x00002b399cb1b23a in timeout_cb () at WebCore/platform/gtk/SharedTimerGtk.cpp:48
#41 0x00002b39a7ae563b in ?? () from /usr/lib/libglib-2.0.so.0
#42 0x00002b39a7ae1b46 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#43 0x00002b39a7ae3737 in ?? () from /usr/lib/libglib-2.0.so.0
#44 0x00002b39a7ae3a98 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#45 0x00002b39a31c9b59 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
---Type <return> to continue, or q <return> to quit---
#46 0x000000000040ecdb in main ()
Comment 4 ./C 2008-07-16 03:05:20 PDT
As you can see from the bt below.. isFixedPitch is called twice.. I'm not sure if it's the proper fix, but

WebCore/platform/graphics/SimpleFontData.cpp
line 70 and 85
comment out //determinePitch();

I get another segfault after this, but not sure if it's related. when I have a patch for this issue I'll attach it.
Comment 5 Mark Rowe (bdash) 2008-07-20 00:20:44 PDT
*** Bug 20109 has been marked as a duplicate of this bug. ***
Comment 6 Adam Williamson 2008-09-11 12:01:56 PDT
Still valid with nightly 36309. Same error.
Comment 7 Dirk Schulze 2008-09-11 12:19:20 PDT
I think the crash with pango on Acid3 is related to https://bugs.webkit.org/show_bug.cgi?id=18552. As long as pango have no support for downloadable fonts Acid3 won't pass.
But the crash should be avoidable.
Comment 8 nousername 2008-10-20 00:46:40 PDT
Confirmed still present in nightly version 37698. What about comment #7? Can webkit at least not crash when fonts can't be downloaded?

Right now webkit gets to 69% in Acid3 before crashing & killing Midori. So close...

Also, Acid3 isn't the only place where this kind of pango crash occurs, so can the bug's severity be escalated a notch to Major?
Comment 9 Hiroyuki Ikezoe 2008-12-23 19:45:04 PST
Created attachment 26232 [details]
Proposed patch
Comment 10 Holger Freyther 2008-12-24 04:25:47 PST
(In reply to comment #8)

> Also, Acid3 isn't the only place where this kind of pango crash occurs, so can
> the bug's severity be escalated a notch to Major?

This would mean "Major" has any meaning... Would you create a patch if the severity is major?

Comment 11 Holger Freyther 2008-12-24 04:30:42 PST
Comment on attachment 26232 [details]
Proposed patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 39457)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,13 @@
> +2008-12-23  Hiroyuki Ikezoe  <poincare@ikezoe.net>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=18310
> +
> +        * platform/graphics/gtk/SimpleFontDataPango.cpp:
> +        (WebCore::SimpleFontData::determinePitch): Do not invoke
> +        isFixedPitch() the font is a custom font.

                         ^
                       when to be inserted?



>  2008-12-23  Darin Adler  <darin@apple.com>
>  
>          Reviewed by John Sullivan.
> Index: WebCore/platform/graphics/gtk/SimpleFontDataPango.cpp
> ===================================================================
> --- WebCore/platform/graphics/gtk/SimpleFontDataPango.cpp	(revision 39456)
> +++ WebCore/platform/graphics/gtk/SimpleFontDataPango.cpp	(working copy)
> @@ -116,7 +116,10 @@ bool SimpleFontData::containsCharacters(
>  
>  void SimpleFontData::determinePitch()
>  {
> -    m_treatAsFixedPitch = m_font.isFixedPitch();
> +    if (isCustomFont())
> +        m_treatAsFixedPitch = false;
> +    else
> +        m_treatAsFixedPitch = m_font.isFixedPitch();


I prefer the flow of the Windows code (win/SimpleFontDataWin.cpp) a lot more.

    // unlikely event
    if (isCustomFont()) {
        m_treatAsFixedPitch = false;
        return;
    }

    // business as usual...
    m_treatAdFixedPitch = m_font.isFixdPitch();


with the above it looks like both case are equal... which as of 2008 they are not. Would you mind to change the code?
Comment 12 Hiroyuki Ikezoe 2008-12-24 20:44:17 PST
Created attachment 26244 [details]
Revised patch

Addressed Holger's comment.
Comment 13 Holger Freyther 2008-12-25 12:32:01 PST
Comment on attachment 26244 [details]
Revised patch

>
> +    // unlikely event
> +    // business as usual...

oops... these were meant to explain you the intended flow... Please remove these comments when landing.
Comment 14 Holger Freyther 2008-12-31 08:18:08 PST
Landed in r39527.