Bug 690994 - Check for self-dependent times when there are coincident zero-duration intervals; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Mon, 14 Nov 2011 16:58:30 +1300
changeset 80222 dc82c3485d1a66af9e2eff30a9bbdf70d6fd7f31
parent 80221 15cf68a3c027d91c1ada77a35fbcb587eaa80b9f
child 80223 b1e2ee0bbdb5ad97abb7fd546c4fdb08fb9c8d2f
push id323
push userrcampbell@mozilla.com
push dateTue, 15 Nov 2011 21:58:36 +0000
treeherderfx-team@3ea216303184 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs690994
milestone11.0a1
Bug 690994 - Check for self-dependent times when there are coincident zero-duration intervals; r=dholbert
content/smil/crashtests/690994-1.svg
content/smil/crashtests/crashtests.list
content/smil/nsSMILTimedElement.cpp
new file mode 100644
--- /dev/null
+++ b/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>
--- a/content/smil/crashtests/crashtests.list
+++ b/content/smil/crashtests/crashtests.list
@@ -37,9 +37,10 @@ load 615872-1.svg
 load 650732-1.svg
 load 665334-1.svg
 load 669225-1.svg
 load 669225-2.svg
 load 670313-1.svg
 load 678822-1.svg
 load 678847-1.svg
 load 678938-1.svg
+load 690994-1.svg
 load 699325-1.svg
--- a/content/smil/nsSMILTimedElement.cpp
+++ b/content/smil/nsSMILTimedElement.cpp
@@ -1617,20 +1617,16 @@ nsSMILTimedElement::GetNextInterval(cons
 
   // Calc starting point
   nsSMILTimeValue beginAfter;
   bool prevIntervalWasZeroDur = false;
   if (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);
   }
 
   nsRefPtr<nsSMILInstanceTime> tempBegin;
   nsRefPtr<nsSMILInstanceTime> tempEnd;
 
   while (true) {
@@ -1642,90 +1638,96 @@ nsSMILTimedElement::GetNextInterval(cons
       // our ref-counting is not const-correct
       tempBegin = const_cast<nsSMILInstanceTime*>(aFixedBeginTime);
     } else if ((!mAnimationElement ||
                 !mAnimationElement->HasAnimAttr(nsGkAtoms::begin)) &&
                beginAfter <= zeroTime) {
       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());
     }
     NS_ABORT_IF_FALSE(tempBegin && tempBegin->Time().IsDefinite() &&
         tempBegin->Time() >= beginAfter,
         "Got a bad begin time while fetching next interval");
 
     // 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
       //    an end instance provided by a DOM call), OR
       // b) We have no definite end instances (SMIL only says "if the instance
       //    list is empty"--but if we have indefinite/unresolved instance times
       //    then there must be a good reason we haven't used them (since they
       //    will be >= tempBegin) such as avoiding creating a self-referential
       //    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() ||
-                                   EndHasEventConditions();
+                                 !HaveDefiniteEndTimes() ||
+                                 EndHasEventConditions();
       if (!tempEnd && !openEndedIntervalOk)
         return false; // Bad interval
 
       nsSMILTimeValue intervalEnd = tempEnd
                                   ? tempEnd->Time() : nsSMILTimeValue();
       nsSMILTimeValue activeEnd = CalcActiveEnd(tempBegin->Time(), intervalEnd);
 
       if (!tempEnd || intervalEnd != activeEnd) {
         tempEnd = new nsSMILInstanceTime(activeEnd);
       }
     }
     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
-    // infinite loop so we break it here by searching for the next begin time
-    // greater than tempEnd on the next time around.
-    if (tempEnd->Time().IsDefinite() && tempBegin->Time() == tempEnd->Time()) {
+    // When we choose the interval endpoints, we don't allow coincident
+    // zero-duration intervals, so if we arrive here and we have a zero-duration
+    // interval starting at the same point as a previous zero-duration interval,
+    // 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 ||
        (tempBegin->Time() == zeroTime && tempEnd->Time() == zeroTime)) {
       aResult.Set(*tempBegin, *tempEnd);
       return true;
     }