Bug 690994 - Check for self-dependent times when there are coincident zero-duration intervals; r=dholbert

content/smil/crashtests/690994-1.svg

@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+<svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait">
+<script>
+<![CDATA[
+function boom()
+{
+  document.documentElement.removeChild(document.getElementById("a"));
+  document.documentElement.removeAttribute("class");
+}
+]]>
+</script>
+<animate id="a" begin="a.end; -0.1s" end="a.begin+0.2s" onend="boom()"/>
+<animate id="a" begin="a.end; -0.1s" end="a.begin+0.2s"/>
+</svg>

content/smil/crashtests/crashtests.list

@@ -42,4 +42,5 @@ load 670313-1.svg
 load 678822-1.svg
 load 678847-1.svg
 load 678938-1.svg
+load 690994-1.svg
 load 699325-1.svg

content/smil/nsSMILTimedElement.cpp

@@ -1622,10 +1622,6 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
     beginAfter = aPrevInterval->End()->Time();
     prevIntervalWasZeroDur
       = aPrevInterval->End()->Time() == aPrevInterval->Begin()->Time();
-    if (aFixedBeginTime) {
-      prevIntervalWasZeroDur &=
-        aPrevInterval->Begin()->Time() == aFixedBeginTime->Time();
-    }
   } else {
     beginAfter.SetMillis(LL_MININT);
   }
@@ -1647,17 +1643,17 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       tempBegin = new nsSMILInstanceTime(nsSMILTimeValue(0));
     } else {
       PRInt32 beginPos = 0;
-      // If we're updating the current interval then skip any begin time that is
-      // dependent on the current interval's begin time. e.g.
-      //   <animate id="a" begin="b.begin; a.begin+2s"...
-      // If b's interval disappears whilst 'a' is in the waiting state the begin
-      // time at "a.begin+2s" should be skipped since 'a' never begun.
       do {
         tempBegin =
           GetNextGreaterOrEqual(mBeginInstances, beginAfter, beginPos);
         if (!tempBegin || !tempBegin->Time().IsDefinite()) {
           return false;
         }
+      // If we're updating the current interval then skip any begin time that is
+      // dependent on the current interval's begin time. e.g.
+      //   <animate id="a" begin="b.begin; a.begin+2s"...
+      // If b's interval disappears whilst 'a' is in the waiting state the begin
+      // time at "a.begin+2s" should be skipped since 'a' never begun.
       } while (aReplacedInterval &&
                tempBegin->GetBaseTime() == aReplacedInterval->Begin());
     }
@@ -1668,22 +1664,24 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
     // Calculate end time
     {
       PRInt32 endPos = 0;
-      // As above with begin times, avoid creating self-referential loops
-      // between instance times by checking that the newly found end instance
-      // time is not already dependent on the end of the current interval.
       do {
         tempEnd =
           GetNextGreaterOrEqual(mEndInstances, tempBegin->Time(), endPos);
+
+        // SMIL doesn't allow for coincident zero-duration intervals, so if the
+        // previous interval was zero-duration, and tempEnd is going to give us
+        // another zero duration interval, then look for another end to use
+        // instead.
+        if (tempEnd && prevIntervalWasZeroDur &&
+            tempEnd->Time() == beginAfter) {
+          tempEnd = GetNextGreater(mEndInstances, tempBegin->Time(), endPos);
+        }
+      // As above with begin times, avoid creating self-referential loops
+      // between instance times by checking that the newly found end instance
+      // time is not already dependent on the end of the current interval.
       } while (tempEnd && aReplacedInterval &&
                tempEnd->GetBaseTime() == aReplacedInterval->End());
 
-      // If the last interval ended at the same point and was zero-duration and
-      // this one is too, look for another end to use instead
-      if (tempEnd && tempEnd->Time() == tempBegin->Time() &&
-          prevIntervalWasZeroDur) {
-        tempEnd = GetNextGreater(mEndInstances, tempBegin->Time(), endPos);
-      }
-
       // If all the ends are before the beginning we have a bad interval UNLESS:
       // a) We never had any end attribute to begin with (and hence we should
       //    just use the active duration after allowing for the possibility of
@@ -1695,8 +1693,8 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       //    loop. In any case, the interval should be allowed to be open.), OR
       // c) We have end events which leave the interval open-ended.
       bool openEndedIntervalOk = mEndSpecs.IsEmpty() ||
-                                   !HaveDefiniteEndTimes() ||
+                                 !HaveDefiniteEndTimes() ||
-                                   EndHasEventConditions();
+                                 EndHasEventConditions();
       if (!tempEnd && !openEndedIntervalOk)
         return false; // Bad interval
 
@@ -1710,17 +1708,21 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
     }
     NS_ABORT_IF_FALSE(tempEnd, "Failed to get end point for next interval");
 
-    // If we get two zero-length intervals in a row we will potentially have an
+    // When we choose the interval endpoints, we don't allow coincident
-    // infinite loop so we break it here by searching for the next begin time
+    // zero-duration intervals, so if we arrive here and we have a zero-duration
-    // greater than tempEnd on the next time around.
+    // interval starting at the same point as a previous zero-duration interval,
-    if (tempEnd->Time().IsDefinite() && tempBegin->Time() == tempEnd->Time()) {
+    // then it must be because we've applied constraints to the active duration.
+    // In that case, we will potentially run into an infinite loop, so we break
+    // it by searching for the next interval that starts AFTER our current
+    // zero-duration interval.
+    if (prevIntervalWasZeroDur && tempEnd->Time() == beginAfter) {
       if (prevIntervalWasZeroDur) {
-        beginAfter.SetMillis(tempEnd->Time().GetMillis() + 1);
+        beginAfter.SetMillis(tempBegin->Time().GetMillis() + 1);
         prevIntervalWasZeroDur = false;
         continue;
       }
-      prevIntervalWasZeroDur = true;
     }
+    prevIntervalWasZeroDur = tempBegin->Time() == tempEnd->Time();
 
     // Check for valid interval
     if (tempEnd->Time() > zeroTime ||