Created attachment 8497311 [details] testcase Assertion failure: aTiming.mIterationDuration >= zeroDuration (Expecting iteration duration >= 0), at dom/animation/Animation.cpp:97
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.
Or we could fix this particular TimeDuration consumer, but I think the behavior of TimeDuration here is clearly bogus.
Created attachment 8497956 [details] [diff] [review] 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
Attachment #8497956 - Flags: review?(bzbarsky)
Assignee: nobody → birtles
Status: NEW → ASSIGNED
Comment on attachment 8497956 [details] [diff] [review] Detect integer overflow in BaseTimeDuration::TicksFromMilliseconds This seems reasonable, yeah.
Attachment #8497956 - Flags: review?(bzbarsky) → review+
Oops, forgot to include the crash test. Coming up.
Created attachment 8497963 [details] [diff] [review] Detect integer overflow in BaseTimeDuration::TicksFromMilliseconds Now with crashtest. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0ad580198d88
Attachment #8497956 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.