Bug 1630935 - Refactor FontFamilyName + FontFamilyList + nsMathMLChar. r=emilio
authorDelan Azabani <delan.azabani@automattic.com>
Thu, 04 Jun 2020 13:18:21 +0000
changeset 533894 4751eee95d774c322021a4b9c038dfcbe4029ddc
parent 533893 2635d8ec9079b2f1721549098e2831dba23e26b1
child 533895 3e654bf7a1e6d7a4fc4a3a0d4232cdc1f05f3cb2
push id37480
push userncsoregi@mozilla.com
push dateThu, 04 Jun 2020 22:00:12 +0000
treeherdermozilla-central@e33aea19d0c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1630935
milestone79.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 1630935 - Refactor FontFamilyName + FontFamilyList + nsMathMLChar. r=emilio This patch: * extracts family name matching logic from FontFamilyList::Contains into FontFamilyName::IsNamedFamily * simplifies the loop around StretchEnumContext::EnumCallback using a range-based loop with a break * inverts the meaning of StretchEnumContext::EnumCallback’s return value These changes were first reviewed in D73833, but we’re separating them to help us investigate some test regressions. Differential Revision: https://phabricator.services.mozilla.com/D77067
gfx/thebes/gfxFontFamilyList.h
layout/mathml/nsMathMLChar.cpp
--- a/gfx/thebes/gfxFontFamilyList.h
+++ b/gfx/thebes/gfxFontFamilyList.h
@@ -112,16 +112,24 @@ struct FontFamilyName final {
       genericType = StyleGenericFontFamily::Fantasy;
     } else {
       return FontFamilyName(aFamilyOrGenericName, Syntax::Identifiers);
     }
 
     return FontFamilyName(genericType);
   }
 
+  bool IsNamedFamily(const nsAString& aFamilyName) const {
+    if (!IsNamed()) {
+      return false;
+    }
+    nsDependentAtomString name{mName};
+    return name.Equals(aFamilyName, nsCaseInsensitiveStringComparator);
+  }
+
   RefPtr<nsAtom> mName;  // null if mGeneric != Default
   StyleFontFamilyNameSyntax mSyntax = StyleFontFamilyNameSyntax::Quoted;
   StyleGenericFontFamily mGeneric = StyleGenericFontFamily::None;
 };
 
 inline bool operator==(const FontFamilyName& a, const FontFamilyName& b) {
   return a.mName == b.mName && a.mSyntax == b.mSyntax &&
          a.mGeneric == b.mGeneric;
@@ -295,21 +303,17 @@ class FontFamilyList {
         aFamilyList.AppendLiteral("sans-serif");
       }
     }
   }
 
   // searches for a specific non-generic name, case-insensitive comparison
   bool Contains(const nsAString& aFamilyName) const {
     for (const FontFamilyName& name : mFontlist->mNames) {
-      if (!name.IsNamed()) {
-        continue;
-      }
-      nsDependentAtomString listname(name.mName);
-      if (listname.Equals(aFamilyName, nsCaseInsensitiveStringComparator)) {
+      if (name.IsNamedFamily(aFamilyName)) {
         return true;
       }
     }
     return false;
   }
 
   StyleGenericFontFamily GetDefaultFontType() const { return mDefaultFontType; }
   void SetDefaultFontType(StyleGenericFontFamily aType) {
--- a/layout/mathml/nsMathMLChar.cpp
+++ b/layout/mathml/nsMathMLChar.cpp
@@ -1271,17 +1271,18 @@ bool nsMathMLChar::StretchEnumContext::T
   for (int32_t i = 0; i < 4; i++) {
     mChar->mGlyphs[i] = std::move(textRun[i]);
     mChar->mBmData[i] = bmdata[i];
   }
 
   return IsSizeOK(computedSize, mTargetSize, mStretchHint);
 }
 
-// This is called for each family, whether it exists or not
+// Returns true iff stretching succeeded with the given family.
+// This is called for each family, whether it exists or not.
 bool nsMathMLChar::StretchEnumContext::EnumCallback(
     const FontFamilyName& aFamily, bool aGeneric, void* aData) {
   StretchEnumContext* context = static_cast<StretchEnumContext*>(aData);
 
   // for comparisons, force use of unquoted names
   FontFamilyName unquotedFamilyName(aFamily);
   if (unquotedFamilyName.mSyntax == StyleFontFamilyNameSyntax::Quoted) {
     unquotedFamilyName.mSyntax = StyleFontFamilyNameSyntax::Identifiers;
@@ -1292,17 +1293,17 @@ bool nsMathMLChar::StretchEnumContext::E
   ComputedStyle* sc = context->mChar->mComputedStyle;
   nsFont font = sc->StyleFont()->mFont;
   NormalizeDefaultFont(font, context->mFontSizeInflation);
   RefPtr<gfxFontGroup> fontGroup;
   FontFamilyList family(nsTArray<FontFamilyName>{unquotedFamilyName});
   if (!aGeneric &&
       !context->mChar->SetFontFamily(context->mPresContext, nullptr, kNullGlyph,
                                      family, font, &fontGroup))
-    return true;  // Could not set the family
+    return false;  // Could not set the family
 
   // Determine the glyph table to use for this font.
   UniquePtr<nsOpenTypeTable> openTypeTable;
   nsGlyphTable* glyphTable;
   if (aGeneric) {
     // This is a generic font, use the Unicode table.
     glyphTable = &gGlyphTableList->mUnicodeTable;
   } else {
@@ -1316,36 +1317,33 @@ bool nsMathMLChar::StretchEnumContext::E
       nsAutoCString familyName;
       unquotedFamilyName.AppendToString(familyName);
       glyphTable = gGlyphTableList->GetGlyphTableFor(familyName);
     }
   }
 
   if (!openTypeTable) {
     if (context->mTablesTried.Contains(glyphTable))
-      return true;  // already tried this one
+      return false;  // already tried this one
 
     // Only try this table once.
     context->mTablesTried.AppendElement(glyphTable);
   }
 
   // If the unicode table is being used, then search all font families.  If a
   // special table is being used then the font in this family should have the
   // specified glyphs.
   const FontFamilyList& familyList =
       glyphTable == &gGlyphTableList->mUnicodeTable ? context->mFamilyList
                                                     : family;
 
-  if ((context->mTryVariants &&
-       context->TryVariants(glyphTable, &fontGroup, familyList)) ||
-      (context->mTryParts &&
-       context->TryParts(glyphTable, &fontGroup, familyList)))
-    return false;  // no need to continue
-
-  return true;  // true means continue
+  return (context->mTryVariants &&
+          context->TryVariants(glyphTable, &fontGroup, familyList)) ||
+         (context->mTryParts &&
+          context->TryParts(glyphTable, &fontGroup, familyList));
 }
 
 static void AppendFallbacks(nsTArray<FontFamilyName>& aNames,
                             const nsTArray<nsCString>& aFallbacks) {
   for (const nsCString& fallback : aFallbacks) {
     aNames.AppendElement(
         FontFamilyName(fallback, StyleFontFamilyNameSyntax::Identifiers));
   }
@@ -1522,22 +1520,20 @@ nsresult nsMathMLChar::StretchInternal(
     StretchEnumContext enumData(this, presContext, aDrawTarget,
                                 aFontSizeInflation, aStretchDirection,
                                 targetSize, aStretchHint, aDesiredStretchSize,
                                 font.fontlist, glyphFound);
     enumData.mTryParts = !largeopOnly;
 
     const nsTArray<FontFamilyName>& fontlist =
         font.fontlist.GetFontlist()->mNames;
-    uint32_t i, num = fontlist.Length();
-    bool next = true;
-    for (i = 0; i < num && next; i++) {
-      const FontFamilyName& name = fontlist[i];
-      next =
-          StretchEnumContext::EnumCallback(name, name.IsGeneric(), &enumData);
+    for (const FontFamilyName& name : fontlist) {
+      if (StretchEnumContext::EnumCallback(name, name.IsGeneric(), &enumData)) {
+        break;
+      }
     }
   }
 
   if (!maxWidth) {
     // Now, we know how we are going to draw the char. Update the member
     // variables accordingly.
     mUnscaledAscent = aDesiredStretchSize.ascent;
   }