1748216 - background image is not displayed after 3 minutes have passed since the last hovering

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[6k64x4ma]

Attachment: testcase

Steps to reproduce:

1. Open the attached file with a new profile.
2. Hover the cursor over the <div>. The MDN icon appears.
3. Move the cursor out of the <div>, then wait about 3 minutes.
4. Hover over the <div> again.

Actual results:
The image is not displayed after about 3 minutes have passed since the last hovering.

Expected results:
The image should be displayed.

mozregression:
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c4b3480996ec9bdbea7040e5cb9e05215d9acb16&tochange=afed7ee7a5dcee09542f3157224a54057527d842

Bug 1711061 - Part 13. Remove the now unused ImageContainer and related code for images. r=tnikkel
Differential Revision: https://phabricator.services.mozilla.com/D126606

Comment 1

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

You can force this to happen without waiting by going to about:memory and hitting minimize memory usage. The image reappears if I move out and back in again.

Comment 2

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

When the surface cache was cleared, our invalidation logic is a bit wonky. PrepareResult returns ImgDrawResult::SUCCESS because we have previously generated a valid frame (that doesn't mean we have any at the moment.) When we try to fetch that result, we end up on the no provider path here:

https://searchfox.org/mozilla-central/rev/253ae246f642fe9619597f44de3b087f94e45a2d/layout/painting/nsImageRenderer.cpp#630

With ImageContainers we used to return an empty ImageContainer in this case. That would cause us to create a WebRenderImageData object to hang off the display item / frame. When the FRAME_COMPLETE came in from the fresh decode request, we would end up invalidating because of an ImageKey mismatch (we didn't have an image key at all before).

Now we use WebRenderImageProviderData but with no provider, we don't create it in the first place. When the FRAME_COMPLETE comes in, we don't force an invalidation of the frame, because we totally ignore the case without any WebRenderUserData objects:

https://searchfox.org/mozilla-central/rev/253ae246f642fe9619597f44de3b087f94e45a2d/layout/style/ImageLoader.cpp#527

Comment 3

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

Attachment: Bug 1748216 - Ensure that we invalidate previously decoded images without a surface ready.

Part of how invalidation works with WebRender is that we assume frames
with a WebRenderUserData object attached to them are in view. This means
for images that we must ensure we create an empty
WebRenderImageProviderData object even when we have no provider or
surface for display. This will allow us to invalidate properly when we
get the FRAME_COMPLETE notification from imagelib indicating that the
redecode has completed.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8df35e043394
Ensure that we invalidate previously decoded images without a surface ready. r=jrmuizel

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8df35e043394

Comment 6

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

Comment on attachment 9257552 [details]
Bug 1748216 - Ensure that we invalidate previously decoded images without a surface ready.

Beta/Release Uplift Approval Request

• User impact if declined: Images may not always invalidate properly, particularly for long lived tabs where the underlying surfaces get invalidated.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1) Browse to https://bug1748216.bmoattachments.org/attachment.cgi?id=9257318

2. Hover in and out over the div twice. The Mozilla image should appear both times.
3. Open new tab, browse to about:memory, and hit Minimize memory usage. Wait for it to complete.
4. Return to previous tab and repeat step 2. Image should appear both times.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is very surgical where we return to the behaviour we would have had with ImageContainers prior to the regressing bug landing.
• String changes made/needed:

Comment 7

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

I manually verified on nightly build 20220105093353 and confirmed the test case works.

Comment 8

[Dianna Smith [:diannaS]]

Comment on attachment 9257552 [details]
Bug 1748216 - Ensure that we invalidate previously decoded images without a surface ready.

Approved for 96.0rc2

Comment 9

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/a0f9ecea3570

Comment 10

[Giorgia Nichita, Release Desktop QA]

Reproduced the issue with Firefox 97.0a1 (20220103092929) on Ubuntu 20.04 and macOS 10.15 using STR from comment 0.
The issue is no longer reproducible with 97.0a1 (20220105170324) and 96.0rc2 (20220105140734 - treeherder build) on Ubuntu 20.04., macOS 10.15 and Windows 10x64.

Comment 11

[Giorgia Nichita, Release Desktop QA]

Verified fixed on the official 96.0-build2 (20220106144528) on macOS 10.15 and Win 10 x64.