1871625 - Assertion failure: mInitialized, at /builds/worker/checkouts/gecko/dom/base/nsFrameLoader.cpp:3495

Status: VERIFIED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20231202-e901b86e5d89 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid> --repeat 10 --no-harness

The attached test case is not very reliable and is very timing dependant.

Assertion failure: mInitialized, at /builds/worker/checkouts/gecko/dom/base/nsFrameLoader.cpp:3495

#0 0x7fc15b621d5a in nsFrameLoader::GetBrowsingContext() /builds/worker/checkouts/gecko/dom/base/nsFrameLoader.cpp:3495:3
#1 0x7fc15b62618b in nsFrameLoader::GetLoadContext() /builds/worker/checkouts/gecko/dom/base/nsFrameLoader.cpp:3484:20
#2 0x7fc15c70ee52 in mozilla::dom::FrameLoader_Binding::get_loadContext(JSContext*, JS::Handle<JSObject*>, void*, JSJitGetterCallArgs) /builds/worker/workspace/obj-build/dom/bindings/./FrameLoaderBinding.cpp:381:67
#3 0x7fc15c87b0f1 in bool mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3140:13
#4 0x7fc160d6e154 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:479:13
#5 0x7fc160d6daab in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:573:12
#6 0x7fc160d6ed7d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#7 0x7fc160d6ff23 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:794:10
#8 0x7fc160fd2dfe in CallGetter /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2069:12
#9 0x7fc160fd2dfe in bool GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, js::PropertyInfoBase<unsigned int>, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2097:12
#10 0x7fc160fd3720 in bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2245:14
#11 0x7fc160d058c2 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/ObjectOperations-inl.h:124:10
#12 0x7fc160d92eb1 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:4452:10
#13 0x7fc160d7a818 in GetPropertyOperation /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:245:10
#14 0x7fc160d7a818 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:2715:12
#15 0x7fc160d6d032 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:451:13
#16 0x7fc160d6dac8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:605:13
#17 0x7fc160d6ed7d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#18 0x7fc160d6ff23 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:794:10
#19 0x7fc160fd2dfe in CallGetter /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2069:12
#20 0x7fc160fd2dfe in bool GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, js::PropertyInfoBase<unsigned int>, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2097:12
#21 0x7fc160fd3720 in bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) /builds/worker/checkouts/gecko/js/src/vm/NativeObject.cpp:2245:14
#22 0x7fc160d058c2 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/ObjectOperations-inl.h:124:10
#23 0x7fc160d92eb1 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:4452:10
#24 0x7fc160d7a818 in GetPropertyOperation /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:245:10
#25 0x7fc160d7a818 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:2715:12
#26 0x7fc160d6d032 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:451:13
#27 0x7fc160d6dac8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:605:13
#28 0x7fc160d6ed7d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#29 0x7fc160e609b4 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CallAndConstruct.cpp:119:10
#30 0x7fc15c458b9c in mozilla::dom::LifecycleConnectedCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./CustomElementRegistryBinding.cpp:1002:8
#31 0x7fc15b2dff16 in void mozilla::dom::LifecycleConnectedCallback::Call<RefPtr<mozilla::dom::Element>>(RefPtr<mozilla::dom::Element> const&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/CustomElementRegistryBinding.h:953:12
#32 0x7fc15b299225 in Call<RefPtr<mozilla::dom::Element> > /builds/worker/workspace/obj-build/dist/include/mozilla/dom/CustomElementRegistryBinding.h:975:12
#33 0x7fc15b299225 in mozilla::dom::CustomElementCallback::Call() /builds/worker/checkouts/gecko/dom/base/CustomElementRegistry.cpp:219:13
#34 0x7fc15b2a4bb5 in mozilla::dom::CustomElementReactionsStack::InvokeReactions(AutoTArray<RefPtr<mozilla::dom::Element>, 3ul>*, nsIGlobalObject*) /builds/worker/checkouts/gecko/dom/base/CustomElementRegistry.cpp:1586:19
#35 0x7fc15b2a4744 in mozilla::dom::CustomElementReactionsStack::PopAndInvokeElementQueue() /builds/worker/checkouts/gecko/dom/base/CustomElementRegistry.cpp:1478:5
#36 0x7fc15a8b7757 in mozilla::dom::CustomElementReactionsStack::LeaveCEReactions(JSContext*, bool) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/CustomElementRegistry.h:294:7
#37 0x7fc15baaf344 in ~AutoCEReaction /builds/worker/workspace/obj-build/dist/include/mozilla/dom/CustomElementRegistry.h:586:22
#38 0x7fc15baaf344 in ~MaybeStorage /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:269:25
#39 0x7fc15baaf344 in mozilla::dom::Node_Binding::appendChild(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/./NodeBinding.cpp:969:1
#40 0x7fc15c87fede in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3258:13
#41 0x7fc160d6e154 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:479:13
#42 0x7fc160d6daab in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:573:12
#43 0x7fc160d7d3a8 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:645:10
#44 0x7fc160d7d3a8 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3060:16
#45 0x7fc160d6d032 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:451:13
#46 0x7fc160d6dac8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:605:13
#47 0x7fc160d6ed7d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#48 0x7fc1610d4497 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/SelfHosting.cpp:1520:10
#49 0x7fc161a06e12 in js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Value*, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/jit/VMFunctions.cpp:1075:10
#50 0x212e4b54a049  (<unknown module>)

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/qJ98rvYt417AFdbutLCjBQ/index.html

Comment 2

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20231222213932-8989af6649bf.
The bug appears to have been introduced in the following build range:

Start: 6e473b5825d2acd52739d9cc4cdec6ea5e26c9d1 (20231105123102)
End: e4d362eebc148baf8432d0b013b6233995a2f16e (20231105171511)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6e473b5825d2acd52739d9cc4cdec6ea5e26c9d1&tochange=e4d362eebc148baf8432d0b013b6233995a2f16e

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1815339

:gregtatum, since you are the author of the regressor, bug 1815339, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Greg Tatum [:gregtatum]]

It looks like the browsingContext property is being accessed via JavaScript and triggering this assertion.

I can't tell exactly from the C++ stack, but I suspect the following JavaScript code is the execution path, as the fuzzer is trigger a translation offering. Opening a popup requires the current browsing context.

gBrowser.selectedBrowser.browsingContext.top.embedderElement.ownerGlobal

This code is fired from an event handler that responds to the page load, and detects the language. I guess through the fuzzer it's finding an execution path where the initialization is not happening correctly. This happens in the nsFrameLoader::GetBrowsingContext().

My patch in bug 1815339 touched quite a few things, but the offending patches are only JS code, not any platform changes. In addition, this doesn't appear to trigger any of the process creation code, so I strongly suspect it's just the property access of the browsingContext that is causing the assertion to be triggered.

Nika: It looks like you wrote the assertion here. Any ideas on what could be causing this or someone I could redirect my question to? I'm not sure on what the severity should be set to as I'm not super familiar with this part of the code.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Looks like this was regressed in bug 1811195 when the check for AppShutdownConfirmed was added to TryRemoteBrowser (https://searchfox.org/mozilla-central/rev/0f39860036f9b6339e65d485aeb6b6be73d9dbda/dom/base/nsFrameLoader.cpp#2803-2808)

Instead of adding the check into TryRemoteBrowserInternal after the mInitialized check, it was added into the caller. This has two negative effects:

1. The frontend is never notified of process launch failures caused by this check, meaning that things like the tab crashed page would not be displayed, and
2. The mInitialized variable is not set to prevent future launches, causing this assertion to fail.

Because it is a shutdown check, this probably has few practical effects on the browser, as showing the tab crashed page during browser shutdown is unlikely to cause problems, and we never stop shutting down after entering browser shutdown, so the later mInitialized check also won't be encountered. We should fix it to avoid folks making this mistake in the future with other checks though, and perhaps add a comment to avoid folks adding the check in the incorrect place.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1871625 - Move AppShutdownConfirmed check in nsFrameLoader to a more appropriate place, r=smaug!

This check was introduced in bug 1811195, but was added outside of the
normal opening flow, meaning that failures after this check would not be
handled properly, leading to potential assertion failures or unhandled
content process creation failures.

This patch adds a clarifying comment to ensure that we don't add checks
to the wrong place in the future, and moves the check into
ContentParent::CreateBrowser such that it is handled in a similar way
to other content process creation failures.

In addition, extra logic was added to ensure mInitialized gets set on
TryRemoteBrowserInternal failure, as new checks have been added before
the mInitialized check, and this seems likely to happen more in the
future.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a35a50999fa1
Move AppShutdownConfirmed check in nsFrameLoader to a more appropriate place, r=smaug

Comment 8

[Sandor Molnar[:smolnar]]

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

Comment 9

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240109162901-b29d6ace45f4.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

The assertion failure here is debug-only.