1074651 - Assertion failure: "Expecting iteration duration >= 0" with CSS transition numeric overflow

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: aTiming.mIterationDuration >= zeroDuration (Expecting iteration duration >= 0), at dom/animation/Animation.cpp:97

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Boris Zbarsky [:bzbarsky]]

Mmm.  Fun integer overflow issues.   :(

In particular, BaseTimeDurationPlatformUtils::TicksFromMilliseconds should probably clamp out-of-range values to INT64_MIN and Forever() just like TimeDuration::FromMilliseconds tries to do with incoming infinite values.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Or we could fix this particular TimeDuration consumer, but I think the behavior of TimeDuration here is clearly bogus.

Comment 4

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: Detect integer overflow in BaseTimeDuration::TicksFromMilliseconds

This seems to work for me.

Something like std::min(std::max(result, INT64_MIN), INT64_MAX) would be more
compact but, as I understand it, we'd end up resolving the type of the max
template argument as 'double' then convert INT64_MAX to double and back to
int64_t again before returning it in the overflow case. I'm not sure if that
necessarily round-trips safely so I did it the long-way.

Currently running through try:

  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=76598669b058

This does, however, mean that we might end up with Forever values in places
where we weren't expecting them. Animation code, however, should handle this
particular case now using the StickyTimeDuration class added in bug 1039924.

  http://hg.mozilla.org/mozilla-central/file/6c824fbb73e8/dom/animation/Animation.cpp#l226

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8497956 [details] [diff] [review]
Detect integer overflow in BaseTimeDuration::TicksFromMilliseconds

This seems reasonable, yeah.

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Oops, forgot to include the crash test. Coming up.

Comment 7

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: Detect integer overflow in BaseTimeDuration::TicksFromMilliseconds

Now with crashtest.

  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0ad580198d88

Comment 8

[Brian Birtles (:birtles, Feb 26~ parental leave)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/79ea9a38b446

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/79ea9a38b446