1750569 - requestScreenPixels() never completes if GPU process is restarted whilst screenshot request is pending

Status: RESOLVED FIXED

(GeckoView :: Sandboxing, defect)

Description

[Jamie Nicol [:jnicol]]

In RequestScreenPixels() we add a CaptureRequest to mCapturePixelsResults, then call UiCompositorControllerChild->RequestScreenPixels(). When we receive the pixels from the compositor in RecvScreenPixels() we pop the result from mCapturePixelsResults and complete it. If the GPU process crashes whilst this result is pending, RecvScreenPixels() will never be called.

Fixing this will require a slight change to how LayerViewSupport accesses the UiCompositorControllerChild. Currently it just fetches it from the nsWindow's mCompositorSession whenever it needs it. This was fine when the compositor session was initialized once and always remained initialized. But we have since added the ability for webrender to "fallback" from hardware accelerated to software webrender, which means the CompositorSession and UiCompositorControllerChild being destroyed and recreated. With the GPU process this will additionally occur whenever the GPU process crashes. We therefore cannot simply call mCompositorSession->GetUiCompositorControllerChild() from the UI thread.

Instead, the main thread should schedule a task on the UI thread whenever the compositor is destroyed or created. When the compositor is created we will pass a reference to the new UiCompositorControllerChild, which LayerViewSupport can hold on to until the compositor is destroyed. It doesn't matter if the UI thread attempts to use a stale UiCompositorControllerChild, as the object remains alive through the reference and IPC messages will simply be dropped.

With that in place, we can fix this bug by simply checking whether mCapturePixelsResults is non-empty whenever the compositor is initialized. If so, we call mUiCompositorControllerChild->RequestScreenPixels().

Comment 1

[Jamie Nicol [:jnicol]]

Attachment: Bug 1750569 - Ensure screen pixels requests are re-sent following GPU process restart. r?agi

If the GPU process crashes while a screen pixels request is
outstanding, then currently that request will never be completed. This
patch makes it so that we check for outstanding requests when the
compositor is (re)initialized, and send those requests to the new
compositor if they exist.

This includes a refactoring of how the LayerViewSupport class accesses
the UiCompositorControllerChild from the UI thread. Previously it
looked it up through the nsWindow's mCompositorSession whenever it was
required. However, since the compositor session can now be destroyed
and created we instead notify the UI thread whenever that occurs with
LayerViewSupport::NotifyCompositorSessionLost and
LayerViewSupport::NotifyCompositorCreated. The latter of these takes a
reference to the UiCompositorControllerChild, which can safely be used
by the LayerViewSupport on the UI thread.

Comment 2

[Jamie Nicol [:jnicol]]

Attachment: Bug 1750569 - Add test to ensure screen pixels requests succeed if the GPU process crashes. r?agi

Depends on D136151

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ed98a8289f2
Ensure screen pixels requests are re-sent following GPU process restart. r=agi
https://hg.mozilla.org/integration/autoland/rev/44b1a295c186
Add test to ensure screen pixels requests succeed if the GPU process crashes. r=agi

Comment 4

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/6ed98a8289f2
https://hg.mozilla.org/mozilla-central/rev/44b1a295c186

Comment 5

[Chris Peterson [:cpeterson]]

Moving GPU process bugs to the new GeckoView::Sandboxing component.