###!!! ASSERTION: GetMillis() called for unresolved time.: 'mState == STATE_RESOLVED', file content/smil/nsSMILTimeValue.cpp, line 80 ###!!! ABORT: Sample time should not be negative: 'mSampleTime >= 0.0f', file content/smil/nsSMILAnimationFunction.cpp, line 368

Confirmed on a mozilla-central debug build from yesterday. Note that the assertion-failure is from a call one line above the failed NS_ABORT_IF_FALSE, in nsSMILAnimationFunction::InterpolateResult 365 const nsSMILTime& dur = mSimpleDuration.GetMillis(); 366 367 // Sanity Checks 368 NS_ABORT_IF_FALSE(mSampleTime >= 0.0f, "Sample time should not be negative"); So initially, we'll skip this animation, at the beginning of nsSMILAnimationFunction::ComposeResult, because "IsActiveOrFrozen()" returns false. But when we set fill=freeze, that method starts returning true, and we abort on the next sample.

Here's a testcase that triggers the failure onclick instead of onload.

So the general problem here is this: we don't correctly handle setting fill="freeze" when our nsSMILTimedElement is in the "POSTACTIVE" state. This includes the scenario in both testcases posted so far -- when the interval is entirely before document start-time (and is therefore ignored[1]) -- but it also includes simpler cases. Normally, when we run an animation that's got fill="freeze", nsSMILTimedElement::SampleAt() calls SampleFillValue() on our first "frozen" sample. This calls nsSMILAnimationFunction::SampleLastValue, which sets the |mLastValue| variable, which gets used for all subsequent samples. This is what we want. However, if we aren't in "freeze" mode at the end of the animation's interval (i.e. if we set fill="freeze" later on), then SampleFillValue & SampleLastValue never get called, and so |mLastValue| never gets set. And so nsSMILAnimationFunction keeps trying to do normal interpolation, when we don't want it to. I propose we fix this by moving the SampleFillValue call from STATE_ACTIVE to STATE_POSTACTIVE, and we add a flag to keep track of whether we need to call it. Patch to do this coming up...

Comment on attachment 410713 [details] [diff] [review] fix v1: add |nsSMILTimedElement.mNeedToSampleFillValue| flag Brian, how does this look to you?

Looks good. I'm just wondering how urgent this is though because it should already be fixed in the patch for syncbase timing (bug 474743) as I came across this bug in implementing that. The patch for syncbase timing is a major overhaul of the timing model (and fixes several other bugs I've discovered) so I'm just wondering if it's best to land this or just wait? Of course the test cases should be added one way or another. (I've significantly updated the handling of the previous interval since the latest wip patch attached to bug 474743, but that latest wip patch should still fix this issue.) I'm expecting to put the syncbase timing patch up for review in about 3 weeks time. I can publish another wip patch in about 2 weeks time.

Ok -- since this only causes problems in debug builds, and since bug 474743 will be reworking this code anyway, I'm happy to just have this be fixed by in that patch when it's ready. --> Marking dependency.

(In reply to comment #6) > Of course the test cases should be added one way or > another. Here are the test cases from "fix v1", for landing once this has been fixed by bug 474743's patch.

Thanks Daniel. Sorry, that must be pretty frustrating after all your work on this. I would have let you know sooner but I wasn't CC'ed on the bug--not sure if there's some way I can automatically be CC'ed for new SMIL bugs?

No prob -- I probably should've CC'd you once I realized it was time-model stuff. Anyway, it was good to refamiliarize myself with time model a bit. :) > if there's some way I can automatically be CC'ed for new SMIL bugs? You can receive email for all SVG bugs if you go to Bugzilla's "Preferences" | "Email Preferences" section and add the SVG QA Contact ("general@svg.bugs") to your watch-list.

Assigning this to me. Once bug 474743 I'll confirm that it fixes this and check in the test cases from this patch.

Landed this bug's testcases, now that bug 474743 has landed: http://hg.mozilla.org/mozilla-central/rev/f4e56d114546

Resolving this as fixed by bug 474743.