Bug 1366735 part 1 - Change counter functions to use struct rather than nsCSSValue::Array. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Mon, 22 May 2017 22:51:20 +1000
changeset 584111 0a2ef777045603fe591ba3a74259699a2339cede
parent 583357 ce3d615a7dcbb63b45fd45052e09483cbcd19c7a
child 584112 8216769c7ee2a323d2af2080dd97cac3489af82d
push id60638
push userxquan@mozilla.com
push dateWed, 24 May 2017 22:57:13 +0000
reviewersheycam
bugs1366735
milestone55.0a1
Bug 1366735 part 1 - Change counter functions to use struct rather than nsCSSValue::Array. r=heycam MozReview-Commit-ID: 4FiOxCOsjtD
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCounterManager.cpp
layout/base/nsCounterManager.h
layout/style/CounterStyleManager.h
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1766,23 +1766,22 @@ nsCSSFrameConstructor::CreateGeneratedCo
       nsCOMPtr<nsIContent> content;
       NS_NewAttributeContent(mDocument->NodeInfoManager(),
                              attrNameSpace, attrName, getter_AddRefs(content));
       return content.forget();
     }
 
     case eStyleContentType_Counter:
     case eStyleContentType_Counters: {
-      nsCSSValue::Array* counters = data.GetCounters();
-      nsCounterList* counterList = mCounterManager.CounterListFor(
-          nsDependentString(counters->Item(0).GetStringBufferValue()));
+      nsStyleContentData::CounterFunction* counters = data.GetCounters();
+      nsCounterList* counterList =
+        mCounterManager.CounterListFor(counters->mIdent);
 
       nsCounterUseNode* node =
-        new nsCounterUseNode(mPresShell->GetPresContext(),
-                             counters, aContentIndex,
+        new nsCounterUseNode(counters, aContentIndex,
                              type == eStyleContentType_Counters);
 
       nsGenConInitializer* initializer =
         new nsGenConInitializer(node, counterList,
                                 &nsCSSFrameConstructor::CountersDirty);
       return CreateGenConTextNode(aState, EmptyString(), &node->mText,
                                   initializer);
     }
--- a/layout/base/nsCounterManager.cpp
+++ b/layout/base/nsCounterManager.cpp
@@ -38,34 +38,16 @@ nsCounterUseNode::InitTextFrame(nsGenCon
       counterList->SetDirty();
       return true;
     }
   }
 
   return false;
 }
 
-CounterStyle*
-nsCounterUseNode::GetCounterStyle()
-{
-    if (!mCounterStyle) {
-        const nsCSSValue& style = mCounterFunction->Item(mAllCounters ? 2 : 1);
-        CounterStyleManager* manager = mPresContext->CounterStyleManager();
-        if (style.GetUnit() == eCSSUnit_AtomIdent) {
-            mCounterStyle = manager->BuildCounterStyle(style.GetAtomValue());
-        } else if (style.GetUnit() == eCSSUnit_Symbols) {
-            mCounterStyle = new AnonymousCounterStyle(style.GetArrayValue());
-        } else {
-            NS_NOTREACHED("Unknown counter style");
-            mCounterStyle = CounterStyleManager::GetDecimalStyle();
-        }
-    }
-    return mCounterStyle;
-}
-
 // assign the correct |mValueAfter| value to a node that has been inserted
 // Should be called immediately after calling |Insert|.
 void nsCounterUseNode::Calc(nsCounterList *aList)
 {
     NS_ASSERTION(!aList->IsDirty(),
                  "Why are we calculating with a dirty list?");
     mValueAfter = aList->ValueBefore(this);
 }
@@ -93,33 +75,27 @@ nsCounterUseNode::GetText(nsString& aRes
 
     AutoTArray<nsCounterNode*, 8> stack;
     stack.AppendElement(static_cast<nsCounterNode*>(this));
 
     if (mAllCounters && mScopeStart)
         for (nsCounterNode *n = mScopeStart; n->mScopePrev; n = n->mScopeStart)
             stack.AppendElement(n->mScopePrev);
 
-    const char16_t* separator;
-    if (mAllCounters)
-        separator = mCounterFunction->Item(1).GetStringBufferValue();
-
-    CounterStyle* style = GetCounterStyle();
     WritingMode wm = mPseudoFrame ?
         mPseudoFrame->GetWritingMode() : WritingMode();
     for (uint32_t i = stack.Length() - 1;; --i) {
         nsCounterNode *n = stack[i];
         nsAutoString text;
         bool isTextRTL;
-        style->GetCounterText(n->mValueAfter, wm, text, isTextRTL);
+        mCounterStyle->GetCounterText(n->mValueAfter, wm, text, isTextRTL);
         aResult.Append(text);
         if (i == 0)
             break;
-        NS_ASSERTION(mAllCounters, "yikes, separator is uninitialized");
-        aResult.Append(separator);
+        aResult.Append(mSeparator);
     }
 }
 
 void
 nsCounterList::SetScope(nsCounterNode *aNode)
 {
     // This function is responsible for setting |mScopeStart| and
     // |mScopePrev| (whose purpose is described in nsCounterManager.h).
--- a/layout/base/nsCounterManager.h
+++ b/layout/base/nsCounterManager.h
@@ -71,43 +71,36 @@ struct nsCounterNode : public nsGenConNo
     {
     }
 
     // to avoid virtual function calls in the common case
     inline void Calc(nsCounterList* aList);
 };
 
 struct nsCounterUseNode : public nsCounterNode {
-    // The same structure passed through the style system:  an array
-    // containing the values in the counter() or counters() in the order
-    // given in the CSS spec.
-    RefPtr<nsCSSValue::Array> mCounterFunction;
-
-    nsPresContext* mPresContext;
     mozilla::CounterStylePtr mCounterStyle;
+    nsString mSeparator;
 
     // false for counter(), true for counters()
     bool mAllCounters;
 
     // args go directly to member variables here and of nsGenConNode
-    nsCounterUseNode(nsPresContext* aPresContext,
-                     nsCSSValue::Array* aCounterFunction,
+    nsCounterUseNode(nsStyleContentData::CounterFunction* aCounterFunction,
                      uint32_t aContentIndex, bool aAllCounters)
         : nsCounterNode(aContentIndex, USE)
-        , mCounterFunction(aCounterFunction)
-        , mPresContext(aPresContext)
+        , mCounterStyle(aCounterFunction->mCounterStyle)
+        , mSeparator(aCounterFunction->mSeparator)
         , mAllCounters(aAllCounters)
     {
         NS_ASSERTION(aContentIndex <= INT32_MAX, "out of range");
     }
 
     virtual bool InitTextFrame(nsGenConList* aList,
             nsIFrame* aPseudoFrame, nsIFrame* aTextFrame) override;
 
-    mozilla::CounterStyle* GetCounterStyle();
     void SetCounterStyleDirty()
     {
         mCounterStyle = nullptr;
     }
 
     // assign the correct |mValueAfter| value to a node that has been inserted
     // Should be called immediately after calling |Insert|.
     void Calc(nsCounterList* aList);
--- a/layout/style/CounterStyleManager.h
+++ b/layout/style/CounterStyleManager.h
@@ -149,16 +149,21 @@ public:
   CounterStylePtr() : mRaw(0) {}
   CounterStylePtr(const CounterStylePtr& aOther)
     : mRaw(aOther.mRaw)
   {
     if (IsAnonymous()) {
       AsAnonymous()->AddRef();
     }
   }
+  CounterStylePtr(CounterStylePtr&& aOther)
+    : mRaw(aOther.mRaw)
+  {
+    aOther.mRaw = 0;
+  }
   ~CounterStylePtr() { Reset(); }
 
   CounterStylePtr& operator=(const CounterStylePtr& aOther)
   {
     if (this != &aOther) {
       Reset();
       new (this) CounterStylePtr(aOther);
     }
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -1260,16 +1260,56 @@ nsComputedDOMStyle::DoGetColumnRuleStyle
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetColumnRuleColor()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
   SetValueFromComplexColor(val, StyleColumn()->mColumnRuleColor);
   return val.forget();
 }
 
+static void
+AppendCounterStyle(CounterStyle* aStyle, nsAString& aString)
+{
+  AnonymousCounterStyle* anonymous = aStyle->AsAnonymous();
+  if (!anonymous) {
+    // want SetIdent
+    nsString type;
+    aStyle->GetStyleName(type);
+    nsStyleUtil::AppendEscapedCSSIdent(type, aString);
+  } else if (anonymous->IsSingleString()) {
+    const nsTArray<nsString>& symbols = anonymous->GetSymbols();
+    MOZ_ASSERT(symbols.Length() == 1);
+    nsStyleUtil::AppendEscapedCSSString(symbols[0], aString);
+  } else {
+    aString.AppendLiteral("symbols(");
+
+    uint8_t system = anonymous->GetSystem();
+    NS_ASSERTION(system == NS_STYLE_COUNTER_SYSTEM_CYCLIC ||
+                 system == NS_STYLE_COUNTER_SYSTEM_NUMERIC ||
+                 system == NS_STYLE_COUNTER_SYSTEM_ALPHABETIC ||
+                 system == NS_STYLE_COUNTER_SYSTEM_SYMBOLIC ||
+                 system == NS_STYLE_COUNTER_SYSTEM_FIXED,
+                 "Invalid system for anonymous counter style.");
+    if (system != NS_STYLE_COUNTER_SYSTEM_SYMBOLIC) {
+      AppendASCIItoUTF16(nsCSSProps::ValueToKeyword(
+              system, nsCSSProps::kCounterSystemKTable), aString);
+      aString.Append(' ');
+    }
+
+    const nsTArray<nsString>& symbols = anonymous->GetSymbols();
+    NS_ASSERTION(symbols.Length() > 0,
+                 "No symbols in the anonymous counter style");
+    for (size_t i = 0, iend = symbols.Length(); i < iend; i++) {
+      nsStyleUtil::AppendEscapedCSSString(symbols[i], aString);
+      aString.Append(' ');
+    }
+    aString.Replace(aString.Length() - 1, 1, char16_t(')'));
+  }
+}
+
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetContent()
 {
   const nsStyleContent *content = StyleContent();
 
   if (content->ContentCount() == 0) {
     RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
     val->SetIdent(eCSSKeyword_none);
@@ -1318,36 +1358,25 @@ nsComputedDOMStyle::DoGetContent()
         /* FIXME: counters should really use an object */
         nsAutoString str;
         if (type == eStyleContentType_Counter) {
           str.AppendLiteral("counter(");
         }
         else {
           str.AppendLiteral("counters(");
         }
-        // WRITE ME
-        nsCSSValue::Array* a = data.GetCounters();
-
-        nsStyleUtil::AppendEscapedCSSIdent(
-          nsDependentString(a->Item(0).GetStringBufferValue()), str);
-        int32_t typeItem = 1;
+        nsStyleContentData::CounterFunction* counters = data.GetCounters();
+        nsStyleUtil::AppendEscapedCSSIdent(counters->mIdent, str);
         if (type == eStyleContentType_Counters) {
-          typeItem = 2;
           str.AppendLiteral(", ");
-          nsStyleUtil::AppendEscapedCSSString(
-            nsDependentString(a->Item(1).GetStringBufferValue()), str);
+          nsStyleUtil::AppendEscapedCSSString(counters->mSeparator, str);
         }
-        MOZ_ASSERT(eCSSUnit_None != a->Item(typeItem).GetUnit(),
-                   "'none' should be handled as identifier value");
-        nsString type;
-        a->Item(typeItem).AppendToString(eCSSProperty_list_style_type,
-                                         type, nsCSSValue::eNormalized);
-        if (!type.LowerCaseEqualsLiteral("decimal")) {
+        if (counters->mCounterStyle != CounterStyleManager::GetDecimalStyle()) {
           str.AppendLiteral(", ");
-          str.Append(type);
+          AppendCounterStyle(counters->mCounterStyle, str);
         }
 
         str.Append(char16_t(')'));
         val->SetString(str, nsIDOMCSSPrimitiveValue::CSS_COUNTER);
         break;
       }
       case eStyleContentType_OpenQuote:
         val->SetIdent(eCSSKeyword_open_quote);
@@ -3746,53 +3775,18 @@ nsComputedDOMStyle::DoGetListStylePositi
                                    nsCSSProps::kListStylePositionKTable));
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetListStyleType()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-  CounterStyle* style = StyleList()->mCounterStyle;
-  AnonymousCounterStyle* anonymous = style->AsAnonymous();
   nsAutoString tmp;
-  if (!anonymous) {
-    // want SetIdent
-    nsString type;
-    style->GetStyleName(type);
-    nsStyleUtil::AppendEscapedCSSIdent(type, tmp);
-  } else if (anonymous->IsSingleString()) {
-    const nsTArray<nsString>& symbols = anonymous->GetSymbols();
-    MOZ_ASSERT(symbols.Length() == 1);
-    nsStyleUtil::AppendEscapedCSSString(symbols[0], tmp);
-  } else {
-    tmp.AppendLiteral("symbols(");
-
-    uint8_t system = anonymous->GetSystem();
-    NS_ASSERTION(system == NS_STYLE_COUNTER_SYSTEM_CYCLIC ||
-                 system == NS_STYLE_COUNTER_SYSTEM_NUMERIC ||
-                 system == NS_STYLE_COUNTER_SYSTEM_ALPHABETIC ||
-                 system == NS_STYLE_COUNTER_SYSTEM_SYMBOLIC ||
-                 system == NS_STYLE_COUNTER_SYSTEM_FIXED,
-                 "Invalid system for anonymous counter style.");
-    if (system != NS_STYLE_COUNTER_SYSTEM_SYMBOLIC) {
-      AppendASCIItoUTF16(nsCSSProps::ValueToKeyword(
-              system, nsCSSProps::kCounterSystemKTable), tmp);
-      tmp.Append(' ');
-    }
-
-    const nsTArray<nsString>& symbols = anonymous->GetSymbols();
-    NS_ASSERTION(symbols.Length() > 0,
-                 "No symbols in the anonymous counter style");
-    for (size_t i = 0, iend = symbols.Length(); i < iend; i++) {
-      nsStyleUtil::AppendEscapedCSSString(symbols[i], tmp);
-      tmp.Append(' ');
-    }
-    tmp.Replace(tmp.Length() - 1, 1, char16_t(')'));
-  }
+  AppendCounterStyle(StyleList()->mCounterStyle, tmp);
   val->SetString(tmp);
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetImageRegion()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -8897,17 +8897,36 @@ nsRuleNode::ComputeContentData(void* aSt
           data.SetString(type, buffer.get());
           break;
         }
         case eCSSUnit_Counter:
         case eCSSUnit_Counters: {
           nsStyleContentType type =
             unit == eCSSUnit_Counter ? eStyleContentType_Counter
                                      : eStyleContentType_Counters;
-          data.SetCounters(type, value.GetArrayValue());
+          RefPtr<nsStyleContentData::CounterFunction>
+            counterFunc = new nsStyleContentData::CounterFunction();
+          nsCSSValue::Array* arrayValue = value.GetArrayValue();
+          arrayValue->Item(0).GetStringValue(counterFunc->mIdent);
+          if (unit == eCSSUnit_Counters) {
+            arrayValue->Item(1).GetStringValue(counterFunc->mSeparator);
+          }
+          const nsCSSValue& style =
+            value.GetArrayValue()->Item(unit == eCSSUnit_Counters ? 2 : 1);
+          if (style.GetUnit() == eCSSUnit_AtomIdent) {
+            counterFunc->mCounterStyle = mPresContext->
+              CounterStyleManager()->BuildCounterStyle(style.GetAtomValue());
+          } else if (style.GetUnit() == eCSSUnit_Symbols) {
+            counterFunc->mCounterStyle =
+              new AnonymousCounterStyle(style.GetArrayValue());
+          } else {
+            MOZ_ASSERT_UNREACHABLE("Unknown counter style");
+            counterFunc->mCounterStyle = CounterStyleManager::GetDecimalStyle();
+          }
+          data.SetCounters(type, counterFunc.forget());
           break;
         }
         case eCSSUnit_Enumerated:
           switch (value.GetIntValue()) {
             case NS_STYLE_CONTENT_OPEN_QUOTE:
               data.SetKeyword(eStyleContentType_OpenQuote);
               break;
             case NS_STYLE_CONTENT_CLOSE_QUOTE:
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3803,16 +3803,25 @@ nsStyleContentData::nsStyleContentData(c
     mContent.mCounters->AddRef();
   } else if (aOther.mContent.mString) {
     mContent.mString = NS_strdup(aOther.mContent.mString);
   } else {
     mContent.mString = nullptr;
   }
 }
 
+bool
+nsStyleContentData::
+CounterFunction::operator==(const CounterFunction& aOther) const
+{
+  return mIdent == aOther.mIdent &&
+    mSeparator == aOther.mSeparator &&
+    mCounterStyle == aOther.mCounterStyle;
+}
+
 nsStyleContentData&
 nsStyleContentData::operator=(const nsStyleContentData& aOther)
 {
   if (this == &aOther) {
     return *this;
   }
   this->~nsStyleContentData();
   new (this) nsStyleContentData(aOther);
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -3038,17 +3038,34 @@ public:
 
   char16_t* GetString() const
   {
     MOZ_ASSERT(mType == eStyleContentType_String ||
                mType == eStyleContentType_Attr);
     return mContent.mString;
   }
 
-  nsCSSValue::Array* GetCounters() const
+  struct CounterFunction
+  {
+    nsString mIdent;
+    // This is only used when it is a counters() function.
+    nsString mSeparator;
+    mozilla::CounterStylePtr mCounterStyle;
+
+    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CounterFunction)
+
+    bool operator==(const CounterFunction& aOther) const;
+    bool operator!=(const CounterFunction& aOther) const {
+      return !(*this == aOther);
+    }
+  private:
+    ~CounterFunction() {}
+  };
+
+  CounterFunction* GetCounters() const
   {
     MOZ_ASSERT(mType == eStyleContentType_Counter ||
                mType == eStyleContentType_Counters);
     return mContent.mCounters;
   }
 
   nsStyleImageRequest* GetImageRequest() const
   {
@@ -3079,27 +3096,26 @@ public:
                aType == eStyleContentType_Attr);
     MOZ_ASSERT(aString);
     MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
                "should only initialize nsStyleContentData once");
     mType = aType;
     mContent.mString = NS_strdup(aString);
   }
 
-  void SetCounters(nsStyleContentType aType, nsCSSValue::Array* aCounters)
+  void SetCounters(nsStyleContentType aType,
+                   already_AddRefed<CounterFunction> aCounterFunction)
   {
     MOZ_ASSERT(aType == eStyleContentType_Counter ||
                aType == eStyleContentType_Counters);
-    MOZ_ASSERT(aCounters);
-    MOZ_ASSERT(aCounters->Count() == 2 || aCounters->Count() == 3);
     MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
                "should only initialize nsStyleContentData once");
     mType = aType;
-    mContent.mCounters = aCounters;
-    mContent.mCounters->AddRef();
+    mContent.mCounters = aCounterFunction.take();
+    MOZ_ASSERT(mContent.mCounters);
   }
 
   void SetImageRequest(already_AddRefed<nsStyleImageRequest> aRequest)
   {
     MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
                "should only initialize nsStyleContentData once");
     mType = eStyleContentType_Image;
     mContent.mImage = aRequest.take();
@@ -3112,17 +3128,17 @@ public:
     }
   }
 
 private:
   nsStyleContentType mType;
   union {
     char16_t *mString;
     nsStyleImageRequest* mImage;
-    nsCSSValue::Array* mCounters;
+    CounterFunction* mCounters;
   } mContent;
 };
 
 struct nsStyleCounterData
 {
   nsString  mCounter;
   int32_t   mValue;