Bug 1321022 pt 4.2 - Fix up some nits in existing font-feature-settings code as per review comments on the new font-variation-settings code. r=dholbert
authorJonathan Kew <jkew@mozilla.com>
Sat, 03 Dec 2016 12:18:39 +0000
changeset 325215 b649cf92aa2d04afbfe454ccf31097eb41216b76
parent 325214 180d4d40de6b0089b2ccf41f00c43afd93b5d874
child 325216 7878d113285818e7a5ec92e4c43cd43a84728db3
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersdholbert
bugs1321022
milestone53.0a1
Bug 1321022 pt 4.2 - Fix up some nits in existing font-feature-settings code as per review comments on the new font-variation-settings code. r=dholbert
layout/style/nsCSSParser.cpp
layout/style/nsRuleNode.cpp
layout/style/nsRuleNode.h
layout/style/nsStyleUtil.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -15306,17 +15306,19 @@ ValidFontFeatureTag(const nsString& aTag
 bool
 CSSParserImpl::ParseFontFeatureSettings(nsCSSValue& aValue)
 {
   if (ParseSingleTokenVariant(aValue, VARIANT_INHERIT | VARIANT_NORMAL,
                               nullptr)) {
     return true;
   }
 
-  nsCSSValuePairList *cur = aValue.SetPairListValue();
+  auto resultHead = MakeUnique<nsCSSValuePairList>();
+  nsCSSValuePairList* cur = resultHead.get();
+
   for (;;) {
     // feature tag
     if (!GetToken(true)) {
       return false;
     }
 
     if (mToken.mType != eCSSToken_String ||
         !ValidFontFeatureTag(mToken.mIdent)) {
@@ -15349,16 +15351,18 @@ CSSParserImpl::ParseFontFeatureSettings(
     if (!ExpectSymbol(',', true)) {
       break;
     }
 
     cur->mNext = new nsCSSValuePairList;
     cur = cur->mNext;
   }
 
+  aValue.AdoptPairListValue(Move(resultHead));
+
   return true;
 }
 
 bool
 CSSParserImpl::ParseFontVariationSettings(nsCSSValue& aValue)
 {
   if (ParseSingleTokenVariant(aValue, VARIANT_INHERIT | VARIANT_NORMAL,
                               nullptr)) {
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -3960,43 +3960,43 @@ nsRuleNode::SetFont(nsPresContext* aPres
            defaultVariableFont->variantPosition,
            Unused, Unused, /* normal */ 0, systemFont.variantPosition);
 
   // font-feature-settings
   const nsCSSValue* featureSettingsValue =
     aRuleData->ValueForFontFeatureSettings();
 
   switch (featureSettingsValue->GetUnit()) {
-  case eCSSUnit_Null:
-    break;
-
-  case eCSSUnit_Normal:
-  case eCSSUnit_Initial:
-    aFont->mFont.fontFeatureSettings.Clear();
-    break;
-
-  case eCSSUnit_Inherit:
-  case eCSSUnit_Unset:
-    aConditions.SetUncacheable();
-    aFont->mFont.fontFeatureSettings = aParentFont->mFont.fontFeatureSettings;
-    break;
-
-  case eCSSUnit_System_Font:
-    aFont->mFont.fontFeatureSettings = systemFont.fontFeatureSettings;
-    break;
-
-  case eCSSUnit_PairList:
-  case eCSSUnit_PairListDep:
-    ComputeFontFeatures(featureSettingsValue->GetPairListValue(),
-                        aFont->mFont.fontFeatureSettings);
-    break;
-
-  default:
-    MOZ_ASSERT(false, "unexpected value unit");
-    break;
+    case eCSSUnit_Null:
+      break;
+
+    case eCSSUnit_Normal:
+    case eCSSUnit_Initial:
+      aFont->mFont.fontFeatureSettings.Clear();
+      break;
+
+    case eCSSUnit_Inherit:
+    case eCSSUnit_Unset:
+      aConditions.SetUncacheable();
+      aFont->mFont.fontFeatureSettings = aParentFont->mFont.fontFeatureSettings;
+      break;
+
+    case eCSSUnit_System_Font:
+      aFont->mFont.fontFeatureSettings = systemFont.fontFeatureSettings;
+      break;
+
+    case eCSSUnit_PairList:
+    case eCSSUnit_PairListDep:
+      ComputeFontFeatures(featureSettingsValue->GetPairListValue(),
+                          aFont->mFont.fontFeatureSettings);
+      break;
+
+    default:
+      MOZ_ASSERT(false, "unexpected value unit");
+      break;
   }
 
   // font-variation-settings
   const nsCSSValue* variationSettingsValue =
     aRuleData->ValueForFontVariationSettings();
 
   switch (variationSettingsValue->GetUnit()) {
     case eCSSUnit_Null:
@@ -4174,24 +4174,25 @@ AssertValidFontTag(const nsString& aStri
 }
 
 /* static */ void
 nsRuleNode::ComputeFontFeatures(const nsCSSValuePairList *aFeaturesList,
                                 nsTArray<gfxFontFeature>& aFeatureSettings)
 {
   aFeatureSettings.Clear();
   for (const nsCSSValuePairList* p = aFeaturesList; p; p = p->mNext) {
-    gfxFontFeature feat = {0, 0};
+    gfxFontFeature feat;
 
     MOZ_ASSERT(aFeaturesList->mXValue.GetUnit() == eCSSUnit_String,
                "unexpected value unit");
 
     // tag is a 4-byte ASCII sequence
     nsAutoString tag;
     p->mXValue.GetStringValue(tag);
+    AssertValidFontTag(tag);
     if (tag.Length() != 4) {
       continue;
     }
     // parsing validates that these are ASCII chars
     // tags are always big-endian
     feat.mTag = (tag[0] << 24) | (tag[1] << 16) | (tag[2] << 8)  | tag[3];
 
     // value
--- a/layout/style/nsRuleNode.h
+++ b/layout/style/nsRuleNode.h
@@ -1017,17 +1017,17 @@ public:
   }
 
   // Note that this will return false if we have cached conditional
   // style structs.
   bool NodeHasCachedUnconditionalData(const nsStyleStructID aSID) {
     return !!mStyleData.GetStyleData(aSID);
   }
 
-  static void ComputeFontFeatures(const nsCSSValuePairList *aFeaturesList,
+  static void ComputeFontFeatures(const nsCSSValuePairList* aFeaturesList,
                                   nsTArray<gfxFontFeature>& aFeatureSettings);
 
   static void ComputeFontVariations(const nsCSSValuePairList* aVariationsList,
                                     nsTArray<gfxFontVariation>& aVariationSettings);
 
   static nscoord CalcFontPointSize(int32_t aHTMLSize, int32_t aBasePointSize,
                                    nsPresContext* aPresContext,
                                    nsFontSizeType aFontSizeType = eFontSize_HTML);
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -349,17 +349,17 @@ nsStyleUtil::AppendFontTagAsString(uint3
 /* static */ void
 nsStyleUtil::AppendFontFeatureSettings(const nsTArray<gfxFontFeature>& aFeatures,
                                        nsAString& aResult)
 {
   for (uint32_t i = 0, numFeat = aFeatures.Length(); i < numFeat; i++) {
     const gfxFontFeature& feat = aFeatures[i];
 
     if (i != 0) {
-        aResult.AppendLiteral(", ");
+      aResult.AppendLiteral(", ");
     }
 
     AppendFontTagAsString(feat.mTag, aResult);
 
     // output value, if necessary
     if (feat.mValue == 0) {
       // 0 ==> off
       aResult.AppendLiteral(" off");