1633459 - Crash in [@ IPCError-browser | CommitFromIPC Invalid Transaction from Child - CanSet failed for field(s): UserAgentOverride]

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

Attachment: 1dd6e33e-5e45-4b12-8c25-66d4a0200427-upload_file_minidump_browser.log

This bug is for crash report bp-1dd6e33e-5e45-4b12-8c25-66d4a0200427.

Top 10 frames of crashing thread:

0 libxul.so core::ptr::real_drop_in_place src/libcore/ptr/mod.rs:182
1 libxul.so core::ptr::real_drop_in_place src/libcore/ptr/mod.rs:182
2 libxul.so style::invalidation::element::invalidation_map::InvalidationMap::clear servo/components/style/invalidation/element/invalidation_map.rs:246
3 libxul.so style::stylist::CascadeData::clear servo/components/style/stylist.rs:2272
4 libxul.so style::stylist::CascadeData::rebuild 
5 libxul.so Servo_StyleSet_FlushStyleSheets servo/components/style/stylist.rs:290
6 libxul.so mozilla::ServoStyleSet::UpdateStylist layout/style/ServoStyleSet.cpp:1134
7 libxul.so mozilla::ServoStyleSet::ShellDetachedFromDocument layout/style/ServoStyleSet.cpp:143
8 libxul.so mozilla::dom::Document::DeletePresShell dom/base/Document.cpp:6338
9 libxul.so mozilla::PresShell::Destroy layout/base/PresShell.cpp:1377

This is a fission-specific crash. I've attached the stack of the main process for the crash above.

Comment 1

[Andrew McCreight [:mccr8]]

It looks like some of this CanSet code was touched recently in bug 1621192, so maybe it is a regression from that?

Comment 2

[Andrew McCreight [:mccr8]]

Dan, maybe you could take a look? Only two crashes so far, but maybe it'll be interesting.

Comment 3

[u480271]

I'll take a look.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Dan Glastonbury (:djg) | needinfo me from comment #3)

I'll take a look.

Assigning to Dan because he offered to investigate.

Tracking for Fission M5b because this is a crash.

Comment 5

[u480271]

Attachment: Bug 1633459 - Marshal customUserAgent setting via nsDocShell.

Reinstate customUserAgent interface for nsIDocShell. This is so it can be used
as a choke-point to catch setting values on docshells which are in the process
of changing process. We don't want to send changes which will be rejected on the
parent side.

This code should be removed once callers setting customUserAgent are refactored
to only occur from parent process.

Comment 6

[u480271]

Explanation I posted on Bug 1637493:

The issue is that the docShell customUserAgent changes are kinda midway through refactoring. The property was moved from nsDocShell to BrowsingContext and then the callers to docShell.customUserAgent = were updated to docShell.browsingContext.customUserAgent =. This means that the setting still happens from the Content process. BrowsingContext is split across Content/Parent; the setting from Content is forwarded to the Parent. There's a test on the Content side to determine if it's OK to send and a check in the Parent side to also. In Bug 1633459, the assert in firing in the Parent because a change is being made after the BrowsingContext has changed process and the sender is no longer allowed to set the override. (The Content side thought it was OK to send).

I put the IDL attribute back into nsIDocShell so I can reject setting the override if the doc shell knows it's losing rights to change it's BrowsingContext (The check to mWillProcessChange). This is just an interim fix to stop the assert until we can update all the uses of .customUserAgent = ... to originate from the Parent process, which always has rights to set the property on a BrowsingContext.

Comment 7

[Chris Peterson [:cpeterson]]

Moving to Fission Nightly M6a. Nika says this crash doesn't need to block Fission dogfooding (M5), but we want to fix known crashes soon.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0aab0ef96299
Marshal customUserAgent setting via nsDocShell. r=geckoview-reviewers,esawin,nika

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0aab0ef96299