Bug 1642430 - Improve SVGTests implementation r=dholbert

This is should not be a functional change, just making the methods in SVGTests easier to use/understand.
There's a little dead code removal since we can go through the same code path for empty intl.accept_languages as we do when it has a value, and we'd almost always expect it to have a value.

Differential Revision: https://phabricator.services.mozilla.com/D77694

dom/svg/SVGSwitchElement.cpp

@@ -112,62 +112,47 @@ nsIContent* SVGSwitchElement::FindActiveChild() const {
   nsAutoString acceptLangs;
   Preferences::GetLocalizedString("intl.accept_languages", acceptLangs);
 
-  if (!acceptLangs.IsEmpty()) {
-    int32_t bestLanguagePreferenceRank = -1;
-    nsIContent* bestChild = nullptr;
-    nsIContent* defaultChild = nullptr;
-    for (nsIContent* child = nsINode::GetFirstChild(); child;
-         child = child->GetNextSibling()) {
-      if (!child->IsElement()) {
-        continue;
-      }
-      nsCOMPtr<SVGTests> tests(do_QueryInterface(child));
-      if (tests) {
-        if (tests->PassesConditionalProcessingTests(
-                SVGTests::kIgnoreSystemLanguage)) {
-          int32_t languagePreferenceRank =
-              tests->GetBestLanguagePreferenceRank(acceptLangs);
-          switch (languagePreferenceRank) {
-            case 0:
-              // best possible match
-              return child;
-            case -1:
-              // no match
-              break;
-            case -2:
-              // no systemLanguage attribute. If there's nothing better
-              // we'll use the first such child.
-              if (!defaultChild) {
-                defaultChild = child;
-              }
-              break;
-            default:
-              if (bestLanguagePreferenceRank == -1 ||
-                  languagePreferenceRank < bestLanguagePreferenceRank) {
-                bestLanguagePreferenceRank = languagePreferenceRank;
-                bestChild = child;
-              }
-              break;
-          }
-        }
-      } else if (!bestChild) {
-        bestChild = child;
-      }
-    }
-    return bestChild ? bestChild : defaultChild;
-  }
-
+  int32_t bestLanguagePreferenceRank = -1;
+  nsIContent* bestChild = nullptr;
+  nsIContent* defaultChild = nullptr;
   for (nsIContent* child = nsINode::GetFirstChild(); child;
        child = child->GetNextSibling()) {
     if (!child->IsElement()) {
       continue;
     }
     nsCOMPtr<SVGTests> tests(do_QueryInterface(child));
-    if (!tests || tests->PassesConditionalProcessingTests(&acceptLangs)) {
-      return child;
+    if (tests) {
+      if (tests->PassesConditionalProcessingTestsIgnoringSystemLanguage()) {
+        int32_t languagePreferenceRank =
+            tests->GetBestLanguagePreferenceRank(acceptLangs);
+        switch (languagePreferenceRank) {
+          case 0:
+            // best possible match
+            return child;
+          case -1:
+            // no match
+            break;
+          case -2:
+            // no systemLanguage attribute. If there's nothing better
+            // we'll use the first such child.
+            if (!defaultChild) {
+              defaultChild = child;
+            }
+            break;
+          default:
+            if (bestLanguagePreferenceRank == -1 ||
+                languagePreferenceRank < bestLanguagePreferenceRank) {
+              bestLanguagePreferenceRank = languagePreferenceRank;
+              bestChild = child;
+            }
+            break;
+        }
+      }
+    } else if (!bestChild) {
+      bestChild = child;
     }
   }
-  return nullptr;
+  return bestChild ? bestChild : defaultChild;
 }
 
 }  // namespace dom

dom/svg/SVGTests.cpp

@@ -61,8 +61,6 @@ bool SVGTests::IsConditionalProcessingAttribute(
 
 int32_t SVGTests::GetBestLanguagePreferenceRank(
     const nsAString& aAcceptLangs) const {
-  auto caseInsensitiveComparator = nsCaseInsensitiveStringComparator;
-
   if (!mStringListAttributes[LANGUAGE].IsExplicitlySet()) {
     return -2;
   }
@@ -75,12 +73,11 @@ int32_t SVGTests::GetBestLanguagePreferenceRank(
     while (languageTokenizer.hasMoreTokens()) {
       const nsAString& languageToken = languageTokenizer.nextToken();
       bool exactMatch = languageToken.Equals(mStringListAttributes[LANGUAGE][i],
-                                             caseInsensitiveComparator);
+                                             nsCaseInsensitiveStringComparator);
       bool prefixOnlyMatch =
-          !exactMatch &&
-          nsStyleUtil::DashMatchCompare(mStringListAttributes[LANGUAGE][i],
-                                        languageTokenizer.nextToken(),
-                                        caseInsensitiveComparator);
+          !exactMatch && nsStyleUtil::DashMatchCompare(
+                             mStringListAttributes[LANGUAGE][i], languageToken,
+                             nsCaseInsensitiveStringComparator);
       if (index == 0 && exactMatch) {
         // best possible match
         return 0;
@@ -95,10 +92,7 @@ int32_t SVGTests::GetBestLanguagePreferenceRank(
   return lowestRank;
 }
 
-const nsString* const SVGTests::kIgnoreSystemLanguage = (nsString*)0x01;
-
-bool SVGTests::PassesConditionalProcessingTests(
-    const nsString* aAcceptLangs) const {
+bool SVGTests::PassesConditionalProcessingTestsIgnoringSystemLanguage() const {
   // Required Extensions
   //
   // The requiredExtensions  attribute defines a list of required language
@@ -116,9 +110,12 @@ bool SVGTests::PassesConditionalProcessingTests(
       }
     }
   }
+  return true;
+}
 
-  if (aAcceptLangs == kIgnoreSystemLanguage) {
-    return true;
+bool SVGTests::PassesConditionalProcessingTests() const {
+  if (!PassesConditionalProcessingTestsIgnoringSystemLanguage()) {
+    return false;
   }
 
   // systemLanguage
@@ -135,11 +132,7 @@ bool SVGTests::PassesConditionalProcessingTests(
 
     // Get our language preferences
     nsAutoString acceptLangs;
-    if (aAcceptLangs) {
-      acceptLangs.Assign(*aAcceptLangs);
-    } else {
-      Preferences::GetLocalizedString("intl.accept_languages", acceptLangs);
-    }
+    Preferences::GetLocalizedString("intl.accept_languages", acceptLangs);
 
     if (acceptLangs.IsEmpty()) {
       NS_WARNING(

dom/svg/SVGTests.h

@@ -56,24 +56,30 @@ class SVGTests : public nsISupports {
   int32_t GetBestLanguagePreferenceRank(const nsAString& aAcceptLangs) const;
 
   /**
-   * Special value to pass to PassesConditionalProcessingTests to ignore
-   * systemLanguage attributes
+   * Check whether the conditional processing attributes other than
+   * systemLanguage "return true" if they apply to and are specified
+   * on the given element. Returns true if this element should be
+   * rendered, false if it should not.
    */
-  static const nsString* const kIgnoreSystemLanguage;
+  bool PassesConditionalProcessingTestsIgnoringSystemLanguage() const;
 
   /**
-   * Check whether the conditional processing attributes requiredFeatures,
-   * requiredExtensions and systemLanguage all "return true" if they apply to
+   * Check whether the conditional processing attributes requiredExtensions
+   * and systemLanguage both "return true" if they apply to
+   * and are specified on the given element. Returns true if this element
+   * should be rendered, false if it should not.
+   */
+  bool PassesConditionalProcessingTests() const;
+
+  /**
+   * Check whether the conditional processing attributes requiredExtensions
+   * and systemLanguage both "return true" if they apply to
    * and are specified on the given element. Returns true if this element
    * should be rendered, false if it should not.
    *
-   * @param aAcceptLangs Optional parameter to pass in the value of the
-   *   intl.accept_languages preference if the caller has it cached.
-   *   Alternatively, pass in kIgnoreSystemLanguage to skip the systemLanguage
-   *   check if the caller is giving that special treatment.
+   * @param aAcceptLangs The value of the intl.accept_languages preference
    */
-  bool PassesConditionalProcessingTests(
-      const nsString* aAcceptLangs = nullptr) const;
+  bool PassesConditionalProcessingTests(const nsAString& aAcceptLangs) const;
 
   /**
    * Returns true if the attribute is one of the conditional processing