Bug 485157: SMIL event timing, part 2 refactor instance time clearing, r=dholbert, sr=roc, a=roc
authorBrian Birtles <birtles@gmail.com>
Wed, 18 Aug 2010 19:20:24 +0900
changeset 50803 f23bab9fa177
parent 50802 d736c1a4ad2f
child 50804 26e34fda3938
push id15162
push userbbirtles@mozilla.com
push date2010-08-18 10:24 +0000
Treeherderresults
reviewersdholbert, roc, roc
bugs485157
milestone2.0b5pre
Bug 485157: SMIL event timing, part 2 refactor instance time clearing, r=dholbert, sr=roc, a=roc
content/smil/nsSMILTimedElement.cpp
content/smil/nsSMILTimedElement.h
--- a/content/smil/nsSMILTimedElement.cpp
+++ b/content/smil/nsSMILTimedElement.cpp
@@ -181,17 +181,16 @@ const PRUint8 nsSMILTimedElement::sMaxNu
 //----------------------------------------------------------------------
 // Ctor, dtor
 
 nsSMILTimedElement::nsSMILTimedElement()
 :
   mAnimationElement(nsnull),
   mFillMode(FILL_REMOVE),
   mRestartMode(RESTART_ALWAYS),
-  mBeginSpecSet(PR_FALSE),
   mEndHasEventConditions(PR_FALSE),
   mInstanceSerialIndex(0),
   mClient(nsnull),
   mCurrentInterval(nsnull),
   mCurrentRepeatIteration(0),
   mPrevRegisteredMilestone(sMaxMilestone),
   mElementState(STATE_STARTUP),
   mSeekState(SEEK_NOT_SEEKING)
@@ -660,16 +659,31 @@ nsSMILTimedElement::HandleContainerTimeC
   // containers. For now we don't bother because when we re-resolve the time in
   // the nsSMILTimeValueSpec we'll check if anything has changed and if not, we
   // won't go any further.
   if (mElementState == STATE_WAITING || mElementState == STATE_ACTIVE) {
     NotifyChangedInterval();
   }
 }
 
+namespace
+{
+  PRBool
+  RemoveNonDynamic(nsSMILInstanceTime* aInstanceTime)
+  {
+    // Generally dynamically-generated instance times (DOM calls, event-based
+    // times) are not associated with their creator nsSMILTimeValueSpec since
+    // they may outlive them.
+    NS_ABORT_IF_FALSE(!aInstanceTime->IsDynamic() ||
+         !aInstanceTime->GetCreator(),
+        "Dynamic instance time should be unlinked from its creator");
+    return !aInstanceTime->IsDynamic();
+  }
+}
+
 void
 nsSMILTimedElement::Rewind()
 {
   NS_ABORT_IF_FALSE(mAnimationElement,
       "Got rewind request before being attached to an animation element");
   NS_ABORT_IF_FALSE(mSeekState == SEEK_NOT_SEEKING,
       "Got rewind request whilst already seeking");
 
@@ -681,53 +695,62 @@ nsSMILTimedElement::Rewind()
   // time recalculating the current interval
   mElementState = STATE_STARTUP;
   mCurrentRepeatIteration = 0;
 
   // Clear the intervals and instance times except those instance times we can't
   // regenerate (DOM calls etc.)
   RewindTiming();
 
-  UnsetBeginSpec();
-  UnsetEndSpec();
+  UnsetBeginSpec(RemoveNonDynamic);
+  UnsetEndSpec(RemoveNonDynamic);
 
   if (mClient) {
     mClient->Inactivate(PR_FALSE);
   }
 
   if (mAnimationElement->HasAnimAttr(nsGkAtoms::begin)) {
     nsAutoString attValue;
     mAnimationElement->GetAnimAttr(nsGkAtoms::begin, attValue);
-    SetBeginSpec(attValue, &mAnimationElement->Content());
+    SetBeginSpec(attValue, &mAnimationElement->Content(), RemoveNonDynamic);
   }
 
   if (mAnimationElement->HasAnimAttr(nsGkAtoms::end)) {
     nsAutoString attValue;
     mAnimationElement->GetAnimAttr(nsGkAtoms::end, attValue);
-    SetEndSpec(attValue, &mAnimationElement->Content());
+    SetEndSpec(attValue, &mAnimationElement->Content(), RemoveNonDynamic);
   }
 
   mPrevRegisteredMilestone = sMaxMilestone;
   RegisterMilestone();
 }
 
+namespace
+{
+  PRBool
+  RemoveNonDOM(nsSMILInstanceTime* aInstanceTime)
+  {
+    return !aInstanceTime->FromDOM();
+  }
+}
+
 PRBool
 nsSMILTimedElement::SetAttr(nsIAtom* aAttribute, const nsAString& aValue,
                             nsAttrValue& aResult, nsIContent* aContextNode,
                             nsresult* aParseResult)
 {
   PRBool foundMatch = PR_TRUE;
   nsresult parseResult = NS_OK;
 
   if (aAttribute == nsGkAtoms::begin) {
-    parseResult = SetBeginSpec(aValue, aContextNode);
+    parseResult = SetBeginSpec(aValue, aContextNode, RemoveNonDOM);
   } else if (aAttribute == nsGkAtoms::dur) {
     parseResult = SetSimpleDuration(aValue);
   } else if (aAttribute == nsGkAtoms::end) {
-    parseResult = SetEndSpec(aValue, aContextNode);
+    parseResult = SetEndSpec(aValue, aContextNode, RemoveNonDOM);
   } else if (aAttribute == nsGkAtoms::fill) {
     parseResult = SetFillMode(aValue);
   } else if (aAttribute == nsGkAtoms::max) {
     parseResult = SetMax(aValue);
   } else if (aAttribute == nsGkAtoms::min) {
     parseResult = SetMin(aValue);
   } else if (aAttribute == nsGkAtoms::repeatCount) {
     parseResult = SetRepeatCount(aValue);
@@ -750,21 +773,21 @@ nsSMILTimedElement::SetAttr(nsIAtom* aAt
 }
 
 PRBool
 nsSMILTimedElement::UnsetAttr(nsIAtom* aAttribute)
 {
   PRBool foundMatch = PR_TRUE;
 
   if (aAttribute == nsGkAtoms::begin) {
-    UnsetBeginSpec();
+    UnsetBeginSpec(RemoveNonDOM);
   } else if (aAttribute == nsGkAtoms::dur) {
     UnsetSimpleDuration();
   } else if (aAttribute == nsGkAtoms::end) {
-    UnsetEndSpec();
+    UnsetEndSpec(RemoveNonDOM);
   } else if (aAttribute == nsGkAtoms::fill) {
     UnsetFillMode();
   } else if (aAttribute == nsGkAtoms::max) {
     UnsetMax();
   } else if (aAttribute == nsGkAtoms::min) {
     UnsetMin();
   } else if (aAttribute == nsGkAtoms::repeatCount) {
     UnsetRepeatCount();
@@ -779,44 +802,46 @@ nsSMILTimedElement::UnsetAttr(nsIAtom* a
   return foundMatch;
 }
 
 //----------------------------------------------------------------------
 // Setters and unsetters
 
 nsresult
 nsSMILTimedElement::SetBeginSpec(const nsAString& aBeginSpec,
-                                 nsIContent* aContextNode)
+                                 nsIContent* aContextNode,
+                                 RemovalTestFunction aRemove)
 {
-  mBeginSpecSet = PR_TRUE;
-  return SetBeginOrEndSpec(aBeginSpec, aContextNode, PR_TRUE);
+  return SetBeginOrEndSpec(aBeginSpec, aContextNode, PR_TRUE /*isBegin*/,
+                           aRemove);
 }
 
 void
-nsSMILTimedElement::UnsetBeginSpec()
+nsSMILTimedElement::UnsetBeginSpec(RemovalTestFunction aRemove)
 {
-  ClearBeginOrEndSpecs(PR_TRUE);
-  mBeginSpecSet = PR_FALSE;
+  ClearSpecs(mBeginSpecs, mBeginInstances, aRemove);
   UpdateCurrentInterval();
 }
 
 nsresult
 nsSMILTimedElement::SetEndSpec(const nsAString& aEndSpec,
-                               nsIContent* aContextNode)
+                               nsIContent* aContextNode,
+                               RemovalTestFunction aRemove)
 {
   // XXX When implementing events etc., don't forget to ensure
   // mEndHasEventConditions is set if the specification contains conditions that
   // describe event-values, repeat-values or accessKey-values.
-  return SetBeginOrEndSpec(aEndSpec, aContextNode, PR_FALSE);
+  return SetBeginOrEndSpec(aEndSpec, aContextNode, PR_FALSE /*!isBegin*/,
+                           aRemove);
 }
 
 void
-nsSMILTimedElement::UnsetEndSpec()
+nsSMILTimedElement::UnsetEndSpec(RemovalTestFunction aRemove)
 {
-  ClearBeginOrEndSpecs(PR_FALSE);
+  ClearSpecs(mEndSpecs, mEndInstances, aRemove);
   UpdateCurrentInterval();
 }
 
 nsresult
 nsSMILTimedElement::SetSimpleDuration(const nsAString& aDurSpec)
 {
   nsSMILTimeValue duration;
   PRBool isMedia;
@@ -1143,116 +1168,94 @@ nsSMILTimedElement::Unlink()
 }
 
 //----------------------------------------------------------------------
 // Implementation helpers
 
 nsresult
 nsSMILTimedElement::SetBeginOrEndSpec(const nsAString& aSpec,
                                       nsIContent* aContextNode,
-                                      PRBool aIsBegin)
+                                      PRBool aIsBegin,
+                                      RemovalTestFunction aRemove)
 {
-  ClearBeginOrEndSpecs(aIsBegin);
-
   PRInt32 start;
   PRInt32 end = -1;
   PRInt32 length;
   nsresult rv = NS_OK;
   TimeValueSpecList& timeSpecsList = aIsBegin ? mBeginSpecs : mEndSpecs;
+  InstanceTimeList& instances = aIsBegin ? mBeginInstances : mEndInstances;
+
+  ClearSpecs(timeSpecsList, instances, aRemove);
 
   do {
     start = end + 1;
     end = aSpec.FindChar(';', start);
     length = (end == -1) ? -1 : end - start;
     nsAutoPtr<nsSMILTimeValueSpec>
       spec(new nsSMILTimeValueSpec(*this, aIsBegin));
     rv = spec->SetSpec(Substring(aSpec, start, length), aContextNode);
     if (NS_SUCCEEDED(rv)) {
       timeSpecsList.AppendElement(spec.forget());
     }
   } while (end != -1 && NS_SUCCEEDED(rv));
 
   if (NS_FAILED(rv)) {
-    ClearBeginOrEndSpecs(aIsBegin);
+    ClearSpecs(timeSpecsList, instances, aRemove);
   }
 
   UpdateCurrentInterval();
 
   return rv;
 }
 
 namespace
 {
-  class RemoveNonDOM
+  // Adaptor functor for RemoveInstanceTimes that allows us to use function
+  // pointers instead.
+  // Without this we'd have to either templatize ClearSpecs and all its callers
+  // or pass bool flags around to specify which removal function to use here.
+  class RemoveByFunction
   {
   public:
+    RemoveByFunction(nsSMILTimedElement::RemovalTestFunction aFunction)
+      : mFunction(aFunction) { }
     PRBool operator()(nsSMILInstanceTime* aInstanceTime, PRUint32 /*aIndex*/)
     {
-      return !aInstanceTime->FromDOM();
+      return mFunction(aInstanceTime);
     }
+
+  private:
+    nsSMILTimedElement::RemovalTestFunction mFunction;
   };
 }
 
 void
-nsSMILTimedElement::ClearBeginOrEndSpecs(PRBool aIsBegin)
+nsSMILTimedElement::ClearSpecs(TimeValueSpecList& aSpecs,
+                               InstanceTimeList& aInstances,
+                               RemovalTestFunction aRemove)
 {
-  TimeValueSpecList& specs = aIsBegin ? mBeginSpecs : mEndSpecs;
-  specs.Clear();
-
-  // Remove only those instance times generated by the attribute, not those from
-  // DOM calls.
-  InstanceTimeList& instances = aIsBegin ? mBeginInstances : mEndInstances;
-  RemoveNonDOM removeNonDOM;
-  RemoveInstanceTimes(instances, removeNonDOM);
+  aSpecs.Clear();
+  RemoveByFunction removeByFunction(aRemove);
+  RemoveInstanceTimes(aInstances, removeByFunction);
 }
 
 void
 nsSMILTimedElement::RewindTiming()
 {
-  RewindInstanceTimes(mBeginInstances);
-  RewindInstanceTimes(mEndInstances);
-
   if (mCurrentInterval) {
     mCurrentInterval->Unlink();
     mCurrentInterval = nsnull;
   }
 
   for (PRInt32 i = mOldIntervals.Length() - 1; i >= 0; --i) {
     mOldIntervals[i]->Unlink();
   }
   mOldIntervals.Clear();
 }
 
-namespace
-{
-  class RemoveNonDynamic
-  {
-  public:
-    PRBool operator()(nsSMILInstanceTime* aInstanceTime, PRUint32 /*aIndex*/)
-    {
-      // Generally dynamically-generated instance times (DOM calls, event-based
-      // times) are not associated with their creator nsSMILTimeValueSpec.
-      // If that ever changes though we'll need to make sure to disassociate
-      // them here otherwise they'll get removed when we clear the set of
-      // nsSMILTimeValueSpecs later on.
-      NS_ABORT_IF_FALSE(!aInstanceTime->IsDynamic() ||
-           !aInstanceTime->GetCreator(),
-          "Instance time retained during rewind needs to be unlinked");
-      return !aInstanceTime->IsDynamic();
-    }
-  };
-}
-
-void
-nsSMILTimedElement::RewindInstanceTimes(InstanceTimeList& aList)
-{
-  RemoveNonDynamic removeNonDynamic;
-  RemoveInstanceTimes(aList, removeNonDynamic);
-}
-
 void
 nsSMILTimedElement::ApplyEarlyEnd(const nsSMILTimeValue& aSampleTime)
 {
   // This should only be called within DoSampleAt as a helper function
   NS_ABORT_IF_FALSE(mElementState == STATE_ACTIVE,
       "Unexpected state to try to apply an early end");
 
   // Only apply an early end if we're not already ending.
@@ -1354,16 +1357,20 @@ nsSMILTimedElement::DoPostSeek()
     break;
 
   case SEEK_FORWARD_FROM_INACTIVE:
   case SEEK_BACKWARD_FROM_INACTIVE:
     if (mElementState == STATE_ACTIVE) {
       FireTimeEventAsync(NS_SMIL_BEGIN, 0);
     }
     break;
+
+  case SEEK_NOT_SEEKING:
+    /* Do nothing */
+    break;
   }
 
   mSeekState = SEEK_NOT_SEEKING;
 }
 
 void
 nsSMILTimedElement::UnpreserveInstanceTimes(InstanceTimeList& aList)
 {
@@ -1532,17 +1539,19 @@ nsSMILTimedElement::GetNextInterval(cons
 
   while (PR_TRUE) {
     // Calculate begin time
     if (aFixedBeginTime) {
       if (aFixedBeginTime->Time() < beginAfter)
         return NS_ERROR_FAILURE;
       // our ref-counting is not const-correct
       tempBegin = const_cast<nsSMILInstanceTime*>(aFixedBeginTime);
-    } else if (!mBeginSpecSet && beginAfter <= zeroTime) {
+    } else if ((!mAnimationElement ||
+                !mAnimationElement->HasAnimAttr(nsGkAtoms::begin)) &&
+               beginAfter <= zeroTime) {
       tempBegin = new nsSMILInstanceTime(nsSMILTimeValue(0));
     } else {
       PRInt32 beginPos = 0;
       tempBegin = GetNextGreaterOrEqual(mBeginInstances, beginAfter, beginPos);
       if (!tempBegin || !tempBegin->Time().IsResolved())
         return NS_ERROR_FAILURE;
     }
     NS_ABORT_IF_FALSE(tempBegin && tempBegin->Time().IsResolved() && 
--- a/content/smil/nsSMILTimedElement.h
+++ b/content/smil/nsSMILTimedElement.h
@@ -322,16 +322,18 @@ public:
    * references to other elements can be broken.
    */
   void DissolveReferences() { Unlink(); }
 
   // Cycle collection
   void Traverse(nsCycleCollectionTraversalCallback* aCallback);
   void Unlink();
 
+  typedef PRBool (*RemovalTestFunction)(nsSMILInstanceTime* aInstance);
+
 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;
 
@@ -353,43 +355,47 @@ protected:
   template <class TestFunctor>
   void RemoveInstanceTimes(InstanceTimeList& aArray, TestFunctor& aTest);
 
   //
   // Implementation helpers
   //
 
   nsresult          SetBeginSpec(const nsAString& aBeginSpec,
-                                 nsIContent* aContextNode);
+                                 nsIContent* aContextNode,
+                                 RemovalTestFunction aRemove);
   nsresult          SetEndSpec(const nsAString& aEndSpec,
-                               nsIContent* aContextNode);
+                               nsIContent* aContextNode,
+                               RemovalTestFunction aRemove);
   nsresult          SetSimpleDuration(const nsAString& aDurSpec);
   nsresult          SetMin(const nsAString& aMinSpec);
   nsresult          SetMax(const nsAString& aMaxSpec);
   nsresult          SetRestart(const nsAString& aRestartSpec);
   nsresult          SetRepeatCount(const nsAString& aRepeatCountSpec);
   nsresult          SetRepeatDur(const nsAString& aRepeatDurSpec);
   nsresult          SetFillMode(const nsAString& aFillModeSpec);
 
-  void              UnsetBeginSpec();
-  void              UnsetEndSpec();
+  void              UnsetBeginSpec(RemovalTestFunction aRemove);
+  void              UnsetEndSpec(RemovalTestFunction aRemove);
   void              UnsetSimpleDuration();
   void              UnsetMin();
   void              UnsetMax();
   void              UnsetRestart();
   void              UnsetRepeatCount();
   void              UnsetRepeatDur();
   void              UnsetFillMode();
 
   nsresult          SetBeginOrEndSpec(const nsAString& aSpec,
                                       nsIContent* aContextNode,
-                                      PRBool aIsBegin);
-  void              ClearBeginOrEndSpecs(PRBool aIsBegin);
+                                      PRBool aIsBegin,
+                                      RemovalTestFunction aRemove);
+  void              ClearSpecs(TimeValueSpecList& aSpecs,
+                               InstanceTimeList& aInstances,
+                               RemovalTestFunction aRemove);
   void              RewindTiming();
-  void              RewindInstanceTimes(InstanceTimeList& aList);
   void              DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly);
 
   /**
    * Helper function to check for an early end and, if necessary, update the
    * current interval accordingly.
    *
    * See SMIL 3.0, section 5.4.5, Element life cycle, "Active Time - Playing an
    * interval" for a description of ending early.
@@ -519,23 +525,16 @@ protected:
   {
     RESTART_ALWAYS,
     RESTART_WHENNOTACTIVE,
     RESTART_NEVER
   };
   nsSMILRestartMode               mRestartMode;
   static nsAttrValue::EnumTable   sRestartModeTable[];
 
-  //
-  // We need to distinguish between attempting to set the begin spec and failing
-  // (in which case the mBeginSpecs array will be empty) and not attempting to
-  // set the begin spec at all. In the first case, we should act as if the begin
-  // was indefinite, and in the second, we should act as if begin was 0s.
-  //
-  PRPackedBool                    mBeginSpecSet;
   PRPackedBool                    mEndHasEventConditions;
 
   InstanceTimeList                mBeginInstances;
   InstanceTimeList                mEndInstances;
   PRUint32                        mInstanceSerialIndex;
 
   nsSMILAnimationFunction*        mClient;
   nsAutoPtr<nsSMILInterval>       mCurrentInterval;