1692736 - animated gif is stuck if canvas is also rendered

Status: VERIFIED FIXED

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

Description

[Kaiido]

Attachment: test-case.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

https://jazzy-placid-sunspot.glitch.me/

Display an animated gif on the page and draw on a canvas every frame at the same time.

Actual results:

The gif animation stutters, it only animates sometimes when the full page is rerendered (e.g after a scroll or a window resize).

Expected results:

The gif should still animate smoothly.

Running mozregression points to this pushlog, hence asking to [e-mail] if they can have a look, since it seems bug1691065 is gonna land soon.

Comment 1

[Timothy Nikkel (:tnikkel)]

Confirmed the regression range in the first comment.

Comment 2

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

Set release status flags based on info from the regressing bug 1691065

Comment 3

[Robert Longson [:longsonr]]

Attachment: Bug 1692736 - pass namespace id when writing/reading TransactionData

Comment 4

[Timothy Nikkel (:tnikkel)]

Looks like this could have been caught if he had bug 1602142 ?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8c9e66702b5
pass namespace id when writing/reading TransactionData r=aosmond

Comment 6

[Robert Longson [:longsonr]]

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

Beta/Release Uplift Approval Request

• User impact if declined: animated gifs fail to animate
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: The testcase in this bug should animate . Hopefully this also fixes bug 1692916 which has its own 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): Just passes a value to the child process, The bigger change was the bug that caused this issue i.e. bug 1691065 and that's already on beta so your choice would seem to be take this or backout that one. This is a pretty small change and backing out the bigger fix would re-introduce the issues that it fixes so on the whole I'd say taking this is the least risk option.
• String changes made/needed: none

Comment 7

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

Was about to request beta uplift, thanks very much for that and the patch fixing my mistake :).

Comment 9

[Andreea Pavel [:apavel]]

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

Comment 10

[Peter Magyari (Desktop QA)]

Reproduced the issue on Nightly 87.0a1 (20210216094005)
Verified the fix on Nightly 87.0a1 (20210216215129)

Comment 11

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

This would break animated images mostly, some font cases. I think this is pretty important for an RC uplift, so setting to S2. The workaround is disable WebRender for a user which is pretty drastic.

Comment 12

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

(In reply to Robert Longson [:longsonr] from comment #6)

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

Beta/Release Uplift Approval Request

• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just passes a value to the child process, The bigger change was the bug that caused this issue i.e. bug 1691065 and that's already on beta so your choice would seem to be take this or backout that one. This is a pretty small change and backing out the bigger fix would re-introduce the issues that it fixes so on the whole I'd say taking this is the least risk option.

I would concur we must either back out bug 1691065 (which helped a lot with the crash rate otherwise, notwithstanding this oversight) or take the patch in this bug. Keeping bug 1691065 and not taking bug 1692736 is not desirable for the release.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9203355 [details]
Bug 1692736 - pass namespace id when writing/reading TransactionData

I agree that this has the potential to annoy a lot of users and the fix is small and was verified by QA, I consider this as a driver for an RC2, thanks for the feedback!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/6bda263953c0

Comment 15

[Peter Magyari (Desktop QA)]

Verified the fix on Firefox 86.0 (20210217195321)