1527580 - Animation inspector doesn't deal with fast running RefreshDriver?

Status: UNCONFIRMED

(DevTools :: Inspector: Animations, enhancement, P2)

Description

[Olli Pettay [:smaug][[e-mail]]]

bug 1500046, bug 1483525 and bug 1507337 are an issue which starts to happen more often on debug builds when refreshdriver ticks tiny bit more often (if other JS is slow) because of bug 1506376.
Also browser_animation_logic_mutations_properties.js starts to fail occasionally
(with reactjs error 185).

I could interpret logs like https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228117419&repo=mozilla-inbound&lineNumber=66326 wrongly, but those reactjs errors , especially 185, seem to hint that we don't bound at all how often we call into reactjs stuff.

Comment 1

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

Hi Daisuke, any chance you can look into this?

Thanks

Comment 2

[Daisuke Akatsuka (:daisuke)]

Thank you Olli, and thank you for ni, Gabriel.

Since the failing of browser_animation_logic_mutations_fast.js which was caused by React 185 had been seeing sometimes, I’m not sure whether this related directly. In this test, confirm the animation inspector will not be broken even the animations on document have mutations fast (every 15ms)[1]. Perhaps, 185 error occur when next updating comes during rendering, I think.

However, this might be not related though, I want to drop lifecycle functions from React in near future.
So, how should we do? Skip this test for a while untill above changes (dropping the lifecyle functions)?

https://searchfox.org/mozilla-central/source/devtools/client/inspector/animation/test/doc_mutations_fast.html#41

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(Feel free to re-title the bug. The issues happen particularly on debug builds. Bug 1189015 is perhaps also related.)

Comment 4

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

Hey Daisuke, this appeared on our triage again. Based on the information we have, could you add a note about what particular actions we want to take and assign a priority.

Comment 5

[Daisuke Akatsuka (:daisuke)]

Thanks Gabriel!

I discussed with Brian, I will make this to proceed as followings:

1. Remove lifecycle functions from React
   We are using lifecycle function such as componentWillReceiveProps() in animation inspector. However since these functions is deprecated, I want to remove. Then, Then, will use Redux more instead of this.
   (I may rename this bug to fix above action in this bug)

2. Review tests
   If the failures had not fixed by above change, I'll review the tests.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Daisuke, is there anything I could help here to get this fixed sooner.
I'd like to try to land bug 1506376 sooner than later.
(It helps a bit for example with perceived performance on Google Docs)

Comment 7

[Daisuke Akatsuka (:daisuke)]

Oh I’m so sorry, I have not been able to start to fix yet,,
Please skip tests that are failing, then land your patch.
After that, I’d like to fix above and restore the tests.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

ok, great thanks. I'll look which all tests fail.