Closed Bug 1454974 Opened 6 years ago Closed 6 years ago

Does RecvGetSnapshot for content processes actually work?

Categories

(Core :: Graphics: WebRender, enhancement)

Other Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

While reading the code, I noticed that mForceRendering gets set to true at [1] but the WebRenderBridgeParent that holds the mForceRendering might not be the "root" WRBP (i.e. it might be the one for a content process). The ForcePendingComposite call just below it will trigger a CompositeToTarget call on the root WRBP though, because that's the one that's registered as the mVsyncSchedulerOwner at [2]. So when we read mForceRendering at [3] it might be false in the root WRBP even though we want it to be true.

Am I missing something here? 

[1] https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/gfx/layers/wr/WebRenderBridgeParent.cpp#831
[2] https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/gfx/layers/ipc/CompositorVsyncScheduler.cpp#279
[3] https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/gfx/layers/wr/WebRenderBridgeParent.cpp#1213
Flags: needinfo?(sotaro.ikeda.g)
At least in our automation test suite, it looks like we never get a RecvGetSnapshot call [1] for a non-root WRBP, so that's probably why we haven't run into this before. But I don't know if there's scenarios where this might happen, like with canvas drawWindow calls in web content.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=f605032c7123090a6ab479ed36d3c00e17f3d630
I think the Firefox screenshots functionality calls drawWindow in content (but with chrome privileges!), but the flags it uses are such that we use main thread painting instead of compositor readback.
Hm, ok. I guess I can land the assert so that if we do ever end up taking this codepath we'll assert rather than have a subtle race condition.
Flags: needinfo?(sotaro.ikeda.g)
Assignee: nobody → bugmail
Comment on attachment 8969061 [details]
Bug 1454974 - Assert if RecvGetSnapshot ever gets called in a non-root WRBP.

https://reviewboard.mozilla.org/r/237748/#review243558
Attachment #8969061 - Flags: review?(sotaro.ikeda.g) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/86448c712128
Assert if RecvGetSnapshot ever gets called in a non-root WRBP. r=sotaro
https://hg.mozilla.org/mozilla-central/rev/86448c712128
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.