Bug 130572

Created attachment 227408 [details] Patch

> > Source/WebCore/platform/graphics/FontPlatformData.cpp:30 > > +#if OS(DARWIN) > > OS(DARWIN) might be correct, but my first guess would have been to use > PLATFORM(COCOA) If I read Source/WTF/wtf/Platform.h correctly, OS(DARWIN) implies PLATFORM(COCOA). OS(DARWIN) is what is used in FontPlaformData.h.

(In reply to comment #2) > > > Source/WebCore/platform/graphics/FontPlatformData.cpp:30 > > > +#if OS(DARWIN) > > > > OS(DARWIN) might be correct, but my first guess would have been to use > > PLATFORM(COCOA) > > If I read Source/WTF/wtf/Platform.h correctly, OS(DARWIN) implies PLATFORM(COCOA). OS(DARWIN) is what is used in FontPlaformData.h. Ok

Created attachment 227629 [details] Patch This patch addresses bug 130321 comment 13 (except the #if OS(DARWIN)).

Comment on attachment 227629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227629&action=review > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.h:44 > + bool hasMathData() const { return !!m_mathBuffer; } should this just be return m_mathBuffer.get(), instead of !!

Committed r166169: <http://trac.webkit.org/changeset/166169>

Re-opened since this is blocked by bug 130682

So it seems that some platform/font can not generate a hash(). I'll need to add some special handling to ignore these cases.

Created attachment 227733 [details] Patch

Created attachment 227734 [details] Difference from the previous commit So I think openTypeTable should only be defined for "OS(DARWIN) && USE(CG)", as cgFont() is not available otherwise. I've modified the code to leave m_mathData uninitialized when m_font is undefined on Mac/Win, since otherwise the hash() asserts or return 0.

Comment on attachment 227734 [details] Difference from the previous commit View in context: https://bugs.webkit.org/attachment.cgi?id=227734&action=review > Source/WebCore/platform/graphics/SimpleFontData.cpp:66 > + bool tryMathData = true; should this be initialized with false so that it's explicit who is testing for math?

Created attachment 227931 [details] Patch I've refined tryMathData so that only platforms that will have ENABLE(OPENTYPE_MATH) may set it to true. Also for freetype, there is an ASSERT(hash()) when creating the verticalData. I'm not sure when the hash() can be zero on freetype, but I leave tryMathData to false in that case too.

Committed r166364: <http://trac.webkit.org/changeset/166364>

This caused a new assertion http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r166371%20(4179)/svg/text/text-fonts-02-t-crash-log.txt

Re-opened since this is blocked by bug 130872

Created attachment 228073 [details] Patch So the new assertion was ASSERT(!lookupForWriting(Extractor::extract(entry)).second) which is, IIUC, that the entry already exists in the hash table. I now realize that what FontCache does for vertical data is very weird: it uses the hash() of the FontPlatformData as a key for the hash table (and so of course this key is itself hashed again!). So two fonts with different Open Type tables are likely to get the same key... Open Type tables are encoded in the font file, so they should really be per FontPlatformData and I'm even assuming per SimpleFontData (unless two SimpleFontData's can share the same FontPlatformData). So I'm not really sure it is relevant to use a cache for the Open Type tables. Also, there are not many fonts with Open Type MATH tables and the math data are likely to only be used in the MathML code. So trying to initialize the data only the first time the mathData() is called is probably a better strategy. In this patch, I've removed the cache and moved the initialization to mathData().

Comment on attachment 228073 [details] Patch Please watch the bots after landing

Committed r166633: <http://trac.webkit.org/changeset/166633>