Bug 1458458: Cleanup a bit nsDOMCSSAttrDeclaration. r?hiro draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 02 May 2018 09:40:48 +0200
changeset 790383 9b773aca5f1bae27f399dadcd225a4c79e86465c
parent 790382 d1405d9c624fd6bffeb3c6b065c8b27df264ca28
push id108505
push userbmo:emilio@crisal.io
push dateWed, 02 May 2018 07:45:00 +0000
reviewershiro
bugs1458458, 1457353
milestone61.0a1
Bug 1458458: Cleanup a bit nsDOMCSSAttrDeclaration. r?hiro In preparation of removing the serialization roundtrip that SMIL does, which is blocking bug 1457353. Move the SMIL code in there, since we're going to mess with it more in a second. After this we can use Uncompute instead of nsSMILCSSValueType in order to avoid trying to serialize complex colors. MozReview-Commit-ID: GhCyJrXdfZY
dom/smil/moz.build
dom/smil/nsSMILCSSProperty.cpp
layout/style/nsDOMCSSAttrDeclaration.cpp
layout/style/nsDOMCSSAttrDeclaration.h
layout/style/nsDOMCSSDeclaration.cpp
layout/style/nsDOMCSSDeclaration.h
--- a/dom/smil/moz.build
+++ b/dom/smil/moz.build
@@ -11,16 +11,17 @@ MOCHITEST_MANIFESTS += ['test/mochitest.
 
 EXPORTS += [
     'nsISMILAttr.h',
     'nsISMILType.h',
     'nsSMILAnimationController.h',
     'nsSMILAnimationFunction.h',
     'nsSMILCompositorTable.h',
     'nsSMILCSSProperty.h',
+    'nsSMILCSSValueType.h',
     'nsSMILInstanceTime.h',
     'nsSMILInterval.h',
     'nsSMILKeySpline.h',
     'nsSMILMilestone.h',
     'nsSMILNullType.h',
     'nsSMILRepeatCount.h',
     'nsSMILSetAnimationFunction.h',
     'nsSMILTargetIdentifier.h',
--- a/dom/smil/nsSMILCSSProperty.cpp
+++ b/dom/smil/nsSMILCSSProperty.cpp
@@ -102,32 +102,17 @@ nsSMILCSSProperty::ValueFromString(const
   }
   return NS_OK;
 }
 
 nsresult
 nsSMILCSSProperty::SetAnimValue(const nsSMILValue& aValue)
 {
   NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID), NS_ERROR_FAILURE);
-
-  // Convert nsSMILValue to string
-  nsAutoString valStr;
-  nsSMILCSSValueType::ValueToString(aValue, valStr);
-
-  // Use string value to style the target element
-  nsDOMCSSAttributeDeclaration* overrideDecl = mElement->GetSMILOverrideStyle();
-  if (overrideDecl) {
-    nsAutoString oldValStr;
-    overrideDecl->GetPropertyValue(mPropID, oldValStr);
-    if (valStr.Equals(oldValStr)) {
-      return NS_OK;
-    }
-    overrideDecl->SetPropertyValue(mPropID, valStr, nullptr);
-  }
-  return NS_OK;
+  return mElement->GetSMILOverrideStyle()->SetSMILValue(mPropID, aValue);
 }
 
 void
 nsSMILCSSProperty::ClearAnimValue()
 {
   // Put empty string in override style for our property
   nsDOMCSSAttributeDeclaration* overrideDecl = mElement->GetSMILOverrideStyle();
   if (overrideDecl) {
--- a/layout/style/nsDOMCSSAttrDeclaration.cpp
+++ b/layout/style/nsDOMCSSAttrDeclaration.cpp
@@ -13,16 +13,17 @@
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/MutationEventBinding.h"
 #include "mozilla/InternalMutationEvent.h"
 #include "mozilla/ServoDeclarationBlock.h"
 #include "nsContentUtils.h"
 #include "nsIDocument.h"
 #include "nsIURI.h"
 #include "nsNodeUtils.h"
+#include "nsSMILCSSValueType.h"
 #include "nsWrapperCacheInlines.h"
 #include "nsIFrame.h"
 #include "ActiveLayerTracker.h"
 
 using namespace mozilla;
 
 nsDOMCSSAttributeDeclaration::nsDOMCSSAttributeDeclaration(dom::Element* aElement,
                                                            bool aIsSMILOverride)
@@ -175,26 +176,34 @@ nsDOMCSSAttributeDeclaration::GetServoCS
 {
   return {
     mElement->GetURLDataForStyleAttr(aSubjectPrincipal),
     mElement->OwnerDoc()->GetCompatibilityMode(),
     mElement->OwnerDoc()->CSSLoader(),
   };
 }
 
-css::Rule*
-nsDOMCSSAttributeDeclaration::GetParentRule()
+nsresult
+nsDOMCSSAttributeDeclaration::SetSMILValue(const nsCSSPropertyID aPropID,
+                                           const nsSMILValue& aValue)
 {
-  return nullptr;
-}
+  MOZ_ASSERT(mIsSMILOverride);
 
-/* virtual */ nsINode*
-nsDOMCSSAttributeDeclaration::GetParentObject()
-{
-  return mElement;
+  // Convert nsSMILValue to string.
+  //
+  // FIXME(emilio): This roundtrip should go away.
+  nsAutoString valStr;
+  nsSMILCSSValueType::ValueToString(aValue, valStr);
+
+  nsAutoString oldValStr;
+  GetPropertyValue(aPropID, oldValStr);
+  if (valStr.Equals(oldValStr)) {
+    return NS_OK;
+  }
+  return SetPropertyValue(aPropID, valStr, nullptr);
 }
 
 nsresult
 nsDOMCSSAttributeDeclaration::SetPropertyValue(const nsCSSPropertyID aPropID,
                                                const nsAString& aValue,
                                                nsIPrincipal* aSubjectPrincipal)
 {
   // Scripted modifications to style.opacity or style.transform
--- a/layout/style/nsDOMCSSAttrDeclaration.h
+++ b/layout/style/nsDOMCSSAttrDeclaration.h
@@ -9,16 +9,17 @@
 #ifndef nsDOMCSSAttributeDeclaration_h
 #define nsDOMCSSAttributeDeclaration_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/dom/DocGroup.h"
 #include "nsDOMCSSDeclaration.h"
 
 
+class nsSMILValue;
 namespace mozilla {
 namespace dom {
 class DomGroup;
 class Element;
 } // namespace dom
 } // namespace mozilla
 
 class nsDOMCSSAttributeDeclaration final : public nsDOMCSSDeclaration
@@ -33,19 +34,27 @@ public:
 
   // If GetCSSDeclaration returns non-null, then the decl it returns
   // is owned by our current style rule.
   virtual mozilla::DeclarationBlock* GetCSSDeclaration(Operation aOperation) override;
   virtual void GetCSSParsingEnvironment(CSSParsingEnvironment& aCSSParseEnv,
                                         nsIPrincipal* aSubjectPrincipal) override;
   nsDOMCSSDeclaration::ServoCSSParsingEnvironment
   GetServoCSSParsingEnvironment(nsIPrincipal* aSubjectPrincipal) const final;
-  mozilla::css::Rule* GetParentRule() override;
+  mozilla::css::Rule* GetParentRule() override
+  {
+    return nullptr;
+  }
 
-  virtual nsINode* GetParentObject() override;
+  nsINode* GetParentObject() override
+  {
+    return mElement;
+  }
+
+  nsresult SetSMILValue(const nsCSSPropertyID aPropID, const nsSMILValue&);
 
   nsresult SetPropertyValue(const nsCSSPropertyID aPropID,
                             const nsAString& aValue,
                             nsIPrincipal* aSubjectPrincipal) override;
 
 protected:
   ~nsDOMCSSAttributeDeclaration();
 
--- a/layout/style/nsDOMCSSDeclaration.cpp
+++ b/layout/style/nsDOMCSSDeclaration.cpp
@@ -18,19 +18,17 @@
 #include "nsIURI.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "nsContentUtils.h"
 #include "nsQueryObject.h"
 #include "mozilla/layers/ScrollLinkedEffectDetector.h"
 
 using namespace mozilla;
 
-nsDOMCSSDeclaration::~nsDOMCSSDeclaration()
-{
-}
+nsDOMCSSDeclaration::~nsDOMCSSDeclaration() = default;
 
 /* virtual */ JSObject*
 nsDOMCSSDeclaration::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return dom::CSS2PropertiesBinding::Wrap(aCx, this, aGivenProto);
 }
 
 NS_IMPL_QUERY_INTERFACE(nsDOMCSSDeclaration,
@@ -265,43 +263,42 @@ nsDOMCSSDeclaration::GetServoCSSParsingE
 
   return {
     sheet->URLData(),
     eCompatibility_FullStandards,
     nullptr,
   };
 }
 
-template<typename GeckoFunc, typename ServoFunc>
+template<typename Func>
 nsresult
 nsDOMCSSDeclaration::ModifyDeclaration(nsIPrincipal* aSubjectPrincipal,
-                                       GeckoFunc aGeckoFunc,
-                                       ServoFunc aServoFunc)
+                                       Func aFunc)
 {
   DeclarationBlock* olddecl = GetCSSDeclaration(eOperation_Modify);
   if (!olddecl) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // 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<DeclarationBlock> decl = olddecl->EnsureMutable();
 
   bool changed;
-  ServoCSSParsingEnvironment servoEnv = GetServoCSSParsingEnvironment(
-      aSubjectPrincipal);
+  ServoCSSParsingEnvironment servoEnv =
+    GetServoCSSParsingEnvironment(aSubjectPrincipal);
   if (!servoEnv.mUrlExtraData) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  changed = aServoFunc(decl->AsServo(), servoEnv);
+  changed = aFunc(decl->AsServo(), servoEnv);
 
   if (!changed) {
     // Parsing failed -- but we don't throw an exception for that.
     return NS_OK;
   }
 
   return SetCSSDeclaration(decl);
 }
@@ -309,19 +306,16 @@ nsDOMCSSDeclaration::ModifyDeclaration(n
 nsresult
 nsDOMCSSDeclaration::ParsePropertyValue(const nsCSSPropertyID aPropID,
                                         const nsAString& aPropValue,
                                         bool aIsImportant,
                                         nsIPrincipal* aSubjectPrincipal)
 {
   return ModifyDeclaration(
     aSubjectPrincipal,
-    [&](css::Declaration* decl, CSSParsingEnvironment& env, bool* changed) {
-      MOZ_CRASH("old style system disabled");
-    },
     [&](ServoDeclarationBlock* decl, ServoCSSParsingEnvironment& env) {
       NS_ConvertUTF16toUTF8 value(aPropValue);
       return Servo_DeclarationBlock_SetPropertyById(
         decl->Raw(), aPropID, &value, aIsImportant, env.mUrlExtraData,
         ParsingMode::Default, env.mCompatMode, env.mLoader);
     });
 }
 
@@ -329,19 +323,16 @@ nsresult
 nsDOMCSSDeclaration::ParseCustomPropertyValue(const nsAString& aPropertyName,
                                               const nsAString& aPropValue,
                                               bool aIsImportant,
                                               nsIPrincipal* aSubjectPrincipal)
 {
   MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aPropertyName));
   return ModifyDeclaration(
     aSubjectPrincipal,
-    [&](css::Declaration* decl, CSSParsingEnvironment& env, bool* changed) {
-      MOZ_CRASH("old style system disabled");
-    },
     [&](ServoDeclarationBlock* decl, ServoCSSParsingEnvironment& env) {
       NS_ConvertUTF16toUTF8 property(aPropertyName);
       NS_ConvertUTF16toUTF8 value(aPropValue);
       return Servo_DeclarationBlock_SetProperty(
         decl->Raw(), &property, &value, aIsImportant, env.mUrlExtraData,
         ParsingMode::Default, env.mCompatMode, env.mLoader);
     });
 }
--- a/layout/style/nsDOMCSSDeclaration.h
+++ b/layout/style/nsDOMCSSDeclaration.h
@@ -223,14 +223,14 @@ protected:
 
   nsresult RemovePropertyInternal(nsCSSPropertyID aPropID);
   nsresult RemovePropertyInternal(const nsAString& aProperty);
 
 protected:
   virtual ~nsDOMCSSDeclaration();
 
 private:
-  template<typename GeckoFunc, typename ServoFunc>
+  template<typename ServoFunc>
   inline nsresult ModifyDeclaration(nsIPrincipal* aSubjectPrincipal,
-                                    GeckoFunc aGeckoFunc, ServoFunc aServoFunc);
+                                    ServoFunc aServoFunc);
 };
 
 #endif // nsDOMCSSDeclaration_h___