Bug 1255276 part 2: Make nsSVGAnimatedTransformList remember its previous HasTransform() value, so we can detect attr changes which are effectively creating the attr. r=longsonr
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 14 Mar 2016 17:37:36 -0700
changeset 288664 9483041fedeeb95464e6713a9b98ef947bf7d268
parent 288663 0b31695a67bfc56ef22390f531ee3d381cc42c57
child 288665 e375b8bea68969e9749f129f364139c98a2d4a16
push id30087
push usercbook@mozilla.com
push dateTue, 15 Mar 2016 09:43:43 +0000
treeherdermozilla-central@5e14887312d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1255276
milestone48.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 1255276 part 2: Make nsSVGAnimatedTransformList remember its previous HasTransform() value, so we can detect attr changes which are effectively creating the attr. r=longsonr
dom/svg/SVGTransformableElement.cpp
dom/svg/nsSVGAnimatedTransformList.cpp
dom/svg/nsSVGAnimatedTransformList.h
layout/reftests/svg/dynamic-attr-change-2.svg
layout/reftests/svg/reftest.list
--- a/dom/svg/SVGTransformableElement.cpp
+++ b/dom/svg/SVGTransformableElement.cpp
@@ -67,18 +67,19 @@ SVGTransformableElement::GetAttributeCha
     bool isAdditionOrRemoval = false;
     if (aModType == nsIDOMMutationEvent::ADDITION ||
         aModType == nsIDOMMutationEvent::REMOVAL) {
       isAdditionOrRemoval = true;
     } else {
       MOZ_ASSERT(aModType == nsIDOMMutationEvent::MODIFICATION,
                  "Unknown modification type.");
       if (!mTransforms ||
-          !mTransforms->HasTransform()) {
-        // New value is empty -- this is effectively an attribute-removal.
+          !mTransforms->HasTransform() ||
+          !mTransforms->HadTransformBeforeLastBaseValChange()) {
+        // New or old value is empty; this is effectively addition or removal.
         isAdditionOrRemoval = true;
       }
     }
 
     if (isAdditionOrRemoval) {
       // Reconstruct the frame tree to handle stacking context changes:
       NS_UpdateHint(retval, nsChangeHint_ReconstructFrame);
     } else {
--- a/dom/svg/nsSVGAnimatedTransformList.cpp
+++ b/dom/svg/nsSVGAnimatedTransformList.cpp
@@ -41,34 +41,40 @@ nsSVGAnimatedTransformList::SetBaseValue
     // We must send this notification *before* changing mBaseVal! If the length
     // of our baseVal is being reduced, our baseVal's DOM wrapper list may have
     // to remove DOM items from itself, and any removed DOM items need to copy
     // their internal counterpart values *before* we change them.
     //
     domWrapper->InternalBaseValListWillChangeLengthTo(aValue.Length());
   }
 
+  // (This bool will be copied to our member-var, if attr-change succeeds.)
+  bool hadTransform = HasTransform();
+
   // We don't need to call DidChange* here - we're only called by
   // nsSVGElement::ParseAttribute under Element::SetAttr,
   // which takes care of notifying.
 
   nsresult rv = mBaseVal.CopyFrom(aValue);
   if (NS_FAILED(rv) && domWrapper) {
     // Attempting to increase mBaseVal's length failed - reduce domWrapper
     // back to the same length:
     domWrapper->InternalBaseValListWillChangeLengthTo(mBaseVal.Length());
   } else {
     mIsAttrSet = true;
+    mHadTransformBeforeLastBaseValChange = hadTransform;
   }
   return rv;
 }
 
 void
 nsSVGAnimatedTransformList::ClearBaseValue()
 {
+  mHadTransformBeforeLastBaseValChange = HasTransform();
+
   SVGAnimatedTransformList *domWrapper =
     SVGAnimatedTransformList::GetDOMWrapperIfExists(this);
   if (domWrapper) {
     // We must send this notification *before* changing mBaseVal! (See above.)
     domWrapper->InternalBaseValListWillChangeLengthTo(0);
   }
   mBaseVal.Clear();
   mIsAttrSet = false;
--- a/dom/svg/nsSVGAnimatedTransformList.h
+++ b/dom/svg/nsSVGAnimatedTransformList.h
@@ -39,17 +39,19 @@ class SVGTransform;
  */
 class nsSVGAnimatedTransformList
 {
   // friends so that they can get write access to mBaseVal
   friend class dom::SVGTransform;
   friend class DOMSVGTransformList;
 
 public:
-  nsSVGAnimatedTransformList() : mIsAttrSet(false) { }
+  nsSVGAnimatedTransformList()
+    : mIsAttrSet(false),
+      mHadTransformBeforeLastBaseValChange(false) { }
 
   /**
    * Because it's so important that mBaseVal and its DOMSVGTransformList wrapper
    * (if any) be kept in sync (see the comment in
    * SVGAnimatedTransformList::InternalBaseValListWillChangeTo), this method
    * returns a const reference. Only our friend classes may get mutable
    * references to mBaseVal.
    */
@@ -87,29 +89,46 @@ public:
    */
   bool HasTransform() const
     { return (mAnimVal && !mAnimVal->IsEmpty()) || !mBaseVal.IsEmpty(); }
 
   bool IsAnimating() const {
     return !!mAnimVal;
   }
 
+  /**
+   * Returns true iff "HasTransform" returned true just before our most recent
+   * SetBaseValue/SetBaseValueString/ClearBaseValue change.
+   *
+   * (This is used as part of an optimization in
+   * SVGTransformableElement::GetAttributeChangeHint. That function reports an
+   * inexpensive nsChangeHint when a transform has just modified -- but this
+   * accessor lets it detect cases where the "modification" is actually adding
+   * a transform where we previously had none. These cases require a more
+   * thorough nsChangeHint.)
+   */
+  bool HadTransformBeforeLastBaseValChange() const {
+    return mHadTransformBeforeLastBaseValChange;
+  }
+
   /// Callers own the returned nsISMILAttr
   nsISMILAttr* ToSMILAttr(nsSVGElement* aSVGElement);
 
 private:
 
   // mAnimVal is a pointer to allow us to determine if we're being animated or
   // not. Making it a non-pointer member and using mAnimVal.IsEmpty() to check
   // if we're animating is not an option, since that would break animation *to*
   // the empty string (<set to="">).
 
   SVGTransformList mBaseVal;
   nsAutoPtr<SVGTransformList> mAnimVal;
   bool mIsAttrSet;
+   // (See documentation for accessor, HadTransformBeforeLastBaseValChange.)
+  bool mHadTransformBeforeLastBaseValChange;
 
   struct SMILAnimatedTransformList : public nsISMILAttr
   {
   public:
     SMILAnimatedTransformList(nsSVGAnimatedTransformList* aVal,
                               nsSVGElement* aSVGElement)
       : mVal(aVal)
       , mElement(aSVGElement)
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/dynamic-attr-change-2.svg
@@ -0,0 +1,26 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1" class="reftest-wait">
+  <script>
+  function doTest() {
+    var target = document.querySelector(".target");
+    target.setAttribute("transform", "translate(20,20)");
+    document.documentElement.removeAttribute("class");
+  }
+
+  document.addEventListener("MozReftestInvalidate", doTest, false);
+  </script>
+
+  <!-- Lime background to match pass.svg -->
+  <rect height="100%" width="100%" fill="lime"/>
+
+  <!-- Red rect, which we'll have to cover up to pass the test: -->
+  <rect x="20" y="20" width="100" height="100" fill="red"/>
+
+  <!-- Lime rect, which we'll try to transform to cover up the red rect: -->
+  <g class="target" transform="">
+    <rect width="100" height="100" fill="lime"/>
+  </g>
+</svg>
--- a/layout/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -57,16 +57,17 @@ fuzzy-if(skiaContent,1,320) == condition
 == currentColor-02.svg pass.svg
 == currentColor-03.svg pass.svg
 == data-uri-with-filter-01.xhtml data-uri-with-filter-01-ref.svg
 == data-uri-with-gradient-01.xhtml data-uri-with-gradient-01-ref.svg
 == data-uri-with-pattern-01.xhtml pass.svg
 == dynamic-attr-removal-1.svg pass.svg
 == dynamic-attr-removal-2.svg pass.svg
 == dynamic-attr-change-1.svg pass.svg
+== dynamic-attr-change-2.svg pass.svg
 == dynamic-class-01.svg pass.svg
 fuzzy-if(Android,4,87) == dynamic-clipPath-01.svg pass.svg
 == dynamic-clipPath-02.svg pass.svg
 == dynamic-clipPath-clip-rule-01.svg pass.svg
 == dynamic-conditions-01.svg pass.svg
 random-if(Mulet) == dynamic-conditions-02.svg about:blank # bug 1178062
 == dynamic-conditions-03.svg pass.svg
 random-if(Mulet) == dynamic-conditions-04.svg about:blank # bug 1178062