Bug 22861

Summary: Turn the FontCache into a singleton
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mario.bensi, mitz, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed fix
none
Same as the first version, with corrected ChangeLogs darin: review+

Description Julien Chaffraix 2008-12-14 18:52:22 PST
As discussed on the mailing list, this is part of the initial clean-up for the fontcache.

Patch forthcoming.
Comment 1 Julien Chaffraix 2008-12-14 18:58:34 PST
Created attachment 26020 [details]
Proposed fix
Comment 2 Sam Weinig 2008-12-15 14:00:19 PST
Why is it a good idea to turn the FontCache into a singleton?  What does this get us?
Comment 3 Julien Chaffraix 2008-12-16 02:09:18 PST
(In reply to comment #2)
> Why is it a good idea to turn the FontCache into a singleton?  What does this
> get us?
> 

First it makes FontCache coherent with the rest of WebCore: I have not found another class that only bears static methods inside WebCore. IMHO it is also particularly relevant to make this class close to the Cache.
Then it makes it cristal clear that we are using a singleton: static methods + static HashMaps / HashList inside the .cpp sounds like a hidden singleton.
Finally when you start hacking the FontCache as it is now, you have to find the different definitions mixed with the code in FontCache.cpp. Making it a real class is a first step into moving those definitons in their own files and move the static HashMaps / HashList in the header.

So to answer the benefits: right now, it adds a little overhead to the compiled code. But within a few changes, we should have a more readable code.
Comment 4 Julien Chaffraix 2008-12-16 13:17:08 PST
Created attachment 26065 [details]
Same as the first version, with corrected ChangeLogs

I had forgotten to update the ChangeLog in the WebKit directory, so attached is an updated version.
Comment 5 Darin Adler 2009-01-02 10:56:14 PST
Comment on attachment 26065 [details]
Same as the first version, with corrected ChangeLogs

I'm not entirely convinced this is an improvement, but it seems OK. r=me
Comment 6 Julien Chaffraix 2009-01-12 02:52:18 PST
Landed patch in r39823.