From 98fd010267430139cc15ad37ed01297ef73bd8f0 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Wed, 13 Oct 2010 09:20:12 +0900 Subject: [PATCH] Bug 596796 - SVG SMIL: Fix inconsistent state when resetting current interval; r=dholbert; a=roc --- content/smil/crashtests/596796-1.svg | 15 +++++++ content/smil/crashtests/crashtests.list | 1 + content/smil/nsSMILTimedElement.cpp | 58 ++++++++----------------- content/smil/nsSMILTimedElement.h | 14 +++++- 4 files changed, 48 insertions(+), 40 deletions(-) create mode 100644 content/smil/crashtests/596796-1.svg diff --git a/content/smil/crashtests/596796-1.svg b/content/smil/crashtests/596796-1.svg new file mode 100644 index 000000000000..52a66fd58255 --- /dev/null +++ b/content/smil/crashtests/596796-1.svg @@ -0,0 +1,15 @@ + + + + + + + diff --git a/content/smil/crashtests/crashtests.list b/content/smil/crashtests/crashtests.list index ec8bb0a592bd..c5303ccf613d 100644 --- a/content/smil/crashtests/crashtests.list +++ b/content/smil/crashtests/crashtests.list @@ -20,3 +20,4 @@ load 588287-1.svg load 588287-2.svg load 592477-1.xhtml load 594653-1.svg +load 596796-1.svg diff --git a/content/smil/nsSMILTimedElement.cpp b/content/smil/nsSMILTimedElement.cpp index 12466395c9ef..7b2caf7e3a38 100644 --- a/content/smil/nsSMILTimedElement.cpp +++ b/content/smil/nsSMILTimedElement.cpp @@ -202,9 +202,6 @@ nsSMILTimedElement::nsSMILTimedElement() 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(); @@ -218,10 +215,8 @@ nsSMILTimedElement::~nsSMILTimedElement() // 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->Unlink(); - mCurrentInterval = nsnull; - } + mElementState = STATE_POSTACTIVE; + ResetCurrentInterval(); for (PRInt32 i = mOldIntervals.Length() - 1; i >= 0; --i) { mOldIntervals[i]->Unlink(); @@ -703,14 +698,7 @@ nsSMILTimedElement::Rewind() mSeekState == SEEK_BACKWARD_FROM_ACTIVE, "Rewind in the middle of a forwards seek?"); - // Set the STARTUP state first so that if we get any callbacks we won't waste - // 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(); + ClearIntervalProgress(); UnsetBeginSpec(RemoveNonDynamic); UnsetEndSpec(RemoveNonDynamic); @@ -1116,6 +1104,13 @@ nsSMILTimedElement::IsTimeDependent(const nsSMILTimedElement& aOther) const void nsSMILTimedElement::BindToTree(nsIContent* aContextNode) { + // If we were already active then clear all our timing information and start + // afresh + if (mElementState != STATE_STARTUP) { + mSeekState = SEEK_NOT_SEEKING; + Rewind(); + } + // Resolve references to other parts of the tree PRUint32 count = mBeginSpecs.Length(); for (PRUint32 i = 0; i < count; ++i) { @@ -1127,23 +1122,9 @@ nsSMILTimedElement::BindToTree(nsIContent* aContextNode) mEndSpecs[j]->ResolveReferences(aContextNode); } - // If this element was already in play then we may need to do a local rewind. - nsSMILTime containerTime = GetTimeContainer()->GetCurrentTime(); - PRBool localRewind = - mElementState != STATE_STARTUP && mCurrentInterval && - mCurrentInterval->Begin()->Time().GetMillis() > containerTime; - - if (localRewind) { - Rewind(); - // Put the time container in the seeking state -- this is necessary so we - // don't generate unnecessary events whilst doing the backwards seek. - GetTimeContainer()->SetCurrentTime(containerTime); - } else { - // Otherwise, re-register any previous milestone since it might be been - // processed whilst we were not bound to the tree. - mPrevRegisteredMilestone = sMaxMilestone; - RegisterMilestone(); - } + // Register new milestone + mPrevRegisteredMilestone = sMaxMilestone; + RegisterMilestone(); } void @@ -1269,13 +1250,13 @@ nsSMILTimedElement::ClearSpecs(TimeValueSpecList& aSpecs, } void -nsSMILTimedElement::RewindTiming() +nsSMILTimedElement::ClearIntervalProgress() { - if (mCurrentInterval) { - mCurrentInterval->Unlink(); - mCurrentInterval = nsnull; - } + mElementState = STATE_STARTUP; + mCurrentRepeatIteration = 0; + ResetCurrentInterval(); + // Remove old intervals for (PRInt32 i = mOldIntervals.Length() - 1; i >= 0; --i) { mOldIntervals[i]->Unlink(); } @@ -1893,8 +1874,7 @@ nsSMILTimedElement::UpdateCurrentInterval(PRBool aForceChangeNotice) if (mElementState == STATE_ACTIVE || mElementState == STATE_WAITING) { mElementState = STATE_POSTACTIVE; - mCurrentInterval->Unlink(); - mCurrentInterval = nsnull; + ResetCurrentInterval(); } } } diff --git a/content/smil/nsSMILTimedElement.h b/content/smil/nsSMILTimedElement.h index eebc8ab73755..3492b96da4a7 100644 --- a/content/smil/nsSMILTimedElement.h +++ b/content/smil/nsSMILTimedElement.h @@ -409,7 +409,7 @@ protected: void ClearSpecs(TimeValueSpecList& aSpecs, InstanceTimeList& aInstances, RemovalTestFunction aRemove); - void RewindTiming(); + void ClearIntervalProgress(); void DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly); /** @@ -513,6 +513,18 @@ protected: PRBool HasPlayed() const { return !mOldIntervals.IsEmpty(); } PRBool EndHasEventConditions() const; + // Reset the current interval by first passing ownership to a temporary + // variable so that if Unlink() results in us receiving a callback, + // mCurrentInterval will be nsnull and we will be in a consistent state. + void ResetCurrentInterval() + { + if (mCurrentInterval) { + // Transfer ownership to temp var. (This sets mCurrentInterval to null.) + nsAutoPtr interval(mCurrentInterval); + interval->Unlink(); + } + } + // Hashtable callback methods PR_STATIC_CALLBACK(PLDHashOperator) NotifyNewIntervalCallback( TimeValueSpecPtrKey* aKey, void* aData);