Bug 697640 - Ignore self-dependent end instance times when determining if an open-ended interval is ok; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Mon, 14 Nov 2011 16:58:30 +1300
changeset 80223 b1e2ee0bbdb5ad97abb7fd546c4fdb08fb9c8d2f
parent 80222 dc82c3485d1a66af9e2eff30a9bbdf70d6fd7f31
child 80224 677fe017e6a03295ef9351dec38b77a8ad317ab4
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
bugs697640
milestone11.0a1
Bug 697640 - Ignore self-dependent end instance times when determining if an open-ended interval is ok; r=dholbert
content/smil/crashtests/697640-1.svg
content/smil/crashtests/crashtests.list
content/smil/nsSMILTimedElement.cpp
content/smil/nsSMILTimedElement.h
new file mode 100644
--- /dev/null
+++ b/content/smil/crashtests/697640-1.svg
@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <animate id="b" end="b.end" dur="3s" />
+</svg>
--- a/content/smil/crashtests/crashtests.list
+++ b/content/smil/crashtests/crashtests.list
@@ -38,9 +38,10 @@ 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 697640-1.svg
 load 699325-1.svg
--- a/content/smil/nsSMILTimedElement.cpp
+++ b/content/smil/nsSMILTimedElement.cpp
@@ -1677,31 +1677,47 @@ nsSMILTimedElement::GetNextInterval(cons
           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 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();
-      if (!tempEnd && !openEndedIntervalOk)
-        return false; // Bad interval
+      if (!tempEnd) {
+        // If all the ends are before the beginning we have a bad interval
+        // UNLESS:
+        // a) We never had any end attribute to begin with (the SMIL pseudocode
+        //    places this condition earlier in the flow but that fails to allow
+        //    for DOM calls when no "indefinite" condition is given), OR
+        // b) We never had any end instance times to begin with, OR
+        // c) We have end events which leave the interval open-ended.
+        bool openEndedIntervalOk = mEndSpecs.IsEmpty() ||
+                                   mEndInstances.IsEmpty() ||
+                                   EndHasEventConditions();
+
+        // The above conditions correspond with the SMIL pseudocode but SMIL
+        // doesn't address self-dependent instance times which we choose to
+        // ignore.
+        //
+        // Therefore we add a qualification of (b) above that even if
+        // there are end instance times but they all depend on the end of the
+        // current interval we should act as if they didn't exist and allow the
+        // open-ended interval.
+        //
+        // In the following condition we don't use |= because it doesn't provide
+        // short-circuit behavior.
+        openEndedIntervalOk = openEndedIntervalOk ||
+                             (aReplacedInterval &&
+                              AreEndTimesDependentOn(aReplacedInterval->End()));
+
+        if (!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);
       }
@@ -2250,36 +2266,40 @@ const nsSMILInterval*
 nsSMILTimedElement::GetPreviousInterval() const
 {
   return mOldIntervals.IsEmpty()
     ? nsnull
     : mOldIntervals[mOldIntervals.Length()-1].get();
 }
 
 bool
-nsSMILTimedElement::HaveDefiniteEndTimes() const
-{
-  if (mEndInstances.IsEmpty())
-    return false;
-
-  // mEndInstances is sorted so if the first time is not definite then none of
-  // them are
-  return mEndInstances[0]->Time().IsDefinite();
-}
-
-bool
 nsSMILTimedElement::EndHasEventConditions() const
 {
   for (PRUint32 i = 0; i < mEndSpecs.Length(); ++i) {
     if (mEndSpecs[i]->IsEventBased())
       return true;
   }
   return false;
 }
 
+bool
+nsSMILTimedElement::AreEndTimesDependentOn(
+  const nsSMILInstanceTime* aBase) const
+{
+  if (mEndInstances.IsEmpty())
+    return false;
+
+  for (PRUint32 i = 0; i < mEndInstances.Length(); ++i) {
+    if (mEndInstances[i]->GetBaseTime() != aBase) {
+      return false;
+    }
+  }
+  return true;
+}
+
 //----------------------------------------------------------------------
 // Hashtable callback functions
 
 /* static */ PR_CALLBACK PLDHashOperator
 nsSMILTimedElement::NotifyNewIntervalCallback(TimeValueSpecPtrKey* aKey,
                                               void* aData)
 {
   NS_ABORT_IF_FALSE(aKey, "Null hash key for time container hash table");
--- a/content/smil/nsSMILTimedElement.h
+++ b/content/smil/nsSMILTimedElement.h
@@ -520,18 +520,19 @@ protected:
   void              NotifyChangedInterval(nsSMILInterval* aInterval,
                                           bool aBeginObjectChanged,
                                           bool aEndObjectChanged);
 
   void              FireTimeEventAsync(PRUint32 aMsg, PRInt32 aDetail);
   const nsSMILInstanceTime* GetEffectiveBeginInstance() const;
   const nsSMILInterval* GetPreviousInterval() const;
   bool              HasPlayed() const { return !mOldIntervals.IsEmpty(); }
-  bool              HaveDefiniteEndTimes() const;
   bool              EndHasEventConditions() const;
+  bool              AreEndTimesDependentOn(
+                      const nsSMILInstanceTime* aBase) const;
 
   // Reset the current interval by first passing ownership to a temporary
   // variable so that if Unlink() results in us receiving a callback,
   // mCurrentInterval will be nsnull and we will be in a consistent state.
   void ResetCurrentInterval()
   {
     if (mCurrentInterval) {
       // Transfer ownership to temp var. (This sets mCurrentInterval to null.)