[Webkit-unassigned] [Bug 25770] [chromium] Crash in FontFallbackList::determinePitch(const Font* font)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25770





--- Comment #5 from Hironori Bono <hbono at chromium.org>  2010-11-30 22:03:53 PST ---
(In reply to comment #4)
> (From update of attachment 74209 [details])
> 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.

Thank you for your comment. Even though I realize this is a coding style of Chromium and I don't have any objections, I would like to write an excuse why I put this function there. In brief, I have put this function there just because I thought this is a style for this file. It seems this file arranges its functions (including static ones) in the same order as the ones in 'FontCacheWin.cpp'. To keep this order consistency, I thought I needed to put fontDataFromDescriptionAndLogFont() to the same place (i.e. between FontCache::getSimilarFontPlatformData() and FontCache::getLastResortFallbackFont()).
In fact, this file has some other static functions inserted between FontCache methods, such as charactersAreAllASCII(), lookupAltName(), etc. Do we need refactoring for this file so we can move all these static functions in this file at the beginning of this file?

Regards,

Hironori Bono

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.