1581076 - Crash in [@ mozilla::dom::ContentParent::RecvAttachBrowsingContext] with MOZ_DIAGNOSTIC_ASSERT(false) (Trying to attach to out of process parent context)

Status: RESOLVED FIXED

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

Description

[Ludovic Hirlimann [:Usul]]

This bug is for crash report bp-181bf1fc-517f-458f-8cb0-0853d0190913.

Top 10 frames of crashing thread:

0 libxul.so mozilla::dom::ContentParent::RecvAttachBrowsingContext dom/ipc/ContentParent.cpp:5814
1 libxul.so mozilla::dom::PContentParent::OnMessageReceived ipc/ipdl/PContentParent.cpp:10970
2 libxul.so mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2185
3 libxul.so mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1954
4 libxul.so nsThread::ProcessNextEvent ipc/glue/MessageChannel.cpp:1985
5 libxul.so <name omitted> xpcom/threads/nsThreadUtils.cpp:486
6 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:88
7 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
8 libxul.so nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 libxul.so nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:276

Comment 1

[Laurențiu Nicola]

Possible repro with fission.autostart enabled: load Facebook, wait a little, scroll down the timeline, then quickly type another URL in the address bar and press Enter.

Comment 3

[Neha Kochar [:neha]]

See bug 1580973 for a reduced testcase for this too.

Comment 5

[Neha Kochar [:neha]]

And also bug 1577552 for a fuzzing testcase too.

Comment 6

[Andrew McCreight [:mccr8]]

I'll try to take a look at this once I get back from break. It looks like the most common Fission specific crash.

Comment 7

[Andrew McCreight [:mccr8]]

kmag and I have been looking at this. In the test case from the second duped bug, it seems like the click is somehow clicking on the invalid link, and we end up trying to change frame remoteness to the remote type webIsolated=http://undefined. Somewhere in the frame remoteness switching, we end up changing the process in the canonical browsing context. I assume that later we realize something went wrong and bail out, so we're in this half-transitioned state, so we never tell the child that we switched the process. I'm not sure if the problem is how we're bailing out from whatever failure is happening, or if we should have caught the very bogus URL we're navigating to earlier. I'd assume that there are other failure states where we can't tell until later that it went wrong, so maybe fixing some failure handling path is the way to go?

Comment 8

[Chris Peterson [:cpeterson]]

QA said they hit this bug during their testing of Process Switching (without Fission) in 73 Nightly. The crash volume during 73 and 74 Nightly is low. If we have a fix, we should uplift it. In the meantime, let's monitor the crash volume in 73 Beta.

Comment 9

[Chris Peterson [:cpeterson]]

QA says they hit this crash when testing Screen Reader support with Fission.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I hit this yesterday in bp-95faf49f-690c-4631-8dda-2c1ba0200220 right after hitting the reader mode button while loading an article on ThinkPad firmware problems.

Comment 11

[Chris Peterson [:cpeterson]]

Moving P2 M5 bugs to M5b milestone

Comment 12

[Matt Woodrow (:mattwoodrow)]

This crash seems to happen fairly frequently if you run fission crashtests on try, from dom/base/crashtests/610571-1.html.

Pernosco run: https://pernos.co/debug/dRk1MclFR1s-H8WrMXykqw/index.html#f{m[DXfo,ATE_,t[AQ,Blo_,f{e[DXfo,hA_,s{af6dBybAA,bARU,oGDbf0A,uGA/gYw___

When we get RecvAttachBrowsingContext, the parent isn't owned by the same process, but the in-flight process id for the parent does match.

It looks like the test is creating an iframe, setting the src to a data URI, and appending it to the test document. That should trigger a load of the data URI in the iframe, and a process switch (from the file: process to the default web process).

The test then (synchronously!) appends another iframe inside the first iframe's document, which will still be the initial about:blank document at this point (since we haven't waited for the data: uri to complete).

The second iframe will create a new BC, calling SendAttachBrowsingContext to the parent, but by the time it arrives, we have started processing the outer iframe's process switch, and the IDs don't match.

It seems like for this case at least we could probably silently fail if we match the parent's in-flight process id?

Comment 13

[James Teh [:Jamie]]

I likewise hit this when engaging Reader Mode immediately after loading an article. For me, it was this article. I couldn't reproduce it on a second try, though. bp-f17c9b1b-99bf-47eb-a6d8-643390200324

Comment 14

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1581076 - Don't crash if we try to attach a new BrowsingContext to one that has current process changes in-flight. r?kmag

Comment 15

[Chris Peterson [:cpeterson]]

Matt, is this patch still relevant?

If so, does this bug still need to block Fission Dogfooding? We still see some crash reports with this signature, but the crash volume is very low.

kmag said in Phabricator:

hopefully this has been fixed by @nika's WindowContext changes.

Comment 16

[Matt Woodrow (:mattwoodrow)]

This code no longer exists, as of nika's WindowContext changes, so we can't be crashing here any more.