From 28138b63332b2176ceee9cfc186abec3e113ae63 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Mon, 14 Nov 2011 16:58:30 +1300 Subject: [PATCH] Bug 690994 - Check for self-dependent times when there are coincident zero-duration intervals; r=dholbert --- content/smil/crashtests/690994-1.svg | 14 +++++++ content/smil/crashtests/crashtests.list | 1 + content/smil/nsSMILTimedElement.cpp | 56 +++++++++++++------------ 3 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 content/smil/crashtests/690994-1.svg diff --git a/content/smil/crashtests/690994-1.svg b/content/smil/crashtests/690994-1.svg new file mode 100644 index 000000000000..45c60b90ef8d --- /dev/null +++ b/content/smil/crashtests/690994-1.svg @@ -0,0 +1,14 @@ + + + + + + diff --git a/content/smil/crashtests/crashtests.list b/content/smil/crashtests/crashtests.list index 3289584e4a7a..64bfa0ebb8c6 100644 --- a/content/smil/crashtests/crashtests.list +++ b/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 diff --git a/content/smil/nsSMILTimedElement.cpp b/content/smil/nsSMILTimedElement.cpp index caababa24a35..3b6c59defa57 100644 --- a/content/smil/nsSMILTimedElement.cpp +++ b/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. - // 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. + // 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() || - EndHasEventConditions(); + !HaveDefiniteEndTimes() || + 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 - // 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 ||