1124610 - FF36: Incorrect repainting after scrolling down and up.

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[palo misik]

Attachment: ff37beta-buildbot.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150120155007

Steps to reproduce:

win7x64.
FF36.
For example visit page http://winbot.zope.org/waterfall
Scroll down and up.




Actual results:

Page is incorrectly repainted.


Expected results:

Correctly repainted.

Comment 1

[Matthias Versen [:Matti]]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Got as far as we can go bisecting nightlies...
Last good revision: 74edfbf0e6a3
First bad revision: ced1402861b8
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=74edfbf0e6a3&tochange=ced1402861b8

Last good revision: 4631a7474d8a
First bad revision: bb51e79ff4fa
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4631a7474d8a&tochange=bb51e79ff4fa

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

I can reproduce this bug using the URL in comment 0 on OSX 10.10.1.

Milan - Can you please investigate to see if this is a gfx issue?

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Not seeing this issue on 10.9.5, but I did see a black rectangle at one point.  Given the regression range - Seth, any ideas?  SVG?

Comment 4

[Seth Fowler [:seth] [:s2h]]

I can reproduce as well on 10.10. The background behind the header - where the word "Waterfall" is in a larger font - is initially white, but after scrolling down and back up it becomes a dark gray. Scrolling up and down again fixes it.

Comment 5

[Seth Fowler [:seth] [:s2h]]

Further investigation reveals:

- If I bring up the dev tools and inspect the header, it does not get repainted correctly, even in the areas highlighted by the dev tools inspector.

- If I use the dev tools to delete elements from outside the affected area - like, say, the table - they disappear. However, if I delete elements from inside the affected area - for example, the <h1> element containing the word "Waterfall" - the affected area does not get repainted. The dev tools act as if the <h1> element is gone, but I can still see the word "Waterfall" on the screen.

Comment 6

[Seth Fowler [:seth] [:s2h]]

Looks like body has a repeat-x background image on it - 'bg_gradient.jpg' - which is 120px high, probably approximately the height of the affected area. I'd guess something's going wrong with that image.

Looking at that pushlog, nothing immediately jumps out at me as a logical explanation. The first thing to do is bisect within that pushlog and figure out exactly which commit broke it.

Comment 7

[Seth Fowler [:seth] [:s2h]]

I'll build beta overnight and will try to bisect this to the exact commit tomorrow.

Comment 8

[Seth Fowler [:seth] [:s2h]]

I've bisected this to this commit:

5ff89c5a8393	Seth Fowler — Bug 1102617 - Replace imgIContainer::FrameIsOpaque with imgIContainer::IsOpaque. r=tn

Quite possibly relevant is this message which I noticed we print for this page:

Corrupt JPEG data: 1 extraneous bytes before marker 0xc4

Comment 9

[Seth Fowler [:seth] [:s2h]]

Alright, I just debugged this, and the problem is straightforward. When a decoder bails out of decoding due to an error, it may not have written to its entire frame. The areas of the frame that it hasn't written to are zero-initialized, and since the default frame format is B8G8R8A8, we end up with transparency that we have failed to announce via PostHasTransparency(), which is not allowed. This causes the layout and painting code to believe that the corrupt image on this page is opaque when it isn't, and the result is bad rendering.

The fix is to call PostHasTransparency() if we're completing decoding due to an error.

Comment 10

[Seth Fowler [:seth] [:s2h]]

Attachment: Call PostHasTransparency for corrupt images that we treat as usable

Here's the patch.

Comment 11

[Seth Fowler [:seth] [:s2h]]

Here's a try job:

https://tbpl.mozilla.org/?tree=Try&rev=554f42eea167

Comment 12

[Seth Fowler [:seth] [:s2h]]

Thanks for the review! Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/40d3b44fb02c

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40d3b44fb02c

Comment 14

[Seth Fowler [:seth] [:s2h]]

Attachment: Call PostHasTransparency for corrupt images that we treat as usable. (FF36 Backport)

Here's a backport of this patch to 36, suitable for uplift.

Comment 15

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8560865 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable. (FF36 Backport)

Approval Request Comment
[Feature/regressing bug #]: 1102617
[User impact if declined]: Painting artifacts while scrolling or possibly in other situations, like during animations.
[Describe test coverage new/current, TreeHerder]: On m-c.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Seth, what is the plan about 37/aurora? Thanks

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3347b3531c98

Comment 18

[palo misik]

I've just updated to FF36 Beta 8 (BUILDID:20150209164123) and it problem is fixed.

Comment 19

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to palo misik from comment #18)
> I've just updated to FF36 Beta 8 (BUILDID:20150209164123) and it problem is
> fixed.

Thank you! Marking as Verified on Firefox 36.

Comment 20

[Andrei Vaida [:avaida]]

Managed to reproduce the bug on an affected nightly build from 2015-01-22, using Windows 8.1 (x64) with the link from Comment 0.

This is now verified fixed on Nightly 38.0a1 (2015-02-19) using Windows 8.1 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

Comment 21

[Lawrence Mandel [:lmandel] (use needinfo)]

Seth - Firefox 37 is still affected. Does this require a branch specific patch for Aurora? Can you please get the patch in shape to land on Aurora and submit an uplift request (preferably before the merge on Monday)?

Comment 22

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8558443 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable

Approval Request Comment
[Feature/regressing bug #]: Bug 1102617
[User impact if declined]: Rendering glitches on pages with corrupt images.
[Describe test coverage new/current, TreeHerder]: On m-c for some time.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 23

[Seth Fowler [:seth] [:s2h]]

Pushed to Aurora:

https://hg.mozilla.org/releases/mozilla-aurora/rev/cd3e705640af

Comment 24

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8558443 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable

Verbal approval previously given as this had already landed on 36 and 38. Aurora+