Seth Fowler [:seth] [:s2h] Reporter
	Description • 10 years ago

We have a number of Android intermittent failures right now that are caused by a failure to call imgFrame::Optimize() before the reftest snapshot is taken. (The issue is generally that we use the same image in the test and the reference, and we end up calling Optimize() for one but not the other.) The important ones I know about are:

- layout/reftests/w3c-css/submitted/ui3/box-sizing-replaced-002.xht
- layout/reftests/w3c-css/submitted/ui3/box-sizing-replaced-003.xht
- layout/reftests/text-overflow/marker-basic.html
- layout/reftests/bugs/407111-1.html

Bug 1128225 is impressively effective at turning these intermittent failures into permanent failures; this is presumably because it changes when we perceive an image to be "complete". (In other words, when we no longer feel that we have to sync decode for the reftest snapshot.) Previous to bug 1128225, this was determined by imgIContainer::IsDecoded(), which was determined by the value of RasterImage::mHasBeenDecoded, which got set in RasterImage::DecodingComplete() on the main thread sometime after the image was decoded, making it more likely that we'd have already optimized the image's imgFrame. After bug 1128225, we consider the image to be complete when we draw it successfully, which is determined by (among other things) imgFrame::IsImageComplete() returning true, which can happen well before the imgFrame gets optimized.

The change in behavior in bug 1128225 is not wrong; imgFrame::Optimize() should be an optimization, but we should not depend on it for correctness. We should therefore mark these tests fuzzy.

I do plan to eventually start calling imgFrame::Optimize() off-main-thread; it's possible, but in practice several things block it. When we start doing that, we will be able to remove these fuzzy annotations.

(Note that we also have issues with this problem on Windows; see e.g. bug 1126530. Bug 1128225 doesn't seem to change timing so much on Windows, though, so for now I'm mostly worried about Android.)

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 1 • 10 years ago

Here are some try jobs that show these failures:

https://tbpl.mozilla.org/?tree=Try&rev=49c027bb6dce
https://tbpl.mozilla.org/?tree=Try&rev=18f631156e9b

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 2 • 10 years ago

Here's the patch.

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 3 • 10 years ago

This is purely a fuzzyness tweak, so I think this can go ahead and land without review. Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5903bcfa581c

	Phil Ringnalda (:philor)
	Comment 4 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/5903bcfa581c

	Phil Ringnalda (:philor)
	Comment 5 • 10 years ago

You meant to have this be leave-open, since you want to come back and remove these annotations (and you don't want scripts that look for manifest comments which refer to closed bugs yelling at you, either).

You also want to be reminded of https://hg.mozilla.org/integration/mozilla-inbound/rev/dc9aad9b9ce1 as being the same sort of annotation.

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 10 years ago

Thanks for doing this, Seth.

https://hg.mozilla.org/releases/mozilla-aurora/rev/6947b54d7918
https://hg.mozilla.org/releases/mozilla-beta/rev/58a64dbc82f1

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 7 • 10 years ago

Bug 1125490 and bug 1128769 revealed more tests that suffer from the same issue, so I pushed another round of fuzzification:

https://hg.mozilla.org/integration/mozilla-inbound/rev/393838728c27

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 10 years ago

Windows needed more fuzz.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f918335d622

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 10 years ago

And a little bit more:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d365bfd24986

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/393838728c27
https://hg.mozilla.org/mozilla-central/rev/9f918335d622
https://hg.mozilla.org/mozilla-central/rev/d365bfd24986

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 11 • 10 years ago

Pushed a followup because layout/reftests/w3c-css/submitted/ui3/box-sizing-replaced-002.xht wasn't quite fuzzy enough:

https://hg.mozilla.org/integration/mozilla-inbound/rev/469ff237d9c5

	Phil Ringnalda (:philor)
	Comment 12 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/469ff237d9c5

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 13 • 10 years ago

Pushed even more fuzz for layout/reftests/w3c-css/submitted/ui3/box-sizing-replaced-002.xht. It seems to vary a lot, probably because it contains lots and lots of images.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6393809e286

	Seth Fowler [:seth] [:s2h] Reporter
	Comment 14 • 10 years ago

I pushed every commit in this bug that wasn't already on Aurora, to Aurora.

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/cc3eb5188f65
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/12dde0274d48
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/8e744fc8972c
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/0eabce4d5b3c
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/5030d9cf38b3
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/1a6b4564e1b6

	Carsten Book [:Tomcat]
	Comment 15 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/b6393809e286

	BugBot [:suhaib / :marco/ :calixte]
	Comment 16 • 6 years ago

The leave-open keyword is there and there is no activity for 6 months.
:aosmond, maybe it's time to close this bug?