Bug 537361: Store SMIL intervals with state for restoring. r=dholbert

content/smil/Makefile.in

@@ -61,6 +61,7 @@ CPPSRCS		+= \
 	nsSMILCSSValueType.cpp \
 	nsSMILFloatType.cpp \
 	nsSMILInstanceTime.cpp \
+	nsSMILInterval.cpp \
 	nsSMILNullType.cpp \
 	nsSMILParserUtils.cpp \
 	nsSMILRepeatCount.cpp \

content/smil/nsSMILInstanceTime.cpp

@@ -36,13 +36,49 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsSMILInstanceTime.h"
+#include "nsSMILInterval.h"
+#include "nsSMILTimeValueSpec.h"
+
+//----------------------------------------------------------------------
+// Helper classes
+
+namespace
+{
+  // Utility class to set a PRPackedBool value to PR_TRUE whilst it is in scope.
+  // Saves us having to remember to clear the flag at every possible return.
+  class AutoBoolSetter
+  {
+  public:
+    AutoBoolSetter(PRPackedBool& aValue)
+    : mValue(aValue)
+    {
+      mValue = PR_TRUE;
+    }
+ 
+    ~AutoBoolSetter()
+    {
+      mValue = PR_FALSE;
+    }
+
+  private:
+    PRPackedBool&   mValue;
+  };
+}
+
+//----------------------------------------------------------------------
+// Implementation
 
 nsSMILInstanceTime::nsSMILInstanceTime(const nsSMILTimeValue& aTime,
-    const nsSMILInstanceTime* aDependentTime,
+                                       nsSMILInstanceTimeSource aSource,
-    nsSMILInstanceTimeSource aSource)
+                                       nsSMILTimeValueSpec* aCreator,
-: mTime(aTime),
+                                       nsSMILInterval* aBaseInterval)
-  mFlags(0),
+  : mTime(aTime),
-  mSerial(0)
+    mFlags(0),
+    mSerial(0),
+    mVisited(PR_FALSE),
+    mChainEnd(PR_FALSE),
+    mCreator(aCreator),
+    mBaseInterval(nsnull)
 {
   switch (aSource) {
     case SOURCE_NONE:
@@ -62,43 +98,66 @@ nsSMILInstanceTime::nsSMILInstanceTime(const nsSMILTimeValue& aTime,
       break;
   }
 
-  SetDependentTime(aDependentTime);
+  SetBaseInterval(aBaseInterval);
+}
+
+nsSMILInstanceTime::~nsSMILInstanceTime()
+{
+  NS_ABORT_IF_FALSE(!mBaseInterval && !mCreator,
+      "Destroying instance time without first calling Unlink()");
 }
 
 void
-nsSMILInstanceTime::SetDependentTime(const nsSMILInstanceTime* aDependentTime)
+nsSMILInstanceTime::Unlink()
 {
-  // We must make the dependent time mutable because our ref-counting isn't
+  nsRefPtr<nsSMILInstanceTime> deathGrip(this);
-  // const-correct and BreakPotentialCycle may update dependencies (which should
+  if (mBaseInterval) {
-  // be considered 'mutable')
+    mBaseInterval->RemoveDependentTime(*this);
-  nsSMILInstanceTime* mutableDependentTime =
+    mBaseInterval = nsnull;
-    const_cast<nsSMILInstanceTime*>(aDependentTime);
-
-  // Make sure we don't end up creating a cycle between the dependent time
-  // pointers. (Note that this is not the same as detecting syncbase dependency
-  // cycles. That is done by nsSMILTimeValueSpec. mDependentTime is used ONLY
-  // for ensuring correct ordering within the animation sandwich.)
-  if (aDependentTime) {
-    mutableDependentTime->BreakPotentialCycle(this);
   }
-
+  mCreator = nsnull;
-  mDependentTime = mutableDependentTime;
 }
 
 void
-nsSMILInstanceTime::BreakPotentialCycle(const nsSMILInstanceTime* aNewTail)
+nsSMILInstanceTime::HandleChangedInterval(
+    const nsSMILTimeContainer* aSrcContainer,
+    PRBool aBeginObjectChanged,
+    PRBool aEndObjectChanged)
 {
-  if (!mDependentTime)
+  NS_ABORT_IF_FALSE(mBaseInterval,
-    return;
+      "Got call to HandleChangedInterval on an independent instance time.");
+  NS_ABORT_IF_FALSE(mCreator, "Base interval is set but creator is not.");
 
-  if (mDependentTime == aNewTail) {
+  if (mVisited || mChainEnd) {
-    // Making aNewTail the new tail of the chain would create a cycle so we
+    // We're breaking the cycle here but we need to ensure that if we later
-    // prevent this by unlinking the pointer to aNewTail.
+    // receive a change notice in a different context (e.g. due to a time
-    mDependentTime = nsnull;
+    // container change) that we don't end up following the chain further and so
+    // we set a flag to that effect.
+    mChainEnd = PR_TRUE;
     return;
   }
 
-  mDependentTime->BreakPotentialCycle(aNewTail);
+  PRBool objectChanged = mCreator->DependsOnBegin() ? aBeginObjectChanged
+                                                    : aEndObjectChanged;
+  AutoBoolSetter setVisited(mVisited);
+
+  nsRefPtr<nsSMILInstanceTime> deathGrip(this);
+  mCreator->HandleChangedInstanceTime(*GetBaseTime(), aSrcContainer, *this,
+                                      objectChanged);
+}
+
+void
+nsSMILInstanceTime::HandleDeletedInterval()
+{
+  NS_ABORT_IF_FALSE(mBaseInterval,
+      "Got call to HandleDeletedInterval on an independent instance time.");
+  NS_ABORT_IF_FALSE(mCreator, "Base interval is set but creator is not.");
+
+  mBaseInterval = nsnull;
+
+  nsRefPtr<nsSMILInstanceTime> deathGrip(this);
+  mCreator->HandleDeletedInstanceTime(*this);
+  mCreator = nsnull;
 }
 
 PRBool
@@ -108,11 +167,71 @@ nsSMILInstanceTime::IsDependent(const nsSMILInstanceTime& aOther,
   NS_ABORT_IF_FALSE(aRecursionDepth < 1000,
       "We seem to have created a cycle between instance times");
 
-  if (!mDependentTime)
+  const nsSMILInstanceTime* myBaseTime = GetBaseTime();
+  if (!myBaseTime)
     return PR_FALSE;
 
-  if (mDependentTime == &aOther)
+  if (myBaseTime == &aOther)
     return PR_TRUE;
 
-  return mDependentTime->IsDependent(aOther, ++aRecursionDepth);
+  return myBaseTime->IsDependent(aOther, ++aRecursionDepth);
+}
+
+void
+nsSMILInstanceTime::SetBaseInterval(nsSMILInterval* aBaseInterval)
+{
+  NS_ABORT_IF_FALSE(!mBaseInterval,
+      "Attepting to reassociate an instance time with a different interval.");
+
+  // Make sure we don't end up creating a cycle between the dependent time
+  // pointers.
+  if (aBaseInterval) {
+    NS_ABORT_IF_FALSE(mCreator,
+        "Attempting to create a dependent instance time without reference "
+        "to the creating nsSMILTimeValueSpec object.");
+    if (!mCreator)
+      return;
+
+    const nsSMILInstanceTime* dependentTime = mCreator->DependsOnBegin()
+                                            ? aBaseInterval->Begin()
+                                            : aBaseInterval->End();
+    dependentTime->BreakPotentialCycle(this);
+    aBaseInterval->AddDependentTime(*this);
+  }
+
+  mBaseInterval = aBaseInterval;
+}
+
+const nsSMILInstanceTime*
+nsSMILInstanceTime::GetBaseTime() const
+{
+  if (!mBaseInterval) {
+    return nsnull;
+  }
+
+  NS_ABORT_IF_FALSE(mCreator, "Base interval is set but there is no creator.");
+  if (!mCreator) {
+    return nsnull;
+  }
+
+  return mCreator->DependsOnBegin() ? mBaseInterval->Begin()
+                                    : mBaseInterval->End();
+}
+
+void
+nsSMILInstanceTime::BreakPotentialCycle(
+    const nsSMILInstanceTime* aNewTail) const
+{
+  const nsSMILInstanceTime* myBaseTime = GetBaseTime();
+  if (!myBaseTime)
+    return;
+
+  if (myBaseTime == aNewTail) {
+    // Making aNewTail the new tail of the chain would create a cycle so we
+    // prevent this by unlinking the pointer to aNewTail.
+    mBaseInterval->RemoveDependentTime(*this);
+    return;
+  }
+
+  myBaseTime->BreakPotentialCycle(aNewTail);
 }

content/smil/nsSMILInstanceTime.h

@@ -41,6 +41,8 @@
 #include "nsSMILTimeValue.h"
 #include "nsAutoPtr.h"
 
+class nsSMILInterval;
+class nsSMILTimeContainer;
 class nsSMILTimeValueSpec;
 
 //----------------------------------------------------------------------
@@ -83,22 +85,24 @@ public:
   };
 
   nsSMILInstanceTime(const nsSMILTimeValue& aTime,
-                     const nsSMILInstanceTime* aDependentTime,
+                     nsSMILInstanceTimeSource aSource = SOURCE_NONE,
-                     nsSMILInstanceTimeSource aSource = SOURCE_NONE);
+                     nsSMILTimeValueSpec* aCreator = nsnull,
+                     nsSMILInterval* aBaseInterval = nsnull);
+  ~nsSMILInstanceTime();
+  void Unlink();
+  void HandleChangedInterval(const nsSMILTimeContainer* aSrcContainer,
+                             PRBool aBeginObjectChanged,
+                             PRBool aEndObjectChanged);
+  void HandleDeletedInterval();
 
   const nsSMILTimeValue& Time() const { return mTime; }
-
+  const nsSMILTimeValueSpec* GetCreator() const { return mCreator; }
-  const nsSMILInstanceTime* GetDependentTime() const { return mDependentTime; }
-  void SetDependentTime(const nsSMILInstanceTime* aDependentTime);
 
   PRBool ClearOnReset() const { return !!(mFlags & kClearOnReset); }
   PRBool MayUpdate() const { return !!(mFlags & kMayUpdate); }
   PRBool FromDOM() const { return !!(mFlags & kFromDOM); }
 
-  void MarkNoLongerUpdating()
+  void MarkNoLongerUpdating() { mFlags &= ~kMayUpdate; }
-  {
-    mFlags &= ~kMayUpdate;
-  }
 
   void DependentUpdate(const nsSMILTimeValue& aNewTime)
   {
@@ -110,9 +114,9 @@ public:
   PRBool IsDependent(const nsSMILInstanceTime& aOther,
                      PRUint32 aRecursionDepth = 0) const;
 
-  PRBool SameTimeAndDependency(const nsSMILInstanceTime& aOther) const
+  PRBool SameTimeAndBase(const nsSMILInstanceTime& aOther) const
   {
-    return mTime == aOther.mTime && mDependentTime == aOther.mDependentTime;
+    return mTime == aOther.mTime && GetBaseTime() == aOther.GetBaseTime();
   }
 
   // Get and set a serial number which may be used by a containing class to
@@ -152,7 +156,9 @@ public:
   }
 
 protected:
-  void BreakPotentialCycle(const nsSMILInstanceTime* aNewTail);
+  void SetBaseInterval(nsSMILInterval* aBaseInterval);
+  void BreakPotentialCycle(const nsSMILInstanceTime* aNewTail) const;
+  const nsSMILInstanceTime* GetBaseTime() const;
 
   nsSMILTimeValue mTime;
 
@@ -179,13 +185,18 @@ protected:
     // DOM.
     kFromDOM = 4
   };
-  PRUint8  mFlags; // Combination of kClearOnReset, kMayUpdate, etc.
+  PRUint8       mFlags; // Combination of kClearOnReset, kMayUpdate, etc.
-  PRUint32 mSerial; // A serial number used by the containing class to specify
+  PRUint32      mSerial; // A serial number used by the containing class to
-                    // the sort order for instance times with the same mTime.
+                         // specify the sort order for instance times with the
+                         // same mTime.
+  PRPackedBool  mVisited;
+  PRPackedBool  mChainEnd;
 
-  // The instance time upon which this instance time is based (if any). This is
+  nsSMILTimeValueSpec* mCreator; // The nsSMILTimeValueSpec object that created
-  // ONLY used for determining the compositing order of animations.
+                                 // us. (currently only needed for syncbase
-  nsRefPtr<nsSMILInstanceTime> mDependentTime;
+                                 // instance times.)
+  nsSMILInterval* mBaseInterval; // Interval from which this time is derived
+                                 // (only used for syncbase instance times)
 };
 
 #endif // NS_SMILINSTANCETIME_H_

content/smil/nsSMILInterval.cpp

@@ -0,0 +1,141 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is the Mozilla SMIL module.
+ *
+ * The Initial Developer of the Original Code is Brian Birtles.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Brian Birtles <birtles@gmail.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "nsSMILInterval.h"
+
+nsSMILInterval::nsSMILInterval()
+:
+  mBeginObjectChanged(PR_FALSE),
+  mEndObjectChanged(PR_FALSE)
+{
+}
+
+nsSMILInterval::nsSMILInterval(const nsSMILInterval& aOther)
+:
+  mBegin(aOther.mBegin),
+  mEnd(aOther.mEnd),
+  mBeginObjectChanged(PR_FALSE),
+  mEndObjectChanged(PR_FALSE)
+{
+  NS_ABORT_IF_FALSE(aOther.mDependentTimes.IsEmpty(),
+      "Attempting to copy-construct an interval with dependent times, "
+      "this will lead to instance times being shared between intervals.");
+}
+
+nsSMILInterval::~nsSMILInterval()
+{
+  NS_ABORT_IF_FALSE(mDependentTimes.IsEmpty(),
+      "Destroying interval without disassociating dependent instance times. "
+      "NotifyDeleting was not called.");
+}
+
+void
+nsSMILInterval::NotifyChanged(const nsSMILTimeContainer* aContainer)
+{
+  for (PRInt32 i = mDependentTimes.Length() - 1; i >= 0; --i) {
+    mDependentTimes[i]->HandleChangedInterval(aContainer,
+        mBeginObjectChanged, mEndObjectChanged);
+  }
+  mBeginObjectChanged = PR_FALSE;
+  mEndObjectChanged = PR_FALSE;
+}
+
+void
+nsSMILInterval::NotifyDeleting()
+{
+  for (PRInt32 i = mDependentTimes.Length() - 1; i >= 0; --i) {
+    mDependentTimes[i]->HandleDeletedInterval();
+  }
+  mDependentTimes.Clear();
+}
+
+nsSMILInstanceTime*
+nsSMILInterval::Begin()
+{
+  NS_ABORT_IF_FALSE(mBegin && mEnd,
+      "Requesting Begin() on un-initialized instance time.");
+  return mBegin;
+}
+
+nsSMILInstanceTime*
+nsSMILInterval::End()
+{
+  NS_ABORT_IF_FALSE(mBegin && mEnd,
+      "Requesting End() on un-initialized instance time.");
+  return mEnd;
+}
+
+void
+nsSMILInterval::SetBegin(nsSMILInstanceTime& aBegin)
+{
+  NS_ABORT_IF_FALSE(aBegin.Time().IsResolved(),
+      "Attempting to set unresolved begin time on interval.");
+
+  if (mBegin == &aBegin)
+    return;
+
+  mBegin = &aBegin;
+  mBeginObjectChanged = PR_TRUE;
+}
+
+void
+nsSMILInterval::SetEnd(nsSMILInstanceTime& aEnd)
+{
+  if (mEnd == &aEnd)
+    return;
+
+  mEnd = &aEnd;
+  mEndObjectChanged = PR_TRUE;
+}
+
+void
+nsSMILInterval::AddDependentTime(nsSMILInstanceTime& aTime)
+{
+  nsRefPtr<nsSMILInstanceTime>* inserted =
+    mDependentTimes.InsertElementSorted(&aTime);
+  if (!inserted) {
+    NS_WARNING("Insufficient memory to insert instance time.");
+  }
+}
+
+void
+nsSMILInterval::RemoveDependentTime(const nsSMILInstanceTime& aTime)
+{
+  PRBool found = mDependentTimes.RemoveElementSorted(&aTime);
+  NS_ABORT_IF_FALSE(found, "Couldn't find instance time to delete.");
+  (void)found;
+}

content/smil/nsSMILInterval.h

@@ -39,6 +39,7 @@
 #define NS_SMILINTERVAL_H_
 
 #include "nsSMILInstanceTime.h"
+#include "nsTArray.h"
 
 //----------------------------------------------------------------------
 // nsSMILInterval class
@@ -52,84 +53,88 @@
 class nsSMILInterval
 {
 public:
-  void Set(nsSMILInstanceTime& aBegin, nsSMILInstanceTime& aEnd)
+  nsSMILInterval();
-  {
+  nsSMILInterval(const nsSMILInterval& aOther);
-    NS_ABORT_IF_FALSE(aBegin.Time().IsResolved(),
+  ~nsSMILInterval();
-        "Attempting to set unresolved begin time on an interval");
+  void NotifyChanged(const nsSMILTimeContainer* aContainer);
-    mBegin = &aBegin;
+  void NotifyDeleting();
-    mEnd = &aEnd;
-  }
-
-  PRBool IsSet() const
-  {
-    NS_ABORT_IF_FALSE(!mBegin == !mEnd, "Bad interval: only one endpoint set");
-    return !!mBegin;
-  }
-
-  void Reset()
-  {
-    mBegin = nsnull;
-    mEnd = nsnull;
-  }
-
-  // Begin() and End() will be non-null so long as IsSet() is true. Otherwise,
-  // they probably shouldn't be called.
 
   const nsSMILInstanceTime* Begin() const
   {
-    NS_ABORT_IF_FALSE(mBegin, "Calling Begin() on un-set interval");
+    NS_ABORT_IF_FALSE(mBegin && mEnd,
-    return mBegin;
+        "Requesting Begin() on un-initialized instance time");
-  }
-
-  nsSMILInstanceTime* Begin()
-  {
-    NS_ABORT_IF_FALSE(mBegin, "Calling Begin() on un-set interval");
     return mBegin;
   }
+  nsSMILInstanceTime* Begin();
 
   const nsSMILInstanceTime* End() const
   {
-    NS_ABORT_IF_FALSE(mEnd, "Calling End() on un-set interval");
+    NS_ABORT_IF_FALSE(mBegin && mEnd,
+        "Requesting End() on un-initialized instance time");
     return mEnd;
   }
+  nsSMILInstanceTime* End();
 
-  nsSMILInstanceTime* End()
+  void SetBegin(nsSMILInstanceTime& aBegin);
+  void SetEnd(nsSMILInstanceTime& aEnd);
+  void Set(nsSMILInstanceTime& aBegin, nsSMILInstanceTime& aEnd)
   {
-    NS_ABORT_IF_FALSE(mEnd, "Calling End() on un-set interval");
+    SetBegin(aBegin);
-    return mEnd;
+    SetEnd(aEnd);
-  }
-
-  void SetBegin(nsSMILInstanceTime& aBegin)
-  {
-    NS_ABORT_IF_FALSE(mBegin, "Calling SetBegin() on un-set interval");
-    NS_ABORT_IF_FALSE(aBegin.Time().IsResolved(),
-        "Attempting to set unresolved begin time on interval");
-    mBegin = &aBegin;
-  }
-
-  void SetEnd(nsSMILInstanceTime& aEnd)
-  {
-    NS_ABORT_IF_FALSE(mEnd, "Calling SetEnd() on un-set interval");
-    mEnd = &aEnd;
   }
 
   void FreezeBegin()
   {
-    NS_ABORT_IF_FALSE(mBegin, "Calling FreezeBegin() on un-set interval");
+    NS_ABORT_IF_FALSE(mBegin && mEnd,
+        "Freezing Begin() on un-initialized instance time");
     mBegin->MarkNoLongerUpdating();
   }
 
   void FreezeEnd()
   {
-    NS_ABORT_IF_FALSE(mEnd, "Calling FreezeEnd() on un-set interval");
+    NS_ABORT_IF_FALSE(mBegin && mEnd,
+        "Freezing End() on un-initialized instance time");
     NS_ABORT_IF_FALSE(!mBegin->MayUpdate(),
         "Freezing the end of an interval without a fixed begin");
     mEnd->MarkNoLongerUpdating();
   }
 
+  // XXX Backwards seeking support
+  void Unfreeze()
+  {
+    // XXX
+    UnfreezeEnd();
+  }
+
+  void UnfreezeEnd()
+  {
+    // XXX
+  }
+
+  void AddDependentTime(nsSMILInstanceTime& aTime);
+  void RemoveDependentTime(const nsSMILInstanceTime& aTime);
+
 private:
   nsRefPtr<nsSMILInstanceTime> mBegin;
   nsRefPtr<nsSMILInstanceTime> mEnd;
+
+  typedef nsTArray<nsRefPtr<nsSMILInstanceTime> > InstanceTimeList;
+  InstanceTimeList mDependentTimes;
+
+  // When change notifications are passed around the timing model we try to
+  // filter out all changes where there is no observable difference to an
+  // instance time. Changes that may produce an observable difference are:
+  //
+  // * Changes to the time of an interval endpoint
+  // * Changes in the relative times of different time containers
+  // * Changes to the dependency chain (which may affect the animation sandwich)
+  //
+  // The nsSMILTimeValueSpec can detect the first two changes by recalculating
+  // the time but in order to help detect the third change we simply set a flag
+  // whenever the mBegin or mEnd pointers are changed. These flags are reset
+  // when the next change notification is sent.
+  PRPackedBool mBeginObjectChanged;
+  PRPackedBool mEndObjectChanged;
 };
 
 #endif // NS_SMILINTERVAL_H_

content/smil/nsSMILTimeValueSpec.cpp

@@ -46,32 +46,6 @@
 #include "nsContentUtils.h"
 #include "nsString.h"
 
-//----------------------------------------------------------------------
-// Helper classes
-
-namespace
-{
-  // Utility class to set a PRPackedBool value to PR_TRUE whilst it is in scope.
-  // Saves us having to remember to clear the flag at every possible return.
-  class AutoBoolSetter
-  {
-  public:
-    AutoBoolSetter(PRPackedBool& aValue)
-    : mValue(aValue)
-    {
-      mValue = PR_TRUE;
-    }
-
-    ~AutoBoolSetter()
-    {
-      mValue = PR_FALSE;
-    }
-
-  private:
-    PRPackedBool&   mValue;
-  };
-}
-
 //----------------------------------------------------------------------
 // Implementation
 
@@ -86,8 +60,6 @@ nsSMILTimeValueSpec::nsSMILTimeValueSpec(nsSMILTimedElement& aOwner,
                                          PRBool aIsBegin)
   : mOwner(&aOwner),
     mIsBegin(aIsBegin),
-    mVisited(PR_FALSE),
-    mChainEnd(PR_FALSE),
     mTimebase(this)
 #ifdef _MSC_VER
 #pragma warning(pop)
@@ -111,14 +83,6 @@ nsSMILTimeValueSpec::SetSpec(const nsAString& aStringSpec,
   if (NS_FAILED(rv))
     return rv;
 
-  // Currently we don't allow nsSMILTimeValueSpec objects to be re-used. When
-  // the 'begin' or 'end' attribute on an nsSMILTimedElement is set,
-  // nsSMILTimedElement will just throw away all the old spec objects and create
-  // new ones.
-  NS_ABORT_IF_FALSE(!mLatestInstanceTime,
-      "Attempting to re-use nsSMILTimeValueSpec object. "
-      "Last instance time is non-null");
-
   mParams = params;
 
   // According to SMIL 3.0:
@@ -128,7 +92,7 @@ nsSMILTimeValueSpec::SetSpec(const nsAString& aStringSpec,
   if (mParams.mType == nsSMILTimeValueSpecParams::OFFSET ||
       (!mIsBegin && mParams.mType == nsSMILTimeValueSpecParams::INDEFINITE)) {
     nsRefPtr<nsSMILInstanceTime> instance =
-      new nsSMILInstanceTime(mParams.mOffset, nsnull);
+      new nsSMILInstanceTime(mParams.mOffset);
     if (!instance)
       return NS_ERROR_OUT_OF_MEMORY;
     mOwner->AddInstanceTime(instance, mIsBegin);
@@ -158,7 +122,6 @@ nsSMILTimeValueSpec::ResolveReferences(nsIContent* aContextNode)
   nsRefPtr<nsIContent> oldTimebaseContent = mTimebase.get();
 
   NS_ABORT_IF_FALSE(mParams.mDependentElemID, "NULL syncbase element id");
-
   nsString idStr;
   mParams.mDependentElemID->ToString(idStr);
   mTimebase.ResetWithID(aContextNode, idStr);
@@ -166,84 +129,49 @@ nsSMILTimeValueSpec::ResolveReferences(nsIContent* aContextNode)
 }
 
 void
-nsSMILTimeValueSpec::HandleNewInterval(const nsSMILInterval& aInterval,
+nsSMILTimeValueSpec::HandleNewInterval(nsSMILInterval& aInterval,
                                        const nsSMILTimeContainer* aSrcContainer)
 {
-  NS_ABORT_IF_FALSE(aInterval.IsSet(),
-      "Received notification of new interval that is not set");
-
   const nsSMILInstanceTime& baseInstance = mParams.mSyncBegin
     ? *aInterval.Begin() : *aInterval.End();
   nsSMILTimeValue newTime =
     ConvertBetweenTimeContainers(baseInstance.Time(), aSrcContainer);
 
-  // If we're a begin spec but the time we've been given is not resolved we
-  // won't make an instance time. If the time becomes resolved later we'll
-  // create the instance time when we get the change notice.
-  if (mIsBegin && !newTime.IsResolved())
-    return;
-
   // Apply offset
   if (newTime.IsResolved()) {
     newTime.SetMillis(newTime.GetMillis() + mParams.mOffset.GetMillis());
   }
 
+  // Create the instance time and register it with the interval
   nsRefPtr<nsSMILInstanceTime> newInstance =
-    new nsSMILInstanceTime(newTime, &baseInstance,
+    new nsSMILInstanceTime(newTime, nsSMILInstanceTime::SOURCE_SYNCBASE, this,
-                           nsSMILInstanceTime::SOURCE_SYNCBASE);
+                           &aInterval);
   if (!newInstance)
     return;
 
-  if (mLatestInstanceTime) {
+  // If we are a begin spec but the time we've got is not resolved, we won't add
-    mLatestInstanceTime->MarkNoLongerUpdating();
+  // it to the owner just yet. When the time later becomes resolved we'll add it
-  }
+  // at that point.
+  if (mIsBegin && !newTime.IsResolved())
+    return;
 
-  mLatestInstanceTime = newInstance;
-  mChainEnd = PR_FALSE;
   mOwner->AddInstanceTime(newInstance, mIsBegin);
 }
 
 void
-nsSMILTimeValueSpec::HandleChangedInterval(const nsSMILInterval& aInterval,
+nsSMILTimeValueSpec::HandleChangedInstanceTime(
-   const nsSMILTimeContainer* aSrcContainer)
+    const nsSMILInstanceTime& aBaseTime,
+    const nsSMILTimeContainer* aSrcContainer,
+    nsSMILInstanceTime& aInstanceTimeToUpdate,
+    PRBool aObjectChanged)
 {
-  NS_ABORT_IF_FALSE(aInterval.IsSet(),
+  // If the instance time is fixed (e.g. because it's being used as the begin
-      "Received notification of changed interval that is not set");
+  // time of an active interval) we just ignore the change.
-
+  if (!aInstanceTimeToUpdate.MayUpdate())
-  if (mVisited || mChainEnd) {
-    // We're breaking the cycle here but we need to ensure that if we later
-    // receive a change notice in a different context (e.g. due to a time
-    // container change) that we don't end up following the chain further and so
-    // we set a flag to that effect.
-    mChainEnd = PR_TRUE;
     return;
-  }
-
-  AutoBoolSetter setVisited(mVisited);
-
-  // If there's no latest interval to update it must mean that we decided not to
-  // make one when the got the new interval notification (because we're a begin
-  // spec and the time wasn't resolved) or we deleted it because the source time
-  // container was paused. So now we just act like this was a new interval
-  // notification.
-  if (!mLatestInstanceTime) {
-    HandleNewInterval(aInterval, aSrcContainer);
-    return;
-  }
-
-  const nsSMILInstanceTime& baseInstance = mParams.mSyncBegin
-    ? *aInterval.Begin() : *aInterval.End();
-  NS_ABORT_IF_FALSE(mLatestInstanceTime != &baseInstance,
-      "Instance time is dependent on itself");
 
   nsSMILTimeValue updatedTime =
-    ConvertBetweenTimeContainers(baseInstance.Time(), aSrcContainer);
+    ConvertBetweenTimeContainers(aBaseTime.Time(), aSrcContainer);
-
-  // If we're a begin spec but the time is now unresolved, delete the interval.
-  if (mIsBegin && !updatedTime.IsResolved()) {
-    HandleDeletedInterval();
-    return;
-  }
 
   // Apply offset
   if (updatedTime.IsResolved()) {
@@ -251,46 +179,56 @@ nsSMILTimeValueSpec::HandleChangedInterval(const nsSMILInterval& aInterval,
                           mParams.mOffset.GetMillis());
   }
 
-  // Note that if the instance time is fixed (e.g. because it's being used as
+  // Since we never add unresolved begin times to the owner we must detect if
-  // the begin time of an active interval) we just ignore the change.
+  // this change requires adding a newly-resolved time, removing
-  // See SMIL 3 section 5.4.5:
+  // a previously-resolved time, or doing nothing
-  //
+  if (mIsBegin) {
-  //   "In contrast, when an instance time in the begin list changes because the
+    // Add newly-resolved time
-  //   syncbase (current interval) time moves, this does not invoke restart
+    if (!aInstanceTimeToUpdate.Time().IsResolved() &&
-  //   semantics, but may change the current begin time: If the current interval
+        updatedTime.IsResolved()) {
-  //   has not yet begun, a change to an instance time in the begin list will
+      aInstanceTimeToUpdate.DependentUpdate(updatedTime);
-  //   cause a re-evaluation of the begin instance lists, which may cause the
+      mOwner->AddInstanceTime(&aInstanceTimeToUpdate, mIsBegin);
-  //   interval begin time to change."
+      return;
-  //
+    }
-  if (!mLatestInstanceTime->MayUpdate())
+    // Remove previously-resolved time
-    return;
+    if (aInstanceTimeToUpdate.Time().IsResolved() &&
+        !updatedTime.IsResolved()) {
+      aInstanceTimeToUpdate.DependentUpdate(updatedTime);
+      mOwner->RemoveInstanceTime(&aInstanceTimeToUpdate, mIsBegin);
+      return;
+    }
+    // Do nothing (but update in case we're updating an 'unresolved' time to an
+    // 'indefinite' time or vice versa, both of which return PR_FALSE for
+    // IsResolved() and neither of which should be added to the owner).
+    if (!aInstanceTimeToUpdate.Time().IsResolved() &&
+        !updatedTime.IsResolved()) {
+      aInstanceTimeToUpdate.DependentUpdate(updatedTime);
+      return;
+    }
+  }
 
   // The timed element that owns the instance time does the updating so it can
   // re-sort its array of instance times more efficiently
-  if (mLatestInstanceTime->Time() != updatedTime ||
+  if (aInstanceTimeToUpdate.Time() != updatedTime || aObjectChanged) {
-      mLatestInstanceTime->GetDependentTime() != &baseInstance) {
+    mOwner->UpdateInstanceTime(&aInstanceTimeToUpdate, updatedTime, mIsBegin);
-    mOwner->UpdateInstanceTime(mLatestInstanceTime, updatedTime,
-                               &baseInstance, mIsBegin);
   }
 }
 
 void
-nsSMILTimeValueSpec::HandleDeletedInterval()
+nsSMILTimeValueSpec::HandleDeletedInstanceTime(
+    nsSMILInstanceTime &aInstanceTime)
 {
-  // If we don't have an instance time it must mean we decided not to create one
+  // If it's an unresolved begin time then we won't have added it
-  // when we got a new interval notice (because we're a begin spec and the time
+  if (mIsBegin && !aInstanceTime.Time().IsResolved())
-  // was unresolved).
-  if (!mLatestInstanceTime)
     return;
 
-  // Since we don't know if calling RemoveInstanceTime will result in further
+  mOwner->RemoveInstanceTime(&aInstanceTime, mIsBegin);
-  // calls to us, we ensure that we're in a consistent state before handing over
+}
-  // control.
-  nsRefPtr<nsSMILInstanceTime> oldInstanceTime = mLatestInstanceTime;
-  mLatestInstanceTime = nsnull;
-  mChainEnd = PR_FALSE;
 
-  mOwner->RemoveInstanceTime(oldInstanceTime, mIsBegin);
+PRBool
+nsSMILTimeValueSpec::DependsOnBegin() const
+{
+  return mParams.mSyncBegin;
 }
 
 void
@@ -330,7 +268,7 @@ nsSMILTimeValueSpec::UnregisterFromTimebase(nsSMILTimedElement* aTimedElement)
     return;
 
   aTimedElement->RemoveDependent(*this);
-  HandleDeletedInterval();
+  mOwner->RemoveInstanceTimesForCreator(this, mIsBegin);
 }
 
 nsSMILTimedElement*

content/smil/nsSMILTimeValueSpec.h

@@ -69,11 +69,16 @@ public:
   nsresult SetSpec(const nsAString& aStringSpec, nsIContent* aContextNode);
   void     ResolveReferences(nsIContent* aContextNode);
 
-  void     HandleNewInterval(const nsSMILInterval& aInterval,
+  void     HandleNewInterval(nsSMILInterval& aInterval,
                              const nsSMILTimeContainer* aSrcContainer);
-  void     HandleChangedInterval(const nsSMILInterval& aInterval,
+
-                                 const nsSMILTimeContainer* aSrcContainer);
+  // For created nsSMILInstanceTime objects
-  void     HandleDeletedInterval();
+  PRBool   DependsOnBegin() const;
+  void     HandleChangedInstanceTime(const nsSMILInstanceTime& aBaseTime,
+                                     const nsSMILTimeContainer* aSrcContainer,
+                                     nsSMILInstanceTime& aInstanceTimeToUpdate,
+                                     PRBool aObjectChanged);
+  void     HandleDeletedInstanceTime(nsSMILInstanceTime& aInstanceTime);
 
   // Cycle-collection support
   void Traverse(nsCycleCollectionTraversalCallback* aCallback);
@@ -93,14 +98,8 @@ protected:
                                           // mParams.mSyncBegin which indicates
                                           // if we're synced with the begin of
                                           // the target.
-  PRPackedBool                  mVisited;
-  PRPackedBool                  mChainEnd;
   nsSMILTimeValueSpecParams     mParams;
 
-  // The latest instance time we have generated. Only used for syncbase timing
-  // where the instance time might actually change.
-  nsRefPtr<nsSMILInstanceTime>  mLatestInstanceTime;
-
   class TimebaseElement : public nsReferencedElement {
   public:
     TimebaseElement(nsSMILTimeValueSpec* aOwner) : mSpec(aOwner) { }

content/smil/nsSMILTimedElement.cpp

@@ -120,6 +120,7 @@ nsSMILTimedElement::nsSMILTimedElement()
   mEndHasEventConditions(PR_FALSE),
   mInstanceSerialIndex(0),
   mClient(nsnull),
+  mCurrentInterval(nsnull),
   mPrevRegisteredMilestone(sMaxMilestone),
   mElementState(STATE_STARTUP)
 {
@@ -129,6 +130,35 @@ nsSMILTimedElement::nsSMILTimedElement()
   mTimeDependents.Init();
 }
 
+nsSMILTimedElement::~nsSMILTimedElement()
+{
+  // Put us in a consistent state in case we get any callbacks
+  mElementState = STATE_POSTACTIVE;
+
+  // Unlink all instance times from dependent intervals
+  for (PRUint32 i = 0; i < mBeginInstances.Length(); ++i) {
+    mBeginInstances[i]->Unlink();
+  }
+  mBeginInstances.Clear();
+  for (PRUint32 i = 0; i < mEndInstances.Length(); ++i) {
+    mEndInstances[i]->Unlink();
+  }
+  mEndInstances.Clear();
+
+  // Notify anyone listening to our intervals that they're gone
+  // (We shouldn't get any callbacks from this because all our instance times
+  // are now disassociated with any intervals)
+  if (mCurrentInterval) {
+    mCurrentInterval->NotifyDeleting();
+    mCurrentInterval = nsnull;
+  }
+
+  for (PRInt32 i = mOldIntervals.Length() - 1; i >= 0; --i) {
+    mOldIntervals[i]->NotifyDeleting();
+  }
+  mOldIntervals.Clear();
+}
+
 void
 nsSMILTimedElement::SetAnimationElement(nsISMILAnimationElement* aElement)
 {
@@ -228,7 +258,7 @@ nsSMILTimeValue
 nsSMILTimedElement::GetStartTime() const
 {
   return mElementState == STATE_WAITING || mElementState == STATE_ACTIVE
-         ? mCurrentInterval.Begin()->Time()
+         ? mCurrentInterval->Begin()->Time()
          : nsSMILTimeValue();
 }
 
@@ -258,17 +288,10 @@ nsSMILTimedElement::AddInstanceTime(nsSMILInstanceTime* aInstanceTime,
 void
 nsSMILTimedElement::UpdateInstanceTime(nsSMILInstanceTime* aInstanceTime,
                                        nsSMILTimeValue& aUpdatedTime,
-                                       const nsSMILInstanceTime* aDependentTime,
                                        PRBool aIsBegin)
 {
   NS_ABORT_IF_FALSE(aInstanceTime, "Attempting to update null instance time");
 
-  NS_ABORT_IF_FALSE(aInstanceTime->Time() != aUpdatedTime ||
-      aInstanceTime->GetDependentTime() != aDependentTime,
-      "Got call to UpdateInstanceTime but there's nothing to change");
-
-  aInstanceTime->SetDependentTime(aDependentTime);
-
   // The reason we update the time here and not in the nsSMILTimeValueSpec is
   // that it means we *could* re-sort more efficiently by doing a sorted remove
   // and insert but currently this doesn't seem to be necessary given how
@@ -288,9 +311,9 @@ nsSMILTimedElement::UpdateInstanceTime(nsSMILInstanceTime* aInstanceTime,
   // the current interval but this introduces other complications (particularly
   // detecting which instance time is being used to define the begin of the
   // current interval when doing a Reset).
-  PRBool changedCurrentInterval = mCurrentInterval.IsSet() &&
+  PRBool changedCurrentInterval = mCurrentInterval &&
-    (mCurrentInterval.Begin() == aInstanceTime ||
+    (mCurrentInterval->Begin() == aInstanceTime ||
-     mCurrentInterval.End() == aInstanceTime);
+     mCurrentInterval->End() == aInstanceTime);
 
   UpdateCurrentInterval(changedCurrentInterval);
 }
@@ -311,6 +334,26 @@ nsSMILTimedElement::RemoveInstanceTime(nsSMILInstanceTime* aInstanceTime,
   UpdateCurrentInterval();
 }
 
+void
+nsSMILTimedElement::RemoveInstanceTimesForCreator(
+    const nsSMILTimeValueSpec* aCreator, PRBool aIsBegin)
+{
+  NS_ABORT_IF_FALSE(aCreator, "Creator not set");
+  InstanceTimeList& instances = aIsBegin ? mBeginInstances : mEndInstances;
+
+  PRInt32 count = instances.Length();
+  for (PRInt32 i = count - 1; i >= 0; --i) {
+    nsSMILInstanceTime* instance = instances[i].get();
+    NS_ABORT_IF_FALSE(instance, "NULL instance in instances array");
+    if (instance->GetCreator() == aCreator) {
+      instance->Unlink();
+      instances.RemoveElementAt(i);
+    }
+  }
+
+  UpdateCurrentInterval();
+}
+
 void
 nsSMILTimedElement::SetTimeClient(nsSMILAnimationFunction* aClient)
 {
@@ -385,29 +428,36 @@ nsSMILTimedElement::DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly)
     {
     case STATE_STARTUP:
       {
+        nsSMILInterval firstInterval;
         mElementState =
-         (NS_SUCCEEDED(GetNextInterval(nsnull, nsnull, mCurrentInterval)))
+         NS_SUCCEEDED(GetNextInterval(nsnull, nsnull, firstInterval))
          ? STATE_WAITING
          : STATE_POSTACTIVE;
         stateChanged = PR_TRUE;
         if (mElementState == STATE_WAITING) {
-          NotifyNewInterval();
+          mCurrentInterval = new nsSMILInterval(firstInterval);
+          if (!mCurrentInterval) {
+            NS_WARNING("Failed to allocate memory for new interval");
+            mElementState = STATE_POSTACTIVE;
+          } else {
+            NotifyNewInterval();
+          }
         }
       }
       break;
 
     case STATE_WAITING:
       {
-        if (mCurrentInterval.Begin()->Time() <= sampleTime) {
+        if (mCurrentInterval->Begin()->Time() <= sampleTime) {
           mElementState = STATE_ACTIVE;
-          mCurrentInterval.FreezeBegin();
+          mCurrentInterval->FreezeBegin();
-          if (mPrevInterval.IsSet()) {
+          if (HasPlayed()) {
             Reset(); // Apply restart behaviour
           }
           if (mClient) {
-            mClient->Activate(mCurrentInterval.Begin()->Time().GetMillis());
+            mClient->Activate(mCurrentInterval->Begin()->Time().GetMillis());
           }
-          if (mPrevInterval.IsSet()) {
+          if (HasPlayed()) {
             // The call to Reset() may mean that the end point of our current
             // interval should be changed and so we should update the interval
             // now. However, calling UpdateCurrentInterval could result in the
@@ -425,35 +475,39 @@ nsSMILTimedElement::DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly)
     case STATE_ACTIVE:
       {
         // Only apply an early end if we're not already ending.
-        if (mCurrentInterval.End()->Time() > sampleTime) {
+        if (mCurrentInterval->End()->Time() > sampleTime) {
           nsSMILInstanceTime* earlyEnd = CheckForEarlyEnd(sampleTime);
           if (earlyEnd) {
-            mCurrentInterval.SetEnd(*earlyEnd);
+            mCurrentInterval->SetEnd(*earlyEnd);
             NotifyChangedInterval();
           }
         }
 
-        if (mCurrentInterval.End()->Time() <= sampleTime) {
+        if (mCurrentInterval->End()->Time() <= sampleTime) {
           nsSMILInterval newInterval;
           mElementState =
-            NS_SUCCEEDED(GetNextInterval(&mCurrentInterval, nsnull,
+            NS_SUCCEEDED(GetNextInterval(mCurrentInterval, nsnull, newInterval))
-                                          newInterval))
             ? STATE_WAITING
             : STATE_POSTACTIVE;
           if (mClient) {
             mClient->Inactivate(mFillMode == FILL_FREEZE);
           }
-          mCurrentInterval.FreezeEnd();
+          mCurrentInterval->FreezeEnd();
-          mPrevInterval = mCurrentInterval;
+          mOldIntervals.AppendElement(mCurrentInterval.forget());
-          mCurrentInterval = newInterval;
+          // We must update mOldIntervals before calling SampleFillValue
-          // We must update mPrevInterval before calling SampleFillValue
           SampleFillValue();
           if (mElementState == STATE_WAITING) {
-            NotifyNewInterval();
+            mCurrentInterval = new nsSMILInterval(newInterval);
+            if (!mCurrentInterval) {
+              NS_WARNING("Failed to allocate memory for new interval");
+              mElementState = STATE_POSTACTIVE;
+            } else {
+              NotifyNewInterval();
+            }
           }
           stateChanged = PR_TRUE;
         } else {
-          nsSMILTime beginTime = mCurrentInterval.Begin()->Time().GetMillis();
+          nsSMILTime beginTime = mCurrentInterval->Begin()->Time().GetMillis();
           nsSMILTime activeTime = aContainerTime - beginTime;
           SampleSimpleTime(activeTime);
         }
@@ -503,7 +557,8 @@ nsSMILTimedElement::Reset()
     nsSMILInstanceTime* instance = mBeginInstances[i].get();
     NS_ABORT_IF_FALSE(instance, "NULL instance in begin instances array");
     if (instance->ClearOnReset() &&
-       (!mCurrentInterval.IsSet() || instance != mCurrentInterval.Begin())) {
+       (!mCurrentInterval || instance != mCurrentInterval->Begin())) {
+      instance->Unlink();
       mBeginInstances.RemoveElementAt(i);
     }
   }
@@ -513,6 +568,7 @@ nsSMILTimedElement::Reset()
     nsSMILInstanceTime* instance = mEndInstances[j].get();
     NS_ABORT_IF_FALSE(instance, "NULL instance in end instances array");
     if (instance->ClearOnReset()) {
+      instance->Unlink();
       mEndInstances.RemoveElementAt(j);
     }
   }
@@ -825,10 +881,12 @@ nsSMILTimedElement::SetFillMode(const nsAString& aFillModeSpec)
             ? nsSMILFillMode(temp.GetEnumValue())
             : FILL_REMOVE;
 
-  PRBool hasPlayed = mPrevInterval.IsSet() &&
+  // Check if we're in a fill-able state: i.e. we've played at least one
+  // interval and are now between intervals or at the end of all intervals
+  PRBool isFillable = HasPlayed() &&
     (mElementState == STATE_WAITING || mElementState == STATE_POSTACTIVE);
 
-  if (mClient && mFillMode != previousFillMode && hasPlayed) {
+  if (mClient && mFillMode != previousFillMode && isFillable) {
     mClient->Inactivate(mFillMode == FILL_FREEZE);
     SampleFillValue();
   }
@@ -842,7 +900,7 @@ nsSMILTimedElement::UnsetFillMode()
   PRUint16 previousFillMode = mFillMode;
   mFillMode = FILL_REMOVE;
   if ((mElementState == STATE_WAITING || mElementState == STATE_POSTACTIVE) &&
-      previousFillMode == FILL_FREEZE && mClient && mPrevInterval.IsSet())
+      previousFillMode == FILL_FREEZE && mClient && HasPlayed())
     mClient->Inactivate(PR_FALSE);
 }
 
@@ -855,10 +913,15 @@ nsSMILTimedElement::AddDependent(nsSMILTimeValueSpec& aDependent)
       "nsSMILTimeValueSpec is already registered as a dependency");
   mTimeDependents.PutEntry(&aDependent);
 
-  if (mCurrentInterval.IsSet()) {
+  // Add old and current intervals
-    // Not necessary to call SyncPauseTime here as we're dealing with
+  //
-    // historical instance times not newly added ones.
+  // It's not necessary to call SyncPauseTime since we're dealing with
-    aDependent.HandleNewInterval(mCurrentInterval, GetTimeContainer());
+  // historical instance times not newly added ones.
+  for (PRUint32 i = 0; i < mOldIntervals.Length(); ++i) {
+    aDependent.HandleNewInterval(*mOldIntervals[i], GetTimeContainer());
+  }
+  if (mCurrentInterval) {
+    aDependent.HandleNewInterval(*mCurrentInterval, GetTimeContainer());
   }
 }
 
@@ -991,6 +1054,7 @@ nsSMILTimedElement::ClearBeginOrEndSpecs(PRBool aIsBegin)
     nsSMILInstanceTime* instance = instances[i].get();
     NS_ABORT_IF_FALSE(instance, "NULL instance in instances array");
     if (!instance->FromDOM()) {
+      instance->Unlink();
       instances.RemoveElementAt(i);
     }
   }
@@ -1011,13 +1075,13 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       "Unresolved begin time specified for interval start");
   static nsSMILTimeValue zeroTime(0L);
 
-  if (mRestartMode == RESTART_NEVER && aPrevInterval && aPrevInterval->IsSet())
+  if (mRestartMode == RESTART_NEVER && aPrevInterval)
     return NS_ERROR_FAILURE;
 
   // Calc starting point
   nsSMILTimeValue beginAfter;
   PRBool prevIntervalWasZeroDur = PR_FALSE;
-  if (aPrevInterval && aPrevInterval->IsSet()) {
+  if (aPrevInterval) {
     beginAfter = aPrevInterval->End()->Time();
     prevIntervalWasZeroDur
       = aPrevInterval->End()->Time() == aPrevInterval->Begin()->Time();
@@ -1040,7 +1104,7 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       // our ref-counting is not const-correct
       tempBegin = const_cast<nsSMILInstanceTime*>(aFixedBeginTime);
     } else if (!mBeginSpecSet && beginAfter <= zeroTime) {
-      tempBegin = new nsSMILInstanceTime(nsSMILTimeValue(0), nsnull);
+      tempBegin = new nsSMILInstanceTime(nsSMILTimeValue(0));
       if (!tempBegin)
         return NS_ERROR_OUT_OF_MEMORY;
     } else {
@@ -1088,7 +1152,7 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       nsSMILTimeValue activeEnd = CalcActiveEnd(tempBegin->Time(), intervalEnd);
 
       if (!tempEnd || intervalEnd != activeEnd) {
-        tempEnd = new nsSMILInstanceTime(activeEnd, nsnull);
+        tempEnd = new nsSMILInstanceTime(activeEnd);
       }
       if (!tempEnd)
         return NS_ERROR_OUT_OF_MEMORY;
@@ -1289,18 +1353,19 @@ nsSMILInstanceTime*
 nsSMILTimedElement::CheckForEarlyEnd(
     const nsSMILTimeValue& aContainerTime) const
 {
-  NS_ABORT_IF_FALSE(mCurrentInterval.IsSet(),
+  NS_ABORT_IF_FALSE(mCurrentInterval,
       "Checking for an early end but the current interval is not set");
   if (mRestartMode != RESTART_ALWAYS)
     return nsnull;
 
   PRInt32 position = 0;
   nsSMILInstanceTime* nextBegin =
-    GetNextGreater(mBeginInstances, mCurrentInterval.Begin()->Time(), position);
+    GetNextGreater(mBeginInstances, mCurrentInterval->Begin()->Time(),
+                   position);
 
   if (nextBegin &&
-      nextBegin->Time() > mCurrentInterval.Begin()->Time() &&
+      nextBegin->Time() > mCurrentInterval->Begin()->Time() &&
-      nextBegin->Time() < mCurrentInterval.End()->Time() &&
+      nextBegin->Time() < mCurrentInterval->End()->Time() &&
       nextBegin->Time() <= aContainerTime) {
     return nextBegin;
   }
@@ -1323,18 +1388,23 @@ nsSMILTimedElement::UpdateCurrentInterval(PRBool aForceChangeNotice)
 
   // If the interval is active the begin time is fixed.
   const nsSMILInstanceTime* beginTime = mElementState == STATE_ACTIVE
-                                      ? mCurrentInterval.Begin()
+                                      ? mCurrentInterval->Begin()
                                       : nsnull;
   nsSMILInterval updatedInterval;
-  nsresult rv = GetNextInterval(&mPrevInterval, beginTime, updatedInterval);
+  nsresult rv =
+    GetNextInterval(GetPreviousInterval(), beginTime, updatedInterval);
 
   if (NS_SUCCEEDED(rv)) {
 
     if (mElementState == STATE_POSTACTIVE) {
 
-      NS_ABORT_IF_FALSE(!mCurrentInterval.IsSet(),
+      NS_ABORT_IF_FALSE(!mCurrentInterval,
           "In postactive state but the interval has been set");
-      mCurrentInterval.Set(*updatedInterval.Begin(), *updatedInterval.End());
+      mCurrentInterval = new nsSMILInterval(updatedInterval);
+      if (!mCurrentInterval) {
+        NS_WARNING("Failed to allocate memory for new interval.");
+        return;
+      }
       mElementState = STATE_WAITING;
       NotifyNewInterval();
 
@@ -1343,15 +1413,14 @@ nsSMILTimedElement::UpdateCurrentInterval(PRBool aForceChangeNotice)
       PRBool changed = PR_FALSE;
 
       if (mElementState != STATE_ACTIVE &&
-          !updatedInterval.Begin()->SameTimeAndDependency(
+          !updatedInterval.Begin()->SameTimeAndBase(
-            *mCurrentInterval.Begin())) {
+            *mCurrentInterval->Begin())) {
-        mCurrentInterval.SetBegin(*updatedInterval.Begin());
+        mCurrentInterval->SetBegin(*updatedInterval.Begin());
         changed = PR_TRUE;
       }
 
-      if (!updatedInterval.End()->SameTimeAndDependency(
+      if (!updatedInterval.End()->SameTimeAndBase(*mCurrentInterval->End())) {
-            *mCurrentInterval.End())) {
+        mCurrentInterval->SetEnd(*updatedInterval.End());
-        mCurrentInterval.SetEnd(*updatedInterval.End());
         changed = PR_TRUE;
       }
 
@@ -1367,14 +1436,14 @@ nsSMILTimedElement::UpdateCurrentInterval(PRBool aForceChangeNotice)
     if (mElementState == STATE_ACTIVE && mClient) {
       // Only apply a fill if it was already being applied before the (now
       // deleted) interval was created
-      PRBool applyFill = mPrevInterval.IsSet() && mFillMode == FILL_FREEZE;
+      PRBool applyFill = HasPlayed() && mFillMode == FILL_FREEZE;
       mClient->Inactivate(applyFill);
     }
 
     if (mElementState == STATE_ACTIVE || mElementState == STATE_WAITING) {
       mElementState = STATE_POSTACTIVE;
-      mCurrentInterval.Reset();
+      mCurrentInterval->NotifyDeleting();
-      NotifyDeletedInterval();
+      mCurrentInterval = nsnull;
     }
   }
 }
@@ -1393,22 +1462,20 @@ nsSMILTimedElement::SampleSimpleTime(nsSMILTime aActiveTime)
 void
 nsSMILTimedElement::SampleFillValue()
 {
-  NS_ABORT_IF_FALSE(mPrevInterval.IsSet(),
+  NS_ABORT_IF_FALSE(!mOldIntervals.IsEmpty(),
       "Attempting to sample fill value but there is no previous interval");
 
-  if (mFillMode != FILL_FREEZE)
+  if (mFillMode != FILL_FREEZE || !mClient)
     return;
 
-  if (!mClient)
+  const nsSMILInterval& prevInterval = *GetPreviousInterval();
-    return;
+  NS_ABORT_IF_FALSE(prevInterval.End()->Time().IsResolved() &&
-
+      !prevInterval.End()->MayUpdate(),
-  NS_ABORT_IF_FALSE(mPrevInterval.End()->Time().IsResolved() &&
-      !mPrevInterval.End()->MayUpdate(),
       "Attempting to sample fill value but the endpoint of the previous "
       "interval is not resolved and frozen");
 
-  nsSMILTime activeTime = mPrevInterval.End()->Time().GetMillis() -
+  nsSMILTime activeTime = prevInterval.End()->Time().GetMillis() -
-                          mPrevInterval.Begin()->Time().GetMillis();
+                          prevInterval.Begin()->Time().GetMillis();
 
   PRUint32 repeatIteration;
   nsSMILTime simpleTime =
@@ -1433,7 +1500,7 @@ nsSMILTimedElement::AddInstanceTimeFromCurrentTime(nsSMILTime aCurrentTime,
   // XXX If we re-use this method for event-based timing we'll need to change it
   // so we don't end up setting SOURCE_DOM for event-based times.
   nsRefPtr<nsSMILInstanceTime> instanceTime =
-    new nsSMILInstanceTime(timeVal, nsnull, nsSMILInstanceTime::SOURCE_DOM);
+    new nsSMILInstanceTime(timeVal, nsSMILInstanceTime::SOURCE_DOM);
   if (!instanceTime) {
     NS_WARNING("Insufficient memory to create instance time");
     return;
@@ -1498,10 +1565,10 @@ nsSMILTimedElement::GetNextMilestone(nsSMILMilestone& aNextMilestone) const
     return PR_TRUE;
 
   case STATE_WAITING:
-    NS_ABORT_IF_FALSE(mCurrentInterval.IsSet(),
+    NS_ABORT_IF_FALSE(mCurrentInterval,
         "In waiting state but the current interval has not been set");
     aNextMilestone.mIsEnd = PR_FALSE;
-    aNextMilestone.mTime = mCurrentInterval.Begin()->Time().GetMillis();
+    aNextMilestone.mTime = mCurrentInterval->Begin()->Time().GetMillis();
     return PR_TRUE;
 
   case STATE_ACTIVE:
@@ -1511,7 +1578,7 @@ nsSMILTimedElement::GetNextMilestone(nsSMILMilestone& aNextMilestone) const
 
       // Check for an early end
       nsSMILInstanceTime* earlyEnd =
-        CheckForEarlyEnd(mCurrentInterval.End()->Time());
+        CheckForEarlyEnd(mCurrentInterval->End()->Time());
       if (earlyEnd) {
         aNextMilestone.mIsEnd = PR_TRUE;
         aNextMilestone.mTime = earlyEnd->Time().GetMillis();
@@ -1519,9 +1586,9 @@ nsSMILTimedElement::GetNextMilestone(nsSMILMilestone& aNextMilestone) const
       }
 
       // Otherwise it's just the next interval end
-      if (mCurrentInterval.End()->Time().IsResolved()) {
+      if (mCurrentInterval->End()->Time().IsResolved()) {
         aNextMilestone.mIsEnd = PR_TRUE;
-        aNextMilestone.mTime = mCurrentInterval.End()->Time().GetMillis();
+        aNextMilestone.mTime = mCurrentInterval->End()->Time().GetMillis();
         return PR_TRUE;
       }
 
@@ -1540,7 +1607,7 @@ nsSMILTimedElement::GetNextMilestone(nsSMILMilestone& aNextMilestone) const
 void
 nsSMILTimedElement::NotifyNewInterval()
 {
-  NS_ABORT_IF_FALSE(mCurrentInterval.IsSet(),
+  NS_ABORT_IF_FALSE(mCurrentInterval,
       "Attempting to notify dependents of a new interval but the interval "
       "is not set");
 
@@ -1549,14 +1616,14 @@ nsSMILTimedElement::NotifyNewInterval()
     container->SyncPauseTime();
   }
 
-  NotifyTimeDependentsParams params = { &mCurrentInterval, container };
+  NotifyTimeDependentsParams params = { mCurrentInterval, container };
   mTimeDependents.EnumerateEntries(NotifyNewIntervalCallback, &params);
 }
 
 void
 nsSMILTimedElement::NotifyChangedInterval()
 {
-  NS_ABORT_IF_FALSE(mCurrentInterval.IsSet(),
+  NS_ABORT_IF_FALSE(mCurrentInterval,
       "Attempting to notify dependents of a changed interval but the interval "
       "is not set--perhaps we should be deleting the interval instead?");
 
@@ -1565,14 +1632,7 @@ nsSMILTimedElement::NotifyChangedInterval()
     container->SyncPauseTime();
   }
 
-  NotifyTimeDependentsParams params = { &mCurrentInterval, container };
+  mCurrentInterval->NotifyChanged(container);
-  mTimeDependents.EnumerateEntries(NotifyChangedIntervalCallback, &params);
-}
-
-void
-nsSMILTimedElement::NotifyDeletedInterval()
-{
-  mTimeDependents.EnumerateEntries(NotifyDeletedIntervalCallback, nsnull);
 }
 
 const nsSMILInstanceTime*
@@ -1584,11 +1644,14 @@ nsSMILTimedElement::GetEffectiveBeginInstance() const
     return nsnull;
 
   case STATE_ACTIVE:
-    return mCurrentInterval.Begin();
+    return mCurrentInterval->Begin();
 
   case STATE_WAITING:
   case STATE_POSTACTIVE:
-    return mPrevInterval.IsSet() ? mPrevInterval.Begin() : nsnull;
+    {
+      const nsSMILInterval* prevInterval = GetPreviousInterval();
+      return prevInterval ? prevInterval->Begin() : nsnull;
+    }
 
   default:
     NS_NOTREACHED("Invalid element state");
@@ -1596,6 +1659,14 @@ nsSMILTimedElement::GetEffectiveBeginInstance() const
   }
 }
 
+const nsSMILInterval*
+nsSMILTimedElement::GetPreviousInterval() const
+{
+  return mOldIntervals.IsEmpty()
+    ? nsnull
+    : mOldIntervals[mOldIntervals.Length()-1].get();
+}
+
 //----------------------------------------------------------------------
 // Hashtable callback functions
 
@@ -1603,51 +1674,16 @@ nsSMILTimedElement::GetEffectiveBeginInstance() const
 nsSMILTimedElement::NotifyNewIntervalCallback(TimeValueSpecPtrKey* aKey,
                                               void* aData)
 {
+  NS_ABORT_IF_FALSE(aKey, "Null hash key for time container hash table");
+  NS_ABORT_IF_FALSE(aKey->GetKey(),
+                    "null nsSMILTimeValueSpec in set of time dependents");
+
   NotifyTimeDependentsParams* params =
     static_cast<NotifyTimeDependentsParams*>(aData);
-  SanityCheckTimeDependentCallbackArgs(aKey, params, PR_TRUE);
+  NS_ABORT_IF_FALSE(params, "null data ptr while enumerating hashtable");
+  NS_ABORT_IF_FALSE(params->mCurrentInterval, "null current-interval ptr");
 
   nsSMILTimeValueSpec* spec = aKey->GetKey();
   spec->HandleNewInterval(*params->mCurrentInterval, params->mTimeContainer);
   return PL_DHASH_NEXT;
 }
-
-/* static */ PR_CALLBACK PLDHashOperator
-nsSMILTimedElement::NotifyChangedIntervalCallback(TimeValueSpecPtrKey* aKey,
-                                                  void* aData)
-{
-  NotifyTimeDependentsParams* params =
-    static_cast<NotifyTimeDependentsParams*>(aData);
-  SanityCheckTimeDependentCallbackArgs(aKey, params, PR_TRUE);
-
-  nsSMILTimeValueSpec* spec = aKey->GetKey();
-  spec->HandleChangedInterval(*params->mCurrentInterval,
-                              params->mTimeContainer);
-  return PL_DHASH_NEXT;
-}
-
-/* static */ PR_CALLBACK PLDHashOperator
-nsSMILTimedElement::NotifyDeletedIntervalCallback(TimeValueSpecPtrKey* aKey,
-                                                  void* /* unused */)
-{
-  SanityCheckTimeDependentCallbackArgs(aKey, nsnull, PR_FALSE);
-
-  nsSMILTimeValueSpec* spec = aKey->GetKey();
-  spec->HandleDeletedInterval();
-  return PL_DHASH_NEXT;
-}
-
-/* static */ void
-nsSMILTimedElement::SanityCheckTimeDependentCallbackArgs(
-    TimeValueSpecPtrKey* aKey,
-    NotifyTimeDependentsParams* aParams,
-    PRBool aExpectingParams)
-{
-  NS_ABORT_IF_FALSE(aKey, "Null hash key for time container hash table");
-  NS_ABORT_IF_FALSE(aKey->GetKey(),
-                    "null nsSMILTimeValueSpec in set of time dependents");
-  if (aExpectingParams) {
-    NS_ABORT_IF_FALSE(aParams, "null data ptr while enumerating hashtable");
-    NS_ABORT_IF_FALSE(aParams->mCurrentInterval, "null current-interval ptr");
-  }
-}

content/smil/nsSMILTimedElement.h

@@ -63,6 +63,7 @@ class nsSMILTimedElement
 {
 public:
   nsSMILTimedElement();
+  ~nsSMILTimedElement();
 
   /*
    * Sets the owning animation element which this class uses to convert between
@@ -156,7 +157,6 @@ public:
    */
   void UpdateInstanceTime(nsSMILInstanceTime* aInstanceTime,
                           nsSMILTimeValue& aUpdatedTime,
-                          const nsSMILInstanceTime* aDependentTime,
                           PRBool aIsBegin);
 
   /**
@@ -170,6 +170,19 @@ public:
    */
   void RemoveInstanceTime(nsSMILInstanceTime* aInstanceTime, PRBool aIsBegin);
 
+  /**
+   * Removes all the instance times associated with the given
+   * nsSMILTimeValueSpec object. Used when an ID assignment changes and hence
+   * all the previously associated instance times become invalid.
+   *
+   * @param aSpec    The nsSMILTimeValueSpec object whose created
+   *                 nsSMILInstanceTime's should be removed.
+   * @param aIsBegin PR_TRUE if the times to be removed represent begin
+   *                 times or PR_FALSE if they are end times.
+   */
+  void RemoveInstanceTimesForCreator(const nsSMILTimeValueSpec* aSpec,
+                                     PRBool aIsBegin);
+
   /**
    * Sets the object that will be called by this timed element each time it is
    * sampled.
@@ -314,6 +327,7 @@ protected:
   // Typedefs
   typedef nsTArray<nsAutoPtr<nsSMILTimeValueSpec> > TimeValueSpecList;
   typedef nsTArray<nsRefPtr<nsSMILInstanceTime> >   InstanceTimeList;
+  typedef nsTArray<nsAutoPtr<nsSMILInterval> >      IntervalList;
   typedef nsPtrHashKey<nsSMILTimeValueSpec> TimeValueSpecPtrKey;
   typedef nsTHashtable<TimeValueSpecPtrKey> TimeValueSpecHashSet;
 
@@ -410,19 +424,13 @@ protected:
 
   void              NotifyNewInterval();
   void              NotifyChangedInterval();
-  void              NotifyDeletedInterval();
   const nsSMILInstanceTime* GetEffectiveBeginInstance() const;
+  const nsSMILInterval* GetPreviousInterval() const;
+  PRBool            HasPlayed() const { return !mOldIntervals.IsEmpty(); }
 
   // Hashtable callback methods
   PR_STATIC_CALLBACK(PLDHashOperator) NotifyNewIntervalCallback(
       TimeValueSpecPtrKey* aKey, void* aData);
-  PR_STATIC_CALLBACK(PLDHashOperator) NotifyChangedIntervalCallback(
-      TimeValueSpecPtrKey* aKey, void* aData);
-  PR_STATIC_CALLBACK(PLDHashOperator) NotifyDeletedIntervalCallback(
-      TimeValueSpecPtrKey* aKey, void* /* unused */);
-  static inline void SanityCheckTimeDependentCallbackArgs(
-      TimeValueSpecPtrKey* aKey, NotifyTimeDependentsParams* aParams,
-      PRBool aExpectingParams);
 
   //
   // Members
@@ -471,8 +479,8 @@ protected:
   PRUint32                        mInstanceSerialIndex;
 
   nsSMILAnimationFunction*        mClient;
-  nsSMILInterval                  mCurrentInterval;
+  nsAutoPtr<nsSMILInterval>       mCurrentInterval;
-  nsSMILInterval                  mPrevInterval;
+  IntervalList                    mOldIntervals;
   nsSMILMilestone                 mPrevRegisteredMilestone;
   static const nsSMILMilestone    sMaxMilestone;
 

content/smil/test/Makefile.in

@@ -52,6 +52,7 @@ _TEST_FILES = \
 	  smilTestUtils.js \
 	  smilXHR_helper.svg \
 	  test_smilChangeAfterFrozen.xhtml \
+	  test_smilContainerBinding.xhtml \
 	  test_smilCrossContainer.xhtml \
 	  test_smilCSSFontStretchRelative.xhtml \
 	  test_smilCSSFromBy.xhtml \

content/smil/test/test_smilContainerBinding.xhtml

@@ -0,0 +1,99 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+  <title>Test for adding and removing animations from a time container</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<p id="display"></p>
+<div id="content" style="display: none">
+<svg id="svg" xmlns="http://www.w3.org/2000/svg" width="120px" height="120px">
+  <circle cx="-20" cy="20" r="15" fill="blue" id="circle">
+    <set attributeName="cy" to="120" begin="0s; 2s" dur="1s" id="b"/>
+  </circle>
+</svg>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+<![CDATA[
+/** Test for adding and removing animations from a time container **/
+
+SimpleTest.waitForExplicitFinish();
+
+function main() {
+  var svg = getElement("svg");
+  svg.pauseAnimations();
+  svg.setCurrentTime(0);
+
+  // Create animation and check initial state
+  var anim = createAnim();
+  anim.setAttribute('begin','b.begin+2s; 6s');
+  ok(noStart(anim), "Animation has start time before attaching to document.");
+
+  // Attach animation to container
+  var circle = getElement("circle");
+  circle.appendChild(anim);
+
+  // Check state after attaching
+  is(anim.getStartTime(), 2);
+
+  // Unbind from tree -- the syncbase instance time(s) should become unresolved
+  // but the offset time should remain
+  removeElement(anim);
+  is(anim.getStartTime(), 6);
+
+  // Rebind and check everything is re-resolved
+  circle.appendChild(anim);
+  is(anim.getStartTime(), 2);
+
+  // Advance document time to t=1s
+  // Now the current interval for b is 2s-3s but the current interval for anim
+  // is still 2s-2.5s based on b's previous interval
+  svg.setCurrentTime(1);
+  is(anim.getStartTime(), 2);
+
+  // Unbind
+  removeElement(anim);
+  is(anim.getStartTime(), 6);
+
+  // Rebind
+  // At this point all the old intervals should be re-added to anim. If they're
+  // not and only the current interval is added to anim we'll get a start time
+  // of 4s instead of 2s.
+  circle.appendChild(anim);
+  is(anim.getStartTime(), 2);
+
+  SimpleTest.finish();
+}
+
+function createAnim() {
+  const svgns="http://www.w3.org/2000/svg";
+  var anim = document.createElementNS(svgns,'set');
+  anim.setAttribute('attributeName','cx');
+  anim.setAttribute('to','100');
+  anim.setAttribute('dur','0.5s');
+  return anim;
+}
+
+function noStart(elem) {
+  var exceptionCaught = false;
+
+  try {
+    elem.getStartTime();
+  } catch(e) {
+    exceptionCaught = true;
+    is (e.code, DOMException.INVALID_STATE_ERR,
+        "Unexpected exception code from getStartTime.");
+  }
+
+  return exceptionCaught;
+}
+
+window.addEventListener("load", main, false);
+]]>
+</script>
+</pre>
+</body>
+</html>