Closed Bug 1147491 Opened 5 years ago Closed 5 years ago

Slowing down an animation using animationPlayer.playbackRate crashes the tab


(Core :: DOM: Core & HTML, defect)

Not set



Tracking Status
firefox40 --- fixed
firefox44 --- verified


(Reporter: pbro, Assigned: jwatt)




(2 files)


- visit
- open the devtools webconsole (ctrl/cmd+alt+K)
- enter:
document.querySelector("#box1").getAnimationPlayers()[0].playbackRate = 10
==> This works fine, the animation speeds up

- enter:
document.querySelector("#box1").getAnimationPlayers()[0].playbackRate = 0.5
==> BOOM! The tab crashes.
Blocks: 1120339
So in a debug build I get:

Assertion failure: aDivisor != 0 (Division by zero), at ../../dist/include/mozilla/TimeStamp.h:211
#01: mozilla::BaseTimeDuration<mozilla::TimeDurationValueCalculator>::operator/(long long) const[/Users/bzbarsky/mozilla/inbound/obj-firefox/dist/ +0x15874b3]
#02: mozilla::dom::AnimationPlayer::SilentlySetCurrentTime(mozilla::BaseTimeDuration<mozilla::TimeDurationValueCalculator> const&)[/Users/bzbarsky/mozilla/inbound/obj-firefox/dist/ +0x17cfd8f]
#03: mozilla::dom::AnimationPlayer::SetCurrentTime(mozilla::BaseTimeDuration<mozilla::TimeDurationValueCalculator> const&)[/Users/bzbarsky/mozilla/inbound/obj-firefox/dist/ +0x17cfdff]
#04: mozilla::dom::AnimationPlayer::SetPlaybackRate(double)[/Users/bzbarsky/mozilla/inbound/obj-firefox/dist/ +0x17cfee6]

The relevant code in AnimationPlayer::SilentlySetCurrentTime is:

112         mStartTime.SetValue(mTimeline->GetCurrentTime().Value() -
113                               (aSeekTime / mPlaybackRate));

here aSeekTime is a TimeDuration and the issue is that TimeDuration's operator/ takes an _integer_ argument.  So passing in 0.5 ends up passing floor(0.5), so 0.

This code has looked like this since bug 1127380 landed.

It seems like we should use aSeekTime.MultDouble(1 / mPlaybackRate) or something?
Flags: needinfo?(jwatt)
Assignee: nobody → jwatt
Flags: needinfo?(jwatt)
Thanks for the helpful debug notes.
Attachment #8586236 - Flags: review?(ehsan)
Comment on attachment 8586231 [details] [diff] [review]
part 1 - Fix playbackRate crash due to integer rounding causing divide-by-zero

>diff --git a/dom/animation/AnimationPlayer.cpp b/dom/animation/AnimationPlayer.cpp
>--- a/dom/animation/AnimationPlayer.cpp
>+++ b/dom/animation/AnimationPlayer.cpp
>@@ -101,17 +101,17 @@ AnimationPlayer::SilentlySetCurrentTime(
>       mPlaybackRate == 0.0
>       /*or, once supported, if we have a pending pause task*/) {
>     mHoldTime.SetValue(aSeekTime);
>     if (!mTimeline || mTimeline->GetCurrentTime().IsNull()) {
>       mStartTime.SetNull();
>     }
>   } else {
>     mStartTime.SetValue(mTimeline->GetCurrentTime().Value() -
>-                          (aSeekTime / mPlaybackRate));
>+                          (aSeekTime.MultDouble(1 / mPlaybackRate)));

If mPlaybackRate were *actually* zero here, then we'd still divide by 0 and run into trouble here.  Can you assert that it's nonzero here? (That's a real question; I'm not sure if we know it's nonzero).  If so, great; if not, we need to handle the 0 case.

(The other two chunks are fine; they've got explicit mPlaybackRate!=0 checks protecting this division.)
Comment on attachment 8586231 [details] [diff] [review]
part 1 - Fix playbackRate crash due to integer rounding causing divide-by-zero

Sorry, I got the logic confused; if mPlaybackRate is 0, we'll take the first case, and we won't hit this code. So, we should be fine. (and an assertion would probably be overkill given the "if" condition)

Attachment #8586231 - Flags: review?(dholbert) → review+
Attachment #8586236 - Flags: review?(ehsan) → review+
I was able to reproduce this issue on Mac OS X 10.10.5 on Nightly 39.0a1(2015-03-29) using STR from Comment 0.
The issue is no longer reproducible on Mac OS x 10.10.5, Ubuntu 14.04 x86 and Windows 10 x86 , using Latest Nightly 44.0a1 (2015-10-25) and STR from Comment 0 with the updated functions: 
- document.querySelector("#box1").getAnimations()[0].playbackRate = 10
- document.querySelector("#box1").getAnimations()[0].playbackRate = 0.5
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.