1249212 - Make ComputedTiming.endTime handling match updated spec

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

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

I spoke with Google about how end-time should be defined and we agreed it should be possible to have an end time before the start time. When I went to implement this and check it (before speccing), however, I found a number of other issues in our code we need to fix. This bug covers all those other issues too.

Comment 1

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

Attachment: part 1 - Simplify various keyframe-effect tests

This patch just simplifies the keyframe-effect tests so that we don't have to
repeat default values. This makes the tests shorter, easier to scan, and
easier to understand what is being tested.

In some cases we still repeat the default values in order to indicate that
we're testing that we get a particular default value.

Comment 2

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

Attachment: part 2 - Fix infinity handling in StickyTimeDurationValueCalculator::Multiply

This patch brings the behavior of this method into line with the comment that
describes it and other methods in this class that have a similar test.

I discovered this bug while exercising this code from animation code and was
getting test failures due to returning -Infinity instead of the expected
Infinity.

Comment 3

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

Attachment: part 3 - Fix active duration calculation when iteration duration is infinity

With the added tests in part 4 we crash without this change because we end up
trying to multiply an infinite iteration duration by a zero iteration count
which trips an assertion in StickyTimeDuration. Hence we fix this behavior
before adding the tests.

Comment 4

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

Attachment: part 4 - Add tests for activeDuration

Before we go fixing endTime, we should add tests that activeDuration (which
endTime builds on) is being calculated correctly. (Spoiler: it wasn't, hence
parts 2 and 3 in this patch series.)

Comment 5

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

Attachment: part 5 - Remove max() clamping from endTime calculation

Based on discussion at: https://github.com/w3c/web-animations/issues/86

Comment 6

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

Attachment: part 6 - Add tests for endTime calculation

Comment 7

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

Attachment: part 7 - Calculate the endTime in GetComputedTimingAt

Current endTime is calculated when getComputedTiming() is called. As a result,
the value returned there doesn't necessarily reflect what we are using in
the model. It would be more simple, consistent and useful if we simply
calculate this as part of GetComputedTimingAt and use it both internally
and in the result to getComputedTiming().

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8720675 [details] [diff] [review]
part 2 - Fix infinity handling in StickyTimeDurationValueCalculator::Multiply

Review of attachment 8720675 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch, thanks for the patch!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e17bca1849ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed71e0a2f711
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e4a17c84ae3
https://hg.mozilla.org/integration/mozilla-inbound/rev/85e66410c01b
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9130dcb9d01
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f7b038c23f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/99f8242bbd92

Comment 10

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

Spec has now been updated.

https://github.com/w3c/web-animations/commit/082696c1db5fad92226b6d8daf05242fa665df09

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e17bca1849ad
https://hg.mozilla.org/mozilla-central/rev/ed71e0a2f711
https://hg.mozilla.org/mozilla-central/rev/2e4a17c84ae3
https://hg.mozilla.org/mozilla-central/rev/85e66410c01b
https://hg.mozilla.org/mozilla-central/rev/e9130dcb9d01
https://hg.mozilla.org/mozilla-central/rev/2f7b038c23f1
https://hg.mozilla.org/mozilla-central/rev/99f8242bbd92