1486962 - Background image sometimes missing when tabbing back after a long period of time

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Bobby Holley (:bholley)]

I've been using my blog [1] to test WR. Sometimes when it's been in the background for a while, I can tab back to it and the background is white. Anything that triggers a repaint (interacting with the page, tabbing the window to/from background, switching tabs) will cause the background image to reappear.

I've only noticed this in my primary browsing environment (OSX). I haven't been able to reproduce this reliably, but wanted to get it on file in case it rings any bells.

What diagnostic steps should I take next time it happens? I've tried capturing with ctrl+shift+3, but that doesn't seem to work when Firefox isn't launched from the command line.

[1] https://bholley.net/blog/latest/

Comment 1

[Dzmitry Malyshau [:kvark]]

A WR capture would be most useful for me. Could you run FF from command line once, so that we can get one?

Comment 2

[Bobby Holley (:bholley)]

(In reply to Dzmitry Malyshau [:kvark] from comment #1)
> A WR capture would be most useful for me. Could you run FF from command line
> once, so that we can get one?

I can do that.

In general though, it seems like we'd empower a lot more people to generate captures if we didn't have this requirement. Is there any reason we can't generate captures in /tmp or equivalent if the CWD isn't writable?

Comment 3

[Dzmitry Malyshau [:kvark]]

No particular reason, at all. It just happens to work so smooth on Linux that I tend to forget the difficulties of capturing on other platforms. We should totally make it configurable through a pref.

Comment 4

[Bobby Holley (:bholley)]

Managed to capture a trace: https://www.dropbox.com/s/y9p1jjhnnfnpdvd/bholley-missing-bg-capture.zip?dl=0

Comment 5

[Bobby Holley (:bholley)]

(In reply to Dzmitry Malyshau [:kvark] from comment #3)
> No particular reason, at all. It just happens to work so smooth on Linux
> that I tend to forget the difficulties of capturing on other platforms. We
> should totally make it configurable through a pref.

Filed bug 1487462 for this.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

If it has been tabbed away from for a while, then the image cache got cleared. I suspect the ImageRenderer state still has the original state cleared, so it attempts to draw and it will result in NOT_READY. If the FRAME_UPDATE event got suppressed like in the nsImageBoxFrame case, that could cause it to not repaint when it should. But I would have expected it to be reproducible from an about:memory minimization in that case.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

(nsImageBoxFrame case refers to bug 1487120.)

Comment 8

[Bobby Holley (:bholley)]

(In reply to Andrew Osmond [:aosmond] from comment #6)
> If it has been tabbed away from for a while, then the image cache got
> cleared. I suspect the ImageRenderer state still has the original state
> cleared, so it attempts to draw and it will result in NOT_READY. If the
> FRAME_UPDATE event got suppressed like in the nsImageBoxFrame case, that
> could cause it to not repaint when it should. But I would have expected it
> to be reproducible from an about:memory minimization in that case.

Minimizing memory usage via about:memory does reliably reproduce this issue on my machine (MBP). Checking Windows next.

Comment 9

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) from comment #8)
> Minimizing memory usage via about:memory does reliably reproduce this issue
> on my machine (MBP). Checking Windows next.

Same, same thing on a Quantum reference device. Glad this is reproducible!

Comment 10

[Bobby Holley (:bholley)]

I think this probably blocks Nightly, feel free to change if anyone disagrees.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Hmmm, very interesting! Digging a little further, this is the code I expected to schedule the paint:

https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/layout/style/ImageLoader.cpp#458

The WebRenderUserData was probably missing until we tried the first paint after returning, and it would have gotten created here:

https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/layout/painting/nsImageRenderer.cpp#633

If the FRAME_UPDATE came too early, the first paint would have painted the actual image, and if it came "too late", it should have triggered a repaint. Hm. Maybe this could be a race on the decoder thread, e.g. if it finishes redecoding soon enough, then something else triggers the paint on our behalf, hence why I can't reproduce (yet).

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Okay, reproduced in rr with a forced decoding delay, thanks Bobby for confirming the about:memory STR!

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

So the nsDisplayBackgroundImage is associated with an nsCanvasFrame, which gets the WebRenderImageData associated with it. When we get the FRAME_UPDATE however, it has a different object, an nsBlockFrame, in the frame set which doesn't have any WebRenderUserData bound to it. This causes us to ignore the FRAME_UPDATE and not schedule a paint.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

It looks like the only frame that ever gets associated with it is the nsBlockFrame. On the non-WebRender path, I'm guessing this would normally cause the association:

https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/layout/painting/nsCSSRendering.cpp#2777

We did remember to do something similar for border images on the WebRender path:

https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/layout/painting/nsCSSRenderingBorders.cpp#3487

Comment 15

[Andrew Osmond [:aosmond] (he/him)]

Attachment: 0001-Bug-1486962-Ensure-frame-is-associated-with-backgrou.patch, v1

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f170796353d49344a5ba2a6ee152106e0a437608

This ought to do it.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5c7d67c2b5b
Ensure frame is associated with background images with WebRender. r=mstange

Comment 17

[Raul Gurzau (:RaulG)]

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