1710687 - Crash in [@ nsDocShell::SetVisibility]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

These crashes are interesting because 50-80 reports per release until 87, then 1 for 88.0 releases and the 88 betas, and for the 89 betas 4-5 reports per beta seem to be the median.

99% of crashes on Windows 10, >80% on x86-64, 60% in first minute after launch.

Crash report: https://crash-stats.mozilla.org/report/index/4866a848-dec2-43d5-914d-4cc6d0210511

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll nsDocShell::SetVisibility docshell/base/nsDocShell.cpp:5048
1 xul.dll mozilla::AppWindow::SetVisibility xpfe/appshell/AppWindow.cpp:965
2 xul.dll mozilla::AppWindow::OnStateChange xpfe/appshell/AppWindow.cpp:3197
3 xul.dll nsDocLoader::DoFireOnStateChange uriloader/base/nsDocLoader.cpp:1332
4 xul.dll nsDocLoader::doStopDocumentLoad uriloader/base/nsDocLoader.cpp:949
5 xul.dll nsDocLoader::DocLoaderIsEmpty uriloader/base/nsDocLoader.cpp:757
6 xul.dll nsDocLoader::OnStopRequest uriloader/base/nsDocLoader.cpp:640
7 xul.dll mozilla::net::nsLoadGroup::NotifyRemovalObservers netwerk/base/nsLoadGroup.cpp:616
8 xul.dll mozilla::net::nsLoadGroup::Cancel netwerk/base/nsLoadGroup.cpp:250
9 xul.dll nsDocLoader::Stop uriloader/base/nsDocLoader.cpp:253

Comment 1

[Chris Peterson [:cpeterson]]

Assigning to Olli because he has a patch. Since this is a long-standing crash and the patch will be simple, we should consider uplifting this fix to 89 Beta.

We seem to be trying to close the first browser window very early and then crashing. Should we ignore visibility changes when the window is destroyed (perhaps checking whether mDocShell is null)?

mDocShell is null. The first XUL browser window tried to load a JSM but then called Stop here?

https://searchfox.org/mozilla-central/rev/cecdac0aa5733fee515a166b6e31e38cc58abf32/browser/components/BrowserGlue.jsm#831-832

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1710687, null check mDocShell before using it, r=peterv

The patch is based on code inspection. SizeShell may run scripts
https://searchfox.org/mozilla-central/rev/cecdac0aa5733fee515a166b6e31e38cc58abf32/xpfe/appshell/AppWindow.cpp#1180,1182

The patch is very conservative. It keeps returning success code in the cases where it would return such without the patch and only
adds null check right before accessing mDocShell. mDebuting value stays false after the method call.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b586b87e1135
null check mDocShell before using it, r=peterv

Comment 4

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/b586b87e1135