Bug 1458814: Make SMIL values not roundtrip through strings. r=hiro

MozReview-Commit-ID: DpbFSutIv3t

dom/smil/nsSMILCSSValueType.cpp

@@ -16,7 +16,9 @@
 #include "nsCSSValue.h"
 #include "nsColor.h"
 #include "nsPresContext.h"
+#include "mozilla/DeclarationBlockInlines.h"
 #include "mozilla/ServoBindings.h"
+#include "mozilla/ServoDeclarationBlock.h"
 #include "mozilla/StyleAnimationValue.h" // For AnimationValue
 #include "mozilla/ServoCSSParser.h"
 #include "mozilla/ServoStyleSet.h"
@@ -630,6 +632,28 @@ nsSMILCSSValueType::ValueToString(const nsSMILValue& aValue,
                                  &aString);
 }
 
+// static
+bool
+nsSMILCSSValueType::SetPropertyValues(const nsSMILValue& aValue,
+                                      DeclarationBlock& aDecl)
+{
+  MOZ_ASSERT(aValue.mType == &nsSMILCSSValueType::sSingleton,
+             "Unexpected SMIL value type");
+  const ValueWrapper* wrapper = ExtractValueWrapper(aValue);
+  if (!wrapper) {
+    return false;
+  }
+
+  bool changed = false;
+  for (const auto& value : wrapper->mServoValues) {
+    changed |=
+      Servo_DeclarationBlock_SetPropertyToAnimationValue(
+        aDecl.AsServo()->Raw(), value);
+  }
+
+  return changed;
+}
+
 // static
 nsCSSPropertyID
 nsSMILCSSValueType::PropertyFromValue(const nsSMILValue& aValue)

dom/smil/nsSMILCSSValueType.h

@@ -16,6 +16,7 @@
 
 namespace mozilla {
 struct AnimationValue;
+class DeclarationBlock;
 namespace dom {
 class Element;
 } // namespace dom
@@ -114,6 +115,13 @@ public:
    */
   static void ValueToString(const nsSMILValue& aValue, nsAString& aString);
 
+  /**
+   * Sets the relevant property values in the declaration block.
+   *
+   * Returns whether the declaration changed.
+   */
+  static bool SetPropertyValues(const nsSMILValue&, mozilla::DeclarationBlock&);
+
   /**
    * Return the CSS property animated by the specified value.
    *

layout/style/ServoBindingList.h

@@ -557,6 +557,9 @@ SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetProperty, bool,
                    mozilla::ParsingMode parsing_mode,
                    nsCompatibility quirks_mode,
                    mozilla::css::Loader* loader)
+SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetPropertyToAnimationValue, bool,
+                   RawServoDeclarationBlockBorrowed declarations,
+                   RawServoAnimationValueBorrowed animation_value)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetPropertyById, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    nsCSSPropertyID property,

layout/style/nsDOMCSSAttrDeclaration.cpp

@@ -14,6 +14,7 @@
 #include "mozilla/dom/MutationEventBinding.h"
 #include "mozilla/InternalMutationEvent.h"
 #include "mozilla/ServoDeclarationBlock.h"
+#include "mozAutoDocUpdate.h"
 #include "nsContentUtils.h"
 #include "nsIDocument.h"
 #include "nsIURI.h"
@@ -172,19 +173,20 @@ nsDOMCSSAttributeDeclaration::SetSMILValue(const nsCSSPropertyID aPropID,
                                            const nsSMILValue& aValue)
 {
   MOZ_ASSERT(mIsSMILOverride);
-
-  // 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;
+  // No need to do the ActiveLayerTracker / ScrollLinkedEffectDetector bits,
+  // since we're in a SMIL animation anyway, no need to try to detect we're a
+  // scripted animation.
+  DeclarationBlock* olddecl = GetCSSDeclaration(eOperation_Modify);
+  if (!olddecl) {
+    return NS_ERROR_NOT_AVAILABLE;
   }
-  return SetPropertyValue(aPropID, valStr, nullptr);
+  mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), true);
+  RefPtr<DeclarationBlock> decl = olddecl->EnsureMutable();
+  bool changed = nsSMILCSSValueType::SetPropertyValues(aValue, *decl);
+  if (changed) {
+    SetCSSDeclaration(decl);
+  }
+  return NS_OK;
 }
 
 nsresult

servo/ports/geckolib/glue.rs

@@ -3609,6 +3609,20 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetProperty(
     )
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyToAnimationValue(
+    declarations: RawServoDeclarationBlockBorrowed,
+    animation_value: RawServoAnimationValueBorrowed,
+) -> bool {
+    write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
+        decls.push(
+            AnimationValue::as_arc(&animation_value).uncompute(),
+            Importance::Normal,
+            DeclarationSource::CssOm,
+        )
+    })
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyById(
     declarations: RawServoDeclarationBlockBorrowed,