1582832 - Crash in [@ mozilla::dom::BrowsingContext::CreateFromIPC]

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-094af257-082d-4684-a095-cd35a0190920.

Seen while looking at nightly crashes - these seem to be with Fission enabled: https://bit.ly/2kM5Jy3. The 71 crashes are macOS and Linux only.

Top 10 frames of crashing thread:

0 XUL mozilla::dom::BrowsingContext::CreateFromIPC docshell/base/BrowsingContext.cpp:163
1 XUL mozilla::dom::ContentChild::RecvRegisterBrowsingContextGroup dom/ipc/ContentChild.cpp:3907
2 XUL mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:12063
3 XUL mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2185
4 XUL mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1985
5 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
6 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
7 XUL mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110
8 XUL nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 XUL nsAppShell::Run widget/cocoa/nsAppShell.mm:705

Comment 1

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

Hmm, perhaps bug 1532661 didn't actually regress this, but it is the code from bug 1532661 which crashes.

Comment 2

[Neha Kochar [:neha]]

Andreas, this seems to be the cause of dom/tests/mochitest/general/test_vibrator.html test failing with Fission. Could you look into this?

Comment 4

[Andreas Farre [:farre]]

(In reply to Neha Kochar [:neha] from comment #2)

Andreas, this seems to be the cause of dom/tests/mochitest/general/test_vibrator.html test failing with Fission. Could you look into this?

I cannot get dom/tests/mochitest/general/test_vibrator.html to fail, how hard I try.

Comment 5

[Chris Peterson [:cpeterson]]

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Comment 6

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

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

No crashes on beta, Fission is disabled by default

Comment 8

[Anny Gakhokidze [:annyG]]

I was able to reproduce this on latest nightly https://crash-stats.mozilla.org/report/index/c03c4a84-c17c-40fa-84fb-1ce160191210

STR - Steps to reproduce (don't forget to disable any ad blockers you have)

1. Go to https://www.reddit.com/r/videos/
2. Scroll down a little bit, wait until you see the thumbnails for first few youtube videos
3. Refresh the page. The video iframes have crashed and you have new pending crash reports to submit in about:crashes :-)

Comment 9

[u608768]

I also see this sometimes when trying to open attachments or links from a pinned Gmail tab (only after it's been open and pinned for some amount of time). Closing and re-opening the tab makes the crash go away (seems like the tab gets into some state over time which causes the crash, unsure if it has anything to do with being pinned).

Comment 10

[Neha Kochar [:neha]]

I tried download from my pinned Gmail tab (has been a pinned tab forever) and it didn't reproduce for me.
It reproduced for me with Anny's STR from comment 8.

Comment 11

[Neha Kochar [:neha]]

Anny, this crash was also intermittently happening for some mochitests:

• toolkit/components/url-classifier/tests/mochitest/test_classify_by_default.html
• dom/security/test/mixedcontentblocker/test_main.html (although this is also being tracked by Bug 1580771, for error "object did not follow block_active_content pref" so may require more work)

so we may be able to re-enable these tests in bug 1580565 once this is fixed.

Comment 12

[Anny Gakhokidze [:annyG]]

Assigning this to Kris as he mentioned that he has outstanding patches that will fix this

Comment 14

[Andrew McCreight [:mccr8]]

Between these two signatures, I think this is the top Fission-specific crash, by a good margin, with about 11% of all Fission enabled Nightly 73 crashes.

Comment 15

[Anny Gakhokidze [:annyG]]

Kashav and I came up with this example (so one does not have to go to reddit) https://annygakh.github.io/test/. You can follow the same STR as I outlined above, with this page.

Comment 16

[Marcia Knous [:marcia]]

This is #4 overall on 73 nightly, but there are usually only 1-2 crashes being generated per build.

Comment 17

[Kris Maglione [:kmag]]

Attachment: Bug 1582832: Part 1 - Make FrameLoader owner rather than DocShell responsible for discarding a BC. r=nika

There are all sorts of lifecycle issues which arise from making DocShell
responsible for discarding BrowsingContexts. In this particular bug, we tend
to run into them in cases where we create a BrowsingContext for a FrameLoader,
and then never create a DocShell for it, leading to it never being destroyed.
But there are myriad other issues as well.

This patch moves the responsibility for BrowsingContext lifecycle management
to the FrameLoader/FrameLoaderOwner, rather than the DocShell, which makes
things more consistent, and more closely aligns with spec-defined behavior.

Comment 18

[Kris Maglione [:kmag]]

Attachment: Bug 1582832: Part 2 - Don't attach a BrowsingContext before a FrameLoader is initialized. r=nika

There are cases where the initial BrowsingContext that we create for a
FrameLoader will never have a DocShell created for it. In the particular case
when the frame it belongs to is part of an inactive document, trying to
eagerly attach the BrowsingContext ends up attaching it as an active child of
its parent BrowsingContext, when it should be either cached or detached. Which
causes no end of problems.

This patch delays attaching the BrowsingContext until the FrameLoader is
initialized, which solves most of these problems.

Comment 19

[Jonathan Kingston [:jkt] he/him]

It seems this crash is still existing with the work from Bug 1594529 applied on top:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e2188df2531ed78998e3510d6803a29370a63f4

Comment 20

[Jeff Walden [:Waldo]]

A friend just ran into this crash in the wild in a January 16 nightly, push date was ~9h ago. I can get a crashid if it's desired, but right now I'm too lazy to have him dictate it to me. :-)

Comment 21

[Kris Maglione [:kmag]]

Attachment: Bug 1582832: Part 3 - Correctly handle windowless BCs created for printing. r=nika

Comment 22

[Chris Peterson [:cpeterson]]

QA says they hit this crash when testing Office365 and Reddit with Fission.

Comment 23

[Paul Bone [:pbone]]

I just got this crash when I followed a link from gmail. The link opens a new tab and that tab crashes. I tried twice and it reproduces reliably.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fd57b83bb52
Part 1 - Make FrameLoader owner rather than DocShell responsible for discarding a BC. r=nika
https://hg.mozilla.org/integration/autoland/rev/cefac6461948
Part 2 - Don't attach a BrowsingContext before a FrameLoader is initialized. r=nika
https://hg.mozilla.org/integration/autoland/rev/aea4783f8b48
Part 3 - Correctly handle windowless BCs created for printing. r=nika

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d190ee669ee
Update expectation for closed-attribute.window.html on fission a=test-only

Comment 26

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6fd57b83bb52
https://hg.mozilla.org/mozilla-central/rev/cefac6461948
https://hg.mozilla.org/mozilla-central/rev/aea4783f8b48
https://hg.mozilla.org/mozilla-central/rev/1d190ee669ee

Comment 27

[Pascal Chevrel:pascalc (PTO until April 26)]

This crash fix caused two known regressions in 74 beta, one a bigger crash on beta (bug 1614259), the other a functional regression (bug 1615508).
Kris, should we evaluate a backout of Bug 1582832 on the beta branch? Thanks

Comment 28

[Kris Maglione [:kmag]]

(In reply to Pascal Chevrel:pascalc from comment #27)

This crash fix caused two known regressions in 74 beta, one a bigger crash on beta (bug 1614259), the other a functional regression (bug 1615508).
Kris, should we evaluate a backout of Bug 1582832 on the beta branch? Thanks

Yes, let's backout from beta. This should only really matter when Fission is enabled, and it can't be enabled on beta.

Comment 29

[Arthur Iakab [arthur_iakab]]

Backed out from beta:

Backed out 4 changesets (bug 1582832) for causing Bug 1614259 and Bug 1615508 a=backout

Backout revision https://hg.mozilla.org/releases/mozilla-beta/rev/4d0fa357ec250d5628963820a446e8a02b48fdcf