[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 #31 from ningxin hu <ningxin.hu at gmail.com>  2010-08-09 18:44:23 PST ---
(In reply to comment #30)
> This cache looks like it will grow without ever shrinking.
> Won't it include a copy of every character ever rendered?
The memory usage of this typeface cache is similar to unicode_font_families_ cache in RendererWebKitClientImpl::SandboxSupport::getFontFamilyForCharacters which is currently used. They are destroyed with the render (usually switching to another site will destroy the render.)
In my page_cycler test, chromium with my patch doesn't increase memory usage comparing to HEAD. And I also tested different sites switch cases, the memory usage behavior is not different. 
For another instance, FontCacheChromiumWin also has a FontCmapCache which doesn't shrink. It is acceptable.

> It's also not correct to not include the font information in the cache (for example, this cache will return the wrong font when asked for the same characters in bold or not bold).
Yes. This is what I want to get feedbacks from you. To combine the style info to cache key is better. If you think OK, I will add it. 

> 
> More broadly, I think you need to spend some time understanding FontCache.  Maybe it has the appropriate setup to cache what you need.  (I don't understand it completely myself.)
I agree with you that I don't understand font cache well. But I just want to fix the bug. The complex font selection bug on Linux is annoying for non-English users. 
As my knowledge and test, I think your fix at r59483 works. But it is rolled out according to performance regression. I just tried to find the root cause and provide a fix. Besides that, I did the test to make sure my fix doesn't bring other regressions such as memory leaks.  
I did investigate other ways, such as passing the character info to createFontPlatformData, but it involves WebKit API change, other platforms are impacted. 
I submitted this patch for the three reasons:
1. fix the complex font selection problem (https://bugs.webkit.org/show_bug.cgi?id=37904), pass the test case for that. (the cache key need to update)
2. fix the performance loss of page_cycler
3. no more memory leaks comparing to current implementation. 

So could you share with me what your most concerns? And in your mind, what's the right way to fix it? Thanks in advance.

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