1590908 - Crash in [@ mozilla::dom::BrowsingContext::LoadURI]

Status: RESOLVED FIXED

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

Description

[Asif Youssuff]

This bug is for crash report bp-105388b8-4377-4f49-bb06-b639e0191023.

Top 10 frames of crashing thread:

0 XUL mozilla::dom::BrowsingContext::LoadURI docshell/base/BrowsingContext.cpp:886
1 XUL mozilla::dom::LocationBase::SetURI dom/base/LocationBase.cpp:150
2 XUL mozilla::dom::LocationBase::SetHrefWithBase dom/base/LocationBase.cpp:212
3 XUL mozilla::dom::Location_Binding::replace dom/bindings/LocationBinding.cpp
4 XUL bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::CrossOriginThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3198
5 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:550
6 XUL Interpret js/src/vm/Interpreter.cpp:623
7 XUL js::RunScript js/src/vm/Interpreter.cpp:425
8 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:591
9 XUL js::jit::InvokeFunction js/src/jit/VMFunctions.cpp:260

This crash is back in 72. 52 crashes in the last 7 days over 8 installs. Reported https://www.reddit.com/r/firefox/comments/dk2am2/weekly_nightly_discussion_for_20191019_20191025/f4xkrot/

Fission seems to be enabled in most of the crash reports, so that may make it worse.

Comment 1

[exprezdev]

This is occurring on my computer. Disabling Fission causes the problem to disappear.

Related: refreshing the tab crashed page (Cmd-R) causes all of Firefox to crash (20019d93-49b5-4838-84a6-024850191023).

Comment 2

[Pulse]

I'm also seeing this bug when I have a combination of fission and uMatrix enabled. For me it's an instant tab crash on old.redd.com. With one or the other disabled no crashes..

[48bc74d4-f3ea-4db8-b5bc-9a06e0191024]{https://crash-stats.mozilla.org/report/index/48bc74d4-f3ea-4db8-b5bc-9a06e0191024}

Comment 3

[Pulse]

(In reply to Pulse from comment #2)

I'm also seeing this bug when I have a combination of fission and uMatrix enabled. For me it's an instant tab crash on old.redd.com. With one or the other disabled no crashes..

[48bc74d4-f3ea-4db8-b5bc-9a06e0191024](https://crash-stats.mozilla.org/report/index/48bc74d4-f3ea-4db8-b5bc-9a06e0191024}

Comment 4

[Pulse]

(In reply to Pulse from comment #3)

(In reply to Pulse from comment #2)

I'm also seeing this bug when I have a combination of fission and uMatrix enabled. For me it's an instant tab crash on old.redd.com. With one or the other disabled no crashes..

48bc74d4-f3ea-4db8-b5bc-9a06e0191024

Comment 5

[Andrew McCreight [:mccr8]]

This is crashing on MOZ_DIAGNOSTIC_ASSERT(aAccessor).

Comment 6

[Andrew McCreight [:mccr8]]

This is a top crash on Linux for the 10/23 nightlies.

Comment 7

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

Attachment: Bug 1590908 - Part 1: Move parent-only LoadURI method to CanonicalBrowsingContext,

These methods are only callable from the parent process, so it doesn't make
sense to have the method available driectly on BrowsingContext.

Comment 8

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

Attachment: Bug 1590908 - Part 2: Don't require passing JSContext* to CallerInnerWindow,

The JSContext* is already fetched from within GetIncumbentGlobal, so the get is
guaranteed not to fail. This simplifies the callsite, making it easier to call.

Comment 9

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

Attachment: Bug 1590908 - Part 3: Use CallerInnerWindow in LocationBase::SetURI,

This doesn't fix every scenario, as chrome JS can still try to call one of these
methods, which will cause a crash. We would need to move SendLoadURI to PContent
so that chrome JS can navigate arbitrary contexts if we wanted to be certain no
crash occurred.

Unfortunately, chrome JS navigates in-process BrowsingContext objects very
frequently in tests (etc), so we can't make location navigations which don't have
an accessor fail.

I considered making the method just produce an error, rather than doing a
diagnostic assert, but I figured we should make that decision in another bug.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f54c4d6a55df
Part 1: Move parent-only LoadURI method to CanonicalBrowsingContext, r=kmag
https://hg.mozilla.org/integration/autoland/rev/1c60e2224eea
Part 2: Don't require passing JSContext* to CallerInnerWindow, r=kmag
https://hg.mozilla.org/integration/autoland/rev/7842d89fdccf
Part 3: Use CallerInnerWindow in LocationBase::SetURI, r=kmag

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/f54c4d6a55df
https://hg.mozilla.org/mozilla-central/rev/1c60e2224eea
https://hg.mozilla.org/mozilla-central/rev/7842d89fdccf

Comment 12

[Chris Peterson [:cpeterson]]

Retroactively moving fixed bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to an appropriate Fission 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 13

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

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

Comment 14

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Hi Assif, is qa needed here? And if so, could you provide some steps ? Thanks !

Comment 15

[Asif Youssuff]

I don't have STR, unfortunately.

The original reporter said that a crash occurred when loading govtrack.us and with Fission enabled.