Bug 1294299 part 8 - Refactor interface provided by css::Declaration. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Fri, 21 Oct 2016 17:54:57 +1100
changeset 432589 db19b2ba7f1bbc24700ac0df210fe55fafcd5ca5
parent 432588 ce5b3dad0fcf0ff757e0120ef33fea5ec60863f6
child 432590 de45a64c715d17421dc8a0f7717531a94f4c1f32
push id34373
push userxquan@mozilla.com
push dateWed, 02 Nov 2016 11:29:39 +0000
reviewersheycam
bugs1294299
milestone52.0a1
Bug 1294299 part 8 - Refactor interface provided by css::Declaration. r=heycam The main targets of this refactor are: 1. Move most of the logic of distinguishing properties and custom properties from nsDOMCSSDeclaration into css::Declaration, which gives ServoDeclarationBlock more flexibility to implement. 2. Rename those methods of css::Declaration to provide a clear interface which makes sense for implementing in ServoDeclarationBlock, and also avoid method overload, which can impede the forward macro, on them. MozReview-Commit-ID: 2cCqF855TVK
dom/base/nsTreeSanitizer.cpp
dom/canvas/CanvasRenderingContext2D.cpp
editor/libeditor/CSSEditUtils.cpp
layout/style/Declaration.cpp
layout/style/Declaration.h
layout/style/nsCSSParser.cpp
layout/style/nsDOMCSSDeclaration.cpp
layout/style/nsDOMCSSDeclaration.h
--- a/dom/base/nsTreeSanitizer.cpp
+++ b/dom/base/nsTreeSanitizer.cpp
@@ -1063,17 +1063,17 @@ nsTreeSanitizer::MustPrune(int32_t aName
   return false;
 }
 
 bool
 nsTreeSanitizer::SanitizeStyleDeclaration(mozilla::css::Declaration* aDeclaration,
                                           nsAutoString& aRuleText)
 {
   bool didSanitize = aDeclaration->HasProperty(eCSSProperty_binding);
-  aDeclaration->RemoveProperty(eCSSProperty_binding);
+  aDeclaration->RemovePropertyByID(eCSSProperty_binding);
   aDeclaration->ToString(aRuleText);
   return didSanitize;
 }
 
 bool
 nsTreeSanitizer::SanitizeStyleSheet(const nsAString& aOriginal,
                                     nsAString& aSanitized,
                                     nsIDocument* aDocument,
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -2681,17 +2681,17 @@ GetFontStyleContext(Element* aElement, c
 
   RefPtr<nsStyleContext> sc =
     styleSet->ResolveStyleForRules(parentContext, rules);
 
   // The font getter is required to be reserialized based on what we
   // parsed (including having line-height removed).  (Older drafts of
   // the spec required font sizes be converted to pixels, but that no
   // longer seems to be required.)
-  decl->GetValue(eCSSProperty_font, aOutUsedFont);
+  decl->GetPropertyValueByID(eCSSProperty_font, aOutUsedFont);
 
   return sc.forget();
 }
 
 static already_AddRefed<Declaration>
 CreateFilterDeclaration(const nsAString& aFilter,
                         nsINode* aNode,
                         bool* aOutFilterChanged)
--- a/editor/libeditor/CSSEditUtils.cpp
+++ b/editor/libeditor/CSSEditUtils.cpp
@@ -551,17 +551,17 @@ CSSEditUtils::GetCSSInlinePropertyBase(n
   if (decl->IsServo()) {
     MOZ_CRASH("stylo: not implemented");
     return NS_ERROR_NOT_IMPLEMENTED;
   }
   nsCSSPropertyID prop =
     nsCSSProps::LookupProperty(nsDependentAtomString(aProperty),
                                CSSEnabledState::eForAllContent);
   MOZ_ASSERT(prop != eCSSProperty_UNKNOWN);
-  decl->AsGecko()->GetValue(prop, aValue);
+  decl->AsGecko()->GetPropertyValueByID(prop, aValue);
 
   return NS_OK;
 }
 
 already_AddRefed<nsComputedDOMStyle>
 CSSEditUtils::GetComputedStyle(Element* aElement)
 {
   MOZ_ASSERT(aElement);
--- a/layout/style/Declaration.cpp
+++ b/layout/style/Declaration.cpp
@@ -111,17 +111,17 @@ Declaration::MightMapInheritedStyleData(
   }
   return mData->HasInheritedStyleData();
 }
 
 /* virtual */ bool
 Declaration::GetDiscretelyAnimatedCSSValue(nsCSSPropertyID aProperty,
                                            nsCSSValue* aValue)
 {
-  nsCSSCompressedDataBlock* data = GetValueIsImportant(aProperty)
+  nsCSSCompressedDataBlock* data = GetPropertyIsImportantByID(aProperty)
                                    ? mImportantData : mData;
   const nsCSSValue* value = data->ValueFor(aProperty);
   if (!value) {
     return false;
   }
   *aValue = *value;
   return true;
 }
@@ -146,18 +146,79 @@ Declaration::ValueAppended(nsCSSProperty
              "should only be called while expanded");
   MOZ_ASSERT(!nsCSSProps::IsShorthand(aProperty),
              "shorthands forbidden");
   // order IS important for CSS, so remove and add to the end
   mOrder.RemoveElement(static_cast<uint32_t>(aProperty));
   mOrder.AppendElement(static_cast<uint32_t>(aProperty));
 }
 
+template<typename PropFunc, typename CustomPropFunc>
+inline void
+DispatchPropertyOperation(const nsAString& aProperty,
+                          PropFunc aPropFunc, CustomPropFunc aCustomPropFunc)
+{
+  nsCSSPropertyID propID =
+    nsCSSProps::LookupProperty(aProperty, CSSEnabledState::eForAllContent);
+  if (propID != eCSSProperty_UNKNOWN) {
+    if (propID != eCSSPropertyExtra_variable) {
+      aPropFunc(propID);
+    } else {
+      aCustomPropFunc(Substring(aProperty, CSS_CUSTOM_NAME_PREFIX_LENGTH));
+    }
+  }
+}
+
 void
-Declaration::RemoveProperty(nsCSSPropertyID aProperty)
+Declaration::GetPropertyValue(const nsAString& aProperty,
+                              nsAString& aValue) const
+{
+  DispatchPropertyOperation(aProperty,
+    [&](nsCSSPropertyID propID) { GetPropertyValueByID(propID, aValue); },
+    [&](const nsAString& name) { GetVariableValue(name, aValue); });
+}
+
+void
+Declaration::GetPropertyValueByID(nsCSSPropertyID aPropID,
+                                  nsAString& aValue) const
+{
+  GetPropertyValueInternal(aPropID, aValue, nsCSSValue::eNormalized);
+}
+
+void
+Declaration::GetAuthoredPropertyValue(const nsAString& aProperty,
+                                      nsAString& aValue) const
+{
+  DispatchPropertyOperation(aProperty,
+    [&](nsCSSPropertyID propID) {
+      GetPropertyValueInternal(propID, aValue, nsCSSValue::eAuthorSpecified);
+    },
+    [&](const nsAString& name) { GetVariableValue(name, aValue); });
+}
+
+bool
+Declaration::GetPropertyIsImportant(const nsAString& aProperty) const
+{
+  bool r = false;
+  DispatchPropertyOperation(aProperty,
+    [&](nsCSSPropertyID propID) { r = GetPropertyIsImportantByID(propID); },
+    [&](const nsAString& name) { r = GetVariableIsImportant(name); });
+  return r;
+}
+
+void
+Declaration::RemoveProperty(const nsAString& aProperty)
+{
+  DispatchPropertyOperation(aProperty,
+    [&](nsCSSPropertyID propID) { RemovePropertyByID(propID); },
+    [&](const nsAString& name) { RemoveVariable(name); });
+}
+
+void
+Declaration::RemovePropertyByID(nsCSSPropertyID aProperty)
 {
   MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT);
 
   nsCSSExpandedDataBlock data;
   ExpandTo(&data);
   MOZ_ASSERT(!mData && !mImportantData, "Expand didn't null things out");
 
   if (nsCSSProps::IsShorthand(aProperty)) {
@@ -175,53 +236,41 @@ Declaration::RemoveProperty(nsCSSPropert
 }
 
 bool
 Declaration::HasProperty(nsCSSPropertyID aProperty) const
 {
   MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT_no_shorthands,
              "property ID out of range");
 
-  nsCSSCompressedDataBlock *data = GetValueIsImportant(aProperty)
+  nsCSSCompressedDataBlock *data = GetPropertyIsImportantByID(aProperty)
                                       ? mImportantData : mData;
   const nsCSSValue *val = data->ValueFor(aProperty);
   return !!val;
 }
 
 bool
 Declaration::AppendValueToString(nsCSSPropertyID aProperty,
                                  nsAString& aResult,
                                  nsCSSValue::Serialization aSerialization) const
 {
   MOZ_ASSERT(0 <= aProperty && aProperty < eCSSProperty_COUNT_no_shorthands,
              "property ID out of range");
 
-  nsCSSCompressedDataBlock *data = GetValueIsImportant(aProperty)
+  nsCSSCompressedDataBlock *data = GetPropertyIsImportantByID(aProperty)
                                       ? mImportantData : mData;
   const nsCSSValue *val = data->ValueFor(aProperty);
   if (!val) {
     return false;
   }
 
   val->AppendToString(aProperty, aResult, aSerialization);
   return true;
 }
 
-void
-Declaration::GetValue(nsCSSPropertyID aProperty, nsAString& aValue) const
-{
-  GetValue(aProperty, aValue, nsCSSValue::eNormalized);
-}
-
-void
-Declaration::GetAuthoredValue(nsCSSPropertyID aProperty, nsAString& aValue) const
-{
-  GetValue(aProperty, aValue, nsCSSValue::eAuthorSpecified);
-}
-
 static void
 AppendSingleImageLayerPositionValue(const nsCSSValue& aPositionX,
                                     const nsCSSValue& aPositionY,
                                     const nsCSSPropertyID aTable[],
                                     nsAString& aValue,
                                     nsCSSValue::Serialization aSerialization)
 {
   // We need to make sure that we don't serialize to an invalid 3-value form.
@@ -484,18 +533,19 @@ Declaration::GetImageLayerPositionValue(
       return;
     }
     aValue.Append(char16_t(','));
     aValue.Append(char16_t(' '));
   }
 }
 
 void
-Declaration::GetValue(nsCSSPropertyID aProperty, nsAString& aValue,
-                      nsCSSValue::Serialization aSerialization) const
+Declaration::GetPropertyValueInternal(
+    nsCSSPropertyID aProperty, nsAString& aValue,
+    nsCSSValue::Serialization aSerialization) const
 {
   aValue.Truncate(0);
 
   // simple properties are easy.
   if (!nsCSSProps::IsShorthand(aProperty)) {
     AppendValueToString(aProperty, aValue, aSerialization);
     return;
   }
@@ -1495,33 +1545,17 @@ Declaration::GetValue(nsCSSPropertyID aP
       break;
     default:
       MOZ_ASSERT(false, "no other shorthands");
       break;
   }
 }
 
 bool
-Declaration::GetValueIsImportant(const nsAString& aProperty) const
-{
-  nsCSSPropertyID propID = nsCSSProps::
-    LookupProperty(aProperty, CSSEnabledState::eIgnoreEnabledState);
-  if (propID == eCSSProperty_UNKNOWN) {
-    return false;
-  }
-  if (propID == eCSSPropertyExtra_variable) {
-    const nsSubstring& variableName =
-      Substring(aProperty, CSS_CUSTOM_NAME_PREFIX_LENGTH);
-    return GetVariableValueIsImportant(variableName);
-  }
-  return GetValueIsImportant(propID);
-}
-
-bool
-Declaration::GetValueIsImportant(nsCSSPropertyID aProperty) const
+Declaration::GetPropertyIsImportantByID(nsCSSPropertyID aProperty) const
 {
   if (!mImportantData)
     return false;
 
   // Calling ValueFor is inefficient, but we can assume '!important' is rare.
 
   if (!nsCSSProps::IsShorthand(aProperty)) {
     return mImportantData->ValueFor(aProperty) != nullptr;
@@ -1550,17 +1584,17 @@ Declaration::AppendPropertyAndValueToStr
   MOZ_ASSERT((aProperty < eCSSProperty_COUNT_no_shorthands) == aValue.IsEmpty(),
              "aValue should be given for shorthands but not longhands");
   AppendASCIItoUTF16(nsCSSProps::GetStringValue(aProperty), aResult);
   aResult.AppendLiteral(": ");
   if (aValue.IsEmpty())
     AppendValueToString(aProperty, aResult, nsCSSValue::eNormalized);
   else
     aResult.Append(aValue);
-  if (GetValueIsImportant(aProperty)) {
+  if (GetPropertyIsImportantByID(aProperty)) {
     aResult.AppendLiteral(" ! important");
   }
   aResult.AppendLiteral("; ");
 }
 
 void
 Declaration::AppendVariableAndValueToString(const nsAString& aName,
                                             nsAString& aResult) const
@@ -1617,17 +1651,18 @@ Declaration::AppendVariableAndValueToStr
 void
 Declaration::ToString(nsAString& aString) const
 {
   // Someone cares about this declaration's contents, so don't let it
   // change from under them.  See e.g. bug 338679.
   SetImmutable();
 
   nsCSSCompressedDataBlock *systemFontData =
-    GetValueIsImportant(eCSSProperty__x_system_font) ? mImportantData : mData;
+    GetPropertyIsImportantByID(eCSSProperty__x_system_font) ? mImportantData
+                                                            : mData;
   const nsCSSValue *systemFont =
     systemFontData->ValueFor(eCSSProperty__x_system_font);
   const bool haveSystemFont = systemFont &&
                                 systemFont->GetUnit() != eCSSUnit_None &&
                                 systemFont->GetUnit() != eCSSUnit_Null;
   bool didSystemFont = false;
 
   int32_t count = mOrder.Length();
@@ -1666,27 +1701,27 @@ Declaration::ToString(nsAString& aString
     for (const nsCSSPropertyID *shorthands =
            nsCSSProps::ShorthandsContaining(property);
          *shorthands != eCSSProperty_UNKNOWN; ++shorthands) {
       // ShorthandsContaining returns the shorthands in order from those
       // that contain the most subproperties to those that contain the
       // least, which is exactly the order we want to test them.
       nsCSSPropertyID shorthand = *shorthands;
 
-      GetValue(shorthand, value);
+      GetPropertyValueByID(shorthand, value);
 
       // in the system font case, skip over font-variant shorthand, since all
       // subproperties are already dealt with via the font shorthand
       if (shorthand == eCSSProperty_font_variant &&
           value.EqualsLiteral("-moz-use-system-font")) {
         continue;
       }
 
-      // If GetValue gives us a non-empty string back, we can use that
-      // value; otherwise it's not possible to use this shorthand.
+      // If GetPropertyValueByID gives us a non-empty string back, we can
+      // use that value; otherwise it's not possible to use this shorthand.
       if (!value.IsEmpty()) {
         AppendPropertyAndValueToString(shorthand, value, aString);
         shorthandsUsed.AppendElement(shorthand);
         doneProperty = true;
         break;
       }
 
       if (shorthand == eCSSProperty_font) {
@@ -1803,18 +1838,17 @@ Declaration::SizeOfIncludingThis(mozilla
   }
   if (mImportantVariables) {
     n += mImportantVariables->SizeOfIncludingThis(aMallocSizeOf);
   }
   return n;
 }
 
 void
-Declaration::GetVariableDeclaration(const nsAString& aName,
-                                    nsAString& aValue) const
+Declaration::GetVariableValue(const nsAString& aName, nsAString& aValue) const
 {
   aValue.Truncate();
 
   CSSVariableDeclarations::Type type;
   nsString value;
 
   if ((mImportantVariables && mImportantVariables->Get(aName, type, value)) ||
       (mVariables && mVariables->Get(aName, type, value))) {
@@ -1837,21 +1871,21 @@ Declaration::GetVariableDeclaration(cons
 
       default:
         MOZ_ASSERT(false, "unexpected variable value type");
     }
   }
 }
 
 void
-Declaration::AddVariableDeclaration(const nsAString& aName,
-                                    CSSVariableDeclarations::Type aType,
-                                    const nsString& aValue,
-                                    bool aIsImportant,
-                                    bool aOverrideImportant)
+Declaration::AddVariable(const nsAString& aName,
+                         CSSVariableDeclarations::Type aType,
+                         const nsString& aValue,
+                         bool aIsImportant,
+                         bool aOverrideImportant)
 {
   MOZ_ASSERT(IsMutable());
 
   nsTArray<nsString>::index_type index = mVariableOrder.IndexOf(aName);
   if (index == nsTArray<nsString>::NoIndex) {
     index = mVariableOrder.Length();
     mVariableOrder.AppendElement(aName);
   }
@@ -1905,30 +1939,30 @@ Declaration::AddVariableDeclaration(cons
   }
 
   uint32_t propertyIndex = index + eCSSProperty_COUNT;
   mOrder.RemoveElement(propertyIndex);
   mOrder.AppendElement(propertyIndex);
 }
 
 void
-Declaration::RemoveVariableDeclaration(const nsAString& aName)
+Declaration::RemoveVariable(const nsAString& aName)
 {
   if (mVariables) {
     mVariables->Remove(aName);
   }
   if (mImportantVariables) {
     mImportantVariables->Remove(aName);
   }
   nsTArray<nsString>::index_type index = mVariableOrder.IndexOf(aName);
   if (index != nsTArray<nsString>::NoIndex) {
     mOrder.RemoveElement(index + eCSSProperty_COUNT);
   }
 }
 
 bool
-Declaration::GetVariableValueIsImportant(const nsAString& aName) const
+Declaration::GetVariableIsImportant(const nsAString& aName) const
 {
   return mImportantVariables && mImportantVariables->Has(aName);
 }
 
 } // namespace css
 } // namespace mozilla
--- a/layout/style/Declaration.h
+++ b/layout/style/Declaration.h
@@ -116,69 +116,70 @@ public:
 
   /**
    * |ValueAppended| must be called to maintain this declaration's
    * |mOrder| whenever a property is parsed into an expanded data block
    * for this declaration.  aProperty must not be a shorthand.
    */
   void ValueAppended(nsCSSPropertyID aProperty);
 
-  void RemoveProperty(nsCSSPropertyID aProperty);
+  void GetPropertyValue(const nsAString& aProperty, nsAString& aValue) const;
+  void GetPropertyValueByID(nsCSSPropertyID aPropID, nsAString& aValue) const;
+  void GetAuthoredPropertyValue(const nsAString& aProperty,
+                                nsAString& aValue) const;
+  bool GetPropertyIsImportant(const nsAString& aProperty) const;
+  void RemoveProperty(const nsAString& aProperty);
+  void RemovePropertyByID(nsCSSPropertyID aProperty);
 
   bool HasProperty(nsCSSPropertyID aProperty) const;
 
-  void GetValue(nsCSSPropertyID aProperty, nsAString& aValue) const;
-  void GetAuthoredValue(nsCSSPropertyID aProperty, nsAString& aValue) const;
-
   bool HasImportantData() const {
     return mImportantData || mImportantVariables;
   }
-  bool GetValueIsImportant(nsCSSPropertyID aProperty) const;
-  bool GetValueIsImportant(const nsAString& aProperty) const;
 
   /**
    * Adds a custom property declaration to this object.
    *
    * @param aName The variable name (i.e., without the "--" prefix).
    * @param aType The type of value the variable has.
    * @param aValue The value of the variable, if aType is
    *   CSSVariableDeclarations::eTokenStream.
    * @param aIsImportant Whether the declaration is !important.
    * @param aOverrideImportant When aIsImportant is false, whether an
    *   existing !important declaration will be overridden.
    */
-  void AddVariableDeclaration(const nsAString& aName,
-                              CSSVariableDeclarations::Type aType,
-                              const nsString& aValue,
-                              bool aIsImportant,
-                              bool aOverrideImportant);
+  void AddVariable(const nsAString& aName,
+                   CSSVariableDeclarations::Type aType,
+                   const nsString& aValue,
+                   bool aIsImportant,
+                   bool aOverrideImportant);
 
   /**
    * Removes a custom property declaration from this object.
    *
    * @param aName The variable name (i.e., without the "--" prefix).
    */
-  void RemoveVariableDeclaration(const nsAString& aName);
+  void RemoveVariable(const nsAString& aName);
 
   /**
    * Gets the string value for a custom property declaration of a variable
    * with a given name.
    *
    * @param aName The variable name (i.e., without the "--" prefix).
    * @param aValue Out parameter into which the variable's value will be
    *   stored.  If the value is 'initial' or 'inherit', that exact string
    *   will be stored in aValue.
    */
-  void GetVariableDeclaration(const nsAString& aName, nsAString& aValue) const;
+  void GetVariableValue(const nsAString& aName, nsAString& aValue) const;
 
   /**
    * Returns whether the custom property declaration for a variable with
    * the given name was !important.
    */
-  bool GetVariableValueIsImportant(const nsAString& aName) const;
+  bool GetVariableIsImportant(const nsAString& aName) const;
 
   uint32_t Count() const {
     return mOrder.Length();
   }
 
   // Returns whether we actually had a property at aIndex
   bool GetNthProperty(uint32_t aIndex, nsAString& aReturn) const;
 
@@ -304,18 +305,20 @@ public:
     }
     return nullptr;
   }
 
 private:
   Declaration& operator=(const Declaration& aCopy) = delete;
   bool operator==(const Declaration& aCopy) const = delete;
 
-  void GetValue(nsCSSPropertyID aProperty, nsAString& aValue,
-                nsCSSValue::Serialization aValueSerialization) const;
+  void GetPropertyValueInternal(nsCSSPropertyID aProperty, nsAString& aValue,
+                                nsCSSValue::Serialization aValueSerialization)
+    const;
+  bool GetPropertyIsImportantByID(nsCSSPropertyID aProperty) const;
 
   static void AppendImportanceToString(bool aIsImportant, nsAString& aString);
   // return whether there was a value in |aValue| (i.e., it had a non-null unit)
   bool AppendValueToString(nsCSSPropertyID aProperty, nsAString& aResult) const;
   bool AppendValueToString(nsCSSPropertyID aProperty, nsAString& aResult,
                            nsCSSValue::Serialization aValueSerialization) const;
   // Helper for ToString with strange semantics regarding aValue.
   void AppendPropertyAndValueToString(nsCSSPropertyID aProperty,
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -2103,18 +2103,18 @@ CSSParserImpl::ParseVariable(const nsASt
 
   if (!parsedOK) {
     REPORT_UNEXPECTED_P(PEValueParsingError, NS_LITERAL_STRING("--") +
                                              aVariableName);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
   } else {
     CLEAR_ERROR();
-    aDeclaration->AddVariableDeclaration(aVariableName, variableType,
-                                         variableValue, aIsImportant, true);
+    aDeclaration->AddVariable(aVariableName, variableType,
+                              variableValue, aIsImportant, true);
     *aChanged = true;
   }
 
   mTempData.AssertInitialState();
 
   ReleaseScanner();
 }
 
@@ -7487,18 +7487,18 @@ CSSParserImpl::ParseDeclaration(css::Dec
     return false;
   }
 
   if (customProperty) {
     MOZ_ASSERT(Substring(propertyName, 0,
                          CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
     // remove '--'
     nsDependentString varName(propertyName, CSS_CUSTOM_NAME_PREFIX_LENGTH);
-    aDeclaration->AddVariableDeclaration(varName, variableType, variableValue,
-                                         status == ePriority_Important, false);
+    aDeclaration->AddVariable(varName, variableType, variableValue,
+                              status == ePriority_Important, false);
   } else {
     *aChanged |= mData.TransferFromBlock(mTempData, propID, EnabledState(),
                                          status == ePriority_Important,
                                          false, aMustCallValueAppended,
                                          aDeclaration, GetDocument());
   }
 
   return true;
--- a/layout/style/nsDOMCSSDeclaration.cpp
+++ b/layout/style/nsDOMCSSDeclaration.cpp
@@ -43,43 +43,23 @@ NS_INTERFACE_MAP_END
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetPropertyValue(const nsCSSPropertyID aPropID,
                                       nsAString& aValue)
 {
   NS_PRECONDITION(aPropID != eCSSProperty_UNKNOWN,
                   "Should never pass eCSSProperty_UNKNOWN around");
 
-  css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko();
-
   aValue.Truncate();
-  if (decl) {
-    decl->GetValue(aPropID, aValue);
+  if (css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko()) {
+    decl->GetPropertyValueByID(aPropID, aValue);
   }
   return NS_OK;
 }
 
-void
-nsDOMCSSDeclaration::GetCustomPropertyValue(const nsAString& aPropertyName,
-                                            nsAString& aValue)
-{
-  MOZ_ASSERT(Substring(aPropertyName, 0,
-                       CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
-
-  css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko();
-  if (!decl) {
-    aValue.Truncate();
-    return;
-  }
-
-  decl->GetVariableDeclaration(Substring(aPropertyName,
-                                         CSS_CUSTOM_NAME_PREFIX_LENGTH),
-                               aValue);
-}
-
 NS_IMETHODIMP
 nsDOMCSSDeclaration::SetPropertyValue(const nsCSSPropertyID aPropID,
                                       const nsAString& aValue)
 {
   switch (aPropID) {
     case eCSSProperty_background_position:
     case eCSSProperty_background_position_x:
     case eCSSProperty_background_position_y:
@@ -101,17 +81,17 @@ nsDOMCSSDeclaration::SetPropertyValue(co
       break;
     default:
       break;
   }
 
   if (aValue.IsEmpty()) {
     // If the new value of the property is an empty string we remove the
     // property.
-    return RemoveProperty(aPropID);
+    return RemovePropertyInternal(aPropID);
   }
 
   return ParsePropertyValue(aPropID, aValue, false);
 }
 
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetCssText(nsAString& aCssText)
@@ -197,92 +177,66 @@ nsDOMCSSDeclaration::IndexedGetter(uint3
   DeclarationBlock* decl = GetCSSDeclaration(eOperation_Read);
   aFound = decl && decl->GetNthProperty(aIndex, aPropName);
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetPropertyValue(const nsAString& aPropertyName,
                                       nsAString& aReturn)
 {
-  const nsCSSPropertyID propID =
-    nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
-  if (propID == eCSSProperty_UNKNOWN) {
-    aReturn.Truncate();
-    return NS_OK;
+  aReturn.Truncate();
+  if (css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko()) {
+    decl->GetPropertyValue(aPropertyName, aReturn);
   }
-
-  if (propID == eCSSPropertyExtra_variable) {
-    GetCustomPropertyValue(aPropertyName, aReturn);
-    return NS_OK;
-  }
-
-  return GetPropertyValue(propID, aReturn);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetAuthoredPropertyValue(const nsAString& aPropertyName,
                                               nsAString& aReturn)
 {
-  const nsCSSPropertyID propID =
-    nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
-  if (propID == eCSSProperty_UNKNOWN) {
-    aReturn.Truncate();
-    return NS_OK;
+  if (css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko()) {
+    decl->GetAuthoredPropertyValue(aPropertyName, aReturn);
   }
-
-  if (propID == eCSSPropertyExtra_variable) {
-    GetCustomPropertyValue(aPropertyName, aReturn);
-    return NS_OK;
-  }
-
-  css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko();
-  if (!decl) {
-    return NS_ERROR_FAILURE;
-  }
-
-  decl->GetAuthoredValue(propID, aReturn);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetPropertyPriority(const nsAString& aPropertyName,
                                          nsAString& aReturn)
 {
   css::Declaration* decl = GetCSSDeclaration(eOperation_Read)->AsGecko();
 
   aReturn.Truncate();
-  if (decl && decl->GetValueIsImportant(aPropertyName)) {
+  if (decl && decl->GetPropertyIsImportant(aPropertyName)) {
     aReturn.AssignLiteral("important");
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::SetProperty(const nsAString& aPropertyName,
                                  const nsAString& aValue,
                                  const nsAString& aPriority)
 {
+  if (aValue.IsEmpty()) {
+    // If the new value of the property is an empty string we remove the
+    // property.
+    // XXX this ignores the priority string, should it?
+    return RemovePropertyInternal(aPropertyName);
+  }
+
   // In the common (and fast) cases we can use the property id
   nsCSSPropertyID propID =
     nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
   if (propID == eCSSProperty_UNKNOWN) {
     return NS_OK;
   }
 
-  if (aValue.IsEmpty()) {
-    // If the new value of the property is an empty string we remove the
-    // property.
-    // XXX this ignores the priority string, should it?
-    if (propID == eCSSPropertyExtra_variable) {
-      return RemoveCustomProperty(aPropertyName);
-    }
-    return RemoveProperty(propID);
-  }
-
   bool important;
   if (aPriority.IsEmpty()) {
     important = false;
   } else if (aPriority.EqualsLiteral("important")) {
     important = true;
   } else {
     // XXX silent failure?
     return NS_OK;
@@ -293,32 +247,19 @@ nsDOMCSSDeclaration::SetProperty(const n
   }
   return ParsePropertyValue(propID, aValue, important);
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::RemoveProperty(const nsAString& aPropertyName,
                                     nsAString& aReturn)
 {
-  const nsCSSPropertyID propID =
-    nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent);
-  if (propID == eCSSProperty_UNKNOWN) {
-    aReturn.Truncate();
-    return NS_OK;
-  }
-
-  if (propID == eCSSPropertyExtra_variable) {
-    RemoveCustomProperty(aPropertyName);
-    return NS_OK;
-  }
-
-  nsresult rv = GetPropertyValue(propID, aReturn);
+  nsresult rv = GetPropertyValue(aPropertyName, aReturn);
   NS_ENSURE_SUCCESS(rv, rv);
-
-  return RemoveProperty(propID);
+  return RemovePropertyInternal(aPropertyName);
 }
 
 /* static */ void
 nsDOMCSSDeclaration::GetCSSParsingEnvironmentForRule(css::Rule* aRule,
                                                      CSSParsingEnvironment& aCSSParseEnv)
 {
   CSSStyleSheet* sheet = aRule ? aRule->GetStyleSheet() : nullptr;
   if (!sheet) {
@@ -406,52 +347,48 @@ nsDOMCSSDeclaration::ParseCustomProperty
     // Parsing failed -- but we don't throw an exception for that.
     return NS_OK;
   }
 
   return SetCSSDeclaration(decl);
 }
 
 nsresult
-nsDOMCSSDeclaration::RemoveProperty(const nsCSSPropertyID aPropID)
+nsDOMCSSDeclaration::RemovePropertyInternal(nsCSSPropertyID aPropID)
 {
   css::Declaration* olddecl =
     GetCSSDeclaration(eOperation_RemoveProperty)->AsGecko();
   if (!olddecl) {
     return NS_OK; // no decl, so nothing to remove
   }
 
   // For nsDOMCSSAttributeDeclaration, SetCSSDeclaration will lead to
   // Attribute setting code, which leads in turn to BeginUpdate.  We
   // need to start the update now so that the old rule doesn't get used
   // between when we mutate the declaration and when we set the new
   // rule (see stack in bug 209575).
   mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), true);
 
   RefPtr<css::Declaration> decl = olddecl->EnsureMutable();
-  decl->RemoveProperty(aPropID);
+  decl->RemovePropertyByID(aPropID);
   return SetCSSDeclaration(decl);
 }
 
 nsresult
-nsDOMCSSDeclaration::RemoveCustomProperty(const nsAString& aPropertyName)
+nsDOMCSSDeclaration::RemovePropertyInternal(const nsAString& aPropertyName)
 {
-  MOZ_ASSERT(Substring(aPropertyName, 0,
-                       CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
-
   css::Declaration* olddecl =
     GetCSSDeclaration(eOperation_RemoveProperty)->AsGecko();
   if (!olddecl) {
     return NS_OK; // no decl, so nothing to remove
   }
 
   // For nsDOMCSSAttributeDeclaration, SetCSSDeclaration will lead to
   // Attribute setting code, which leads in turn to BeginUpdate.  We
   // need to start the update now so that the old rule doesn't get used
   // between when we mutate the declaration and when we set the new
   // rule (see stack in bug 209575).
   mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), true);
 
   RefPtr<css::Declaration> decl = olddecl->EnsureMutable();
-  decl->RemoveVariableDeclaration(Substring(aPropertyName,
-                                            CSS_CUSTOM_NAME_PREFIX_LENGTH));
+  decl->RemoveProperty(aPropertyName);
   return SetCSSDeclaration(decl);
 }
--- a/layout/style/nsDOMCSSDeclaration.h
+++ b/layout/style/nsDOMCSSDeclaration.h
@@ -155,23 +155,20 @@ protected:
   // an css::Rule.
   static void GetCSSParsingEnvironmentForRule(mozilla::css::Rule* aRule,
                                               CSSParsingEnvironment& aCSSParseEnv);
 
   nsresult ParsePropertyValue(const nsCSSPropertyID aPropID,
                               const nsAString& aPropValue,
                               bool aIsImportant);
 
-  // Prop-id based version of RemoveProperty.  Note that this does not
-  // return the old value; it just does a straight removal.
-  nsresult RemoveProperty(const nsCSSPropertyID aPropID);
-
-  void GetCustomPropertyValue(const nsAString& aPropertyName, nsAString& aValue);
-  nsresult RemoveCustomProperty(const nsAString& aPropertyName);
   nsresult ParseCustomPropertyValue(const nsAString& aPropertyName,
                                     const nsAString& aPropValue,
                                     bool aIsImportant);
 
+  nsresult RemovePropertyInternal(nsCSSPropertyID aPropID);
+  nsresult RemovePropertyInternal(const nsAString& aProperty);
+
 protected:
   virtual ~nsDOMCSSDeclaration();
 };
 
 #endif // nsDOMCSSDeclaration_h___