Bug 1366735 part 1 - Change counter functions to use struct rather than nsCSSValue::Array. r=heycam
authorXidorn Quan <me@upsuper.org>
Mon, 22 May 2017 22:51:20 +1000
changeset 408656 787d3a0c5dd103199466107efce043c715f8a613
parent 408655 f7bb8feabd69573a1b44b877e39014c57af38984
child 408657 3da40efe9851348f5a92be871b25d4312c81f4d4
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1366735
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;