Bug 537361 followup: Address review suggestions. r=birtles
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 01 Mar 2010 11:31:52 -0800
changeset 38819 02c434f2fd4a5c7e11873ae6d1ee2995f7c75008
parent 38818 8bac3bfd59a339398eed4c81664b0a82dfaeddb5
child 38820 dbb530e1ac89abc923c8907e1ca16fc6b57e4e85
push id11876
push userdholbert@mozilla.com
push dateMon, 01 Mar 2010 19:34:18 +0000
treeherderautoland@02c434f2fd4a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs537361
milestone1.9.3a3pre
Bug 537361 followup: Address review suggestions. r=birtles
content/smil/nsSMILInstanceTime.cpp
content/smil/nsSMILInterval.cpp
content/smil/nsSMILInterval.h
content/smil/nsSMILTimedElement.cpp
content/smil/nsSMILTimedElement.h
--- a/content/smil/nsSMILInstanceTime.cpp
+++ b/content/smil/nsSMILInstanceTime.cpp
@@ -73,17 +73,18 @@ nsSMILInstanceTime::nsSMILInstanceTime(c
                                        nsSMILTimeValueSpec* aCreator,
                                        nsSMILInterval* aBaseInterval)
   : mTime(aTime),
     mFlags(0),
     mSerial(0),
     mVisited(PR_FALSE),
     mChainEnd(PR_FALSE),
     mCreator(aCreator),
-    mBaseInterval(nsnull)
+    mBaseInterval(nsnull) // This will get set to aBaseInterval in a call to
+                          // SetBaseInterval() at end of constructor
 {
   switch (aSource) {
     case SOURCE_NONE:
       // No special flags
       break;
 
     case SOURCE_DOM:
       mFlags = kClearOnReset | kFromDOM;
@@ -132,18 +133,19 @@ nsSMILInstanceTime::HandleChangedInterva
     // 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;
   }
 
-  PRBool objectChanged = mCreator->DependsOnBegin() ? aBeginObjectChanged
-                                                    : aEndObjectChanged;
+  PRBool objectChanged = mCreator->DependsOnBegin() ? aBeginObjectChanged :
+                                                      aEndObjectChanged;
+
   AutoBoolSetter setVisited(mVisited);
 
   nsRefPtr<nsSMILInstanceTime> deathGrip(this);
   mCreator->HandleChangedInstanceTime(*GetBaseTime(), aSrcContainer, *this,
                                       objectChanged);
 }
 
 void
@@ -187,19 +189,19 @@ nsSMILInstanceTime::SetBaseInterval(nsSM
   // 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();
+    const nsSMILInstanceTime* dependentTime = mCreator->DependsOnBegin() ?
+                                              aBaseInterval->Begin() :
+                                              aBaseInterval->End();
     dependentTime->BreakPotentialCycle(this);
     aBaseInterval->AddDependentTime(*this);
   }
 
   mBaseInterval = aBaseInterval;
 }
 
 const nsSMILInstanceTime*
@@ -209,18 +211,18 @@ nsSMILInstanceTime::GetBaseTime() const
     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();
+  return mCreator->DependsOnBegin() ? mBaseInterval->Begin() :
+                                      mBaseInterval->End();
 }
 
 void
 nsSMILInstanceTime::BreakPotentialCycle(
     const nsSMILInstanceTime* aNewTail) const
 {
   const nsSMILInstanceTime* myBaseTime = GetBaseTime();
   if (!myBaseTime)
--- a/content/smil/nsSMILInterval.cpp
+++ b/content/smil/nsSMILInterval.cpp
@@ -63,17 +63,18 @@ nsSMILInterval::~nsSMILInterval()
       "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,
+                                              mEndObjectChanged);
   }
   mBeginObjectChanged = PR_FALSE;
   mEndObjectChanged = PR_FALSE;
 }
 
 void
 nsSMILInterval::NotifyDeleting()
 {
@@ -130,12 +131,14 @@ nsSMILInterval::AddDependentTime(nsSMILI
   if (!inserted) {
     NS_WARNING("Insufficient memory to insert instance time.");
   }
 }
 
 void
 nsSMILInterval::RemoveDependentTime(const nsSMILInstanceTime& aTime)
 {
-  PRBool found = mDependentTimes.RemoveElementSorted(&aTime);
+#ifdef DEBUG
+  PRBool found =
+#endif
+    mDependentTimes.RemoveElementSorted(&aTime);
   NS_ABORT_IF_FALSE(found, "Couldn't find instance time to delete.");
-  (void)found;
 }
--- a/content/smil/nsSMILInterval.h
+++ b/content/smil/nsSMILInterval.h
@@ -94,17 +94,17 @@ public:
   {
     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
+  // XXX Backwards seeking support (bug 492458)
   void Unfreeze()
   {
     // XXX
     UnfreezeEnd();
   }
 
   void UnfreezeEnd()
   {
--- a/content/smil/nsSMILTimedElement.cpp
+++ b/content/smil/nsSMILTimedElement.cpp
@@ -417,16 +417,27 @@ nsSMILTimedElement::DoSampleAt(nsSMILTim
   NS_ABORT_IF_FALSE(mElementState != STATE_STARTUP || aEndOnly,
       "Got a regular sample during startup state, expected an end sample"
       " instead");
 
   PRBool          stateChanged;
   nsSMILTimeValue sampleTime(aContainerTime);
 
   do {
+#ifdef DEBUG
+    // Check invariant
+    if (mElementState == STATE_STARTUP || mElementState == STATE_POSTACTIVE) {
+      NS_ABORT_IF_FALSE(!mCurrentInterval,
+          "Shouldn't have current interval in startup or postactive states");
+    } else {
+      NS_ABORT_IF_FALSE(mCurrentInterval,
+          "Should have current interval in waiting and active states");
+    }
+#endif
+
     stateChanged = PR_FALSE;
 
     switch (mElementState)
     {
     case STATE_STARTUP:
       {
         nsSMILInterval firstInterval;
         mElementState =
@@ -912,21 +923,22 @@ nsSMILTimedElement::AddDependent(nsSMILT
   NS_ABORT_IF_FALSE(!mTimeDependents.GetEntry(&aDependent),
       "nsSMILTimeValueSpec is already registered as a dependency");
   mTimeDependents.PutEntry(&aDependent);
 
   // Add old and current intervals
   //
   // It's not necessary to call SyncPauseTime since we're dealing with
   // historical instance times not newly added ones.
+  nsSMILTimeContainer* container = GetTimeContainer();
   for (PRUint32 i = 0; i < mOldIntervals.Length(); ++i) {
-    aDependent.HandleNewInterval(*mOldIntervals[i], GetTimeContainer());
+    aDependent.HandleNewInterval(*mOldIntervals[i], container);
   }
   if (mCurrentInterval) {
-    aDependent.HandleNewInterval(*mCurrentInterval, GetTimeContainer());
+    aDependent.HandleNewInterval(*mCurrentInterval, container);
   }
 }
 
 void
 nsSMILTimedElement::RemoveDependent(nsSMILTimeValueSpec& aDependent)
 {
   mTimeDependents.RemoveEntry(&aDependent);
 }
@@ -1064,17 +1076,17 @@ nsSMILTimedElement::ClearBeginOrEndSpecs
 // This method is based on the pseudocode given in the SMILANIM spec.
 //
 // See:
 // http://www.w3.org/TR/2001/REC-smil-animation-20010904/#Timing-BeginEnd-LC-Start
 //
 nsresult
 nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
                                     const nsSMILInstanceTime* aFixedBeginTime,
-                                    nsSMILInterval& aResult)
+                                    nsSMILInterval& aResult) const
 {
   NS_ABORT_IF_FALSE(!aFixedBeginTime || aFixedBeginTime->Time().IsResolved(),
       "Unresolved begin time specified for interval start");
   static nsSMILTimeValue zeroTime(0L);
 
   if (mRestartMode == RESTART_NEVER && aPrevInterval)
     return NS_ERROR_FAILURE;
 
@@ -1457,30 +1469,29 @@ nsSMILTimedElement::SampleSimpleTime(nsS
       ActiveTimeToSimpleTime(aActiveTime, repeatIteration);
     mClient->SampleAt(simpleTime, mSimpleDur, repeatIteration);
   }
 }
 
 void
 nsSMILTimedElement::SampleFillValue()
 {
-  NS_ABORT_IF_FALSE(!mOldIntervals.IsEmpty(),
-      "Attempting to sample fill value but there is no previous interval");
-
   if (mFillMode != FILL_FREEZE || !mClient)
     return;
 
-  const nsSMILInterval& prevInterval = *GetPreviousInterval();
-  NS_ABORT_IF_FALSE(prevInterval.End()->Time().IsResolved() &&
-      !prevInterval.End()->MayUpdate(),
+  const nsSMILInterval* prevInterval = GetPreviousInterval();
+  NS_ABORT_IF_FALSE(prevInterval,
+      "Attempting to sample fill value but there is no previous interval");
+  NS_ABORT_IF_FALSE(prevInterval->End()->Time().IsResolved() &&
+      !prevInterval->End()->MayUpdate(),
       "Attempting to sample fill value but the endpoint of the previous "
       "interval is not resolved and frozen");
 
-  nsSMILTime activeTime = prevInterval.End()->Time().GetMillis() -
-                          prevInterval.Begin()->Time().GetMillis();
+  nsSMILTime activeTime = prevInterval->End()->Time().GetMillis() -
+                          prevInterval->Begin()->Time().GetMillis();
 
   PRUint32 repeatIteration;
   nsSMILTime simpleTime =
     ActiveTimeToSimpleTime(activeTime, repeatIteration);
 
   if (simpleTime == 0L && repeatIteration) {
     mClient->SampleLastValue(--repeatIteration);
   } else {
--- a/content/smil/nsSMILTimedElement.h
+++ b/content/smil/nsSMILTimedElement.h
@@ -394,17 +394,17 @@ protected:
    * @param[out] aResult    The next interval. Will be unchanged if no suitable
    *                        interval was found (in which case NS_ERROR_FAILURE
    *                        will be returned).
    * @return  NS_OK if a suitable interval was found, NS_ERROR_FAILURE
    * otherwise.
    */
   nsresult          GetNextInterval(const nsSMILInterval* aPrevInterval,
                                     const nsSMILInstanceTime* aFixedBeginTime,
-                                    nsSMILInterval& aResult);
+                                    nsSMILInterval& aResult) const;
   nsSMILInstanceTime* GetNextGreater(const InstanceTimeList& aList,
                                      const nsSMILTimeValue& aBase,
                                      PRInt32& aPosition) const;
   nsSMILInstanceTime* GetNextGreaterOrEqual(const InstanceTimeList& aList,
                                             const nsSMILTimeValue& aBase,
                                             PRInt32& aPosition) const;
   nsSMILTimeValue   CalcActiveEnd(const nsSMILTimeValue& aBegin,
                                   const nsSMILTimeValue& aEnd) const;