Bug 1348606 - Use a threadsafe array for counters. r=xidorn

MozReview-Commit-ID: KgTgcD5mGqr

layout/base/nsCSSFrameConstructor.cpp

@@ -1741,7 +1741,7 @@ nsCSSFrameConstructor::CreateGeneratedContent(nsFrameConstructorState& aState,
 
     case eStyleContentType_Counter:
     case eStyleContentType_Counters: {
-      nsCSSValue::Array* counters = data.GetCounters();
+      nsCSSValue::ThreadSafeArray* counters = data.GetCounters();
       nsCounterList* counterList = mCounterManager.CounterListFor(
           nsDependentString(counters->Item(0).GetStringBufferValue()));
 

layout/base/nsCounterManager.h

@@ -79,7 +79,7 @@ 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;
+    RefPtr<nsCSSValue::ThreadSafeArray> mCounterFunction;
 
     nsPresContext* mPresContext;
     RefPtr<mozilla::CounterStyle> mCounterStyle;
@@ -89,7 +89,7 @@ struct nsCounterUseNode : public nsCounterNode {
 
     // args go directly to member variables here and of nsGenConNode
     nsCounterUseNode(nsPresContext* aPresContext,
-                     nsCSSValue::Array* aCounterFunction,
+                     nsCSSValue::ThreadSafeArray* aCounterFunction,
                      uint32_t aContentIndex, bool aAllCounters)
         : nsCounterNode(aContentIndex, USE)
         , mCounterFunction(aCounterFunction)

layout/style/ServoBindings.cpp

@@ -958,7 +958,7 @@ void
 Gecko_SetContentDataArray(nsStyleContentData* aContent,
                           nsStyleContentType aType, uint32_t aLen)
 {
-  nsCSSValue::Array* arr = nsCSSValue::Array::Create(aLen);
+  nsCSSValue::ThreadSafeArray* arr = nsCSSValue::ThreadSafeArray::Create(aLen);
   aContent->SetCounters(aType, arr);
 }
 

layout/style/nsCSSParser.cpp

@@ -8026,8 +8026,8 @@ CSSParserImpl::ParseCounter(nsCSSValue& aValue)
       break;
     }
 
-    RefPtr<nsCSSValue::Array> val =
+    RefPtr<nsCSSValue::ThreadSafeArray> val =
-      nsCSSValue::Array::Create(unit == eCSSUnit_Counter ? 2 : 3);
+      nsCSSValue::ThreadSafeArray::Create(unit == eCSSUnit_Counter ? 2 : 3);
 
     val->Item(0).SetStringValue(mToken.mIdent, eCSSUnit_Ident);
 
@@ -8058,7 +8058,7 @@ CSSParserImpl::ParseCounter(nsCSSValue& aValue)
       break;
     }
 
-    aValue.SetArrayValue(val, unit);
+    aValue.SetThreadSafeArrayValue(val, unit);
     return true;
   }
 

layout/style/nsCSSValue.cpp

@@ -172,6 +172,10 @@ nsCSSValue::nsCSSValue(const nsCSSValue& aCopy)
     mValue.mArray = aCopy.mValue.mArray;
     mValue.mArray->AddRef();
   }
+  else if (UnitHasThreadSafeArrayValue()) {
+    mValue.mThreadSafeArray = aCopy.mValue.mThreadSafeArray;
+    mValue.mThreadSafeArray->AddRef();
+  }
   else if (eCSSUnit_URL == mUnit) {
     mValue.mURL = aCopy.mValue.mURL;
     mValue.mURL->AddRef();
@@ -288,6 +292,9 @@ bool nsCSSValue::operator==(const nsCSSValue& aOther) const
     else if (UnitHasArrayValue()) {
       return *mValue.mArray == *aOther.mValue.mArray;
     }
+    else if (UnitHasThreadSafeArrayValue()) {
+      return *mValue.mThreadSafeArray == *aOther.mValue.mThreadSafeArray;
+    }
     else if (eCSSUnit_URL == mUnit) {
       return mValue.mURL->Equals(*aOther.mValue.mURL);
     }
@@ -436,6 +443,9 @@ void nsCSSValue::DoReset()
     DO_RELEASE(mComplexColor);
   } else if (UnitHasArrayValue()) {
     DO_RELEASE(mArray);
+  } else if (UnitHasThreadSafeArrayValue()) {
+    // ThreadSafe arrays are ok to release on any thread.
+    mValue.mThreadSafeArray->Release();
   } else if (eCSSUnit_URL == mUnit) {
     DO_RELEASE(mURL);
   } else if (eCSSUnit_Image == mUnit) {
@@ -571,6 +581,15 @@ void nsCSSValue::SetArrayValue(nsCSSValue::Array* aValue, nsCSSUnit aUnit)
   mValue.mArray->AddRef();
 }
 
+void nsCSSValue::SetThreadSafeArrayValue(nsCSSValue::ThreadSafeArray* aValue, nsCSSUnit aUnit)
+{
+  Reset();
+  mUnit = aUnit;
+  MOZ_ASSERT(UnitHasThreadSafeArrayValue(), "bad unit");
+  mValue.mThreadSafeArray = aValue;
+  mValue.mThreadSafeArray->AddRef();
+}
+
 void nsCSSValue::SetURLValue(mozilla::css::URLValue* aValue)
 {
   Reset();
@@ -1339,10 +1358,29 @@ nsCSSValue::AppendToString(nsCSSPropertyID aProperty, nsAString& aResult,
       nsStyleUtil::AppendEscapedCSSIdent(buffer, aResult);
     }
   }
-  else if (eCSSUnit_Array <= unit && unit <= eCSSUnit_Symbols) {
+  else if (eCSSUnit_Counter <= unit && unit <= eCSSUnit_Counters) {
     switch (unit) {
       case eCSSUnit_Counter:  aResult.AppendLiteral("counter(");  break;
       case eCSSUnit_Counters: aResult.AppendLiteral("counters("); break;
+      default: MOZ_ASSERT_UNREACHABLE("bad enum");
+    }
+
+    nsCSSValue::ThreadSafeArray *array = GetThreadSafeArrayValue();
+    bool mark = false;
+    for (size_t i = 0, i_end = array->Count(); i < i_end; ++i) {
+      if (mark && array->Item(i).GetUnit() != eCSSUnit_Null) {
+        aResult.AppendLiteral(", ");
+      }
+      nsCSSPropertyID prop = (i == array->Count() - 1)
+        ? eCSSProperty_list_style_type : aProperty;
+      if (array->Item(i).GetUnit() != eCSSUnit_Null) {
+        array->Item(i).AppendToString(prop, aResult, aSerialization);
+        mark = true;
+      }
+    }
+  }
+  else if (eCSSUnit_Array <= unit && unit <= eCSSUnit_Symbols) {
+    switch (unit) {
       case eCSSUnit_Cubic_Bezier: aResult.AppendLiteral("cubic-bezier("); break;
       case eCSSUnit_Steps: aResult.AppendLiteral("steps("); break;
       case eCSSUnit_Symbols: aResult.AppendLiteral("symbols("); break;
@@ -1386,10 +1424,7 @@ nsCSSValue::AppendToString(nsCSSPropertyID aProperty, nsAString& aResult,
         }
         continue;
       }
-      nsCSSPropertyID prop =
+      nsCSSPropertyID prop = aProperty;
-        ((eCSSUnit_Counter <= unit && unit <= eCSSUnit_Counters) &&
-         i == array->Count() - 1)
-        ? eCSSProperty_list_style_type : aProperty;
       if (array->Item(i).GetUnit() != eCSSUnit_Null) {
         array->Item(i).AppendToString(prop, aResult, aSerialization);
         mark = true;

layout/style/nsCSSValue.h

@@ -443,9 +443,9 @@ enum nsCSSUnit {
   eCSSUnit_Font_Format  = 16,     // (char16_t*) a font format name
   eCSSUnit_Element      = 17,     // (char16_t*) an element id
 
-  eCSSUnit_Array        = 20,     // (nsCSSValue::Array*) a list of values
+  eCSSUnit_Counter      = 20,     // (nsCSSValue::ThreadSafeArray*) a counter(string,[string]) value
-  eCSSUnit_Counter      = 21,     // (nsCSSValue::Array*) a counter(string,[string]) value
+  eCSSUnit_Counters     = 21,     // (nsCSSValue::ThreadSafeArray*) a counters(string,string[,string]) value
-  eCSSUnit_Counters     = 22,     // (nsCSSValue::Array*) a counters(string,string[,string]) value
+  eCSSUnit_Array        = 22,     // (nsCSSValue::Array*) a list of values
   eCSSUnit_Cubic_Bezier = 23,     // (nsCSSValue::Array*) a list of float values
   eCSSUnit_Steps        = 24,     // (nsCSSValue::Array*) a list of (integer, enumerated)
   eCSSUnit_Symbols      = 25,     // (nsCSSValue::Array*) a symbols(enumerated, symbols) value
@@ -582,6 +582,8 @@ class nsCSSValue {
 public:
   struct Array;
   friend struct Array;
+  struct ThreadSafeArray;
+  friend struct ThreadSafeArray;
 
   friend struct mozilla::css::URLValueData;
 
@@ -679,6 +681,8 @@ public:
 
   bool      UnitHasStringValue() const
     { return eCSSUnit_String <= mUnit && mUnit <= eCSSUnit_Element; }
+  bool      UnitHasThreadSafeArrayValue() const
+    { return eCSSUnit_Counter <= mUnit && mUnit <= eCSSUnit_Counters; }
   bool      UnitHasArrayValue() const
     { return eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Calc_Divided; }
 
@@ -783,6 +787,12 @@ public:
     return mValue.mArray;
   }
 
+  ThreadSafeArray* GetThreadSafeArrayValue() const
+  {
+    MOZ_ASSERT(UnitHasThreadSafeArrayValue(), "not a threadsafe array value");
+    return mValue.mThreadSafeArray;
+  }
+
   nsIURI* GetURLValue() const
   {
     MOZ_ASSERT(mUnit == eCSSUnit_URL || mUnit == eCSSUnit_Image,
@@ -922,6 +932,7 @@ public:
   void SetComplexColorValue(
     already_AddRefed<mozilla::css::ComplexColorValue> aValue);
   void SetArrayValue(nsCSSValue::Array* aArray, nsCSSUnit aUnit);
+  void SetThreadSafeArrayValue(nsCSSValue::ThreadSafeArray* aArray, nsCSSUnit aUnit);
   void SetURLValue(mozilla::css::URLValue* aURI);
   void SetImageValue(mozilla::css::ImageValue* aImage);
   void SetGradientValue(nsCSSValueGradient* aGradient);
@@ -1022,6 +1033,7 @@ protected:
     nscolor    mColor;
     nsIAtom* MOZ_OWNING_REF mAtom;
     Array* MOZ_OWNING_REF mArray;
+    ThreadSafeArray* MOZ_OWNING_REF mThreadSafeArray;
     mozilla::css::URLValue* MOZ_OWNING_REF mURL;
     mozilla::css::ImageValue* MOZ_OWNING_REF mImage;
     mozilla::css::GridTemplateAreasValue* MOZ_OWNING_REF mGridTemplateAreas;
@@ -1041,6 +1053,9 @@ protected:
   } mValue;
 };
 
+// We use this macro to declare equivalent logic for Array and ThreadSafeArray.
+// It would be much nicer to use a superclass, but the dynamically-sized nature
+// of the interesting part of the class makes that tricky.
 #define DECLARE_CSS_ARRAY(className, refcntMacro)                             \
 struct nsCSSValue::className final {                                          \
                                                                               \
@@ -1133,6 +1148,7 @@ private:                                                                      \
 };
 
 DECLARE_CSS_ARRAY(Array, NS_INLINE_DECL_REFCOUNTING)
+DECLARE_CSS_ARRAY(ThreadSafeArray, NS_INLINE_DECL_THREADSAFE_REFCOUNTING)
 #undef DECLARE_CSS_ARRAY
 
 // Prefer nsCSSValue::Array for lists of fixed size.

layout/style/nsComputedDOMStyle.cpp

@@ -1315,7 +1315,7 @@ nsComputedDOMStyle::DoGetContent()
           str.AppendLiteral("counters(");
         }
         // WRITE ME
-        nsCSSValue::Array* a = data.GetCounters();
+        nsCSSValue::ThreadSafeArray* a = data.GetCounters();
 
         nsStyleUtil::AppendEscapedCSSIdent(
           nsDependentString(a->Item(0).GetStringBufferValue()), str);

layout/style/nsRuleNode.cpp

@@ -9027,7 +9027,7 @@ nsRuleNode::ComputeContentData(void* aStartStruct,
           nsStyleContentType type =
             unit == eCSSUnit_Counter ? eStyleContentType_Counter
                                      : eStyleContentType_Counters;
-          data.SetCounters(type, value.GetArrayValue());
+          data.SetCounters(type, value.GetThreadSafeArrayValue());
           break;
         }
         case eCSSUnit_Enumerated:

layout/style/nsStyleStruct.h

@@ -3020,7 +3020,7 @@ public:
     return mContent.mString;
   }
 
-  nsCSSValue::Array* GetCounters() const
+  nsCSSValue::ThreadSafeArray* GetCounters() const
   {
     MOZ_ASSERT(mType == eStyleContentType_Counter ||
                mType == eStyleContentType_Counters);
@@ -3061,7 +3061,7 @@ public:
     mContent.mString = NS_strdup(aString);
   }
 
-  void SetCounters(nsStyleContentType aType, nsCSSValue::Array* aCounters)
+  void SetCounters(nsStyleContentType aType, nsCSSValue::ThreadSafeArray* aCounters)
   {
     MOZ_ASSERT(aType == eStyleContentType_Counter ||
                aType == eStyleContentType_Counters);
@@ -3094,7 +3094,9 @@ private:
   union {
     char16_t *mString;
     nsStyleImageRequest* mImage;
-    nsCSSValue::Array* mCounters;
+    // NB: We need threadsafe refcounts here to enable inheritance in the
+    // parallel style traversal.
+    nsCSSValue::ThreadSafeArray* mCounters;
   } mContent;
 };