1797055 - Crash early if we detect the Surface has been abandoned

Status: RESOLVED FIXED

(GeckoView :: General, task)

Description

[Jamie Nicol [:jnicol]]

In bug 1772839 we were seeing a large number of crashes due to the compositor being resumed with an android Surface that was already in an abandoned state. This meant we were unable to create an EGL surface, meaning webrender raised an error causing us to disable the GPU process / fallback to SWGL and try again. Attempting to resume in each new configuration would still hit this error as it is because the Surface is fundamentally invalid, and eventually we would hard crash because we still could not resume after exhausting all fallback configurations.

We added a check for this condition and a workaround very early in java code. However, it was not possible to perform this check in all circumstances. We are still seeing crash reports in FallbackFromAcceleration, and recently a report from a user in bug 1796947. In order so that we can determine whether these are also due to the Surface being abandoned, or due to some other reason, we should add a check for the Surface being abandoned much earlier in the pipeline and hard crash if so.

Comment 1

[Jamie Nicol [:jnicol]]

Attachment: Bug 1797055 - Force crash early if compositor is resumed with abandoned Surface. r?#geckoview-reviewers

In bug 1772839 we were seeing a large number of crashes due to
encountering a webrender error after exhausting all fallback
configurations. At least in some cases, this was due to the compositor
being resumed with an Android Surface that was already in an abandoned
state, meaning we can never succeed in creating an EGL Surface.

We added a check for this condition, and a workaround, to the
GeckoView java code. However, we are still seeing crash reports
matching this signature. To help determine whether these are also due
to the Surface being abandoned, or due to some other reason, this
patch adds a deliberate crash much earlier in the pipeline if we
detect an abandoned Surface.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c22bb48805b
Force crash early if compositor is resumed with abandoned Surface. r=geckoview-reviewers,m_kato

Comment 3

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7c22bb48805b

Comment 4

[Chris Peterson [:cpeterson]]

Too late to uplift to Beta 107

Comment 5

[Jamie Nicol [:jnicol]]

Comment on attachment 9299855 [details]
Bug 1797055 - Force crash early if compositor is resumed with abandoned Surface. r?#geckoview-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Needed to uplift bug 1798714
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just makes us crash earlier in a situation we will definitely crash in anyway
• String changes made/needed:
• Is Android affected?: Yes

Comment 6

[Donal Meehan [:dmeehan]]

Comment on attachment 9299855 [details]
Bug 1797055 - Force crash early if compositor is resumed with abandoned Surface. r?#geckoview-reviewers

Approved for Desktop 107.0b9 and Fenix/Focus 107.0b6

Comment 7

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1eb5442484c4