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

@@ -117,6 +117,14 @@ struct FontFamilyName final {
     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;
@@ -300,11 +308,7 @@ class FontFamilyList {
   // 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;
       }
     }

layout/mathml/nsMathMLChar.cpp

@@ -1276,7 +1276,8 @@ bool nsMathMLChar::StretchEnumContext::TryParts(
   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);
@@ -1297,7 +1298,7 @@ bool nsMathMLChar::StretchEnumContext::EnumCallback(
   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;
@@ -1321,7 +1322,7 @@ bool nsMathMLChar::StretchEnumContext::EnumCallback(
 
   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);
@@ -1334,13 +1335,10 @@ bool nsMathMLChar::StretchEnumContext::EnumCallback(
       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,
@@ -1527,12 +1525,10 @@ nsresult nsMathMLChar::StretchInternal(
 
     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;
+      }
     }
   }