bug 874053 - ReadCMAP() must not leave the font entry's mCharacterMap pointer NULL, even if cmap table can't be read. r=roc
authorJonathan Kew <jkew@mozilla.com>
Tue, 21 May 2013 12:25:47 +0800
changeset 139604 f7beb3f7dceb06c3b176fca84dc947f613c0b7c3
parent 139603 3322189ff1db90ca807fa38ac636edf68af59c08
child 139605 762982dc7b78af09f4289b5e4eb25f85b96e9c48
push id3911
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 20:17:26 +0000
treeherdermozilla-aurora@7e26ca8db92b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs874053
milestone24.0a1
bug 874053 - ReadCMAP() must not leave the font entry's mCharacterMap pointer NULL, even if cmap table can't be read. r=roc
gfx/thebes/gfxDWriteFontList.cpp
gfx/thebes/gfxFont.h
gfx/thebes/gfxMacPlatformFontList.mm
gfx/thebes/gfxPangoFonts.cpp
--- a/gfx/thebes/gfxDWriteFontList.cpp
+++ b/gfx/thebes/gfxDWriteFontList.cpp
@@ -405,16 +405,18 @@ gfxDWriteFontEntry::ReadCMAP()
         bool unicodeFont = false, symbolFont = false; // currently ignored
         uint32_t cmapLen;
         const uint8_t* cmapData =
             reinterpret_cast<const uint8_t*>(hb_blob_get_data(cmapTable,
                                                               &cmapLen));
         rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen,
                                     *charmap, mUVSOffset,
                                     unicodeFont, symbolFont);
+    } else {
+        rv = NS_ERROR_NOT_AVAILABLE;
     }
 
     mHasCmapTable = NS_SUCCEEDED(rv);
     if (mHasCmapTable) {
         gfxPlatformFontList *pfl = gfxPlatformFontList::PlatformFontList();
         mCharacterMap = pfl->FindCharMap(charmap);
     } else {
         // if error occurred, initialize to null cmap
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -305,16 +305,23 @@ public:
         }
         return TestCharacterMap(ch);
     }
 
     virtual bool SkipDuringSystemFallback() { return false; }
     virtual bool TestCharacterMap(uint32_t aCh);
     nsresult InitializeUVSMap();
     uint16_t GetUVSGlyph(uint32_t aCh, uint32_t aVS);
+
+    // All concrete gfxFontEntry subclasses (except gfxProxyFontEntry) need
+    // to override this, otherwise the font will never be used as it will
+    // be considered to support no characters.
+    // ReadCMAP() must *always* set the mCharacterMap pointer to a valid
+    // gfxCharacterMap, even if empty, as other code assumes this pointer
+    // can be safely dereferenced.
     virtual nsresult ReadCMAP();
 
     bool TryGetSVGData();
     bool HasSVGGlyph(uint32_t aGlyphId);
     bool GetSVGGlyphExtents(gfxContext *aContext, uint32_t aGlyphId,
                             gfxRect *aResult);
     bool RenderSVGGlyph(gfxContext *aContext, uint32_t aGlyphId, int aDrawMode,
                         gfxTextObjectPaint *aObjectPaint);
--- a/gfx/thebes/gfxMacPlatformFontList.mm
+++ b/gfx/thebes/gfxMacPlatformFontList.mm
@@ -227,28 +227,30 @@ MacOSFontEntry::ReadCMAP()
         return NS_OK;
     }
 
     nsRefPtr<gfxCharacterMap> charmap = new gfxCharacterMap();
 
     uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p');
 
     AutoTable cmapTable(this, kCMAP);
-    if (!cmapTable) {
-        return NS_OK; // leave it empty
+    nsresult rv;
+
+    if (cmapTable) {
+        bool unicodeFont = false, symbolFont = false; // currently ignored
+
+        uint32_t cmapLen;
+        const char* cmapData = hb_blob_get_data(cmapTable, &cmapLen);
+        rv = gfxFontUtils::ReadCMAP((const uint8_t*)cmapData, cmapLen,
+                                    *charmap, mUVSOffset,
+                                    unicodeFont, symbolFont);
+    } else {
+        rv = NS_ERROR_NOT_AVAILABLE;
     }
 
-    bool unicodeFont = false, symbolFont = false; // currently ignored
-
-    uint32_t cmapLen;
-    const char* cmapData = hb_blob_get_data(cmapTable, &cmapLen);
-    nsresult rv = gfxFontUtils::ReadCMAP((const uint8_t*)cmapData, cmapLen,
-                                         *charmap, mUVSOffset,
-                                         unicodeFont, symbolFont);
-  
     if (NS_SUCCEEDED(rv) && !HasGraphiteTables()) {
         // We assume a Graphite font knows what it's doing,
         // and provides whatever shaping is needed for the
         // characters it supports, so only check/clear the
         // complex-script ranges for non-Graphite fonts
 
         // for layout support, check for the presence of mort/morx and/or
         // opentype layout tables
--- a/gfx/thebes/gfxPangoFonts.cpp
+++ b/gfx/thebes/gfxPangoFonts.cpp
@@ -405,24 +405,24 @@ gfxSystemFcFontEntry::MaybeReleaseFTFace
     }
     mFTFaceInitialized = false;
 }
 
 void
 gfxSystemFcFontEntry::ForgetHBFace()
 {
     gfxFontEntry::ForgetHBFace();
-    MaybeReleaseFTFace();        
+    MaybeReleaseFTFace();
 }
 
 void
 gfxSystemFcFontEntry::ReleaseGrFace(gr_face* aFace)
 {
     gfxFontEntry::ReleaseGrFace(aFace);
-    MaybeReleaseFTFace();        
+    MaybeReleaseFTFace();
 }
 
 // A namespace for @font-face family names in FcPatterns so that fontconfig
 // aliases do not pick up families from @font-face rules and so that
 // fontconfig rules can distinguish between web fonts and platform fonts.
 // http://lists.freedesktop.org/archives/fontconfig/2008-November/003037.html
 #define FONT_FACE_FAMILY_PREFIX "@font-face:"