Closed Bug 1436466 Opened 2 years ago Closed 2 years ago

Switching apps blanks canvases

Categories

(Core :: Canvas: 2D, defect, P1)

57 Branch
All
Android
defect

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
fennec + ---
firefox-esr52 --- unaffected
firefox58 + wontfix
firefox59 + verified
firefox60 + verified

People

(Reporter: sander.verdonschot+1github, Assigned: sotaro)

References

Details

(Keywords: regression)

Attachments

(2 files)

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).
Tested on Nexus 4 running Android 5.1.1
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.
Blocks: 1379920
Status: UNCONFIRMED → NEW
Component: General → Canvas: 2D
Ever confirmed: true
Keywords: regression
OS: Unspecified → Android
Product: Firefox for Android → Core
Hardware: Unspecified → All
See Also: → 1435546
Version: 58 Branch → 57 Branch
[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.
tracking-fennec: --- → ?
Milan this looks similar to bug 1435546 and is a recent regression in 58.
tracking-fennec: ? → +
Flags: needinfo?(milan)
Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(milan)
Priority: -- → P1
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.
Flags: needinfo?(sotaro.ikeda.g)
I confirmed the problem on my android.
Flags: needinfo?(sotaro.ikeda.g)
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 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
The patch addressed the problem for me.
Attachment #8949648 - Flags: review?(jmuizelaar)
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.
Attachment #8949648 - Flags: review?(jmuizelaar) → review+
Thanks. I will include it in a commit message.
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00e6bf10a75b
Set dirty in CopyableCanvasRenderer::ClearCachedResources() r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/00e6bf10a75b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Please request beta uplift when you get a chance.
Flags: needinfo?(sotaro.ikeda.g)
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
Flags: needinfo?(sotaro.ikeda.g)
Attachment #8949648 - Flags: approval-mozilla-beta?
Duplicate of this bug: 1435546
Hi Sander, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(sander.verdonschot+1github)
Comment on attachment 8949648 [details] [diff] [review]
patch - Set dirty in CopyableCanvasRenderer::ClearCachedResources()

Low risk fix, Beta59+
Attachment #8949648 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I also experienced the bug and can confirm it's fixed.
Yep, it's working as expected in Nightly. Thanks for the quick response!
Flags: needinfo?(sander.verdonschot+1github)
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?
Flags: needinfo?(sotaro.ikeda.g)
Attached image canvas2d.png
(In reply to roxana.leitan@softvision.ro 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!
Flags: needinfo?(sotaro.ikeda.g)
I filed bug 1439516.
Thank you!
See Also: → 1439516
Pretty unlikely we'll be shipping another 58 dot release at this point given where we are in the cycle.
Duplicate of this bug: 1418552
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.