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
treeherdermozilla-central@ca457b5758e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, roc, roc
bugs485157
milestone2.0b5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;