Bug 537361 followup: Address review suggestions. r=birtles
--- 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;