See https://bugzilla.mozilla.org/show_bug.cgi?id=822231#c12

This fixes the testcase. I'd kind of like to get this in to b2g18, since this is a bug that could break Web content that's using CSS animations. I don't think we have particularly good testing of Web content on B2G, and I'd rather avoid having different sets of bugs when we can.

Oh, and the above patch is against b2g18; the trunk patch (also on top of bug 827717) is: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/321c7aa169d8/omta-animationiteration

Comment on attachment 700709 [details] [diff] [review] patch Review of attachment 700709 [details] [diff] [review]: ----------------------------------------------------------------- r=me with these things checked. ::: layout/style/nsAnimationManager.cpp @@ +167,4 @@ > ElementAnimation &anim = mAnimations[animIdx]; > > if (anim.mProperties.Length() == 0 || > + anim.mIterationDuration.ToMilliseconds() <= 0.0) { Is it worth trying to avoid the main work in this method if animations are paused? i.e., by checking for anim.isPaused() in the second |if| ? @@ +191,5 @@ > // XXX We shouldn't really be using mLastNotification as a general > // indicator that the animation has finished, it should be reserved for > // events. If we use it differently in the future this use might need > // changing. > + if (anim.mLastNotification == ElementAnimation::LAST_NOTIFICATION_END && Do you mean != here?

(In reply to Nick Cameron [:nrc] from comment #4) > Comment on attachment 700709 [details] [diff] [review] > patch > > Review of attachment 700709 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me with these things checked. > > ::: layout/style/nsAnimationManager.cpp > @@ +167,4 @@ > > ElementAnimation &anim = mAnimations[animIdx]; > > > > if (anim.mProperties.Length() == 0 || > > + anim.mIterationDuration.ToMilliseconds() <= 0.0) { > > Is it worth trying to avoid the main work in this method if animations are > paused? i.e., by checking for anim.isPaused() in the second |if| ? I don't think so; it would add differences in behavior between the OMTA and non-OMTA codepaths (in particular, for what happens if the timing parameters of a paused animation are adjusted). > @@ +191,5 @@ > > // XXX We shouldn't really be using mLastNotification as a general > > // indicator that the animation has finished, it should be reserved for > > // events. If we use it differently in the future this use might need > > // changing. > > + if (anim.mLastNotification == ElementAnimation::LAST_NOTIFICATION_END && > > Do you mean != here? No. GetPositionInIteration is what adjusts mLastNotification, so I've changed this comparison from being before mLastNotification is adjusted for this tick to being after it's adjusted for this tick. That's what allowed simplifying the conditions -- now all I need to do is check that the new mLastNotification is end, but we changed it to that during this tick.

Comment on attachment 700709 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 780692 (throttling of off-main-thread animations) User impact if declined: A Web platform feature that works on other Firefox releases since (I think) Firefox 5 will be broken on B2G (without being broken on any other software we release, since OMTA is enabled only on B2G). It's not clear how many Web sites depend on this feature, but I'd rather not take that gamble. Testing completed: Tested that patch fixes behavior on attached testcase and doesn't appear to break primary B2G UI in any obvious ways. Risk to taking this patch (and alternatives if risky): Relatively low, although there's always a chance it'll break something, either because I got the logic wrong, or because something landed in B2G since bug 780692 that depends on this feature being broken. String or UUID changes made by this patch: none

Since the two other flags in this bug haven't been triaged for a few days, adding another flag that might be the right one to request.

Thanks. https://hg.mozilla.org/releases/mozilla-b2g18/rev/b37780f94be7 (oops, took me a while to hit submit on that comment; got stuck in a background tab)