Bug 1395061 - patch 4 - Refactor checks in the gfxPlatformFontList::GetFontList loop to use a single virtual method call instead of three separate calls. r=jrmuizel
authorJonathan Kew <jkew@mozilla.com>
Mon, 11 Sep 2017 19:24:01 +0100
changeset 429603 3ef47d9eabf7316d890319587a7fb795c89bff92
parent 429602 395a64d67308a9451fd7eadaa6cca0f58ae82470
child 429604 034c6b09eb50bbcf8c5256750be7f6c56f76b646
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1395061
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1395061 - patch 4 - Refactor checks in the gfxPlatformFontList::GetFontList loop to use a single virtual method call instead of three separate calls. r=jrmuizel
gfx/thebes/gfxDWriteFontList.h
gfx/thebes/gfxFcPlatformFontList.h
gfx/thebes/gfxFontEntry.h
gfx/thebes/gfxGDIFontList.h
gfx/thebes/gfxPlatformFontList.cpp
--- a/gfx/thebes/gfxDWriteFontList.h
+++ b/gfx/thebes/gfxDWriteFontList.h
@@ -50,24 +50,30 @@ public:
     void LocalizedName(nsAString& aLocalizedName) final;
 
     void ReadFaceNames(gfxPlatformFontList *aPlatformFontList,
                        bool aNeedFullnamePostscriptNames,
                        FontInfoData *aFontInfoData = nullptr) final;
 
     void SetForceGDIClassic(bool aForce) { mForceGDIClassic = aForce; }
 
-    bool IsSymbolFontFamily() const final;
-
     void AddSizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf,
                                 FontListSizes* aSizes) const final;
     void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
                                 FontListSizes* aSizes) const final;
 
+    bool FilterForFontList(nsIAtom* aLangGroup,
+                           const nsACString& aGeneric) const final {
+        return !IsSymbolFontFamily();
+    }
+
 protected:
+    // helper for FilterForFontList
+    bool IsSymbolFontFamily() const;
+
     /** This font family's directwrite fontfamily object */
     RefPtr<IDWriteFontFamily> mDWFamily;
     bool mForceGDIClassic;
 };
 
 /**
  * \brief Class representing DirectWrite FontEntry (a unique font style/family)
  */
--- a/gfx/thebes/gfxFcPlatformFontList.h
+++ b/gfx/thebes/gfxFcPlatformFontList.h
@@ -194,21 +194,27 @@ public:
     }
 
     void
     FindAllFontsForStyle(const gfxFontStyle& aFontStyle,
                          nsTArray<gfxFontEntry*>& aFontEntryList,
                          bool& aNeedsSyntheticBold,
                          bool aIgnoreSizeTolerance) override;
 
-    bool SupportsLangGroup(nsIAtom *aLangGroup) const override;
+    bool FilterForFontList(nsIAtom* aLangGroup,
+                           const nsACString& aGeneric) const final {
+        return SupportsLangGroup(aLangGroup);
+    }
 
 protected:
     virtual ~gfxFontconfigFontFamily();
 
+    // helper for FilterForFontList
+    bool SupportsLangGroup(nsIAtom *aLangGroup) const;
+
     nsTArray<nsCountedRef<FcPattern> > mFontPatterns;
 
     bool      mContainsAppFonts;
     bool      mHasNonScalableFaces;
     bool      mForceScalable;
 };
 
 class gfxFontconfigFont : public gfxFT2FontBase {
--- a/gfx/thebes/gfxFontEntry.h
+++ b/gfx/thebes/gfxFontEntry.h
@@ -748,28 +748,27 @@ public:
     // Only used for debugging checks - does a linear search
     bool ContainsFace(gfxFontEntry* aFontEntry);
 #endif
 
     void SetSkipSpaceFeatureCheck(bool aSkipCheck) {
         mSkipDefaultFeatureSpaceCheck = aSkipCheck;
     }
 
-    virtual bool MatchesGenericFamily(const nsACString& aGeneric) const {
+    // Check whether this family is appropriate to include in the Preferences
+    // font list for the given langGroup and CSS generic, if the platform lets
+    // us determine this.
+    // Return true if the family should be included in the list, false to omit.
+    // Default implementation returns true for everything, so no filtering
+    // will occur; individual platforms may override.
+    virtual bool FilterForFontList(nsIAtom* aLangGroup,
+                                   const nsACString& aGeneric) const {
         return true;
     }
 
-    virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const {
-        return true;
-    }
-
-    virtual bool IsSymbolFontFamily() const {
-        return false;
-    }
-
 protected:
     // Protected destructor, to discourage deletion outside of Release():
     virtual ~gfxFontFamily();
 
     bool ReadOtherFamilyNamesForFace(gfxPlatformFontList *aPlatformFontList,
                                      hb_blob_t           *aNameTable,
                                      bool                 useFullName = false);
 
--- a/gfx/thebes/gfxGDIFontList.h
+++ b/gfx/thebes/gfxGDIFontList.h
@@ -215,20 +215,32 @@ public:
         gfxFontFamily(aName),
         mWindowsFamily(0),
         mWindowsPitch(0),
         mCharset()
     {}
 
     virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr);
 
-    uint8_t mWindowsFamily;
-    uint8_t mWindowsPitch;
+    bool FilterForFontList(nsIAtom* aLangGroup,
+                           const nsACString& aGeneric) const final {
+        return !IsSymbolFontFamily() &&
+               SupportsLangGroup(aLangGroup) &&
+               MatchesGenericFamily(aGeneric);
+    }
 
-    virtual bool MatchesGenericFamily(const nsACString& aGeneric) const {
+protected:
+    friend class gfxGDIFontList;
+
+    // helpers for FilterForFontList
+    bool IsSymbolFontFamily() const {
+        return mCharset.test(SYMBOL_CHARSET);
+    }
+
+    bool MatchesGenericFamily(const nsACString& aGeneric) const {
         if (aGeneric.IsEmpty()) {
             return true;
         }
 
         // Japanese 'Mincho' fonts do not belong to FF_MODERN even if
         // they are fixed pitch because they have variable stroke width.
         if (mWindowsFamily == FF_ROMAN && mWindowsPitch & FIXED_PITCH) {
             return aGeneric.EqualsLiteral("monospace");
@@ -254,19 +266,17 @@ public:
             return aGeneric.EqualsLiteral("cursive");
         case FF_DECORATIVE:
             return aGeneric.EqualsLiteral("fantasy");
         }
 
         return false;
     }
 
-    gfxSparseBitSet mCharset;
-
-    virtual bool SupportsLangGroup(nsIAtom* aLangGroup) const override {
+    bool SupportsLangGroup(nsIAtom* aLangGroup) const {
         if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) {
             return true;
         }
 
         int16_t bit = -1;
 
         /* map our langgroup names in to Windows charset bits */
         if (aLangGroup == nsGkAtoms::x_western) {
@@ -293,19 +303,20 @@ public:
 
         if (bit != -1) {
             return mCharset.test(bit);
         }
 
         return false;
     }
 
-    bool IsSymbolFontFamily() const final {
-        return mCharset.test(SYMBOL_CHARSET);
-    }
+    uint8_t mWindowsFamily;
+    uint8_t mWindowsPitch;
+
+    gfxSparseBitSet mCharset;
 
 private:
     static int CALLBACK FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe,
                                             const NEWTEXTMETRICEXW *nmetrics,
                                             DWORD fontType, LPARAM data);
 };
 
 class gfxGDIFontList : public gfxPlatformFontList {
--- a/gfx/thebes/gfxPlatformFontList.cpp
+++ b/gfx/thebes/gfxPlatformFontList.cpp
@@ -478,19 +478,17 @@ gfxPlatformFontList::UpdateFontList()
 
 void
 gfxPlatformFontList::GetFontList(nsIAtom *aLangGroup,
                                  const nsACString& aGenericFamily,
                                  nsTArray<nsString>& aListOfFonts)
 {
     for (auto iter = mFontFamilies.Iter(); !iter.Done(); iter.Next()) {
         RefPtr<gfxFontFamily>& family = iter.Data();
-        if (!family->IsSymbolFontFamily() &&
-            family->SupportsLangGroup(aLangGroup) &&
-            family->MatchesGenericFamily(aGenericFamily)) {
+        if (family->FilterForFontList(aLangGroup, aGenericFamily)) {
             nsAutoString localizedFamilyName;
             family->LocalizedName(localizedFamilyName);
             aListOfFonts.AppendElement(localizedFamilyName);
         }
     }
 
     aListOfFonts.Sort();
     aListOfFonts.Compact();