[webkit-reviews] review granted: [Bug 25770] [chromium] Crash in FontFallbackList::determinePitch(const Font* font) : [Attachment 74209] A copy-and-paste fix

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Hironori Bono
<hbono at chromium.org>'s request for review:
Bug 25770: [chromium] Crash in FontFallbackList::determinePitch(const Font*
font)
https://bugs.webkit.org/show_bug.cgi?id=25770

Attachment 74209: A copy-and-paste fix
https://bugs.webkit.org/attachment.cgi?id=74209&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74209&action=review

R=me, but it sure would be nice to find a way to share code with the
PLATFORM(WIN) port.

> WebCore/platform/graphics/chromium/FontCacheChromiumWin.cpp:426
> +static SimpleFontData* fontDataFromDescriptionAndLogFont(FontCache*
fontCache, const FontDescription& fontDescription, const LOGFONT& font,
wchar_t* outFontFamilyName)

nit: it is normally nice to put helper functions at the top of the file, so as
not
to break up the flow of class method implementations.  this doesn't cause a
readability
problem in this case, but often once a static method is added like this, it
becomes
an invitation for more people to add static functions like this, and pretty
soon the
organization of a .cpp file is lost.