1599498 - Assertion failure: XRE_IsParentProcess(), at src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:702

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reproduced with m-c:
BuildID=20191125161209
SourceStamp=b4755981c1382cb88fed4e4fcff3ba73779b2080

Assertion failure: XRE_IsParentProcess(), at src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:702

#0 nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, bool, bool, bool, nsDocShellLoadState*, mozilla::dom::BrowsingContext**) src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:672:27
#1 nsWindowWatcher::OpenWindow(mozIDOMWindowProxy*, char const*, char const*, char const*, nsISupports*, mozIDOMWindowProxy**) src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:294:3
#2 NS_InvokeByIndex src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
#3 CallMethodHelper::Call() src/js/xpconnect/src/XPCWrappedNative.cpp:1183:19
#4 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) src/js/xpconnect/src/XPCWrappedNative.cpp:1149:23
#5 XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946:10
#6 CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:456:13
#7 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:548:12
#8 InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:617:10
#9 Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3117:16
#10 js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:423:10
#11 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:589:13
#12 InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:617:10
#13 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:634:8
#14 JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2647:10
#15 nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:956:17
#16 PrepareAndDispatch src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:125:37
#17 SharedStub (/home/user/workspace/browsers/m-c-20191125161209-asan-debug/libxul.so+0x4b4760a)
#18 nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*) src/uriloader/base/nsURILoader.cpp:477:30
#19 nsDocumentOpenInfo::OnStartRequest(nsIRequest*) src/uriloader/base/nsURILoader.cpp:292:8
#20 nsBaseChannel::OnStartRequest(nsIRequest*) src/netwerk/base/nsBaseChannel.cpp:814:23
#21 nsInputStreamPump::OnStateStart() src/netwerk/base/nsInputStreamPump.cpp:487:21
#22 nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) src/netwerk/base/nsInputStreamPump.cpp:396:21
#23 non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) src/netwerk/base/nsInputStreamPump.cpp
#24 mozilla::NonBlockingAsyncInputStream::RunAsyncWaitCallback(mozilla::NonBlockingAsyncInputStream::AsyncWaitRunnable*, already_AddRefed<nsIInputStreamCallback>) src/xpcom/io/NonBlockingAsyncInputStream.cpp:411:13
#25 mozilla::NonBlockingAsyncInputStream::AsyncWaitRunnable::Run() src/xpcom/io/NonBlockingAsyncInputStream.cpp:29:14
#26 mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:295:32
#27 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1250:14
#28 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
#29 bool mozilla::SpinEventLoopUntil<(mozilla::ProcessFailureBehavior)1, mozilla::dom::XMLHttpRequestMainThread::SendInternal(mozilla::dom::BodyExtractorBase const*, bool)::$_0>(mozilla::dom::XMLHttpRequestMainThread::SendInternal(mozilla::dom::BodyExtractorBase const*, bool)::$_0&&, nsIThread*) src/obj-firefox/dist/include/nsThreadUtils.h:348:25
#30 mozilla::dom::XMLHttpRequestMainThread::SendInternal(mozilla::dom::BodyExtractorBase const*, bool) src/dom/xhr/XMLHttpRequestMainThread.cpp:2960:12
#31 mozilla::dom::XMLHttpRequestMainThread::Send(JSContext*, mozilla::dom::Nullable<mozilla::dom::DocumentOrBlobOrArrayBufferViewOrArrayBufferOrFormDataOrURLSearchParamsOrUSVString> const&, mozilla::ErrorResult&) src/dom/xhr/XMLHttpRequestMainThread.cpp:2734:11
#32 mozilla::dom::XMLHttpRequest_Binding::send(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XMLHttpRequest*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp:1283:24
#33 bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3153:13

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/h10tBeIC7oImqVIylj6Nsg/index.html
It will expire in 7 days.

Comment 2

[:Gijs (he/him)]

Note: I ran into something that looked similar with https://bugzilla.mozilla.org/show_bug.cgi?id=1595399#c3 .

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to :Gijs (he/him) from comment #2)

Note: I ran into something that looked similar with https://bugzilla.mozilla.org/show_bug.cgi?id=1595399#c3 .

Can you cc me please?

Comment 4

[Honza Bambas (:mayhemer)]

This looks exactly like bug 1587686 which has been fixed few days ago. The build contains the fix, so it was not sufficient.

Kershaw, can you look at this again please?

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

When I load this I get a constantly going spinner in the "Stack of selected thread" pane, and the gdb terminal never comes up if I try to open it. Do you see those two working for you, by any chance?

This should be pretty simple to debug with Pernosco if we get it working...

Comment 6

[Tyson Smith [:tsmith]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5)

When I load this I get a constantly going spinner in the "Stack of selected thread" pane, and the gdb terminal never comes up if I try to open it. Do you see those two working for you, by any chance?

Thanks for pointing that out, I've pinged Rob and Kyle. I'll let you know when it is fixed.

Comment 7

[Kershaw Chang [:kershaw]]

(In reply to Honza Bambas (:mayhemer) from comment #4)

This looks exactly like bug 1587686 which has been fixed few days ago. The build contains the fix, so it was not sufficient.

Kershaw, can you look at this again please?

This seems not share the same root cause as bug 1587686.
According to the Pernosco session, neither HttpChannelChild::Redirect1Begin nor HttpChannelChild::Redirect3Complete was called.
However, HttpChannelChild::CompleteRedirectSetup did get called by DocumentChannelChild.

#0  mozilla::net::HttpChannelChild::CompleteRedirectSetup (this=0x7fc8f1884800, aListener=0x7fc8f18f5be0, aContext=0x0) at /builds/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:1976
#1  0x00007fc8fbe9a3a4 in mozilla::net::DocumentChannelChild::OnRedirectVerifyCallback (this=0x7fc8f27e12e0, aStatusCode=<optimized out>) at /builds/worker/workspace/build/src/netwerk/ipc/DocumentChannelChild.cpp:392
#2  0x00007fc8fbe9a5e6 in non-virtual thunk to mozilla::net::DocumentChannelChild::OnRedirectVerifyCallback(nsresult) () at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:185
#3  0x00007fc8fbac4571 in mozilla::net::nsAsyncVerifyRedirectCallbackEvent::Run (this=0x7fc8f18f6a80) at /builds/worker/workspace/build/src/netwerk/base/nsAsyncRedirectVerifyHelper.cpp:40

Maybe this is more related to DocumentChannel. I am not sure. I'll keep looking.

Comment 8

[:Gijs (he/him)]

(In reply to Honza Bambas (:mayhemer) from comment #3)

(In reply to :Gijs (he/him) from comment #2)

Note: I ran into something that looked similar with https://bugzilla.mozilla.org/show_bug.cgi?id=1595399#c3 .

Can you cc me please?

CC'd you and :kershaw. :-)

Comment 9

[Boris Zbarsky [:bzbarsky]]

This is a pure docshell issue, no necko involved.

The testcase basically goes right up to the recursion limit and does a sync XHR that spins the event loop. While that's spinning, we try to load about:blank in the iframe. This lands us in nsDSURIContentListener::DoContent which does this:

    rv =
        mDocShell->CreateContentViewer(aContentType, aRequest, aContentHandler);
...
  if (NS_FAILED(rv)) {
    // we don't know how to handle the content
    *aContentHandler = nullptr;
    return rv;
  }

The CreateContentViewer call returns error, because the stack depth check in nsGlobalWindowOuter::SetNewDocument fails. See similar issues in bug 1599617, which is what I ran into when running this testcase locally.

Then we end up returning failure from nsDSURIContentListener::DoContent. The caller treats that as "this listener won't handle the content, try someone else", and tries to hand it off to other things, which is what triggers the assert, since those "other things" shouldn't run in a content process.

Now fundamentally, this whole state where we can't even create an inner window is bonkers and no good will come of it. I'm not 100% sure what we should really do here. Some options:

1. Just crash the content process. Not great.
2. Have nsDSURIContentListener::DoContent just return success and set *aAbortProcess to true. That will make uriloader stop trying to dispatch the content and effectively drop the load on the floor. We can cancel the request to be sure; basically do what we do now for NS_ERROR_DOCSHELL_DYING or NS_ERROR_REMOTE_XUL for all error returns from CreateContentViewer.
3. Change the DoContent contract somehow, but then what would the caller do, exactly? Probably the same thing as option 2...

Comment 10

[Tyson Smith [:tsmith]]

(In reply to Tyson Smith [:tsmith] from comment #6)

I'll let you know when it is fixed.

The gdb terminal loads now. The loading spinner is present but everything seems functional.

Comment 11

[Andrew McCreight [:mccr8]]

Boris, how bad of a security issue is this, for the purpose of a security rating? "no good will come of it" certainly does not sound good. Thanks.

Comment 12

[Kris Maglione [:kmag]]

The assertion failure itself isn't a big deal, and would be easy enough to fix by just not registering the BrowserContentHandler component in content processes (which we should do anyway). There may be other potential problems with falling back to other content handlers, but from a quick look at the ones we register, I don't see any obvious ones.

Comment 13

[Kris Maglione [:kmag]]

(In reply to Kris Maglione [:kmag] from comment #12)

The assertion failure itself isn't a big deal, and would be easy enough to fix by just not registering the BrowserContentHandler component in content processes (which we should do anyway).

Filed bug 1601768.

Comment 14

[Boris Zbarsky [:bzbarsky]]

"no good will come of it" certainly does not sound good.

Well, it's hard to have any sort of "sane" behavior in that situation... I don't think there's a security issue here per se, though.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Is this fixed now for 73+ by way of bug 1601768? If so, is it safe to say that fix can ride the trains?