Bug 1497986 - Some more nsCSSValue cleanup. r=mats
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 10 Oct 2018 20:26:45 +0000
changeset 496353 c51832c62e768f6089510fb8c2736957eff811f9
parent 496352 96ae9247bb0f2678cd44b15740da601de25c39ae
child 496354 fee847bb2f45afbe782ffec903004a64ed2169c9
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1497986
milestone64.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 1497986 - Some more nsCSSValue cleanup. r=mats I had this around, I couldn't work on more stuff today, but I may as well land this. Differential Revision: https://phabricator.services.mozilla.com/D8277
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
servo/components/style/gecko_bindings/sugar/ns_css_value.rs
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -76,23 +76,16 @@ nsCSSValue::nsCSSValue(const nsString& a
 nsCSSValue::nsCSSValue(nsCSSValue::Array* aValue, nsCSSUnit aUnit)
   : mUnit(aUnit)
 {
   MOZ_ASSERT(UnitHasArrayValue(), "bad unit");
   mValue.mArray = aValue;
   mValue.mArray->AddRef();
 }
 
-nsCSSValue::nsCSSValue(SharedFontList* aValue)
-  : mUnit(eCSSUnit_FontFamilyList)
-{
-  mValue.mFontFamilyList = aValue;
-  mValue.mFontFamilyList->AddRef();
-}
-
 nsCSSValue::nsCSSValue(const nsCSSValue& aCopy)
   : mUnit(aCopy.mUnit)
 {
   if (mUnit <= eCSSUnit_DummyInherit) {
     // nothing to do, but put this important case first
   }
   else if (eCSSUnit_Percent <= mUnit) {
     mValue.mFloat = aCopy.mValue.mFloat;
@@ -112,34 +105,24 @@ nsCSSValue::nsCSSValue(const nsCSSValue&
   else if (eCSSUnit_Pair == mUnit) {
     mValue.mPair = aCopy.mValue.mPair;
     mValue.mPair->AddRef();
   }
   else if (eCSSUnit_List == mUnit) {
     mValue.mList = aCopy.mValue.mList;
     mValue.mList->AddRef();
   }
-  else if (eCSSUnit_ListDep == mUnit) {
-    mValue.mListDependent = aCopy.mValue.mListDependent;
-  }
   else if (eCSSUnit_SharedList == mUnit) {
     mValue.mSharedList = aCopy.mValue.mSharedList;
     mValue.mSharedList->AddRef();
   }
   else if (eCSSUnit_PairList == mUnit) {
     mValue.mPairList = aCopy.mValue.mPairList;
     mValue.mPairList->AddRef();
   }
-  else if (eCSSUnit_PairListDep == mUnit) {
-    mValue.mPairListDependent = aCopy.mValue.mPairListDependent;
-  }
-  else if (eCSSUnit_FontFamilyList == mUnit) {
-    mValue.mFontFamilyList = aCopy.mValue.mFontFamilyList;
-    mValue.mFontFamilyList->AddRef();
-  }
   else if (eCSSUnit_AtomIdent == mUnit) {
     mValue.mAtom = aCopy.mValue.mAtom;
     mValue.mAtom->AddRef();
   }
   else {
     MOZ_ASSERT(false, "unknown unit");
   }
 }
@@ -163,22 +146,16 @@ nsCSSValue::operator=(nsCSSValue&& aOthe
   mValue = aOther.mValue;
   aOther.mUnit = eCSSUnit_Null;
 
   return *this;
 }
 
 bool nsCSSValue::operator==(const nsCSSValue& aOther) const
 {
-  MOZ_ASSERT(mUnit != eCSSUnit_ListDep &&
-             aOther.mUnit != eCSSUnit_ListDep &&
-             mUnit != eCSSUnit_PairListDep &&
-             aOther.mUnit != eCSSUnit_PairListDep,
-             "don't use operator== with dependent lists");
-
   if (mUnit == aOther.mUnit) {
     if (mUnit <= eCSSUnit_DummyInherit) {
       return true;
     }
     else if (UnitHasStringValue()) {
       return (NS_strcmp(GetBufferValue(mValue.mString),
                         GetBufferValue(aOther.mValue.mString)) == 0);
     }
@@ -196,20 +173,16 @@ bool nsCSSValue::operator==(const nsCSSV
     }
     else if (eCSSUnit_SharedList == mUnit) {
       return *mValue.mSharedList == *aOther.mValue.mSharedList;
     }
     else if (eCSSUnit_PairList == mUnit) {
       return nsCSSValuePairList::Equal(mValue.mPairList,
                                        aOther.mValue.mPairList);
     }
-    else if (eCSSUnit_FontFamilyList == mUnit) {
-      return mValue.mFontFamilyList->mNames ==
-             aOther.mValue.mFontFamilyList->mNames;
-    }
     else if (eCSSUnit_AtomIdent == mUnit) {
       return mValue.mAtom == aOther.mValue.mAtom;
     }
     else {
       return mValue.mFloat == aOther.mValue.mFloat;
     }
   }
   return false;
@@ -289,18 +262,16 @@ void nsCSSValue::DoReset()
   } else if (eCSSUnit_Pair == mUnit) {
     DO_RELEASE(mPair);
   } else if (eCSSUnit_List == mUnit) {
     DO_RELEASE(mList);
   } else if (eCSSUnit_SharedList == mUnit) {
     DO_RELEASE(mSharedList);
   } else if (eCSSUnit_PairList == mUnit) {
     DO_RELEASE(mPairList);
-  } else if (eCSSUnit_FontFamilyList == mUnit) {
-    DO_RELEASE(mFontFamilyList);
   } else if (eCSSUnit_AtomIdent == mUnit) {
     DO_RELEASE(mAtom);
   }
   mUnit = eCSSUnit_Null;
 }
 
 #undef DO_RELEASE
 
@@ -587,58 +558,39 @@ nsCSSValue::SizeOfExcludingThis(mozilla:
     case eCSSUnit_Counter:
     case eCSSUnit_Counters:
     case eCSSUnit_Cubic_Bezier:
     case eCSSUnit_Steps:
     case eCSSUnit_Symbols:
     case eCSSUnit_Function:
     case eCSSUnit_Calc:
     case eCSSUnit_Calc_Plus:
-    case eCSSUnit_Calc_Minus:
-    case eCSSUnit_Calc_Times_L:
-    case eCSSUnit_Calc_Times_R:
-    case eCSSUnit_Calc_Divided:
       break;
 
     // Pair
     case eCSSUnit_Pair:
       n += mValue.mPair->SizeOfIncludingThis(aMallocSizeOf);
       break;
 
     // List
     case eCSSUnit_List:
       n += mValue.mList->SizeOfIncludingThis(aMallocSizeOf);
       break;
 
-    // ListDep: not measured because it's non-owning.
-    case eCSSUnit_ListDep:
-      break;
-
     // SharedList
     case eCSSUnit_SharedList:
       // Makes more sense not to measure, since it most cases the list
       // will be shared.
       break;
 
     // PairList
     case eCSSUnit_PairList:
       n += mValue.mPairList->SizeOfIncludingThis(aMallocSizeOf);
       break;
 
-    // PairListDep: not measured because it's non-owning.
-    case eCSSUnit_PairListDep:
-      break;
-
-    case eCSSUnit_FontFamilyList:
-      // The SharedFontList is a refcounted object, but is unique per
-      // declaration. We don't measure the references from computed
-      // values.
-      n += mValue.mFontFamilyList->SizeOfIncludingThis(aMallocSizeOf);
-      break;
-
     // Atom is always shared, and thus should not be counted.
     case eCSSUnit_AtomIdent:
       break;
 
     // Int: nothing extra to measure.
     case eCSSUnit_Integer:
     case eCSSUnit_Enumerated:
       break;
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -290,32 +290,22 @@ enum nsCSSUnit {
   // eCSSUnit_Calc has an array with exactly 1 element.  eCSSUnit_Calc
   // exists so we can distinguish calc(2em) from 2em as specified values
   // (but we drop this distinction for nsStyleCoord when we store
   // computed values).
   eCSSUnit_Calc         = 30,     // (nsCSSValue::Array*) calc() value
   // Plus, Minus, Times_* and Divided have arrays with exactly 2
   // elements.  a + b + c + d is grouped as ((a + b) + c) + d
   eCSSUnit_Calc_Plus    = 31,     // (nsCSSValue::Array*) + node within calc()
-  eCSSUnit_Calc_Minus   = 32,     // (nsCSSValue::Array*) - within calc
-  eCSSUnit_Calc_Times_L = 33,     // (nsCSSValue::Array*) num * val within calc
-  eCSSUnit_Calc_Times_R = 34,     // (nsCSSValue::Array*) val * num within calc
-  eCSSUnit_Calc_Divided = 35,     // (nsCSSValue::Array*) / within calc
 
   eCSSUnit_Pair         = 50,     // (nsCSSValuePair*) pair of values
   eCSSUnit_List         = 53,     // (nsCSSValueList*) list of values
-  eCSSUnit_ListDep      = 54,     // (nsCSSValueList*) same as List
-                                  //   but does not own the list
   eCSSUnit_SharedList   = 55,     // (nsCSSValueSharedList*) same as list
                                   //   but reference counted and shared
   eCSSUnit_PairList     = 56,     // (nsCSSValuePairList*) list of value pairs
-  eCSSUnit_PairListDep  = 57,     // (nsCSSValuePairList*) same as PairList
-                                  //   but does not own the list
-
-  eCSSUnit_FontFamilyList = 58,   // (SharedFontList*) value
 
   // Atom units
   eCSSUnit_AtomIdent    = 60,     // (nsAtom*) for its string as an identifier
 
   eCSSUnit_Integer      = 70,     // (int) simple value
   eCSSUnit_Enumerated   = 71,     // (int) value has enumerated meaning
 
   eCSSUnit_Percent      = 100,     // (float) 1.0 == 100%) value is percentage of something
@@ -380,17 +370,16 @@ public:
   {
     MOZ_ASSERT(aUnit <= eCSSUnit_DummyInherit, "not a valueless unit");
   }
 
   nsCSSValue(int32_t aValue, nsCSSUnit aUnit);
   nsCSSValue(float aValue, nsCSSUnit aUnit);
   nsCSSValue(const nsString& aValue, nsCSSUnit aUnit);
   nsCSSValue(Array* aArray, nsCSSUnit aUnit);
-  explicit nsCSSValue(mozilla::SharedFontList* aValue);
   nsCSSValue(const nsCSSValue& aCopy);
   nsCSSValue(nsCSSValue&& aOther)
     : mUnit(aOther.mUnit)
     , mValue(aOther.mValue)
   {
     aOther.mUnit = eCSSUnit_Null;
   }
   template<typename T,
@@ -445,22 +434,22 @@ public:
     { return eCSSUnit_Number <= aUnit; }
   bool      IsAngularUnit() const
     { return eCSSUnit_Degree <= mUnit && mUnit <= eCSSUnit_Turn; }
   bool      IsFrequencyUnit() const
     { return eCSSUnit_Hertz <= mUnit && mUnit <= eCSSUnit_Kilohertz; }
   bool      IsTimeUnit() const
     { return eCSSUnit_Seconds <= mUnit && mUnit <= eCSSUnit_Milliseconds; }
   bool      IsCalcUnit() const
-    { return eCSSUnit_Calc <= mUnit && mUnit <= eCSSUnit_Calc_Divided; }
+    { return eCSSUnit_Calc <= mUnit && mUnit <= eCSSUnit_Calc_Plus; }
 
   bool      UnitHasStringValue() const
     { return eCSSUnit_String <= mUnit && mUnit <= eCSSUnit_Element; }
   bool      UnitHasArrayValue() const
-    { return eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Calc_Divided; }
+    { return eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Calc_Plus; }
 
   int32_t GetIntValue() const
   {
     MOZ_ASSERT(mUnit == eCSSUnit_Integer ||
                mUnit == eCSSUnit_Enumerated,
                "not an int value");
     return mValue.mInt;
   }
@@ -519,25 +508,16 @@ public:
   }
 
   nsCSSValueSharedList* GetSharedListValue() const
   {
     MOZ_ASSERT(mUnit == eCSSUnit_SharedList, "not a shared list value");
     return mValue.mSharedList;
   }
 
-  mozilla::NotNull<mozilla::SharedFontList*> GetFontFamilyListValue() const
-  {
-    MOZ_ASSERT(mUnit == eCSSUnit_FontFamilyList,
-               "not a font family list value");
-    NS_ASSERTION(mValue.mFontFamilyList != nullptr,
-                 "font family list value should never be null");
-    return mozilla::WrapNotNull(mValue.mFontFamilyList);
-  }
-
   // bodies of these are below
   inline nsCSSValuePair& GetPairValue();
   inline const nsCSSValuePair& GetPairValue() const;
 
   inline nsCSSValueList* GetListValue();
   inline const nsCSSValueList* GetListValue() const;
 
   inline nsCSSValuePairList* GetPairListValue();
@@ -580,17 +560,16 @@ public:
   }
   void SetPercentValue(float aValue);
   void SetFloatValue(float aValue, nsCSSUnit aUnit);
   void SetStringValue(const nsString& aValue, nsCSSUnit aUnit);
   void SetAtomIdentValue(already_AddRefed<nsAtom> aValue);
   // converts the nscoord to pixels
   void SetIntegerCoordValue(nscoord aCoord);
   void SetArrayValue(nsCSSValue::Array* aArray, nsCSSUnit aUnit);
-  void SetFontFamilyListValue(already_AddRefed<mozilla::SharedFontList> aFontListValue);
   void SetPairValue(const nsCSSValuePair* aPair);
   void SetPairValue(const nsCSSValue& xValue, const nsCSSValue& yValue);
   void SetSharedListValue(nsCSSValueSharedList* aList);
   void SetNoneValue();
 
   nsStyleCoord::CalcValue GetCalcValue() const;
   void SetCalcValue(const nsStyleCoord::CalcValue&);
 
@@ -599,19 +578,16 @@ public:
   nsCSSValueList* SetListValue();
   nsCSSValuePairList* SetPairListValue();
 
   // Returns an already addrefed buffer.  Guaranteed to return non-null.
   // (Will abort on allocation failure.)
   static already_AddRefed<nsStringBuffer>
     BufferFromString(const nsString& aValue);
 
-  // Convert the given Ident value into AtomIdent.
-  void AtomizeIdentValue();
-
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   static void
   AppendAlignJustifyValueToString(int32_t aValue, nsAString& aResult);
 
 private:
   static const char16_t* GetBufferValue(nsStringBuffer* aBuffer) {
     return static_cast<char16_t*>(aBuffer->Data());
@@ -624,21 +600,18 @@ protected:
     float      mFloat;
     // Note: the capacity of the buffer may exceed the length of the string.
     // If we're of a string type, mString is not null.
     nsStringBuffer* MOZ_OWNING_REF mString;
     nsAtom* MOZ_OWNING_REF mAtom;
     Array* MOZ_OWNING_REF mArray;
     nsCSSValuePair_heap* MOZ_OWNING_REF mPair;
     nsCSSValueList_heap* MOZ_OWNING_REF mList;
-    nsCSSValueList* mListDependent;
     nsCSSValueSharedList* MOZ_OWNING_REF mSharedList;
     nsCSSValuePairList_heap* MOZ_OWNING_REF mPairList;
-    nsCSSValuePairList* mPairListDependent;
-    mozilla::SharedFontList* MOZ_OWNING_REF mFontFamilyList;
   } mValue;
 };
 
 struct nsCSSValue::Array final {
 
   // return |Array| with reference count of zero
   static Array* Create(size_t aItemCount) {
     return new (aItemCount) Array(aItemCount);
@@ -795,38 +768,28 @@ public:
   bool operator!=(const nsCSSValueSharedList& aOther) const
   { return !(*this == aOther); }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   nsCSSValueList* mHead;
 };
 
-// This has to be here so that the relationship between nsCSSValueList
-// and nsCSSValueList_heap is visible.
 inline nsCSSValueList*
 nsCSSValue::GetListValue()
 {
-  if (mUnit == eCSSUnit_List)
-    return mValue.mList;
-  else {
-    MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a list value");
-    return mValue.mListDependent;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(mUnit == eCSSUnit_List, "not a list value");
+  return mValue.mList;
 }
 
 inline const nsCSSValueList*
 nsCSSValue::GetListValue() const
 {
-  if (mUnit == eCSSUnit_List)
-    return mValue.mList;
-  else {
-    MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a list value");
-    return mValue.mListDependent;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(mUnit == eCSSUnit_List, "not a list value");
+  return mValue.mList;
 }
 
 struct nsCSSValuePair {
   nsCSSValuePair()
   {
     MOZ_COUNT_CTOR(nsCSSValuePair);
   }
   explicit nsCSSValuePair(nsCSSUnit aUnit)
@@ -972,29 +935,21 @@ private:
   }
 };
 
 // This has to be here so that the relationship between nsCSSValuePairList
 // and nsCSSValuePairList_heap is visible.
 inline nsCSSValuePairList*
 nsCSSValue::GetPairListValue()
 {
-  if (mUnit == eCSSUnit_PairList)
-    return mValue.mPairList;
-  else {
-    MOZ_ASSERT (mUnit == eCSSUnit_PairListDep, "not a pairlist value");
-    return mValue.mPairListDependent;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(mUnit == eCSSUnit_PairList, "not a pairlist value");
+  return mValue.mPairList;
 }
 
 inline const nsCSSValuePairList*
 nsCSSValue::GetPairListValue() const
 {
-  if (mUnit == eCSSUnit_PairList)
-    return mValue.mPairList;
-  else {
-    MOZ_ASSERT (mUnit == eCSSUnit_PairListDep, "not a pairlist value");
-    return mValue.mPairListDependent;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(mUnit == eCSSUnit_PairList, "not a pairlist value");
+  return mValue.mPairList;
 }
 
 #endif /* nsCSSValue_h___ */
 
--- a/servo/components/style/gecko_bindings/sugar/ns_css_value.rs
+++ b/servo/components/style/gecko_bindings/sugar/ns_css_value.rs
@@ -55,17 +55,17 @@ impl nsCSSValue {
         unsafe { *self.mValue.mFloat.as_ref() }
     }
 
     /// Returns this nsCSSValue as a nsCSSValue::Array, unchecked in release
     /// builds.
     pub unsafe fn array_unchecked(&self) -> &nsCSSValue_Array {
         debug_assert!(
             nsCSSUnit::eCSSUnit_Array as u32 <= self.mUnit as u32 &&
-                self.mUnit as u32 <= nsCSSUnit::eCSSUnit_Calc_Divided as u32
+                self.mUnit as u32 <= nsCSSUnit::eCSSUnit_Calc_Plus as u32
         );
         let array = *self.mValue.mArray.as_ref();
         debug_assert!(!array.is_null());
         &*array
     }
 
     /// Sets LengthOrPercentage value to this nsCSSValue.
     pub unsafe fn set_lop(&mut self, lop: LengthOrPercentage) {