Bug 1754334 - Return error when GPU process crash occurs during screen pixels request. r=agi

In bug 1750569 we attempted to ensure that following a GPU process
crash outstanding screen pixels requests would be fulfilled. While
this usually worked there was a race condition between sending the
request to the new compositor and the content process sending the
display list to the new compositor, which meant that sometimes we
would screenshot an empty screen instead of the page content.

As a GPU process crash is an extraordinary circumstance and
screenshots are non-critical, the best solution is to simply return an
error if a GPU process crash occurs while there is an outstanding
request (or if a new request is made whilst the GPU process is
restarting). This patch also updates the junit test to check for this
error rather than expecting a screenshot to be returned.

Differential Revision: https://phabricator.services.mozilla.com/D138323
This commit is contained in:
Jamie Nicol 2022-02-09 19:36:49 +00:00
parent 62398b35a2
commit 8fb7eecd6e
2 changed files with 36 additions and 22 deletions

View File

@ -230,26 +230,19 @@ class ScreenshotTest : BaseSessionTest() {
}
@WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
@Test
fun capturePixelsBeforeGpuProcessCrash() {
@Test(expected = IllegalStateException::class)
fun capturePixelsAfterGpuProcessCrash() {
// We need the GPU process for this test
assumeTrue(sessionRule.usingGpuProcess())
val screenshotFile = getComparisonScreenshot(SCREEN_WIDTH, SCREEN_HEIGHT)
mainSession.loadTestPath(COLORS_HTML_PATH)
sessionRule.waitUntilCalled(object : ContentDelegate {
@AssertCalled(count = 1)
override fun onFirstContentfulPaint(session: GeckoSession) {
}
})
sessionRule.display?.let {
// Request screen pixels then immediately kill the GPU process
val result = it.capturePixels()
// Kill the GPU process then immediately request screen pixels. Requesting the pixels
// *before* killing the process will often result in the same error, but sometimes the
// screenshot request will complete successfully before the crash.
sessionRule.killGpuProcess()
val result = it.capturePixels()
assertScreenshotResult(result, screenshotFile)
sessionRule.waitForResult(result)
}
}

View File

@ -1022,10 +1022,6 @@ class LayerViewSupport final
if (!mCompositorPaused) {
mUiCompositorControllerChild->Resume();
if (!mCapturePixelsResults.empty()) {
mUiCompositorControllerChild->RequestScreenPixels();
}
}
}
@ -1033,6 +1029,20 @@ class LayerViewSupport final
void NotifyCompositorSessionLost() {
MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
mUiCompositorControllerChild = nullptr;
if (auto window = mWindow.Access()) {
while (!mCapturePixelsResults.empty()) {
auto result =
java::GeckoResult::LocalRef(mCapturePixelsResults.front().mResult);
if (result) {
result->CompleteExceptionally(
java::sdk::IllegalStateException::New(
"Compositor session lost during screen pixels request")
.Cast<jni::Throwable>());
}
mCapturePixelsResults.pop();
}
}
}
java::sdk::Surface::Param GetSurface() { return mSurface; }
@ -1338,11 +1348,24 @@ class LayerViewSupport final
int32_t aSrcHeight, int32_t aOutWidth,
int32_t aOutHeight) {
MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
auto result = java::GeckoResult::LocalRef(aResult);
if (!mUiCompositorControllerChild) {
if (result) {
if (auto window = mWindow.Access()) {
result->CompleteExceptionally(
java::sdk::IllegalStateException::New(
"Compositor session lost prior to screen pixels request")
.Cast<jni::Throwable>());
}
}
return;
}
int size = 0;
if (auto window = mWindow.Access()) {
mCapturePixelsResults.push(CaptureRequest(
java::GeckoResult::GlobalRef(java::GeckoResult::LocalRef(aResult)),
java::GeckoResult::GlobalRef(result),
java::sdk::Bitmap::GlobalRef(java::sdk::Bitmap::LocalRef(aTarget)),
ScreenRect(aXOffset, aYOffset, aSrcWidth, aSrcHeight),
IntSize(aOutWidth, aOutHeight)));
@ -1350,9 +1373,7 @@ class LayerViewSupport final
}
if (size == 1) {
if (mUiCompositorControllerChild) {
mUiCompositorControllerChild->RequestScreenPixels();
}
mUiCompositorControllerChild->RequestScreenPixels();
}
}