Bug 572938 - SVG SMIL: Fix infinite recursion on 'display' with '<use>'; r=dholbert; a=blocking-final

content/smil/crashtests/572938-1.svg

@@ -0,0 +1,12 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+  xmlns:xlink="http://www.w3.org/1999/xlink">
+  <defs>
+    <text id="myText">Used Text Element
+      <set attributeName="display" to="none"/>
+    </text>
+  </defs>
+  <use xlink:href="#myText" x="20" y="40"/>
+  <text x="20" y="60">Normal Text Element
+    <set attributeName="display" to="none"/>
+  </text>
+</svg>

content/smil/crashtests/572938-2.svg

@@ -0,0 +1,22 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+  xmlns:xlink="http://www.w3.org/1999/xlink"
+  class="reftest-wait">
+
+    <script>
+    function boom()
+    {
+      document.getElementById("circleID").removeChild(
+        document.getElementById("at"));
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("load", boom, false);
+    </script>
+
+    <circle id="circleID">
+        <animate/>
+        <animateTransform id="at" attributeName="transform"/>
+    </circle>
+    <animate attributeName="stroke-width"/>
+    <use xlink:href="#circleID"/>
+
+</svg>

content/smil/crashtests/572938-3.svg

@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+  xmlns:xlink="http://www.w3.org/1999/xlink">
+  <defs>
+    <text id="a">Text A</text>
+    <text id="b">Text B</text>
+  </defs>
+  <use xlink:href="#a" x="20" y="40">
+    <set attributeName="xlink:href" to="#b" dur="2s"/>
+  </use>
+</svg>

content/smil/crashtests/572938-4.svg

@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+   xmlns:xlink="http://www.w3.org/1999/xlink">
+<g id="a">
+ <path d=""><animate/></path>
+</g>
+<g display="none">
+<use xlink:href="#a" x="80"/>
+<set attributeName="display" to="inline"/>
+</g>
+</svg>

content/smil/crashtests/crashtests.list

@@ -16,6 +16,10 @@ load 554202-2.svg
 load 554141-1.svg
 load 555026-1.svg
 load 556841-1.svg
+load 572938-1.svg
+load 572938-2.svg
+load 572938-3.svg
+load 572938-4.svg
 load 588287-1.svg
 load 588287-2.svg
 load 590425-1.html

content/smil/nsSMILAnimationController.cpp

@@ -74,9 +74,7 @@ GetRefreshDriverForDoc(nsIDocument* aDoc)
 nsSMILAnimationController::nsSMILAnimationController()
   : mResampleNeeded(PR_FALSE),
     mDeferredStartSampling(PR_FALSE),
-#ifdef DEBUG
     mRunningSample(PR_FALSE),
-#endif
     mDocument(nsnull)
 {
   mAnimationElementTable.Init();
@@ -189,7 +187,6 @@ void
 nsSMILAnimationController::RegisterAnimationElement(
                                   nsISMILAnimationElement* aAnimationElement)
 {
-  NS_ASSERTION(!mRunningSample, "Registering content during sample.");
   mAnimationElementTable.PutEntry(aAnimationElement);
   if (mDeferredStartSampling) {
     mDeferredStartSampling = PR_FALSE;
@@ -207,7 +204,6 @@ void
 nsSMILAnimationController::UnregisterAnimationElement(
                                   nsISMILAnimationElement* aAnimationElement)
 {
-  NS_ASSERTION(!mRunningSample, "Unregistering content during sample.");
   mAnimationElementTable.RemoveEntry(aAnimationElement);
 }
 
@@ -362,16 +358,11 @@ nsSMILAnimationController::DoSample()
 void
 nsSMILAnimationController::DoSample(PRBool aSkipUnchangedContainers)
 {
-  // Reset resample flag -- do this before flushing styles since flushing styles
-  // will also flush animation resample requests
   mResampleNeeded = PR_FALSE;
-  mDocument->FlushPendingNotifications(Flush_Style);
-#ifdef DEBUG
+  // Set running sample flag -- do this before flushing styles so that when we
+  // flush styles we don't end up requesting extra samples
   mRunningSample = PR_TRUE;
-#endif
-  // Reset resample flag again -- flushing styles may have set this flag but
-  // since we're about to do a sample now, reset it
-  mResampleNeeded = PR_FALSE;
+  mDocument->FlushPendingNotifications(Flush_Style);
 
   // STEP 1: Bring model up to date
   // (i)  Rewind elements where necessary
@@ -445,9 +436,7 @@ nsSMILAnimationController::DoSample(PRBool aSkipUnchangedContainers)
   // when the inherited value is *also* being animated, we really should be
   // traversing our animated nodes in an ancestors-first order (bug 501183)
   currentCompositorTable->EnumerateEntries(DoComposeAttribute, nsnull);
-#ifdef DEBUG
   mRunningSample = PR_FALSE;
-#endif
 
   // Update last compositor table
   mLastCompositorTable = currentCompositorTable.forget();

content/smil/nsSMILAnimationController.h

@@ -94,7 +94,12 @@ public:
   // (A resample performs the same operations as a sample but doesn't advance
   // the current time and doesn't check if the container is paused)
   void Resample() { DoSample(PR_FALSE); }
-  void SetResampleNeeded() { mResampleNeeded = PR_TRUE; }
+  void SetResampleNeeded()
+  {
+    if (!mRunningSample) {
+      mResampleNeeded = PR_TRUE;
+    }
+  }
   void FlushResampleRequests()
   {
     if (!mResampleNeeded)
@@ -203,9 +208,7 @@ protected:
   // record the time, set the following flag, and then wait until we have an
   // animation element. Then we'll reset this flag and actually start sampling.
   PRPackedBool               mDeferredStartSampling;
-#ifdef DEBUG
   PRPackedBool               mRunningSample;
-#endif
 
   // Store raw ptr to mDocument.  It owns the controller, so controller
   // shouldn't outlive it

content/smil/nsSMILCSSProperty.cpp

@@ -102,8 +102,9 @@ nsSMILCSSProperty::GetBaseValue() const
   // from ALL return points. This function must only return THIS variable:
   nsSMILValue baseValue;
 
-  // SPECIAL CASE: Shorthands
-  if (nsCSSProps::IsShorthand(mPropID)) {
+  // SPECIAL CASE: (a) Shorthands
+  //               (b) 'display'
+  if (nsCSSProps::IsShorthand(mPropID) || mPropID == eCSSProperty_display) {
     // We can't look up the base (computed-style) value of shorthand
     // properties, because they aren't guaranteed to have a consistent computed
     // value.  However, that's not a problem, because it turns out the caller
@@ -112,6 +113,10 @@ nsSMILCSSProperty::GetBaseValue() const
     // properties we know about don't support those operations. So, we can just
     // return a dummy value (initialized with the right type, so as not to
     // indicate failure).
+    // For 'display' we'd like to avoid clearing and setting this property since
+    // it can cause frames to be recreated, so instead we just return a dummy
+    // value. As with shorthand properties this is ok as we never interpolate or
+    // add display properties.
     nsSMILValue tmpVal(&nsSMILCSSValueType::sSingleton);
     baseValue.Swap(tmpVal);
     return baseValue;