1661835 - Crash in [@ nsFrameLoader::MaybeCreateDocShell]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[u608768]

Crash report: https://crash-stats.mozilla.org/report/index/290e4594-dfc3-43b4-9016-bc0a30200827

Top 10 frames of crashing thread:

0 xul.dll nsFrameLoader::MaybeCreateDocShell dom/base/nsFrameLoader.cpp:2209
1 xul.dll nsFrameLoader::ReallyStartLoadingInternal dom/base/nsFrameLoader.cpp:657
2 xul.dll nsFrameLoader::ReallyStartLoading dom/base/nsFrameLoader.cpp:608
3 xul.dll mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders dom/base/Document.cpp:8571
4 xul.dll mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1240
5 xul.dll static nsContentUtils::RemoveScriptBlocker dom/base/nsContentUtils.cpp:5347
6 xul.dll mozilla::dom::Document::EndUpdate dom/base/Document.cpp:7138
7 xul.dll nsHtml5DocumentBuilder::UpdateStyleSheet parser/html/nsHtml5DocumentBuilder.cpp:60
8 xul.dll nsHtml5TreeOperation::Perform parser/html/nsHtml5TreeOperation.cpp:1176
9 xul.dll nsHtml5TreeOpExecutor::RunFlushLoop parser/html/nsHtml5TreeOpExecutor.cpp:494

Seeing a few of these since build 20200826032437 (push log: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a8f04609cdd89e6dd4f6e8c3f24daca3b6dc33be&tochange=e7988afc46d5611771b5e269738e07b9be970046). Maybe bug 1636728?

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia)]

This looks like mOwnerContent misses a nullptr check. Reminds of bug 1660537.

Comment 2

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

But the crash is older. 79 at least seems to have it.
(68 has different crash)

Comment 3

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

nsDocShell::Initialize can run scripts.

Comment 4

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

Attachment: Bug 1661835 - DocShell::Initialize may change the state of nsFrameLoader, r=kmag

This is a guess fix based on possible reason for the crash.
Use of AutoScriptBlocker would be one option too, but better to return as early as possible.

Comment 5

[Mirko Brodesser (:mbrodesser-Igalia)]

But the crash is older. 79 at least seems to have it.
(68 has different crash)

According to the crash stats aggregated by version it seems to be only 80.0 and newer.

(In reply to Olli Pettay [:smaug] from comment #3)

nsDocShell::Initialize can run scripts.

Are you sure? It doesn't seem so. The method isn't declared as such and it's definition also seems to call no functions declared with MOZ_CAN_RUN_SCRIPT_BOUNDARY.

Comment 6

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

Yes it can.
https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/docshell/base/nsDocShell.cpp#492
may run scripts if there are no script blockers on stack.

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia)]

Then it would be helpful to have it annotated with MOZ_CAN_RUN_SCRIPT.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/863a7717219c
DocShell::Initialize may change the state of nsFrameLoader, r=kmag

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/863a7717219c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

No crashes on Nightly since this landed. Please nominate this for Beta approval when you get a chance.

Comment 11

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

Comment on attachment 9173038 [details]
Bug 1661835 - DocShell::Initialize may change the state of nsFrameLoader, r=kmag

Beta/Release Uplift Approval Request

• User impact if declined: Crashes
• 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): It is a null check
• String changes made/needed:

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173038 [details]
Bug 1661835 - DocShell::Initialize may change the state of nsFrameLoader, r=kmag

Approved for 81.0b8.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5e8e24da4af8