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

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Koji Ishii <kojiishi at gmail.com>  2012-07-16 04:02:53 PST ---
(In reply to comment #14)
> > Source/WebCore/platform/graphics/FontCache.cpp:197
> > +        return getCachedFontPlatformData(fontDescription, AtomicString(familyName.impl()->substring(i)), checkingAlternateName);
> 
> I would have renamed familyName to something else like passedFamilyName, and did:
> const AtomicString& familyName = stripLeadingVerticalFlowFlag(familyName);
> where stripLeadingVerticalFlowFlag is an inline function that strips the leading '@' in Windows and no-op on other platforms.

I tried inline function, but returning const AtomicString& of locally created object crashes, so I followed your advice but the code lives within the function.


> > Source/WebCore/platform/graphics/FontCache.cpp:237
> > +typedef HashMap<AtomicString, OpenTypeVerticalData*> FontVerticalDataCache;
> > +
> > +static FontVerticalDataCache* gFontVerticalDataCache = 0;
> 
> We typically use DEFINE_STATIC_LOCAL and declare these variables inside a function.

Thanks for the advise, changed to follow the pattern.


> > Source/WebCore/platform/graphics/FontCache.cpp:251
> > +    OpenTypeVerticalData* verticalData = new OpenTypeVerticalData(platformData);
> 
> We should probably use OwnPtr in the future. Although we might want to stick with
> new/delete for now since the rest of the file uses that pattern.

Yeah, that was what I thought. Glad to know you thought the same way.

The updated patch is uploaded and passed EWS, appreciate your support to go further.

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