Last Comment Bug 781362 - Some SMIL reftests take too long to stop animating
: Some SMIL reftests take too long to stop animating
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla17
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: dlbi 547801
  Show dependency treegraph
 
Reported: 2012-08-08 15:22 PDT by Matt Woodrow (:mattwoodrow)
Modified: 2012-08-13 14:53 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Delay the animation start time (13.53 KB, patch)
2012-08-08 15:22 PDT, Matt Woodrow (:mattwoodrow)
dholbert: review+
Details | Diff | Review

Description Matt Woodrow (:mattwoodrow) 2012-08-08 15:22:30 PDT
Created attachment 650336 [details] [diff] [review]
Delay the animation start time

We should delay the start time of the animations so we don't have to wait for them.

I *think* this patch also fixes bug 547801.
Comment 1 Daniel Holbert [:dholbert] 2012-08-08 15:57:52 PDT
Comment on attachment 650336 [details] [diff] [review]
Delay the animation start time

Yup -- in particular, we don't want there to be any immediately-animating stuff, because any initial animations will delay the firing of MozReftestInvalidate.
Comment 2 Matt Woodrow (:mattwoodrow) 2012-08-13 03:15:39 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/1302dc0adcd5
Comment 3 Jonathan Watt [:jwatt] 2012-08-13 05:41:07 PDT
Just so I understand, the problem here is that the timeline starts before the MozReftestInvalidate event is fired? Why does that matter?
Comment 4 Daniel Holbert [:dholbert] 2012-08-13 09:08:28 PDT
The problem arises from these facts:
 * These tests have animations that start immediately, at time=0

 * The immediate animations trigger continuous paints which (with DLBI) delay the firing of MozReftestInvalidate.  This is because DLBI tickles something about the way we determine "all painting has stopped" for firing that event.

 * This means the MozReftestInvalidate-triggered scripting (including the "reftest-wait" removal) doesn't run until the animations have completely played through, which takes on the order of seconds.

So: at a minimum, this means the reftests take longer than we'd like to run.  In some cases it can makes them time out before they fire MozReftestInvalidate, if we continue invalidating forever due to e.g. some fill="freeze" by-animations that we don't trust to be the same from sample to sample since the underlying value might change.
Comment 5 Jonathan Watt [:jwatt] 2012-08-13 09:26:34 PDT
(In reply to Daniel Holbert [:dholbert] from comment #4)
>  * The immediate animations trigger continuous paints which (with DLBI)
> delay the firing of MozReftestInvalidate.  This is because DLBI tickles
> something about the way we determine "all painting has stopped" for firing
> that event.

If anyone (Matt?) knows more about this part, I'd be interested to hear the details.

>  * This means the MozReftestInvalidate-triggered scripting (including the
> "reftest-wait" removal) doesn't run until the animations have completely
> played through

Wow, okay. Good to know. CC'ing some other people who should be aware of this when writing future SMIL animation tests.

FWIW, to me it seems like it would be a good idea to be calling pauseAnimations() before the load events fire to make tests a bit cleaner. (Doing the pause under setTimeAndSnapshot() in onload seems less great in the face of this bug.)
Comment 6 Daniel Holbert [:dholbert] 2012-08-13 09:42:07 PDT
(In reply to Jonathan Watt [:jwatt] from comment #5)
> (In reply to Daniel Holbert [:dholbert] from comment #4)
> >  * The immediate animations trigger continuous paints which (with DLBI)
> > delay the firing of MozReftestInvalidate.
[...]
> If anyone (Matt?) knows more about this part, I'd be interested to hear the
> details.

(For a bit more discussion on this, see bug 758505 comment 7 and 10-12.)
Comment 7 Ed Morley [:emorley] 2012-08-13 11:10:19 PDT
https://hg.mozilla.org/mozilla-central/rev/1302dc0adcd5
Comment 8 Matt Woodrow (:mattwoodrow) 2012-08-13 14:53:22 PDT
(In reply to Jonathan Watt [:jwatt] from comment #5)
> (In reply to Daniel Holbert [:dholbert] from comment #4)
> >  * The immediate animations trigger continuous paints which (with DLBI)
> > delay the firing of MozReftestInvalidate.  This is because DLBI tickles
> > something about the way we determine "all painting has stopped" for firing
> > that event.
> 
> If anyone (Matt?) knows more about this part, I'd be interested to hear the
> details.

Both with and without DLBI, we always wait until there are no pending paints to be processed before we fire the MozReftestInvalidate event and finish the test.

It appeared with the existing code that we'd often manage to paint the last set of changes and then finish the test before the SMIL animation code scheduled another paint.

All my patches do is change some of the paint timings, which make this no longer true and we have to wait out the entire animation.

Note You need to log in before you can comment on or make changes to this bug.