1565542 - img.decode() never resolved/rejected if src already as html-image and in cache

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P2)

Description

[tobias.buschor]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

html:
<img src="https://imagizer.imageshack.com/v2/285x280q90/c/661/ggZf2A.jpg">
javaascript:
img1.src = 'https://imagizer.imageshack.com/v2/285x280q90/c/661/ggZf2A.jpg';
img1.decode().then(function(){
log('loaded 1')
});
See here:
https://jsbin.com/fudowariki/1/edit?html,js,output

Actual results:

The decode promise will not resolve/reject if a html-image-element with the same src already exists and the image is in the cache.

Comment 1

[tobias.buschor]

As this future is already shipped, it can break websites using this api.
Code like this will fail:

if (!img.decode) img.onload = function(){...}
img.src = 'http....';
if (img.decode) img.decode().then(...)

Comment 2

[tobias.buschor]

Can't always be reproduced.
It occurs when DevTools are open and under network the cache is deactivated and reactivated.

Comment 3

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

I was able to reproduce just by refreshing a few times, no need to fiddle with dev tools or the network cache. This is concerning.

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1565542. We need to check to resolve image decode promises when we get the frame update notification too. r?aosmond

When we reload the document the destruction of the old document triggers a discard request for the image. If timing is right we haven't locked the image in the new document yet so it discards.

We call LoadImage on the image, it returns the existing entry from the image cache, but it needs to validate. When it validates we send out all the progress in the progress tracker already. This includes frame complete and decode complete even though we have no decoded surfaces for this image right now.

The RequestDecodeForSize call in nsImageLoadingContent::MaybeResolveDecodePromises triggers a decode. When the decode finishes we send a frame update notification but we never send frame complete or decode complete because those are permanent once they happen.

Comment 6

[(no longer active)]

Copying flags over from bug 1567535.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b0b216b8675
We need to check to resolve image decode promises when we get the frame update notification too. r=aosmond

Comment 9

[Andreea Pavel [:apavel]]

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

Comment 10

[alexander.pepper]

Here is another proof of this bug: https://codepen.io/anon/pen/eqvbQy . Simply reload the pen. Ideally you should always see the image and a "Decoding done" but more often then not, the image is not seen, because "decode" never finishes.

Tested with User Agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0" - latest public Firefox "68.0.1 (64-Bit)" version on mac os.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta/Release/ESR68 approval when you get a chance.

Comment 12

[Timothy Nikkel (:tnikkel)]

Comment on attachment 9081071 [details]
Bug 1565542. We need to check to resolve image decode promises when we get the frame update notification too. r?aosmond

Beta/Release Uplift Approval Request

• User impact if declined: img.decode promises sometimes don't get resolved. affects google maps loaded in iframes
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We just add another trigger to check if we meet the conditions to resolve the promise.
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: causes google maps in an iframe to not work sometimes
• User impact if declined: google maps in an iframe does not work sometimes
• Fix Landed on Version: 70
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We just add another trigger to check if we meet the conditions to resolve the promise.
• String or UUID changes made by this patch: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9081071 [details]
Bug 1565542. We need to check to resolve image decode promises when we get the frame update notification too. r?aosmond

Fixes a known regression in Fx68 causing problems with Google Maps in some cases. Let's get this into tomorrow's 69.0b10 build so it can get some wider testing before next week's dot releases.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/dca65e12800d

Comment 15

[Cristian Fogel [:cfogel]]

Confirmed the loading/decode issue with 68.0.1 /69.0b9 after several page reloads.
Fix verified with 70.0a1 (2019-07-31), 69.0b10(taskCluster build) on Windows 10, macOS 10.14, Ubuntu 16.04LTS.

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 9081071 [details]
Bug 1565542. We need to check to resolve image decode promises when we get the frame update notification too. r?aosmond

approved for 68.0.2

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/e6651671ca5c
FIREFOX_ESR_68_0_X_RELBRANCH https://hg.mozilla.org/releases/mozilla-esr68/rev/8b03048a3ef2778b232ef921104eed5c3ea8e408

Comment 19

[Cristian Fogel [:cfogel]]

Verified with the task_cluster builds 68.0.2 and the 68.0esr on the avialable OSs.