Closed Bug 1342240 Opened 5 years ago Closed 5 years ago

Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html,315920-7c.html | image comparison, max difference: 229, number of differing pixels: 23,45

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed

People

(Reporter: intermittent-bug-filer, Unassigned)

References

(Depends on 1 open bug)

Details

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

Attachments

(1 file)

Looks like it expects the first checkbox to be checked, but it isn't.
:bz -- You worked on this test, long ago. Any idea what's gone wrong?
Flags: needinfo?(bzbarsky)
Well, the obvious fail is that the checkmark in the checkbox is not drawing.

The actual checked state is correct, since the text is green.  It's just the drawing of the checkmark that's broken.  And it's not broken in general (paints in the reference)....

So presumably some sort of invalidation bug.  But nothing in the pushlog jumps out at me.

This was green on both parents of the merge but broken on the merge?  :(
Flags: needinfo?(bzbarsky)
I mean nothing in the "inbound merge to m-c" pushlog.  On the autoland side, maybe bug 1026804 messes up something in the test harness?  May be worth doing a try with that turned back off.  Past that no sure.
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #6)
> maybe bug 1026804 messes up something in the test harness?  May be worth
> doing a try with that turned back off.  Past that no sure.

I'll check on that - thanks.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b528fced62ea37341048d1dabcae1a858948f585
Another possibility: In all of the failed cases, 315920-12c runs as the first test in the chunk; in all of the recent tests that passed before the merge, 315920-12c ran as the second test in the chunk, after 315920-12b.

OK:

REFTEST SUITE-START | Running 14321 tests
REFTEST INFO | Running chunk 15 out of 48 chunks.  tests 4585-4883/299
REFTEST TEST-START | http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12b.html == http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12-ref.html

Fail:

REFTEST SUITE-START | Running 14323 tests
REFTEST INFO | Running chunk 15 out of 48 chunks.  tests 4586-4884/299
REFTEST TEST-START | http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12c.html == http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12-ref.html
(In reply to Geoff Brown [:gbrown] from comment #7)
> (In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #6)
> > maybe bug 1026804 messes up something in the test harness?  May be worth
> > doing a try with that turned back off.  Past that no sure.
> 
> I'll check on that - thanks.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=b528fced62ea37341048d1dabcae1a858948f585

No, it's not bug 1026804.
On Android, the checkmark is an SVG image.  Perhaps we're firing onload despite the image not having loaded (or maybe it loaded but hasn't gone through the whole drawing pipeline yet)?

https://searchfox.org/mozilla-central/rev/c9ccb8ec1f5acda78e6344ffb87aa3a409031e3d/layout/style/res/forms.css#643
Good news, adding a couple of reftests turned the crank, and it's no longer permaorange on Android opt.

Bad news, if, as that did, you put 315920-13a.html first in a chunk, it'll fail to get the checkmark loaded in time, and now Android debug's permaorange.
Summary: Permafailing android 315920-12c.html == 315920-12-ref.html | image comparison, max difference: 229, number of differing pixels: 23 → Permafailing android 315920-12c.html,315920-13a.html | image comparison, max difference: 229, number of differing pixels: 23
I got the same result on try: I removed some unrelated reftests to shift the tests so that 315920-12c no longer runs first, and all passed.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=209941037623eedefc0e2f281428ce22f78351db
Using MozReftestInvalidate should fix this.  See bug 1334944.
See Also: → 1339062
Yeah, running first in a chunk before anything in that session has had a chance to load the image could have this effect.  After that the image would be cached and loads would complete sync.... That at least explains why this could happen on a merge.
Depends on: 1334944
I saw this failure in 315920-8a.html on my try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9661df4cb88d5e5266bedb934afc61be59e303d4&selectedJob=80356303
Because I added some reftests?
Summary: Permafailing android 315920-12c.html,315920-13a.html | image comparison, max difference: 229, number of differing pixels: 23 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html | image comparison, max difference: 229, number of differing pixels: 23,45
(In reply to Mats Palmgren (:mats) from comment #14)
> Using MozReftestInvalidate should fix this.  See bug 1334944.

It still seems wrong to me that this is required.  Shouldn't the reftest harness be waiting for images to load (perhaps via shouldWaitForPendingPaints() in reftest-content.js calling to nsPresContext::IsDOMPaintEventPending)?
Whiteboard: [stockwell needswork]
Summary: Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html | image comparison, max difference: 229, number of differing pixels: 23,45 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html | image comparison, max difference: 229, number of differing pixels: 23,45
Duplicate of this bug: 1343808
Duplicate of this bug: 1343809
Duplicate of this bug: 1343811
Summary: Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html | image comparison, max difference: 229, number of differing pixels: 23,45 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html,315920-7c.html | image comparison, max difference: 229, number of differing pixels: 23,45
I assume the fix for this is in bug 1334944, I will push on that, with 552 failures in the last week (60%+ of the time), this is a bit more than a nuisance.  If we cannot get bug 1334944 assigned and in progress by end of day tomorrow, I will disable all 315920 bugs until we have time.
this is sort of 'disabling' the tests until bug 1334944 can get completion.
Attachment #8843438 - Flags: review?(gbrown)
Comment on attachment 8843438 [details] [diff] [review]
add fuzzy-if to frequently failing tests.

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

Maybe add a comment pointing back to this bug.
Attachment #8843438 - Flags: review?(gbrown) → review+
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1416b28a89c5
Permafailing android 315920*, add fuzzy-if. r=gbrown
Whiteboard: [stockwell needswork] → [stockwell disabled]
https://hg.mozilla.org/mozilla-central/rev/1416b28a89c5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
This is hitting -9 on Aurora today. I'll fuzzy-if it away like we did on trunk in comment 34 I guess.
https://treeherder.mozilla.org/logviewer.html#?job_id=87311362&repo=mozilla-aurora
Fuzz harder on Aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/a55f31cc0fe1

I suppose I could land this on trunk too, but I guess I'll hold out hope that we'll fix this properly somewhere else before we end up having to.
You need to log in before you can comment on or make changes to this bug.