Bug 720103 - Detect and break create-delete cycles in SMIL timegraphs; r=dholbert

content/smil/nsSMILTimedElement.cpp

@@ -233,7 +233,7 @@ const PRUint8 nsSMILTimedElement::sMaxNumInstanceTimes = 100;
 
 // Detect if we arrive in some sort of undetected recursive syncbase dependency
 // relationship
-const PRUint16 nsSMILTimedElement::sMaxUpdateIntervalRecursionDepth = 20;
+const PRUint8 nsSMILTimedElement::sMaxUpdateIntervalRecursionDepth = 20;
 
 //----------------------------------------------------------------------
 // Ctor, dtor
@@ -252,6 +252,7 @@ nsSMILTimedElement::nsSMILTimedElement()
   mSeekState(SEEK_NOT_SEEKING),
   mDeferIntervalUpdates(false),
   mDoDeferredUpdate(false),
+  mDeleteCount(0),
   mUpdateIntervalRecursionDepth(0)
 {
   mSimpleDur.SetIndefinite();
@@ -1960,12 +1961,29 @@ nsSMILTimedElement::UpdateCurrentInterval(bool aForceChangeNotice)
   if (mElementState == STATE_STARTUP)
     return;
 
+  // Although SMIL gives rules for detecting cycles in change notifications,
+  // some configurations can lead to create-delete-create-delete-etc. cycles
+  // which SMIL does not consider.
+  //
+  // In order to provide consistent behavior in such cases, we detect two
+  // deletes in a row and then refuse to create any further intervals. That is,
+  // we say the configuration is invalid.
+  if (mDeleteCount > 1) {
+    // When we update the delete count we also set the state to post active, so
+    // if we're not post active here then something other than
+    // UpdateCurrentInterval has updated the element state in between and all
+    // bets are off.
+    NS_ABORT_IF_FALSE(mElementState == STATE_POSTACTIVE,
+      "Expected to be in post-active state after performing double delete");
+    return;
+  }
+
   // Check that we aren't stuck in infinite recursion updating some syncbase
   // dependencies. Generally such situations should be detected in advance and
   // the chain broken in a sensible and predictable manner, so if we're hitting
   // this assertion we need to work out how to detect the case that's causing
   // it. In release builds, just bail out before we overflow the stack.
-  AutoRestore<PRUint16> depthRestorer(mUpdateIntervalRecursionDepth);
+  AutoRestore<PRUint8> depthRestorer(mUpdateIntervalRecursionDepth);
   if (++mUpdateIntervalRecursionDepth > sMaxUpdateIntervalRecursionDepth) {
     NS_ABORT_IF_FALSE(false,
         "Update current interval recursion depth exceeded threshold");
@@ -2026,6 +2044,8 @@ nsSMILTimedElement::UpdateCurrentInterval(bool aForceChangeNotice)
       // sample (along with firing end events, clearing intervals etc.)
       RegisterMilestone();
     } else if (mElementState == STATE_WAITING) {
+      AutoRestore<PRUint8> deleteCountRestorer(mDeleteCount);
+      ++mDeleteCount;
       mElementState = STATE_POSTACTIVE;
       ResetCurrentInterval();
     }

content/smil/nsSMILTimedElement.h

@@ -625,13 +625,13 @@ protected:
   // Used to batch updates to the timing model
   class AutoIntervalUpdateBatcher;
   bool mDeferIntervalUpdates;
-  bool mDoDeferredUpdate; // Set if an update to the current interval
-                                  // was requested while mDeferIntervalUpdates
-                                  // was set
+  bool mDoDeferredUpdate; // Set if an update to the current interval was
+                          // requested while mDeferIntervalUpdates was set
 
   // Recursion depth checking
-  PRUint16              mUpdateIntervalRecursionDepth;
-  static const PRUint16 sMaxUpdateIntervalRecursionDepth;
+  PRUint8              mDeleteCount;
+  PRUint8              mUpdateIntervalRecursionDepth;
+  static const PRUint8 sMaxUpdateIntervalRecursionDepth;
 };
 
 #endif // NS_SMILTIMEDELEMENT_H_