Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable. r?dholbert draft
authorXidorn Quan <me@upsuper.org>
Tue, 29 Aug 2017 15:36:32 +1000
changeset 656372 0d70e41ac71beb2653e8e9085bbd9ded25dc1ed8
parent 656371 ee1fef0b9b5af8718792d532185f4229b9f38a23
child 656373 246e4827b3d7f0f7249267d3928eac21989da65c
push id77188
push userxquan@mozilla.com
push dateThu, 31 Aug 2017 04:04:58 +0000
reviewersdholbert
bugs1393189
milestone57.0a1
Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable. r?dholbert The change in CounterStyleManager::BuildCounterStyle converts a case- insensitive comparison to a case-sensitive comparison by comparing atom pointer directly. But this is fine because all names of builtin counter styles should have been lowercased by the parser. For Gecko, it is done in CSSParserImpl::ParseCounterStyleName, and for Servo, it is done in counter_style::parse_counter_style_name. MozReview-Commit-ID: JHHmzEaNIpn
layout/style/CounterStyleManager.cpp
layout/style/CounterStyleManager.h
layout/style/nsRuleNode.cpp
--- a/layout/style/CounterStyleManager.cpp
+++ b/layout/style/CounterStyleManager.cpp
@@ -2039,32 +2039,31 @@ CounterStyle*
 CounterStyleManager::BuildCounterStyle(nsIAtom* aName)
 {
   MOZ_ASSERT(NS_IsMainThread());
   CounterStyle* data = GetCounterStyle(aName);
   if (data) {
     return data;
   }
 
-  // It is intentional that the predefined names are case-insensitive
-  // but the user-defined names case-sensitive.
+  // Names are compared case-sensitively here. Predefined names should
+  // have been lowercased by the parser.
   StyleSetHandle styleSet = mPresContext->StyleSet();
   nsCSSCounterStyleRule* rule = styleSet->CounterStyleRuleForName(aName);
   if (rule) {
     MOZ_ASSERT(rule->Name() == aName);
     data = new (mPresContext) CustomCounterStyle(aName, this, rule);
   } else {
-    int32_t type;
-    nsDependentAtomString name(aName);
-    nsCSSKeyword keyword = nsCSSKeywords::LookupKeyword(name);
-    if (nsCSSProps::FindKeyword(keyword, nsCSSProps::kListStyleKTable, type)) {
-      if (gBuiltinStyleTable[type].IsDependentStyle()) {
-        data = new (mPresContext) DependentBuiltinCounterStyle(type, this);
-      } else {
-        data = GetBuiltinStyle(type);
+    for (const BuiltinCounterStyle& item : gBuiltinStyleTable) {
+      if (item.GetStyleName() == aName) {
+        int32_t style = item.GetStyle();
+        data = item.IsDependentStyle()
+          ? new (mPresContext) DependentBuiltinCounterStyle(style, this)
+          : GetBuiltinStyle(style);
+        break;
       }
     }
   }
   if (!data) {
     data = GetDecimalStyle();
   }
   mStyles.Put(aName, data);
   return data;
@@ -2077,16 +2076,24 @@ CounterStyleManager::GetBuiltinStyle(int
              "Require a valid builtin style constant");
   MOZ_ASSERT(!gBuiltinStyleTable[aStyle].IsDependentStyle(),
              "Cannot get dependent builtin style");
   // No method of BuiltinCounterStyle mutates the struct itself, so it
   // should be fine to cast const away.
   return const_cast<BuiltinCounterStyle*>(&gBuiltinStyleTable[aStyle]);
 }
 
+/* static */ nsIAtom*
+CounterStyleManager::GetStyleNameFromType(int32_t aStyle)
+{
+  MOZ_ASSERT(0 <= aStyle && size_t(aStyle) < sizeof(gBuiltinStyleTable),
+             "Require a valid builtin style constant");
+  return gBuiltinStyleTable[aStyle].GetStyleName();
+}
+
 bool
 CounterStyleManager::NotifyRuleChanged()
 {
   bool changed = false;
   for (auto iter = mStyles.Iter(); !iter.Done(); iter.Next()) {
     CounterStyle* style = iter.Data();
     bool toBeUpdated = false;
     bool toBeRemoved = false;
--- a/layout/style/CounterStyleManager.h
+++ b/layout/style/CounterStyleManager.h
@@ -340,16 +340,18 @@ public:
   {
     return GetBuiltinStyle(NS_STYLE_LIST_STYLE_DECIMAL);
   }
   static CounterStyle* GetDiscStyle()
   {
     return GetBuiltinStyle(NS_STYLE_LIST_STYLE_DISC);
   }
 
+  static nsIAtom* GetStyleNameFromType(int32_t aStyle);
+
   // This method will scan all existing counter styles generated by this
   // manager, and remove or mark data dirty accordingly. It returns true
   // if any counter style is changed, false elsewise. This method should
   // be called when any counter style may be affected.
   bool NotifyRuleChanged();
   // NotifyRuleChanged will evict no longer needed counter styles into
   // mRetiredStyles, and this function destroys all objects listed there.
   // It should be called only after no one may ever use those objects.
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -8088,36 +8088,37 @@ nsRuleNode::ComputeListData(void* aStart
     }
     case eCSSUnit_String: {
       nsString str;
       typeValue->GetStringValue(str);
       list->mCounterStyle = new AnonymousCounterStyle(str);
       break;
     }
     case eCSSUnit_Enumerated: {
-      // For compatibility with html attribute map.
-      // This branch should never be called for value from CSS.
+      // For compatibility with html attribute map. This branch should
+      // never be called for value from CSS. The values can only come
+      // from the items in EnumTable listed in HTMLLIElement.cpp and
+      // HTMLSharedListElement.cpp.
       int32_t intValue = typeValue->GetIntValue();
       nsCOMPtr<nsIAtom> name;
       switch (intValue) {
         case NS_STYLE_LIST_STYLE_LOWER_ROMAN:
           name = nsGkAtoms::lowerRoman;
           break;
         case NS_STYLE_LIST_STYLE_UPPER_ROMAN:
           name = nsGkAtoms::upperRoman;
           break;
         case NS_STYLE_LIST_STYLE_LOWER_ALPHA:
           name = nsGkAtoms::lowerAlpha;
           break;
         case NS_STYLE_LIST_STYLE_UPPER_ALPHA:
           name = nsGkAtoms::upperAlpha;
           break;
         default:
-          name = NS_Atomize(nsCSSProps::ValueToKeyword(
-                  intValue, nsCSSProps::kListStyleKTable));
+          name = CounterStyleManager::GetStyleNameFromType(intValue);
           break;
       }
       setListStyleType(name);
       break;
     }
     case eCSSUnit_Symbols:
       list->mCounterStyle =
         new AnonymousCounterStyle(typeValue->GetArrayValue());