1986191 - temporarily work around crash of bug 1764863

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1986191. Work around a null mScrollContainerFrame in an active scrolled root. r?#gfx-reviewers,#layout-reviewers

We should not have a null mScrollContainerFrame in an asr that is in use, but we are getting crashes in bug 1764863 that show that this is happening at volume. We have not been successful over a long period of time of finding a testcase or some way to reproduce. We have a landed patch in bug 1984898 (based on a pernosco of one instance of this from a few years back) that we hope fixes some or most of these. The patch in bug 1984898 is a little bit big, and we have no evidence that it fixes the problem. So I am proposing this patch as something we can uplift that should work around the problem. We still want to fix any instances of that this we come across so the MOZ_DIAGNOSTIC_ASSERT will fire in nightly and early beta builds. The gfxCriticalNoteOnce will alert us if this is having negative consequences later on.

By returning false we will abort the partial display list update and delete the retained display list that contains an asr that has a null mScrollContainerFrame and we will do a full rebuild. This should avoid the problem and be much better than crashing.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d678d1575184
https://hg.mozilla.org/integration/autoland/rev/23cb5f9ae881
Work around a null mScrollContainerFrame in an active scrolled root. r=layout-reviewers,emilio

Comment 3

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/23cb5f9ae881

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1986191. Work around a null mScrollContainerFrame in an active scrolled root. r?#gfx-reviewers,#layout-reviewers

We should not have a null mScrollContainerFrame in an asr that is in use, but we are getting crashes in bug 1764863 that show that this is happening at volume. We have not been successful over a long period of time of finding a testcase or some way to reproduce. We have a landed patch in bug 1984898 (based on a pernosco of one instance of this from a few years back) that we hope fixes some or most of these. The patch in bug 1984898 is a little bit big, and we have no evidence that it fixes the problem. So I am proposing this patch as something we can uplift that should work around the problem. We still want to fix any instances of that this we come across so the MOZ_DIAGNOSTIC_ASSERT will fire in nightly and early beta builds. The gfxCriticalNoteOnce will alert us if this is having negative consequences later on.

By returning false we will abort the partial display list update and delete the retained display list that contains an asr that has a null mScrollContainerFrame and we will do a full rebuild. This should avoid the problem and be much better than crashing.

Original Revision: https://phabricator.services.mozilla.com/D263224

Comment 5

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: crashes
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: we have no steps
• Risk associated with taking this patch: low
• Explanation of risk level: we detect the null-deref before and throw away the data structure and rebuild it
• String changes made/needed: none
• Is Android affected?: yes

Comment 6

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/c13fb36fd262
https://hg.mozilla.org/releases/mozilla-beta/rev/c55b0327e1a8

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

Should we take this on ESR140 also?

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1986191. Work around a null mScrollContainerFrame in an active scrolled root. r?#gfx-reviewers,#layout-reviewers

We should not have a null mScrollContainerFrame in an asr that is in use, but we are getting crashes in bug 1764863 that show that this is happening at volume. We have not been successful over a long period of time of finding a testcase or some way to reproduce. We have a landed patch in bug 1984898 (based on a pernosco of one instance of this from a few years back) that we hope fixes some or most of these. The patch in bug 1984898 is a little bit big, and we have no evidence that it fixes the problem. So I am proposing this patch as something we can uplift that should work around the problem. We still want to fix any instances of that this we come across so the MOZ_DIAGNOSTIC_ASSERT will fire in nightly and early beta builds. The gfxCriticalNoteOnce will alert us if this is having negative consequences later on.

By returning false we will abort the partial display list update and delete the retained display list that contains an asr that has a null mScrollContainerFrame and we will do a full rebuild. This should avoid the problem and be much better than crashing.

Original Revision: https://phabricator.services.mozilla.com/D263224

Comment 9

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: crashes
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: https://bugzilla.mozilla.org/show_bug.cgi?id=1764863#c21
• Risk associated with taking this patch: low
• Explanation of risk level: we detect the null-deref before and throw away the data structure and rebuild it
• String changes made/needed: none
• Is Android affected?: yes

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/57581b9b56f9

Comment 11

[Ciprian Georgiu, Desktop QA]

I was able to reproduce the crash using an affected Asan fuzzing build on Windows 11 with the test page from Bug 1764863, comment 21.

The issue is verified as fixed on the latest Asan fuzzing builds: Nightly 144.0a1, Beta 143.0b8, and ESR 140.3.0 on Windows 11.