Bug 1395061 - patch 1 - Refactor gfxFontEntry::SupportsLangGroup and MatchesGenericFamily into gfxFontFamily. r=jfkthame

gfx/thebes/gfxFcPlatformFontList.cpp

@@ -325,39 +325,6 @@ gfxFontconfigFontEntry::~gfxFontconfigFontEntry()
 {
 }
 
-static bool
-PatternHasLang(const FcPattern *aPattern, const FcChar8 *aLang)
-{
-    FcLangSet *langset;
-
-    if (FcPatternGetLangSet(aPattern, FC_LANG, 0, &langset) != FcResultMatch) {
-        return false;
-    }
-
-    if (FcLangSetHasLang(langset, aLang) != FcLangDifferentLang) {
-        return true;
-    }
-    return false;
-}
-
-bool
-gfxFontconfigFontEntry::SupportsLangGroup(nsIAtom *aLangGroup) const
-{
-    if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) {
-        return true;
-    }
-
-    nsAutoCString fcLang;
-    gfxFcPlatformFontList* pfl = gfxFcPlatformFontList::PlatformFontList();
-    pfl->GetSampleLangForGroup(aLangGroup, fcLang);
-    if (fcLang.IsEmpty()) {
-        return true;
-    }
-
-    // is lang included in the underlying pattern?
-    return PatternHasLang(mFontPattern, ToFcChar8Ptr(fcLang.get()));
-}
-
 nsresult
 gfxFontconfigFontEntry::ReadCMAP(FontInfoData *aFontInfoData)
 {
@@ -1189,6 +1156,54 @@ gfxFontconfigFontFamily::FindAllFontsForStyle(const gfxFontStyle& aFontStyle,
     }
 }
 
+static bool
+PatternHasLang(const FcPattern *aPattern, const FcChar8 *aLang)
+{
+    FcLangSet *langset;
+
+    if (FcPatternGetLangSet(aPattern, FC_LANG, 0, &langset) != FcResultMatch) {
+        return false;
+    }
+
+    if (FcLangSetHasLang(langset, aLang) != FcLangDifferentLang) {
+        return true;
+    }
+    return false;
+}
+
+bool
+gfxFontconfigFontFamily::SupportsLangGroup(nsIAtom *aLangGroup) const
+{
+    if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) {
+        return true;
+    }
+
+    nsAutoCString fcLang;
+    gfxFcPlatformFontList* pfl = gfxFcPlatformFontList::PlatformFontList();
+    pfl->GetSampleLangForGroup(aLangGroup, fcLang);
+    if (fcLang.IsEmpty()) {
+        return true;
+    }
+
+    // Before FindStyleVariations has been called, mFontPatterns will contain
+    // the font patterns.  Afterward, it'll be empty, but mAvailableFonts
+    // will contain the font entries, each of which holds a reference to its
+    // pattern.  We only check the first pattern in each list, because support
+    // for langs is considered to be consistent across all faces in a family.
+    FcPattern* fontPattern;
+    if (mFontPatterns.Length()) {
+        fontPattern = mFontPatterns[0];
+    } else if (mAvailableFonts.Length()) {
+        fontPattern = static_cast<gfxFontconfigFontEntry*>
+                      (mAvailableFonts[0].get())->GetPattern();
+    } else {
+        return true;
+    }
+
+    // is lang included in the underlying pattern?
+    return PatternHasLang(fontPattern, ToFcChar8Ptr(fcLang.get()));
+}
+
 /* virtual */
 gfxFontconfigFontFamily::~gfxFontconfigFontFamily()
  {

gfx/thebes/gfxFcPlatformFontList.h

@@ -99,8 +99,6 @@ public:
 
     FcPattern* GetPattern() { return mFontPattern; }
 
-    bool SupportsLangGroup(nsIAtom *aLangGroup) const override;
-
     nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr) override;
     bool TestCharacterMap(uint32_t aCh) override;
 
@@ -201,6 +199,8 @@ public:
                          bool& aNeedsSyntheticBold,
                          bool aIgnoreSizeTolerance) override;
 
+    bool SupportsLangGroup(nsIAtom *aLangGroup) const override;
+
 protected:
     virtual ~gfxFontconfigFontFamily();
 

gfx/thebes/gfxFontEntry.h

@@ -207,13 +207,6 @@ public:
                                 nsTArray<uint16_t>& layerGlyphs,
                                 nsTArray<mozilla::gfx::Color>& layerColors);
 
-    virtual bool MatchesGenericFamily(const nsACString& aGeneric) const {
-        return true;
-    }
-    virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const {
-        return true;
-    }
-
     // Access to raw font table data (needed for Harfbuzz):
     // returns a pointer to data owned by the fontEntry or the OS,
     // which will remain valid until the blob is destroyed.
@@ -764,6 +757,14 @@ public:
         mSkipDefaultFeatureSpaceCheck = aSkipCheck;
     }
 
+    virtual bool MatchesGenericFamily(const nsACString& aGeneric) const {
+        return true;
+    }
+
+    virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const {
+        return true;
+    }
+
 protected:
     // Protected destructor, to discourage deletion outside of Release():
     virtual ~gfxFontFamily();

gfx/thebes/gfxGDIFontList.cpp

@@ -120,11 +120,10 @@ GDIFontEntry::GDIFontEntry(const nsAString& aFaceName,
                            gfxUserFontData *aUserFontData,
                            bool aFamilyHasItalicFace)
     : gfxFontEntry(aFaceName),
-      mWindowsFamily(0), mWindowsPitch(0),
       mFontType(aFontType),
       mForceGDI(false),
       mFamilyHasItalicFace(aFamilyHasItalicFace),
-      mCharset(), mUnicodeRanges()
+      mUnicodeRanges()
 {
     mUserFontData.reset(aUserFontData);
     mStyle = aStyle;
@@ -486,7 +485,9 @@ GDIFontFamily::FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe,
         if (fe->mWeight == logFont.lfWeight &&
             fe->IsItalic() == (logFont.lfItalic == 0xFF)) {
             // update the charset bit here since this could be different
-            fe->mCharset.set(metrics.tmCharSet);
+            // XXX Can we still do this now that we store mCharset
+            // on the font family rather than the font entry?
+            ff->mCharset.set(metrics.tmCharSet);
             return 1; 
         }
     }
@@ -505,12 +506,6 @@ GDIFontFamily::FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe,
 
     ff->AddFontEntry(fe);
 
-    // mark the charset bit
-    fe->mCharset.set(metrics.tmCharSet);
-
-    fe->mWindowsFamily = logFont.lfPitchAndFamily & 0xF0;
-    fe->mWindowsPitch = logFont.lfPitchAndFamily & 0x0F;
-
     if (nmetrics->ntmFontSig.fsUsb[0] != 0x00000000 &&
         nmetrics->ntmFontSig.fsUsb[1] != 0x00000000 &&
         nmetrics->ntmFontSig.fsUsb[2] != 0x00000000 &&
@@ -713,6 +708,7 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe,
                                       DWORD fontType,
                                       LPARAM lParam)
 {
+    const NEWTEXTMETRICW& metrics = lpntme->ntmTm;
     const LOGFONTW& lf = lpelfe->elfLogFont;
 
     if (lf.lfFaceName[0] == '@') {
@@ -726,7 +722,7 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe,
 
     if (!fontList->mFontFamilies.GetWeak(name)) {
         nsDependentString faceName(lf.lfFaceName);
-        RefPtr<gfxFontFamily> family = new GDIFontFamily(faceName);
+        RefPtr<GDIFontFamily> family = new GDIFontFamily(faceName);
         fontList->mFontFamilies.Put(name, family);
 
         // if locale is such that CJK font names are the default coming from
@@ -739,6 +735,12 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe,
 
         if (fontList->mBadUnderlineFamilyNames.Contains(name))
             family->SetBadUnderlineFamily();
+
+        family->mWindowsFamily = lf.lfPitchAndFamily & 0xF0;
+        family->mWindowsPitch = lf.lfPitchAndFamily & 0x0F;
+
+        // mark the charset bit
+        family->mCharset.set(metrics.tmCharSet);
     }
 
     return 1;

gfx/thebes/gfxGDIFontList.h

@@ -147,6 +147,83 @@ public:
                 mFontType == GFX_FONT_TYPE_TT_OPENTYPE);
     }
 
+    virtual bool SupportsRange(uint8_t range) {
+        return mUnicodeRanges.test(range);
+    }
+
+    virtual bool SkipDuringSystemFallback() {
+        return !HasCmapTable(); // explicitly skip non-SFNT fonts
+    }
+
+    virtual bool TestCharacterMap(uint32_t aCh);
+
+    virtual void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
+                                        FontListSizes* aSizes) const;
+
+    gfxFontEntry* Clone() const override;
+
+    // create a font entry for a font with a given name
+    static GDIFontEntry* CreateFontEntry(const nsAString& aName,
+                                         gfxWindowsFontType aFontType,
+                                         uint8_t aStyle,
+                                         uint16_t aWeight, int16_t aStretch,
+                                         gfxUserFontData* aUserFontData,
+                                         bool aFamilyHasItalicFace);
+
+    // create a font entry for a font referenced by its fullname
+    static GDIFontEntry* LoadLocalFont(const nsAString& aFontName,
+                                       uint16_t aWeight,
+                                       int16_t aStretch,
+                                       uint8_t aStyle);
+
+    gfxWindowsFontType mFontType;
+    bool mForceGDI    : 1;
+
+    // For src:local user-fonts, we keep track of whether the platform family
+    // contains an italic face, because in this case we can't safely ask GDI
+    // to create synthetic italics (oblique) via the LOGFONT.
+    // (For other types of font, this is just set to false.)
+    bool mFamilyHasItalicFace : 1;
+
+    gfxSparseBitSet mUnicodeRanges;
+
+protected:
+    friend class gfxGDIFont;
+
+    GDIFontEntry(const nsAString& aFaceName, gfxWindowsFontType aFontType,
+                 uint8_t aStyle, uint16_t aWeight, int16_t aStretch,
+                 gfxUserFontData *aUserFontData, bool aFamilyHasItalicFace);
+
+    void InitLogFont(const nsAString& aName, gfxWindowsFontType aFontType);
+
+    virtual gfxFont *CreateFontInstance(const gfxFontStyle *aFontStyle, bool aNeedsBold);
+
+    virtual nsresult CopyFontTable(uint32_t aTableTag,
+                                   nsTArray<uint8_t>& aBuffer) override;
+
+    already_AddRefed<mozilla::gfx::UnscaledFontGDI> LookupUnscaledFont(HFONT aFont);
+
+    LOGFONTW mLogFont;
+
+    mozilla::WeakPtr<mozilla::gfx::UnscaledFont> mUnscaledFont;
+};
+
+// a single font family, referencing one or more faces
+class GDIFontFamily : public gfxFontFamily
+{
+public:
+    explicit GDIFontFamily(const nsAString& aName) :
+        gfxFontFamily(aName),
+        mWindowsFamily(0),
+        mWindowsPitch(0),
+        mCharset()
+    {}
+
+    virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr);
+
+    uint8_t mWindowsFamily;
+    uint8_t mWindowsPitch;
+
     virtual bool MatchesGenericFamily(const nsACString& aGeneric) const {
         if (aGeneric.IsEmpty()) {
             return true;
@@ -183,6 +260,8 @@ public:
         return false;
     }
 
+    gfxSparseBitSet mCharset;
+
     virtual bool SupportsLangGroup(nsIAtom* aLangGroup) const override {
         if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) {
             return true;
@@ -220,80 +299,6 @@ public:
         return false;
     }
 
-    virtual bool SupportsRange(uint8_t range) {
-        return mUnicodeRanges.test(range);
-    }
-
-    virtual bool SkipDuringSystemFallback() { 
-        return !HasCmapTable(); // explicitly skip non-SFNT fonts
-    }
-
-    virtual bool TestCharacterMap(uint32_t aCh);
-
-    virtual void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
-                                        FontListSizes* aSizes) const;
-
-    gfxFontEntry* Clone() const override;
-
-    // create a font entry for a font with a given name
-    static GDIFontEntry* CreateFontEntry(const nsAString& aName,
-                                         gfxWindowsFontType aFontType,
-                                         uint8_t aStyle,
-                                         uint16_t aWeight, int16_t aStretch,
-                                         gfxUserFontData* aUserFontData,
-                                         bool aFamilyHasItalicFace);
-
-    // create a font entry for a font referenced by its fullname
-    static GDIFontEntry* LoadLocalFont(const nsAString& aFontName,
-                                       uint16_t aWeight,
-                                       int16_t aStretch,
-                                       uint8_t aStyle);
-
-    uint8_t mWindowsFamily;
-    uint8_t mWindowsPitch;
-
-    gfxWindowsFontType mFontType;
-    bool mForceGDI    : 1;
-
-    // For src:local user-fonts, we keep track of whether the platform family
-    // contains an italic face, because in this case we can't safely ask GDI
-    // to create synthetic italics (oblique) via the LOGFONT.
-    // (For other types of font, this is just set to false.)
-    bool mFamilyHasItalicFace : 1;
-
-    gfxSparseBitSet mCharset;
-    gfxSparseBitSet mUnicodeRanges;
-
-protected:
-    friend class gfxGDIFont;
-
-    GDIFontEntry(const nsAString& aFaceName, gfxWindowsFontType aFontType,
-                 uint8_t aStyle, uint16_t aWeight, int16_t aStretch,
-                 gfxUserFontData *aUserFontData, bool aFamilyHasItalicFace);
-
-    void InitLogFont(const nsAString& aName, gfxWindowsFontType aFontType);
-
-    virtual gfxFont *CreateFontInstance(const gfxFontStyle *aFontStyle, bool aNeedsBold);
-
-    virtual nsresult CopyFontTable(uint32_t aTableTag,
-                                   nsTArray<uint8_t>& aBuffer) override;
-
-    already_AddRefed<mozilla::gfx::UnscaledFontGDI> LookupUnscaledFont(HFONT aFont);
-
-    LOGFONTW mLogFont;
-
-    mozilla::WeakPtr<mozilla::gfx::UnscaledFont> mUnscaledFont;
-};
-
-// a single font family, referencing one or more faces
-class GDIFontFamily : public gfxFontFamily
-{
-public:
-    explicit GDIFontFamily(const nsAString& aName) :
-        gfxFontFamily(aName) {}
-
-    virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr);
-
 private:
     static int CALLBACK FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe,
                                             const NEWTEXTMETRICEXW *nmetrics,

gfx/thebes/gfxPlatformFontList.cpp

@@ -500,8 +500,8 @@ gfxPlatformFontList::GetFontList(nsIAtom *aLangGroup,
             continue;
         }
 
-        if (fontEntry->SupportsLangGroup(aLangGroup) &&
+        if (family->SupportsLangGroup(aLangGroup) &&
-            fontEntry->MatchesGenericFamily(aGenericFamily)) {
+            family->MatchesGenericFamily(aGenericFamily)) {
             nsAutoString localizedFamilyName;
             family->LocalizedName(localizedFamilyName);
             aListOfFonts.AppendElement(localizedFamilyName);