[Webkit-unassigned] [Bug 38701] [chromium] don't call fontconfig twice in complex text path

[bugzilla-daemon at webkit.org]

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





--- Comment #27 from ningxin hu <ningxin.hu at gmail.com>  2010-08-05 01:19:24 PST ---
(In reply to comment #26)
Thanks for your comments. 

> (From update of attachment 63553 [details])
> Hi, thank you for your patch! I'll try to take a look:
> 
> > --- a/WebCore/platform/graphics/FontCache.h
> > +++ b/WebCore/platform/graphics/FontCache.h
> > @@ -102,6 +102,7 @@ private:
> >      // These methods are implemented by each platform.
> >      SimpleFontData* getSimilarFontPlatformData(const Font&);
> >      FontPlatformData* createFontPlatformData(const FontDescription&, const AtomicString& family);
> > +    void invalidatePlatform();
> Who call this? If chromium side, it's OK, if it will be called inside webkit, such code might be bette to stay in same patch.

In my early thoughts, it is called by FontCache::invalidate. However, FontCache::invalidate is called by MemoryPurger in chromium via task manager. So it is not a big deal. And it changes WebKit API, all platforms are impacted. So I prefer to remove it. In my page_cycler and other tests, the memory usage doesn't increase comparing to HEAD.  Render shutdown will release the typeface cache. 

> 
> > +typedef HashMap<String, SkTypeface*> TypefaceCache;
> > +
> > +static TypefaceCache *gTypefaceChache = 0;
> > +
> Because FontCache instance itself global, 
> It can be a class member field instead of static variable.
> Dynamically allocated static variable typically make valgrind unhappy, 
> we should avoid them if possible.

I just avoid to change WebKit API (FontCache.h). So by referring to FontCacheChromiumWin.cpp::fontCmapCache, I modify the patch in that way.

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