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

@@ -72,8 +72,9 @@ SVGTransformableElement::GetAttributeChangeHint(const nsIAtom* aAttribute,
       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;
       }
     }

dom/svg/nsSVGAnimatedTransformList.cpp

@@ -46,6 +46,9 @@ nsSVGAnimatedTransformList::SetBaseValue(const SVGTransformList& aValue)
     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.
@@ -57,6 +60,7 @@ nsSVGAnimatedTransformList::SetBaseValue(const SVGTransformList& aValue)
     domWrapper->InternalBaseValListWillChangeLengthTo(mBaseVal.Length());
   } else {
     mIsAttrSet = true;
+    mHadTransformBeforeLastBaseValChange = hadTransform;
   }
   return rv;
 }
@@ -64,6 +68,8 @@ nsSVGAnimatedTransformList::SetBaseValue(const SVGTransformList& aValue)
 void
 nsSVGAnimatedTransformList::ClearBaseValue()
 {
+  mHadTransformBeforeLastBaseValChange = HasTransform();
+
   SVGAnimatedTransformList *domWrapper =
     SVGAnimatedTransformList::GetDOMWrapperIfExists(this);
   if (domWrapper) {

dom/svg/nsSVGAnimatedTransformList.h

@@ -44,7 +44,9 @@ class nsSVGAnimatedTransformList
   friend class DOMSVGTransformList;
 
 public:
-  nsSVGAnimatedTransformList() : mIsAttrSet(false) { }
+  nsSVGAnimatedTransformList()
+    : mIsAttrSet(false),
+      mHadTransformBeforeLastBaseValChange(false) { }
 
   /**
    * Because it's so important that mBaseVal and its DOMSVGTransformList wrapper
@@ -92,6 +94,21 @@ public:
     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);
 
@@ -105,6 +122,8 @@ private:
   SVGTransformList mBaseVal;
   nsAutoPtr<SVGTransformList> mAnimVal;
   bool mIsAttrSet;
+   // (See documentation for accessor, HadTransformBeforeLastBaseValChange.)
+  bool mHadTransformBeforeLastBaseValChange;
 
   struct SMILAnimatedTransformList : public nsISMILAttr
   {

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>

layout/reftests/svg/reftest.list

@@ -62,6 +62,7 @@ fuzzy-if(skiaContent,1,320) == conditions-08.svg conditions-08-ref.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