Bug 1383650 - Notify style system when SMIL animation changes length r=birtles,longsonr
☠☠ backed out by 1624c5a31917 ☠ ☠
authorviolet <violet.bugreport@gmail.com>
Thu, 16 May 2019 00:58:56 +0000
changeset 532850 a7b8e6460fb8b1245507e9e15deadc1e62674102
parent 532849 e864696f6cf80910479dce91eed090a6455a4e2f
child 532851 6730776560c028a402e7112569e6ca8bf6cd742b
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles, longsonr
bugs1383650
milestone68.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 1383650 - Notify style system when SMIL animation changes length r=birtles,longsonr When animating geometry attribute, we need to notify style system about the change of SMIL override style. Differential Revision: https://phabricator.services.mozilla.com/D30361
dom/smil/SMILCompositor.cpp
dom/svg/SVGElement.cpp
dom/svg/SVGElement.h
dom/svg/SVGGeometryProperty.cpp
dom/svg/SVGGeometryProperty.h
dom/svg/SVGViewportElement.h
layout/style/nsDOMCSSAttrDeclaration.cpp
layout/style/nsDOMCSSAttrDeclaration.h
--- a/dom/smil/SMILCompositor.cpp
+++ b/dom/smil/SMILCompositor.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SMILCompositor.h"
 
+#include "mozilla/dom/SVGSVGElement.h"
 #include "nsComputedDOMStyle.h"
 #include "nsCSSProps.h"
 #include "nsHashKeys.h"
 #include "SMILCSSProperty.h"
 
 namespace mozilla {
 
 // PLDHashEntryHdr methods
@@ -142,29 +143,31 @@ nsCSSPropertyID SMILCompositor::GetCSSPr
       nsCSSProps::LookupProperty(nsDependentAtomString(mKey.mAttributeName));
 
   if (!SMILCSSProperty::IsPropertyAnimatable(propID)) {
     return eCSSProperty_UNKNOWN;
   }
 
   // If we are animating the 'width' or 'height' of an outer SVG
   // element we should animate it as a CSS property, but for other elements
-  // (e.g. <rect>) we should animate it as a length attribute.
-  // The easiest way to test for an outer SVG element, is to see if it is an
-  // SVG-namespace element mapping its width/height attribute to style.
-  //
-  // If we have animation of 'width' or 'height' on an SVG element that is
-  // NOT mapping that attributes to style then it must not be an outermost SVG
-  // element so we should return eCSSProperty_UNKNOWN to indicate that we
-  // should animate as an attribute instead.
+  // in SVG namespace (e.g. <rect>) we should animate it as a length attribute.
   if ((mKey.mAttributeName == nsGkAtoms::width ||
        mKey.mAttributeName == nsGkAtoms::height) &&
-      mKey.mElement->GetNameSpaceID() == kNameSpaceID_SVG &&
-      !mKey.mElement->IsAttributeMapped(mKey.mAttributeName)) {
-    return eCSSProperty_UNKNOWN;
+      mKey.mElement->GetNameSpaceID() == kNameSpaceID_SVG) {
+    // Not an <svg> element.
+    if (!mKey.mElement->IsSVGElement(nsGkAtoms::svg)) {
+      return eCSSProperty_UNKNOWN;
+    }
+
+    // An inner <svg> element
+    if (static_cast<dom::SVGSVGElement const&>(*mKey.mElement).IsInner()) {
+      return eCSSProperty_UNKNOWN;
+    }
+
+    // Indeed an outer <svg> element, fall through.
   }
 
   return propID;
 }
 
 bool SMILCompositor::MightNeedBaseStyle() const {
   if (GetCSSPropertyToAnimate() == eCSSProperty_UNKNOWN) {
     return false;
--- a/dom/svg/SVGElement.cpp
+++ b/dom/svg/SVGElement.cpp
@@ -25,16 +25,17 @@
 #include "mozilla/SVGContentUtils.h"
 #include "mozilla/Unused.h"
 
 #include "DOMSVGAnimatedEnumeration.h"
 #include "mozAutoDocUpdate.h"
 #include "nsAttrValueOrString.h"
 #include "nsCSSProps.h"
 #include "nsContentUtils.h"
+#include "nsDOMCSSAttrDeclaration.h"
 #include "nsICSSDeclaration.h"
 #include "nsIContentInlines.h"
 #include "mozilla/dom/Document.h"
 #include "nsError.h"
 #include "nsGkAtoms.h"
 #include "nsIFrame.h"
 #include "nsQueryObject.h"
 #include "nsLayoutUtils.h"
@@ -48,16 +49,17 @@
 #include "SVGAnimatedInteger.h"
 #include "SVGAnimatedIntegerPair.h"
 #include "SVGAnimatedLength.h"
 #include "SVGAnimatedNumber.h"
 #include "SVGAnimatedNumberPair.h"
 #include "SVGAnimatedOrient.h"
 #include "SVGAnimatedString.h"
 #include "SVGAnimatedViewBox.h"
+#include "SVGGeometryProperty.h"
 #include "SVGMotionSMILAttr.h"
 #include <stdarg.h>
 
 // This is needed to ensure correct handling of calls to the
 // vararg-list methods in this file:
 //   SVGElement::GetAnimated{Length,Number,Integer}Values
 // See bug 547964 for details:
 static_assert(sizeof(void*) == sizeof(nullptr),
@@ -1015,16 +1017,51 @@ SVGSVGElement* SVGElement::GetOwnerSVGEl
 SVGElement* SVGElement::GetViewportElement() {
   return SVGContentUtils::GetNearestViewportElement(this);
 }
 
 already_AddRefed<DOMSVGAnimatedString> SVGElement::ClassName() {
   return mClassAttribute.ToDOMAnimatedString(this);
 }
 
+/* static */
+bool SVGElement::UpdateDeclarationBlockFromLength(
+    DeclarationBlock& aBlock, nsCSSPropertyID aPropId,
+    const SVGAnimatedLength& aLength, ValToUse aValToUse) {
+  aBlock.AssertMutable();
+
+  float value;
+  if (aValToUse == ValToUse::Anim) {
+    value = aLength.GetAnimValInSpecifiedUnits();
+  } else {
+    MOZ_ASSERT(aValToUse == ValToUse::Base);
+    value = aLength.GetBaseValInSpecifiedUnits();
+  }
+
+  // SVG parser doesn't check non-negativity of some parsed value,
+  // we should not pass those to CSS side.
+  if (value < 0 &&
+      SVGGeometryProperty::IsNonNegativeGeometryProperty(aPropId)) {
+    return false;
+  }
+
+  nsCSSUnit cssUnit = SVGGeometryProperty::SpecifiedUnitTypeToCSSUnit(
+      aLength.GetSpecifiedUnitType());
+
+  if (cssUnit == eCSSUnit_Percent) {
+    Servo_DeclarationBlock_SetPercentValue(aBlock.Raw(), aPropId,
+                                           value / 100.f);
+  } else {
+    Servo_DeclarationBlock_SetLengthValue(aBlock.Raw(), aPropId, value,
+                                          cssUnit);
+  }
+
+  return true;
+}
+
 //------------------------------------------------------------------------
 // Helper class: MappedAttrParser, for parsing values of mapped attributes
 
 namespace {
 
 class MOZ_STACK_CLASS MappedAttrParser {
  public:
   MappedAttrParser(css::Loader* aLoader, nsIURI* aDocURI,
@@ -1386,16 +1423,25 @@ void SVGElement::DidChangeLength(uint8_t
 
   nsAttrValue newValue;
   newValue.SetTo(info.mLengths[aAttrEnum], nullptr);
 
   DidChangeValue(info.mLengthInfo[aAttrEnum].mName, aEmptyOrOldValue, newValue);
 }
 
 void SVGElement::DidAnimateLength(uint8_t aAttrEnum) {
+  if (SVGGeometryProperty::ElementMapsLengthsToStyle(this)) {
+    nsCSSPropertyID propId =
+        SVGGeometryProperty::AttrEnumToCSSPropId(this, aAttrEnum);
+
+    SMILOverrideStyle()->SetSMILValue(propId,
+                                      GetLengthInfo().mLengths[aAttrEnum]);
+    return;
+  }
+
   ClearAnyCachedPath();
 
   nsIFrame* frame = GetPrimaryFrame();
 
   if (frame) {
     LengthAttributesInfo info = GetLengthInfo();
     frame->AttributeChanged(kNameSpaceID_None,
                             info.mLengthInfo[aAttrEnum].mName,
--- a/dom/svg/SVGElement.h
+++ b/dom/svg/SVGElement.h
@@ -160,16 +160,22 @@ class SVGElement : public SVGElementBase
     return GetStringInfo().mStringInfo[aAttrEnum].mIsAnimatable;
   }
   bool NumberAttrAllowsPercentage(uint8_t aAttrEnum) {
     return GetNumberInfo().mNumberInfo[aAttrEnum].mPercentagesAllowed;
   }
   virtual bool HasValidDimensions() const { return true; }
   void SetLength(nsAtom* aName, const SVGAnimatedLength& aLength);
 
+  enum class ValToUse { Base, Anim };
+  static bool UpdateDeclarationBlockFromLength(DeclarationBlock& aBlock,
+                                               nsCSSPropertyID aPropId,
+                                               const SVGAnimatedLength& aLength,
+                                               ValToUse aValToUse);
+
   nsAttrValue WillChangeLength(uint8_t aAttrEnum);
   nsAttrValue WillChangeNumberPair(uint8_t aAttrEnum);
   nsAttrValue WillChangeIntegerPair(uint8_t aAttrEnum);
   nsAttrValue WillChangeOrient();
   nsAttrValue WillChangeViewBox();
   nsAttrValue WillChangePreserveAspectRatio();
   nsAttrValue WillChangeNumberList(uint8_t aAttrEnum);
   nsAttrValue WillChangeLengthList(uint8_t aAttrEnum);
--- a/dom/svg/SVGGeometryProperty.cpp
+++ b/dom/svg/SVGGeometryProperty.cpp
@@ -64,11 +64,24 @@ nsCSSPropertyID AttrEnumToCSSPropId(cons
     return SVGEllipseElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
   }
   if (aElement->IsSVGElement(nsGkAtoms::foreignObject)) {
     return SVGForeignObjectElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
   }
   return eCSSProperty_UNKNOWN;
 }
 
+bool IsNonNegativeGeometryProperty(nsCSSPropertyID aProp) {
+  return aProp == eCSSProperty_r || aProp == eCSSProperty_rx ||
+         aProp == eCSSProperty_ry || aProp == eCSSProperty_width ||
+         aProp == eCSSProperty_height;
+}
+
+bool ElementMapsLengthsToStyle(SVGElement const* aElement) {
+  return aElement->IsSVGElement(nsGkAtoms::rect) ||
+         aElement->IsSVGElement(nsGkAtoms::circle) ||
+         aElement->IsSVGElement(nsGkAtoms::ellipse) ||
+         aElement->IsSVGElement(nsGkAtoms::foreignObject);
+}
+
 }  // namespace SVGGeometryProperty
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/svg/SVGGeometryProperty.h
+++ b/dom/svg/SVGGeometryProperty.h
@@ -147,13 +147,16 @@ bool ResolveAll(const SVGElement* aEleme
   }
   return false;
 }
 
 nsCSSUnit SpecifiedUnitTypeToCSSUnit(uint8_t aSpecifiedUnit);
 nsCSSPropertyID AttrEnumToCSSPropId(const SVGElement* aElement,
                                     uint8_t aAttrEnum);
 
+bool IsNonNegativeGeometryProperty(nsCSSPropertyID aProp);
+bool ElementMapsLengthsToStyle(SVGElement const* aElement);
+
 }  // namespace SVGGeometryProperty
 }  // namespace dom
 }  // namespace mozilla
 
 #endif
--- a/dom/svg/SVGViewportElement.h
+++ b/dom/svg/SVGViewportElement.h
@@ -119,43 +119,43 @@ class SVGViewportElement : public SVGGra
     return svgFloatSize(mViewportWidth, mViewportHeight);
   }
 
   void SetViewportSize(const svgFloatSize& aSize) {
     mViewportWidth = aSize.width;
     mViewportHeight = aSize.height;
   }
 
+  /**
+   * Returns true if either this is an SVG <svg> element that is the child of
+   * another non-foreignObject SVG element, or this is a SVG <symbol> element
+   * this is the root of a use-element shadow tree.
+   */
+  bool IsInner() const {
+    const nsIContent* parent = GetFlattenedTreeParent();
+    return parent && parent->IsSVGElement() &&
+           !parent->IsSVGElement(nsGkAtoms::foreignObject);
+  }
+
   // WebIDL
   already_AddRefed<SVGAnimatedRect> ViewBox();
   already_AddRefed<DOMSVGAnimatedPreserveAspectRatio> PreserveAspectRatio();
   virtual SVGAnimatedViewBox* GetAnimatedViewBox() override;
 
  protected:
   // implementation helpers:
 
   bool IsRoot() const {
     NS_ASSERTION((IsInUncomposedDoc() && !GetParent()) ==
                      (OwnerDoc()->GetRootElement() == this),
                  "Can't determine if we're root");
     return IsInUncomposedDoc() && !GetParent();
   }
 
   /**
-   * Returns true if either this is an SVG <svg> element that is the child of
-   * another non-foreignObject SVG element, or this is a SVG <symbol> element
-   * this is the root of a use-element shadow tree.
-   */
-  bool IsInner() const {
-    const nsIContent* parent = GetFlattenedTreeParent();
-    return parent && parent->IsSVGElement() &&
-           !parent->IsSVGElement(nsGkAtoms::foreignObject);
-  }
-
-  /**
    * Returns the explicit or default preserveAspectRatio, unless we're
    * synthesizing a viewBox, in which case it returns the "none" value.
    */
   virtual SVGPreserveAspectRatio GetPreserveAspectRatioWithOverride() const {
     return mPreserveAspectRatio.GetAnimValue();
   }
 
   /**
--- a/layout/style/nsDOMCSSAttrDeclaration.cpp
+++ b/layout/style/nsDOMCSSAttrDeclaration.cpp
@@ -5,20 +5,22 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* DOM object for element.style */
 
 #include "nsDOMCSSAttrDeclaration.h"
 
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/SVGElement.h"
 #include "mozilla/dom/MutationEventBinding.h"
 #include "mozilla/DeclarationBlock.h"
 #include "mozilla/InternalMutationEvent.h"
 #include "mozilla/SMILCSSValueType.h"
+#include "mozilla/SMILValue.h"
 #include "mozAutoDocUpdate.h"
 #include "nsIURI.h"
 #include "nsNodeUtils.h"
 #include "nsWrapperCacheInlines.h"
 #include "nsIFrame.h"
 #include "ActiveLayerTracker.h"
 
 using namespace mozilla;
@@ -128,39 +130,59 @@ nsDOMCSSAttributeDeclaration::GetParsing
     nsIPrincipal* aSubjectPrincipal) const {
   return {
       mElement->GetURLDataForStyleAttr(aSubjectPrincipal),
       mElement->OwnerDoc()->GetCompatibilityMode(),
       mElement->OwnerDoc()->CSSLoader(),
   };
 }
 
-nsresult nsDOMCSSAttributeDeclaration::SetSMILValue(
-    const nsCSSPropertyID aPropID, const SMILValue& aValue) {
+template <typename SetterFunc>
+nsresult nsDOMCSSAttributeDeclaration::SetSMILValueHelper(SetterFunc aFunc) {
   MOZ_ASSERT(mIsSMILOverride);
+
   // 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.
   RefPtr<DeclarationBlock> created;
   DeclarationBlock* olddecl =
       GetOrCreateCSSDeclaration(eOperation_Modify, getter_AddRefs(created));
   if (!olddecl) {
     return NS_ERROR_NOT_AVAILABLE;
   }
   mozAutoDocUpdate autoUpdate(DocToUpdate(), true);
   RefPtr<DeclarationBlock> decl = olddecl->EnsureMutable();
-  bool changed = SMILCSSValueType::SetPropertyValues(aValue, *decl);
+
+  bool changed = aFunc(*decl);
+
   if (changed) {
     // We can pass nullptr as the latter param, since this is
     // mIsSMILOverride == true case.
     SetCSSDeclaration(decl, nullptr);
   }
   return NS_OK;
 }
 
+nsresult nsDOMCSSAttributeDeclaration::SetSMILValue(
+    const nsCSSPropertyID /*aPropID*/, const SMILValue& aValue) {
+  MOZ_ASSERT(aValue.mType == &SMILCSSValueType::sSingleton,
+             "We should only try setting a CSS value type");
+  return SetSMILValueHelper([&aValue](DeclarationBlock& aDecl) {
+    return SMILCSSValueType::SetPropertyValues(aValue, aDecl);
+  });
+}
+
+nsresult nsDOMCSSAttributeDeclaration::SetSMILValue(
+    const nsCSSPropertyID aPropID, const SVGAnimatedLength& aLength) {
+  return SetSMILValueHelper([aPropID, &aLength](DeclarationBlock& aDecl) {
+    return SVGElement::UpdateDeclarationBlockFromLength(
+        aDecl, aPropID, aLength, SVGElement::ValToUse::Anim);
+  });
+}
+
 nsresult nsDOMCSSAttributeDeclaration::SetPropertyValue(
     const nsCSSPropertyID aPropID, const nsAString& aValue,
     nsIPrincipal* aSubjectPrincipal) {
   // Scripted modifications to style.opacity or style.transform (or other
   // transform-like properties, e.g. style.translate, style.rotate, style.scale)
   // could immediately force us into the animated state if heuristics suggest
   // this is scripted animation.
   // FIXME: This is missing the margin shorthand and the logical versions of
--- a/layout/style/nsDOMCSSAttrDeclaration.h
+++ b/layout/style/nsDOMCSSAttrDeclaration.h
@@ -13,44 +13,48 @@
 #include "mozilla/dom/DocGroup.h"
 #include "nsDOMCSSDeclaration.h"
 
 struct RawServoUnlockedDeclarationBlock;
 
 namespace mozilla {
 
 class SMILValue;
+class SVGAnimatedLength;
 
 namespace dom {
 class DomGroup;
 class Element;
 }  // namespace dom
 }  // namespace mozilla
 
 class nsDOMCSSAttributeDeclaration final : public nsDOMCSSDeclaration {
  public:
   typedef mozilla::dom::Element Element;
   typedef mozilla::SMILValue SMILValue;
+  typedef mozilla::SVGAnimatedLength SVGAnimatedLength;
   nsDOMCSSAttributeDeclaration(Element* aContent, bool aIsSMILOverride);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS_AMBIGUOUS(
       nsDOMCSSAttributeDeclaration, nsICSSDeclaration)
 
   mozilla::DeclarationBlock* GetOrCreateCSSDeclaration(
       Operation aOperation, mozilla::DeclarationBlock** aCreated) final;
 
   nsDOMCSSDeclaration::ParsingEnvironment GetParsingEnvironment(
       nsIPrincipal* aSubjectPrincipal) const final;
 
   mozilla::css::Rule* GetParentRule() override { return nullptr; }
 
   nsINode* GetParentObject() override { return mElement; }
 
-  nsresult SetSMILValue(const nsCSSPropertyID aPropID, const SMILValue&);
+  nsresult SetSMILValue(const nsCSSPropertyID aPropID, const SMILValue& aValue);
+  nsresult SetSMILValue(const nsCSSPropertyID aPropID,
+                        const SVGAnimatedLength& aLength);
 
   nsresult SetPropertyValue(const nsCSSPropertyID aPropID,
                             const nsAString& aValue,
                             nsIPrincipal* aSubjectPrincipal) override;
 
   static void MutationClosureFunction(void* aData);
 
   void GetPropertyChangeClosure(
@@ -74,11 +78,15 @@ class nsDOMCSSAttributeDeclaration final
 
   RefPtr<Element> mElement;
 
   /* If true, this indicates that this nsDOMCSSAttributeDeclaration
    * should interact with mContent's SMIL override style rule (rather
    * than the inline style rule).
    */
   const bool mIsSMILOverride;
+
+ private:
+  template <typename SetterFunc>
+  nsresult SetSMILValueHelper(SetterFunc aFunc);
 };
 
 #endif /* nsDOMCSSAttributeDeclaration_h */