1963915 - Crash in [@ CheckAllowLoadByTriggeringRemoteType] when navigating a Firefox-Nightly-for-Android tab using remote devtools (about:debugging) URL bar

Status: RESOLVED FIXED

(DevTools :: about:debugging, defect, P2)

Description

[Daniel Holbert [:dholbert]]

Crash report: https://crash-stats.mozilla.org/report/index/ffc3857f-c731-417d-b055-6aa2a0250501

MOZ_CRASH Reason:

Blocking load of about uri by content process which may have otherwise succeeded [aboutModule=config, isSystemPrincipal=1]

Top 10 frames:

0  libxul.so  MOZ_CrashSequence(void*, long)  mfbt/Assertions.h:253
0  libxul.so  MOZ_Crash(char const*, int, char const*)  mfbt/Assertions.h:381
0  libxul.so  CheckAllowLoadByTriggeringRemoteType(nsIChannel*)  dom/security/nsContentSecurityManager.cpp:1334
0  libxul.so  nsContentSecurityManager::doContentSecurityCheck(nsIChannel*, nsCOMPtr<nsIStr...  dom/security/nsContentSecurityManager.cpp:1414
1  libxul.so  nsJARChannel::AsyncOpen(nsIStreamListener*)  modules/libjar/nsJARChannel.cpp:1137
2  libxul.so  mozilla::net::DocumentLoadListener::Open(nsDocShellLoadState*, mozilla::net::...  netwerk/ipc/DocumentLoadListener.cpp:937
2  libxul.so  mozilla::MozPromise<bool, bool, false>::InvokeMethod<mozilla::net::DocumentLo...  xpcom/threads/MozPromise.h:652
2  libxul.so  mozilla::MozPromise<bool, bool, false>::InvokeCallbackMethod<false, mozilla::...  xpcom/threads/MozPromise.h:666
2  libxul.so  mozilla::MozPromise<bool, bool, false>::ThenValue<mozilla::net::DocumentLoadL...  xpcom/threads/MozPromise.h:923
3  libxul.so  mozilla::MozPromise<bool, bool, false>::ThenValueBase::ResolveOrRejectRunnabl...  xpcom/threads/MozPromise.h:488

STR:

1. In Firefox Nightly on Android, open a tab at https://example.org/
2. Inspect that tab using about:debugging (documentation here) from Firefox Nightly on a laptop/desktop.
3. In the about:debugging devtools UI, focus the URL bar that's part of the devtools UI, and replace https://example.org/ with about:config and hit Enter

ACTUAL RESULTS: Firefox Nightly on Android crashes (bp-ffc3857f-c731-417d-b055-6aa2a0250501 in my case). This is a full app crash (i.e. "Process Type: parent"), not just a content-process crash.

EXPECTED RESULTS: No such crash.

(Note: We previously had bug 1838686 filed on this crash signature, which was closed several years back as "incomplete". I'm linking that up via see-also; I don't know if what I'm seeing now is the same as what we were seeing there.)

Comment 1

[Daniel Holbert [:dholbert]]

Adding dependency on bug 1538028 where this assertion was added.

Also: if I repeat the STR but I type in file:/// instead of about:config, then I get a content-process crash (i.e. just the one tab crashes), and the crash signature is a bit different: bp-a0f606b5-9cb1-4cee-9a5c-05cf70250501 [@ nsDocShellLoadState::AssertProcessCouldTriggerLoadIfSystem ], with MOZ_CRASH Reason MOZ_CRASH(Unexpected system load of file URI from web content process).

I'm adding that as an additional signature here since it seems like the same basic issue, just manifesting in a slightly different way.

(I think both assertions are warning us that we shouldn't be reusing the same process for this navigation; this might be a sign that the about:debugging DevTools are navigating in the wrong way here?)

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)

Adding dependency on bug 1538028 where this assertion was added.

er, actually adding that dependency now. :) Also adding bug 1838686 as a dependency since that's where we added the code that's intentionally crashing for the file:/// URI variant that I described later in comment 1 (ah right, that's the bug that was originally tracking the main siganture here).

Comment 3

[Daniel Holbert [:dholbert]]

ni=nika who might be interested to look, since she was investigating this previously in bug 1838686.

Comment 4

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

It seems like perhaps the devtools UI for controlling the browser on mobile is starting navigations from the devtools url bar from a content process, instead of starting that navigation directly in the parent process. Doing something like that would trigger a crash like this one, as we don't want to trust the content process to perform navigations like this.

We should update the control interface using about:debugging to start the navigation directly from the parent process, rather than via the content process, so that the UI can be used to navigate to non-web-safe URIs (and perhaps add some tests for this).

Comment 5

[Julian Descottes [:jdescottes]]

The component for this UI is at https://searchfox.org/mozilla-central/source/devtools/client/framework/components/DebugTargetInfo.js#23 and we hit the target actor (content process if page is in content) for navigating to a URL, going forward or backward.

The only exception is for the "reload" actions, which uses the target descriptor (parent process).

One option is to just reuse this pattern for the other navigation actions.

(also checked, and this component is the only one using target's goBack, goForward and navigateTo helpers, so we shouldn't have the same issue in other parts of the UI)

Comment 6

[Julian Descottes [:jdescottes]]

See relevant patch at https://phabricator.services.mozilla.com/D232328

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1963915 - [devtools] Navigate back and forward from the parent process. r=#devtools

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de11fb15a214
[devtools] Navigate back and forward from the parent process. r=devtools-reviewers,devtools-backward-compat-reviewers,jdescottes

Comment 9

[Narcis Beleuzu [:NarcisB]]

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