1480027 - Animations tab crashes when timer is set to long values

Status: VERIFIED FIXED

(DevTools :: Inspector: Animations, defect, P1)

Description

[Cristian Fogel [:cfogel]]

Attachment: devT_animations_crashFFXInspecorOnKeyFrameChange.gif

[Affected versions]:
- 63.0a1 (2018-07-31), 61.0.1, 62.0b13;

[Affected platforms]:
- win 10x64, Ubuntu 16.04LTs, macOS 10.13.4

[Steps to reproduce]:
1. Launch Firefox;
2. Access the following link:
https://rawgit.com/dadaa/3b73f847427025b51ba1ab7333013d0c/raw/77f3f0bb884875a179c3407f73bf8a8dd54751c9/doc_simple_animation.html
3. Open DevTools, Inspector - Animations tab;
4. In the inspector section, click on the <ball negative-delay> div;
5. In the rules section click to modify the animation timer value;
6. Increase the value of the timer adding extra numbers (ex: 10s to 10000000000s or more).

[Expected result]:
- values are added, time-frame indicator is updated. No crashes occur.

[Actual result]:
1. the animations tab and whole right section of the inspector vanishes(including latest beta);

2. on 61.0.1 the whole browser crashes:
- instantly once the value inputed is high enough(nr. of characters)
- if 3-pane is toggled on>off the tab vanishes, ffox doesn't crash fully;
- if about Info window is opened then closed, the browser doesn't crash just 
the inspector right section;


[Regression range]:
- moregression result, after checking strictly after the crashes points out to: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1445825

[Additional notes]:
- attached screen recording with the issues, first part is for nightly/beta behaviour and the second part is for release;
- page refresh, devTools display toggle (or restart) needed to reset the enviroment.

Comment 1

[Marco Castelluccio [:marco]]

When you notice that a bug is a regression, please use the `regression` keyword.

Comment 2

[Cristian Fogel [:cfogel]]

(In reply to Marco Castelluccio [:marco] from comment #1)
> When you notice that a bug is a regression, please use the `regression`
> keyword.

Thanks for the help, I kept forgetting to fill that part.

Comment 3

[Daisuke Akatsuka (:daisuke)]

Thank you for the reporting.
The error was:
  TypeError: 'delay' member of EffectTiming is not a finite floating-point value.
Because 'delay' does not support infinite value[1].
(Also, 'endDelay' and 'iterationStart')

I'll fix this asap.

[1] https://searchfox.org/mozilla-central/source/dom/webidl/AnimationEffect.webidl#29

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1480027 - Part 1: Ignores animations that have big enough delay or endDelay. r=gl

Comment 5

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1480027 - Part 2: Add test for big enough delay and endDelay. r=gl

Depends on D2913

Comment 6

[Daisuke Akatsuka (:daisuke)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c680b8ab68a07c7fbe0574fd25d77d29abb440e

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8998433 [details]
Bug 1480027 - Part 1: Ignores animations that have big enough delay or endDelay. r=gl

Gabriel [:gl] (ΦωΦ) has approved the revision.

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8998435 [details]
Bug 1480027 - Part 2: Add test for big enough delay and endDelay. r=gl

Gabriel [:gl] (ΦωΦ) has approved the revision.

Comment 9

[Daisuke Akatsuka (:daisuke)]

I'll push if the try was green.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=da018910737f7d74589c9d500517bcc6f97db6a5

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a90a87e41d89
Part 1: Ignores animations that have overflowed delay or endDelay. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f8ddc26081f
Part 2: Add test for overflowed delay and endDelay. r=gl

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/a90a87e41d89
https://hg.mozilla.org/mozilla-central/rev/3f8ddc26081f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

This seems edge-case enough that it can ride the trains, but feel free to nominate for Beta approval if you feel strongly otherwise. Thanks for the automated test too :).

Comment 13

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

The issue is still present on 63.0a1 (2018-08-13) across platforms (Windows 10 x64, Ubuntu 18.04 x64, macOS 10.13.4).

Comment 14

[Daisuke Akatsuka (:daisuke)]

(In reply to Iulia Cristescu, QA [:JuliaC] from comment #13)
> The issue is still present on 63.0a1 (2018-08-13) across platforms (Windows
> 10 x64, Ubuntu 18.04 x64, macOS 10.13.4).

I have changed the logic to that if there are overflowing number in delay or end delay field, we ignore the animation displayed. So, in that test case, after changing the delay to big number on the inspector, we can see "No animations ..." message.
I could confirm 63.0a1 (2018-08-13) on OSX 10.13.6.
Could you confirm??

Comment 15

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I can confirm that the message is still NOT displayed across platforms on 63.0a1 (2018-08-13, using the steps provided in comment 0. For more details, see the screencast https://drive.google.com/file/d/1_OS24jK7QRDxUTUyvva0JFPsCBCC10ny/view?usp=sharing

Comment 16

[Daisuke Akatsuka (:daisuke)]

Ah, negative infinity, thanks.

Comment 17

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1480027 - Part 3: Ignore animations that have negative infinity delay or end delay. r=gl

Comment 18

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1480027 - Part 4: Add test case for negative infinity delay and end delay. r=gl

Depends on D3380

Comment 19

[Daisuke Akatsuka (:daisuke)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d311019b65c640cd386ddbc0740bbce060a5339

Comment 20

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 9000130 [details]
Bug 1480027 - Part 3: Ignore animations that have negative infinity delay or end delay. r=gl

Gabriel [:gl] (ΦωΦ) has approved the revision.

Comment 21

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 9000131 [details]
Bug 1480027 - Part 4: Add test case for negative infinity delay and end delay. r=gl

Gabriel [:gl] (ΦωΦ) has approved the revision.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ad987a8861
Part 3: Ignore animations that have negative infinity delay or end delay. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2c0be296f6f
Part 4: Add test case for negative infinity delay and end delay. r=gl

Comment 23

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/a2ad987a8861
https://hg.mozilla.org/mozilla-central/rev/f2c0be296f6f

Comment 24

[Cristian Fogel [:cfogel]]

Issue verified, filled a separate bug for the newly noticed issue.
(bug 1544688)