[webkit-reviews] review denied: [Bug 52422] [chromium] More crash in FontFallbackList::determinePitch(const Font* font) : [Attachment 78889] A blind and speculative fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 13 22:28:52 PST 2011


David Levin <levin at chromium.org> has denied Hironori Bono
<hbono at chromium.org>'s request for review:
Bug 52422: [chromium] More crash in FontFallbackList::determinePitch(const
Font* font)
https://bugs.webkit.org/show_bug.cgi?id=52422

Attachment 78889: A blind and speculative fix
https://bugs.webkit.org/attachment.cgi?id=78889&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78889&action=review

> Source/WebCore/ChangeLog:14
> +	   No new tests. (OOPS!)

oops. :)

This line should be removed.  Ideally there would be a test.

What about doing what you mentioned in the bug or something like that? Would
that cause a crash that is fixed by this change?

> Source/WebCore/platform/graphics/chromium/FontCacheChromiumWin.cpp:593
> +	   if (simpleFont = getCachedFontData(description, fallbackFonts[i])) {


Consider a fail fast approach.

if (!(simpleFont = getCachedFontData(description, fallbackFonts[i])))
    continue;


More information about the webkit-reviews mailing list