1610614 - Crash in [@ mozilla::dom::CanonicalBrowsingContext::PendingRemotenessChange::Complete]: MOZ_DIAGNOSTIC_ASSERT(loadContext->UseRemoteTabs() && loadContext->UseRemoteSubframes())

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P1)

Description

[Chris Peterson [:cpeterson]]

This bug is for crash report bp-2a6db1a3-5c29-4ac7-af8e-2b4890200121.

I have Fission enabled, but I don't know if this CanonicalBrowsingContext might affect non-Fission users. I see only two crash reports with this signature in Socorro. The first is from 2020-01-19.

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(loadContext->UseRemoteTabs() && loadContext->UseRemoteSubframes()) (Not supported without fission)

This assertion was added in subframe process switch bug 1576714.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::CanonicalBrowsingContext::PendingRemotenessChange::Complete docshell/base/CanonicalBrowsingContext.cpp:332
1 xul.dll mozilla::MozPromise<RefPtr<mozilla::dom::ContentParent>, nsresult, 0>::ThenValue<`lambda at z:/task_1579535168/build/src/docshell/base/CanonicalBrowsingContext.cpp:526:11', `lambda at z:/task_1579535168/build/src/docshell/base/CanonicalBrowsingContext.cpp:529:11'>::DoResolveOrRejectInternal xpcom/threads/MozPromise.h:727
2 xul.dll mozilla::MozPromise<RefPtr<AudioDeviceInfo>, nsresult, 1>::ThenValueBase::ResolveOrRejectRunnable::Run xpcom/threads/MozPromise.h:403
3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1220
4 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
5 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
6 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
7 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
8 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:406

Comment 1

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

I think this may be being caused by dragging a fission tab into a non-fission window, or vice-versa. I thought we had blocked that type of interaction, but it appears there is nothing preventing dragging tabs between fission and non-fission windows, as I was able to do so.

:mconley, what do we need to change in the frontend to prevent swapping tabs between fission and non-fission windows?

Comment 2

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

We should also add assertions into nsFrameLoader's swapping code to prevent errors like this in the future.

Comment 3

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

Attachment: Bug 1610614 - Part 1: Ensure remoteSubframes status matches before allowing frameLoader swap,

Comment 4

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

Attachment: Bug 1610614 - Part 2: Check for gFissionBrowser match in swapBrowsersAndCloseOther,

Comment 5

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

Attachment: Bug 1610614 - Part 3: Check for gFissionBrowser in dragover handler,

Comment 6

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

Attachment: Bug 1610614 - Part 4: Add tests for dragging tabs between fission and non-fission windows,

Comment 7

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Nika Layzell (ni? for response) from comment #1)

:mconley, what do we need to change in the frontend to prevent swapping tabs between fission and non-fission windows?

PTO and All-Hands meant I didn't get to this in time. Sorry, and glad you sorted it!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c69aec21a39b
Part 1: Ensure remoteSubframes status matches before allowing frameLoader swap, r=kmag
https://hg.mozilla.org/integration/autoland/rev/2efa86d954cd
Part 2: Check for gFissionBrowser match in swapBrowsersAndCloseOther, r=mconley
https://hg.mozilla.org/integration/autoland/rev/11145723684a
Part 3: Check for gFissionBrowser in dragover handler, r=mconley
https://hg.mozilla.org/integration/autoland/rev/1d70434c1ac7
Part 4: Add tests for dragging tabs between fission and non-fission windows, r=mconley

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c69aec21a39b
https://hg.mozilla.org/mozilla-central/rev/2efa86d954cd
https://hg.mozilla.org/mozilla-central/rev/11145723684a
https://hg.mozilla.org/mozilla-central/rev/1d70434c1ac7

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Does this need an uplift request? DevEdition does have diagnostic asserts enabled, though it's not clear to me if this bug causes problems in the non-Fission case or not.

Comment 11

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

This should theoretically not impact non-fission users at all. If non-fission DevEdition users are running into this issue, then these patches won't fix it, and I've misdiagnosed the source of the issue :-/