[Webkit-unassigned] [Bug 81332] Cache support for OpenTypeVerticalData

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Tony Chang <tony at chromium.org>  2012-07-19 14:08:11 PST ---
(From update of attachment 152501)
View in context: https://bugs.webkit.org/attachment.cgi?id=152501&action=review

> Source/WebCore/platform/graphics/FontCache.cpp:40
> +#if ENABLE(OPENTYPE_VERTICAL)
> +#include "OpenTypeVerticalData.h"
> +#endif

In WebKit, it's more common to put the #if ENABLE check in the .h file and unconditionally include the .h file everywhere.

> Source/WebCore/platform/graphics/FontCache.cpp:234
> +typedef HashMap<AtomicString, OpenTypeVerticalData*> FontVerticalDataCache;

Can this be HashMap<AtomicString, OwnPtr<OpenTypeVerticalData> > ?

> Source/WebCore/platform/graphics/FontCache.cpp:446
> +        for (size_t i = 0, count = keysToRemove.size(); i < count; ++i)
> +            delete fontVerticalDataCache.take(keysToRemove[i]);

I see. We remove entries from fontVerticalDataCache when they are removed from gFontDataCache.  This seems really inefficient. Can we mark the keys for removal or just delete them directly when we're removing entries from gFontDataCache?

> Source/WebCore/platform/graphics/FontCache.h:54
> +#if ENABLE(OPENTYPE_VERTICAL)
> +class OpenTypeVerticalData;
> +#endif

You don't need the #if here. It should be safe to forward declare something that is unused.

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