Closed Bug 1326306 Opened 3 years ago Closed 2 years ago

Intermittent dom/animation/test/chrome/test_restyles.html | CSS animations running on the main-thread should update style on the main thread - got 6, expected 5

Categories

(Core :: DOM: Animation, defect, P5)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: intermittent-bug-filer, Assigned: hiro)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell disabled][fennec-scouting])

Attachments

(2 files)

See Also: → 1332970
This seems to only happen on android-4-2-x86 (assuming it has a different cause than bug 1332970). We only started running mochitest-chrome on android-4-2-x86 about 2 months ago. I think it would be no great loss to skip this test on Android x86. :hiro -- would that be okay?
Blocks: 1315749
Flags: needinfo?(hikezoe)
I did not realize that this failure get frequent recently.  I did check that all of the recent failures happened in the first test case.   One interesting thing is that some of failures received 7 restyles instead of 6 restyles.  I am convinced that one restyle comes from creating a thumbnail (See bug 1276170).  I guess the other one is the root cause of this recent failure.  I think there was a change on Android that causes flush style after the document has been finished to be loaded.

Considering only animations I am OK with skipping this test on Android x86 but I don't think it's good for Android performance since flushing style will spend some amount of time.

Hello Jim Chen,  I am not sure you are a proper person but I saw lots of your name in recent commits for Android. ;-)
Do you see any suspicious changes on Android that cause flushing style (e.g. creating a thumbnail, getComputedStyle() or something, actually I don't know all of them..) around 25 Jan?

Please feel free to toss it me back again if you are not sure.
Thank you!
Flags: needinfo?(hikezoe) → needinfo?(nchen)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> 
> Hello Jim Chen,  I am not sure you are a proper person but I saw lots of
> your name in recent commits for Android. ;-)
> Do you see any suspicious changes on Android that cause flushing style (e.g.
> creating a thumbnail, getComputedStyle() or something, actually I don't know
> all of them..) around 25 Jan?

I don't see anything suspicious around 1/25. Some of the bugs I was working on could have an impact but they landed a while ago. Is there any way for you to log who is causing the flush?
Flags: needinfo?(nchen)
Jim, thanks for the feedback!

Normally we can use devtools performance tool to see restyles but it's hard to track down where the restyle comes from.

If android team is OK with the redundant restyle on the initial page load, I will file a new bug for the redundant restyle and am going to skip this test on android-4-2-x86.

Jim, what do you think?
Flags: needinfo?(nchen)
Sounds good. Can I set a breakpoint somewhere in GDB to see what's causing restyling?
Flags: needinfo?(nchen)
Thanks! I will post a patch soon.

(In reply to Jim Chen [:jchen] [:darchons] from comment #10)
> Sounds good. Can I set a breakpoint somewhere in GDB to see what's causing
> restyling?

If you try to track it down I think start points are here[1] and here[2].

[1] https://hg.mozilla.org/mozilla-central/file/f985243bb630/dom/animation/EffectCompositor.cpp#l609
[2] https://hg.mozilla.org/mozilla-central/file/f985243bb630/dom/animation/EffectCompositor.cpp#l319

Thanks for helping us!
See Also: → 1335986
Comment on attachment 8832739 [details]
Bug 1326306 - Skip test_restyles.html on Android x86.

https://reviewboard.mozilla.org/r/108956/#review110314
Attachment #8832739 - Flags: review?(gbrown) → review+
Keywords: leave-open
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/861fe1977321
Skip test_restyles.html on Android x86. r=gbrown
Looks like all failures since this landed have been on try and mozilla-aurora. Hiro, shall we uplift this to aurora?
Flags: needinfo?(hikezoe)
Yes, Unfortunately the change that causes 7 restyling has been slipped into aurora.. The first failure on aurora is https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&revision=d8d335074d5c896ccacce5e302baae193e62dbaf

Maybe the culprit uplifted there?
Whiteboard: [stockwell disabled]
Whiteboard: [stockwell disabled] → [stockwell disabled][fennec-scouting]
this has been slowly increasing in frequency- we previously skipped this on x64, but at this rate in a few weeks I see us skipping this on all android.
Beginning with https://hg.mozilla.org/integration/autoland/rev/ff0abac31f47f58e53f13c6cad84f6a5c74cefca (bug 1378313), now we are getting failures on Windows and Linux too. That was backed out yesterday and seems to have been effective.

As noted in comment 36, failures continue on Android.
OK to expand the skip to all Android? 

There have been 6 Android (arm) failures so far today.
Attachment #8884333 - Flags: review?(hikezoe)
Attachment #8884333 - Flags: review?(hikezoe) → review+
(In reply to Geoff Brown [:gbrown] from comment #39)
> Beginning with
> https://hg.mozilla.org/integration/autoland/rev/
> ff0abac31f47f58e53f13c6cad84f6a5c74cefca (bug 1378313), now we are getting
> failures on Windows and Linux too. That was backed out yesterday and seems
> to have been effective.

This fact (browser code changes breaks this test) makes me nervous. We might want to migrate this test in content (i.e. not in chrome test) and use SpecialPowers there.
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb0994ed7a97
Skip test_restyles.html on Android; r=hiro
See Also: → 1413370
P5 since this test has already run since bug 1335986.  And I believe the failure case has been fixed by 1418268.  I will close this once after Firefox 59 was released.
Priority: -- → P5
I believe this failure no longer happens.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Assignee: nobody → hikezoe
You need to log in before you can comment on or make changes to this bug.