Bug 1458814: Make SMIL values not roundtrip through strings. r=hiro
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 03 May 2018 18:27:44 +0200
changeset 473160 218a57b6f00e1d8da9660858fb58498b984ddadc
parent 473159 c5329a24af4fdcb5ce8feac93d42630bb2d4e96b
child 473161 ce36f98b098424ee86c13ec1bf6549fe37f09363
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1458814
milestone61.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 1458814: Make SMIL values not roundtrip through strings. r=hiro MozReview-Commit-ID: DpbFSutIv3t
dom/smil/nsSMILCSSValueType.cpp
dom/smil/nsSMILCSSValueType.h
layout/style/ServoBindingList.h
layout/style/nsDOMCSSAttrDeclaration.cpp
servo/ports/geckolib/glue.rs
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -11,17 +11,19 @@
 #include "nsComputedDOMStyle.h"
 #include "nsString.h"
 #include "nsSMILParserUtils.h"
 #include "nsSMILValue.h"
 #include "nsCSSProps.h"
 #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"
 #include "mozilla/dom/BaseKeyframeTypesBinding.h" // For CompositeOperation
 #include "mozilla/dom/Element.h"
 #include "nsDebug.h"
 #include "nsStyleUtil.h"
 #include "nsIDocument.h"
@@ -626,16 +628,38 @@ nsSMILCSSValueType::ValueToString(const 
   }
 
   Servo_AnimationValue_Serialize(wrapper->mServoValues[0],
                                  wrapper->mPropID,
                                  &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)
 {
   if (aValue.mType != &nsSMILCSSValueType::sSingleton) {
     return eCSSProperty_UNKNOWN;
   }
 
   const ValueWrapper* wrapper = ExtractValueWrapper(aValue);
--- a/dom/smil/nsSMILCSSValueType.h
+++ b/dom/smil/nsSMILCSSValueType.h
@@ -11,16 +11,17 @@
 
 #include "nsISMILType.h"
 #include "nsCSSPropertyID.h"
 #include "nsStringFwd.h"
 #include "mozilla/Attributes.h"
 
 namespace mozilla {
 struct AnimationValue;
+class DeclarationBlock;
 namespace dom {
 class Element;
 } // namespace dom
 } // namespace mozilla
 
 /*
  * nsSMILCSSValueType: Represents a SMIL-animated CSS value.
  */
@@ -110,16 +111,23 @@ public:
    * freshly-initialized value the resulting string will be empty.
    *
    * @param       aValue   The nsSMILValue to be converted into a string.
    * @param [out] aString  The string to be populated with the given value.
    */
   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.
    *
    * @param   aValue   The nsSMILValue to examine.
    * @return           The nsCSSPropertyID enum value of the property animated
    *                   by |aValue|, or eCSSProperty_UNKNOWN if the type of
    *                   |aValue| is not nsSMILCSSValueType.
    */
   static nsCSSPropertyID PropertyFromValue(const nsSMILValue& aValue);
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -552,16 +552,19 @@ SERVO_BINDING_FUNC(Servo_DeclarationBloc
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetProperty, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    const nsACString* property,
                    const nsACString* value, bool is_important,
                    RawGeckoURLExtraData* data,
                    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,
                    const nsACString* value, bool is_important,
                    RawGeckoURLExtraData* data,
                    mozilla::ParsingMode parsing_mode,
                    nsCompatibility quirks_mode,
                    mozilla::css::Loader* loader)
--- a/layout/style/nsDOMCSSAttrDeclaration.cpp
+++ b/layout/style/nsDOMCSSAttrDeclaration.cpp
@@ -9,16 +9,17 @@
 #include "nsDOMCSSAttrDeclaration.h"
 
 #include "mozilla/DeclarationBlock.h"
 #include "mozilla/DeclarationBlockInlines.h"
 #include "mozilla/dom/Element.h"
 #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"
 #include "nsNodeUtils.h"
 #include "nsSMILCSSValueType.h"
 #include "nsWrapperCacheInlines.h"
 #include "nsIFrame.h"
 #include "ActiveLayerTracker.h"
@@ -167,29 +168,30 @@ nsDOMCSSAttributeDeclaration::GetServoCS
   };
 }
 
 nsresult
 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
 nsDOMCSSAttributeDeclaration::SetPropertyValue(const nsCSSPropertyID aPropID,
                                                const nsAString& aValue,
                                                nsIPrincipal* aSubjectPrincipal)
 {
   // Scripted modifications to style.opacity or style.transform
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -3605,16 +3605,30 @@ pub unsafe extern "C" fn Servo_Declarati
         data,
         parsing_mode,
         quirks_mode.into(),
         loader,
     )
 }
 
 #[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,
     property: nsCSSPropertyID,
     value: *const nsACString,
     is_important: bool,
     data: *mut URLExtraData,
     parsing_mode: structs::ParsingMode,
     quirks_mode: nsCompatibility,