1566915 - Composition recording with WebRender on macOS crashes due to calling GL methods while not having a GL context

Status: RESOLVED FIXED

(Core :: Performance: General, defect, P2)

Description

[Denis Palmeiro [:denispal]]

With webrender enabled, it doesn't seem like the composition recorder actually starts when invoking windowUtils.setCompositionRecording(1). Everything works as expected with webrender off on Mac.

Comment 1

[Timothy Nikkel (:tnikkel)]

On my machine, using the ctrl shift 4 shortcut (in a nightly or local build) the composition recording starts but when I stop it I crash https://crash-stats.mozilla.org/report/index/9e627659-f80e-49e0-91f8-0276e0190726

Comment 2

[Denis Palmeiro [:denispal]]

(In reply to Timothy Nikkel (:tnikkel) from comment #1)

On my machine, using the ctrl shift 4 shortcut (in a nightly or local build) the composition recording starts but when I stop it I crash https://crash-stats.mozilla.org/report/index/9e627659-f80e-49e0-91f8-0276e0190726

I think that's because it might not actually be starting properly and the crash could be because of bug 1569258.

Comment 3

[Beth Rennie [:beth] (she/her)]

(In reply to Denis Palmeiro [:denispal] from comment #2)

(In reply to Timothy Nikkel (:tnikkel) from comment #1)

On my machine, using the ctrl shift 4 shortcut (in a nightly or local build) the composition recording starts but when I stop it I crash https://crash-stats.mozilla.org/report/index/9e627659-f80e-49e0-91f8-0276e0190726

I think that's because it might not actually be starting properly and the crash could be because of bug 1569258.

That is not the case. I have applied the patch from bug 1569258 and the crash still happens.

The linked crash shows that it occurred in glBindBuffer() called from Device::map_pbo_for_readback

Attempting to bind any buffer to any value (even 0 to clear the binding) at this location seems to cause the same crash.

Comment 4

[Beth Rennie [:beth] (she/her)]

This is caused by not having a current GL context when we are writing the frames to disk. Writing the frames to disk from the render thread solves the issue and also enables some cleanup of the CompositionRecorder structures.

Comment 5

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1566915 - Write collected frames from the composition recorder on the render thread r?kvark

On macOS, if we try to write the collected frames from the
CompositorBridgeParent we will not have an active GL context, resulting in a
crash. Writing the frames from the RenderThread solves this problem.

Depends on D39584

Comment 6

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1566915 - Do not keep a handle to the composition recorder in the CompositorBridgeParent r?mstange

Since we are not writing frames to disk from the CompositorBridgeParent in
the WebRender case, we do not actually need a handle to the
(WebRender)CompositionRecorder there. Instead, the HostLayerManager and
RenderThread can maintain exclusive handles to these objects. This will allow
us to use unique pointers for these objects and delete code in a follow up
patch.

Depends on D39789

Comment 7

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1566915 - Do not ref count the CompositionRecorder r?mstange

Now that there is only ever a single handle to the CompositionRecorder, it no
longer needs to be ref-counted. And since the WebRenderCompositionRecorder is
owned exclusively by the RenderThread, it no longer needs a mutex. All the
code that resulted from having handles to the WebRenderCompositionRecorder on
two different threads is now no longer necessary.

Depends on D39790

Comment 8

[Timothy Nikkel (:tnikkel)]

With these patches I get a recording now! Hooray. The pngs display upside down though.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd6e22d00783
Write collected frames from the composition recorder on the render thread r=kvark
https://hg.mozilla.org/integration/autoland/rev/95669c21b82b
Do not keep a handle to the composition recorder in the CompositorBridgeParent r=mstange
https://hg.mozilla.org/integration/autoland/rev/ffa36846d4ac
Do not ref count the CompositionRecorder r=mstange

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/fd6e22d00783
https://hg.mozilla.org/mozilla-central/rev/95669c21b82b
https://hg.mozilla.org/mozilla-central/rev/ffa36846d4ac