1436466 - Switching apps blanks canvases

Status: VERIFIED FIXED

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

Description

[Sander Verdonschot]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

Go to https://mangara.github.io/Color-Zebra/
Switch to a different app
Switch back to Firefox


Actual results:

All canvases are blank.


Expected results:

Canvases should retain their before-switch contents (this happens with Chrome).

Comment 1

[Sander Verdonschot]

Tested on Nexus 4 running Android 5.1.1

Comment 2

[Jan Henning [:JanH]]

This is a regression from bug 1379920.

When I try to bring Firefox back into the foreground on a debug build, I also get the following assertion:
> 02-06 21:08:30.568 7799-7863/org.mozilla.fennec_jan A/MOZ_Assert: Assertion failure: status != 0 (ClientWaitSync generated an error. Has mSync already been destroyed?), at /home/jan/Mozilla/mozilla-central/gfx/layers/opengl/TextureHostOGL.cpp:687

And the thing that triggers this seems to be the heap minimisation we do when going into the background on Android. If I comment out this line (https://dxr.mozilla.org/mozilla-central/rev/0d806b3230fe4767fa70cdee57db87d1e9a5ba49/widget/android/nsAppShell.cpp#184), I can no longer reproduce this on a local build.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]:  This is quite bad. Recent regression since v57. Might be worth a ride along in a dot release of 58 if we have another one.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Milan this looks similar to bug 1435546 and is a recent regression in 58.

Comment 5

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

Sotaro, you did some reviews for bug 1379920, can you take a look at this as a priority; grab anybody else on the team if you need them.  Also note related bug 1435546.

Comment 6

[Sotaro Ikeda [:sotaro]]

I confirmed the problem on my android.

Comment 7

[Sotaro Ikeda [:sotaro]]

It seems that bug 1379920 changed the way of dirty flag handling. And the dirty flag was not set when CopyableCanvasRenderer::ClearCachedResources() is called. Then ShareableCanvasRenderer::UpdateCompositableClient() did not update canvas.

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> It seems that bug 1379920 changed the way of dirty flag handling. And the
> dirty flag was not set when CopyableCanvasRenderer::ClearCachedResources()
> is called. Then ShareableCanvasRenderer::UpdateCompositableClient() did not
> update canvas.

https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ShareableCanvasRenderer.cpp#221

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Set dirty in CopyableCanvasRenderer::ClearCachedResources()

The patch addressed the problem for me.

Comment 10

[Sotaro Ikeda [:sotaro]]

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

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8949648 [details] [diff] [review]
patch - Set dirty in CopyableCanvasRenderer::ClearCachedResources()

Review of attachment 8949648 [details] [diff] [review]:
-----------------------------------------------------------------

Please include something like "It seems that bug 1379920 changed the way of dirty flag handling. And the dirty flag was not set when CopyableCanvasRenderer::ClearCachedResources() is called. Then ShareableCanvasRenderer::UpdateCompositableClient() did not update canvas." in the commit message.

Comment 12

[Sotaro Ikeda [:sotaro]]

Thanks. I will include it in a commit message.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00e6bf10a75b
Set dirty in CopyableCanvasRenderer::ClearCachedResources() r=jrmuizel

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/00e6bf10a75b

Comment 15

[Julien Cristau [:jcristau]]

Please request beta uplift when you get a chance.

Comment 16

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8949648 [details] [diff] [review]
patch - Set dirty in CopyableCanvasRenderer::ClearCachedResources()

Approval Request Comment
[Feature/Bug causing the regression]:Bug 1379920
[User impact if declined]:Canvas is not rendered correctly when switch to a different app and then switch back to Firefox.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes. The step is in Comment 0.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: The patch just add invalidation when a document of canvas go to background. 
[String changes made/needed]: None

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Sander, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8949648 [details] [diff] [review]
patch - Set dirty in CopyableCanvasRenderer::ClearCachedResources()

Low risk fix, Beta59+

Comment 20

[Ori Avtalion (salty-horse)]

I also experienced the bug and can confirm it's fixed.

Comment 21

[Sander Verdonschot]

Yep, it's working as expected in Nightly. Thanks for the quick response!

Comment 22

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

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

Comment 23

[roxleitan]

Managed to reproduce the issue using the latest Nightly 60.0a1 (Build ID:20180219100221) on Windows 10 x64 following the STR:
1.Open Nightly with a new profile
2.Go to https://mangara.github.io/Color-Zebra/

Actual results:
All canvases are blank.(please see the attached screenshot)

Note: Sometimes the page is correctly displayed after refresh 

Should I open a new bug for this?

Comment 24

[roxleitan]

Attachment: canvas2d.png

Comment 25

[Sotaro Ikeda [:sotaro]]

(In reply to [e-mail] from comment #23)
> Managed to reproduce the issue using the latest Nightly 60.0a1 (Build
> ID:20180219100221) on Windows 10 x64 following the STR:
> 1.Open Nightly with a new profile
> 2.Go to https://mangara.github.io/Color-Zebra/
> 
> Actual results:
> All canvases are blank.(please see the attached screenshot)
> 
> Note: Sometimes the page is correctly displayed after refresh 
> 
> Should I open a new bug for this?

It seems like a different bug. Can you open a new bug? Thanks!

Comment 26

[roxleitan]

I filed bug 1439516.

Comment 27

[Sotaro Ikeda [:sotaro]]

Thank you!

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Pretty unlikely we'll be shipping another 58 dot release at this point given where we are in the cycle.

Comment 30

[roxleitan]

I cannot reproduce the issue on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12 using Nightly 60.0a1 (2018-02-07), the latest Nightly 61.0a1(2018-03-12), Beta 60.0b3 and Firefox Release 59 with the STR from the description.

Please note that bug 1439516 is still reproducible (intermittently).

Based on my results, comment 20 and comment 21 I will mark this bug verified fixed.