2004165 - 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/bc030364-9778-4b1e-baf4-e8b9f0251204

MOZ_CRASH Reason:

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

Top 10 frames:

0  XUL  MOZ_CrashSequence(void*, long)  mfbt/Assertions.h:242
0  XUL  mozilla::dom::BrowsingContextGroup::EnsureUsesOriginAgentClusterInitialized(n...  docshell/base/BrowsingContextGroup.cpp:675
0  XUL  nsDocShell::CreateAboutBlankDocumentViewer(nsIPrincipal*, nsIPrincipal*, nsIP...  docshell/base/nsDocShell.cpp:6710
1  XUL  nsDocShell::CompleteInitialAboutBlankLoad(nsDocShellLoadState*, nsILoadInfo*)  docshell/base/nsDocShell.cpp:11190
1  XUL  nsDocShell::DoURILoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>, nsI...  docshell/base/nsDocShell.cpp:10997
1  XUL  nsDocShell::InternalLoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>)  docshell/base/nsDocShell.cpp:10031
2  XUL  nsDocShell::LoadHistoryEntry(nsDocShellLoadState*, unsigned int, bool)  docshell/base/nsDocShell.cpp:12951
3  XUL  nsDocShell::LoadHistoryEntry(mozilla::dom::LoadingSessionHistoryInfo const&, ...  docshell/base/nsDocShell.cpp:12811
4  XUL  nsDocShell::LoadURI(nsDocShellLoadState*, bool, bool)  docshell/base/nsDocShell.cpp:833
5  XUL  mozilla::dom::BrowsingContext::LoadURI(nsDocShellLoadState*, bool)  docshell/base/BrowsingContext.cpp:2145

While there are a small number of these crashes on ESR and release builds, it looks like this picked up quite a bit on Nightly, starting with the 20251124214625 build. I'm guessing this could be another about:blank regression.

Comment 1

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

:vhilla, since you are the author of the regressor, bug 543435, 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 543435

Comment 3

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

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 AArch64 and ARM crashes on nightly

For more information, please visit BugBot documentation.

Comment 4

[Vincent Hilla [:vhilla]]

If CompleteInitialAboutBlankLoad calls CreateAboutBlankDocumentViewer, it means we mispredicted the principal for initial about:blank, got a null principal or a sandbox. So we create a new document with the correct principal aLoadState->PrincipalToInherit().

It would be interesting to know where aLoadState->PrincipalToInherit() comes from and whether UsesOriginAgentCluster would've been initialized in some way before bug 543435. My hypotheses are

• Going through the normal load path would've involved the parent process initializing UsesOriginAgentCluster for the principal
• The normal load path would've used a different principal than aLoadState->PrincipalToInherit()
• Something broke earlier and changed the principal of a document or so which impacted aLoadState->PrincipalToInherit()

From a quick look, the crashes from ESR vs Nightly have different stacks. All ESR crashes originate in mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders() while the Nightly crashes involve LoadURI from history or parent process.

Comment 5

[Vincent Hilla [:vhilla]]

It reminds me of this review comment for bug 543435

I checked on central, <iframe sandbox></iframe> leads to an entry in nsSHistory whose principal is not sandboxed.

Comment 6

[Vincent Hilla [:vhilla]]

:nika can you share some thoughts and provide context on where UsesOriginAgentCluster should be initialized?

( Feel free to cancel the ni? or forward it. Given the number of about:blank regressions, I want to check in before I dig deeper. )

Comment 7

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

The intention behind UsesOriginAgentCluster is that it is checked and initialized in the parent process, as we cannot perform the checks for it within a content process. The logic for setting this value for most loads happens within TriggerRedirectToRealChannel (https://searchfox.org/firefox-main/rev/9a44e01da517c08e962c78bafd0712690381717a/netwerk/ipc/DocumentLoadListener.cpp#2417-2454). In this case, the network response is analyzed to set the flag on the BrowsingContextGroup within the parent process.

In some cases, we need use CreateAboutBlankDocumentViewer to force creation of a specific principal-ed about:blank document in a content process from e.g. JS. This triggers https://searchfox.org/firefox-main/rev/9a44e01da517c08e962c78bafd0712690381717a/dom/ipc/BrowserHost.cpp#270-274 to ensure that the UsesOriginAgentCluster is configured in the content process before it is resolved.

---

It appears that the specific problem in your case is happening with this code: https://searchfox.org/firefox-main/rev/9a44e01da517c08e962c78bafd0712690381717a/docshell/base/nsDocShell.cpp#11179-11201

Specifically, due to the changes you're now treating a normal "about:blank" load triggered by the parent process with a different principal than the principal of the current about:blank document as-if it was a CreateAboutBlankDocumentViewer load. I expect this will break more things than just UsesOriginAgentCluster, as this will also mean we don't things like permissions for the document before it arrives.

This also could cause process selection issues. In the previous code path we may have chosen to do a process switch in this case, but that's no longer possible.

---

My recommendation for fixing this right now would be to abort doing a synchronous load in this process mismatch case instead of calling createAboutBlankDocumentViewer, and instead do a "normal" non-initial about:blank load as we did before your patches. I think this would involve moving the principal mismatch check back into IsAboutBlankLoadOntoInitialAboutBlank (https://searchfox.org/firefox-main/rev/9a44e01da517c08e962c78bafd0712690381717a/docshell/base/nsDocShell.cpp#10852-10856), and returning false if the principals do not match. This will cause the CompleteInitialAboutBlankLoad call to be skipped (https://searchfox.org/firefox-main/rev/9a44e01da517c08e962c78bafd0712690381717a/docshell/base/nsDocShell.cpp#10994-10998), and mean that we continue with the old load codepath I believe.

Fundamentally, we need to do some async work with at least a round trip to the parent process to configure the process to load an about:blank document with the new principal, though we may even select a different process to finish the load. Using our existing load infrastructure is probably the easiest way to do that.

Comment 8

[Vincent Hilla [:vhilla]]

https://treeherder.mozilla.org/jobs?repo=try&revision=2ac71b5ae89d5abd6e3f1ab6fa0a20419cb97e81

Comment 9

[Ryan VanderMeulen (PTO, back July 27)]

Bug 543435 was reverted from Fx147. Updating the flags accordingly.

Comment 10

[Vincent Hilla [:vhilla]]

I talked to :farre to understand better what's going on. Skipping the sync path as a quick fix makes sense.

We managed to reproduce this.

• Go to example.com
• Do window.open() to open a new about:blank with content principal
• Restart the browser and have the about:blank be restored before example.com.
  • E.g. by going to about:profiles, "Restart normally", then click on the about:blank tab.

The about:blank is restored from SH with an example.com content process, but the example.com hasn't loaded so UsesOriginAgentCluster wasn't initialized for this principal. Maybe we just need to initialize that in nsSHistory::LoadEntry?

Comment 11

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

The bug is marked as tracked for firefox148 (nightly). However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 12

[Vincent Hilla [:vhilla]]

Attachment: Bug 2004165 - Use async load path when restoring initial about:blank from session history. r=#dom-core

Comment 13

[Vincent Hilla [:vhilla]]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=168241

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fa9c48352a4d
https://hg.mozilla.org/integration/autoland/rev/db9bdefd983b
Use async load path when restoring initial about:blank from session history. r=dom-core,hsivonen

Comment 15

[Iulian Moraru]

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