1956954 - Crash in [@ mozilla::dom::BrowsingContextGroup::EnsureUsesOriginAgentClusterInitialized]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/8ce8bf23-a28a-48e7-b256-5cb080250327

MOZ_CRASH Reason:

MOZ_RELEASE_ASSERT(!XRE_IsContentProcess()) (Cannot determine origin-keying in content process!)

Top 10 frames:

0  libxul.so  MOZ_CrashSequence(void*, long)  mfbt/Assertions.h:267
0  libxul.so  mozilla::dom::BrowsingContextGroup::EnsureUsesOriginAgentClusterInitialized(n...  docshell/base/BrowsingContextGroup.cpp:669
1  libxul.so  nsDocShell::CreateAboutBlankDocumentViewer(nsIPrincipal*, nsIPrincipal*, nsIC...  docshell/base/nsDocShell.cpp:6451
2  libxul.so  nsDocShell::CreateAboutBlankDocumentViewer(nsIPrincipal*, nsIPrincipal*, nsIC...  docshell/base/nsDocShell.cpp:6629
2  libxul.so  {virtual override thunk({offset(-400)}, nsDocShell::CreateAboutBlankDocumentV...  docshell/base/nsDocShell.cpp:0
3  libxul.so  mozilla::dom::BrowserChild::RecvCreateAboutBlankDocumentViewer(nsIPrincipal*,...  dom/ipc/BrowserChild.cpp:843
4  libxul.so  mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PBrowserChild.cpp:4821
5  libxul.so  mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentChild.cpp:8822
6  libxul.so  mozilla::dom::ContentChild::OnMessageReceived(IPC::Message const&)  dom/ipc/ContentChild.cpp:3618
6  libxul.so  mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecyc...  ipc/glue/MessageChannel.cpp:1789

Comment 1

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

Set release status flags based on info from the regressing bug 1665474

:nika, since you are the author of the regressor, bug 1665474, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

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

Set release status flags based on info from the regressing bug 1665474

Comment 3

[Dianna Smith [:diannaS]]

:nika, looks like these crashes continued (one crash so hard to assess). thoughts?

Comment 4

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

(In reply to Dianna Smith [:diannaS] from comment #3)

:nika, looks like these crashes continued (one crash so hard to assess). thoughts?

Hmm, that's a bit disconcerting. I suppose that means I must have missed some case here. I believe 138 has the fix for this issue, and I do see 3 crashes in 138.

In 139 we have also landed some extra assertions (https://searchfox.org/mozilla-central/rev/cc231cc166e845deb02d08b175e22236bfad8b84/docshell/base/BrowsingContextGroup.cpp#655-661), which should fire before this particular assertion, so in beta/nightly I expect that assertion will fire instead of this one. In release though that assertion will be disabled.

I'll put up a patch to avoid this crash in the parent process. though I don't know exactly how it's happening right now.

Comment 5

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

Attachment: Bug 1956954 - ValidatePrincipal in CreateAboutBlankDocumentViewer calls, r=smaug!

This should avoid the content crash by blocking the initial
createAboutBlankDocumentViewer call and throwing an exception instead of
crashing the content process.

This also adds a call to LogAndAssertFailedPrincipalValidationInfo which
should help us collect telemetry to detect when these failures happen.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> S2 due to a crash bug. Feel free to change the severity if it's not proper value.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb6568c4b338
ValidatePrincipal in CreateAboutBlankDocumentViewer calls, r=smaug

Comment 8

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/fb6568c4b338

Comment 9

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

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox139 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

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

Comment on attachment 9484641 [details]
Bug 1956954 - ValidatePrincipal in CreateAboutBlankDocumentViewer calls, r=smaug!

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Turns a content process crash with unknown cause into an exception being thrown in the parent process.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The specifics of the situation leading to this crash are not well known, but this patch should ideally stop it from happening anymore (by making it into an exception thrown in the parent).
• String changes made/needed: none
• Is Android affected?: Unknown

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9484641 [details]
Bug 1956954 - ValidatePrincipal in CreateAboutBlankDocumentViewer calls, r=smaug!

Approved for 139.0b6

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/3bf5ed7561f5