2014692 - Crash in [@ IPCError-content | RecvConstructBrowser BrowserChild::Init failed (rv=NS_ERROR_FAILURE)]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/130799e2-b55e-426b-9ef9-689410260204

MOZ_CRASH Reason:

MOZ_CRASH(Content child abort due to IPC error)

Top 10 frames:

0  libxul.so  MOZ_CrashSequence(void*, long)  mfbt/Assertions.h:237
0  libxul.so  mozilla::dom::ContentChild::ProcessingError(mozilla::ipc::HasResultCodes::Res...  dom/ipc/ContentChild.cpp:2326
1  libxul.so  mozilla::ipc::IPCResult::FailImpl(mozilla::NotNull<mozilla::ipc::IProtocol*>,...  ipc/glue/ProtocolUtils.cpp:61
2  libxul.so  mozilla::ipc::IPCResult::FailUnsafePrintfImpl<(unsigned long)21>(mozilla::Not...  ipc/glue/ProtocolUtils.h:390
2  libxul.so  mozilla::dom::ContentChild::RecvConstructBrowser(mozilla::ipc::ManagedEndpoin...  dom/ipc/ContentChild.cpp:1977
2  libxul.so  mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentChild.cpp:8666
3  libxul.so  mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecyc...  ipc/glue/MessageChannel.cpp:1793
3  libxul.so  mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecyclePro...  ipc/glue/MessageChannel.cpp:1719
3  libxul.so  mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, ...  ipc/glue/MessageChannel.cpp:1508
3  libxul.so  mozilla::ipc::MessageChannel::MessageTask::Run()  ipc/glue/MessageChannel.cpp:1610

Comment 1

[BugBot [:suhaib / :marco]]

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

:vhilla, since you are the author of the regressor, bug 2009931, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Vincent Hilla [:vhilla]]

The stack to IPC_FAIL is (per comment and the changes in bug 2013872)
ContentChild::RecvConstructBrowser -> browserChild->Init -> nsWebBrowser::Create -> nsDocShell::InitWindow -> nsDocShell::Initialize -> nsDocShell::CreateInitialDocumentViewer -> nsDocShell::CreateAboutBlankDocumentViewer.

Looking at the code where NS_ERROR_FAILURE could come from:

• nsDocShell::CreateInitialDocumentViewer if mIsBeingDestroyed (then it would've occured before bug 2009931)
• nsDocShell::CreateAboutBlankDocumentViewer if mCreatingDocument, see bug 2013932
  • This is set/cleared via a scope exit and we don't have CreateAboutBlankDocumentViewer on the stack when reaching it from RecvConstructBrowser.
• nsDocShell::CreateAboutBlankDocumentViewer if okToUnload, see bug 2013932
• nsDocShell::CreateAboutBlankDocumentViewer last return if !docFactory or !blankDoc
• nsDocShell::SetupNewViewer if GetInProcessSameTypeParent() fails (always NS_OK)
• nsDocShell::SetupNewViewer if !mDocumentViewer or mDocumentViewer->Init() fails

Comment 3

[BugBot [:suhaib / :marco]]

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

• Top 10 desktop browser crashes on nightly

:vhilla, could you consider increasing the severity of this top-crash bug?

For more information, please visit BugBot documentation.

Comment 4

[Vincent Hilla [:vhilla]]

The many reports come from few installs, so I think S3 is still ok.

Comment 5

[Vincent Hilla [:vhilla]]

A lot of the windows crashes I looked at had >90% memory usage. But not all, e.g. https://crash-stats.mozilla.org/report/index/4e8cfac6-c4c0-4a27-98ec-999b70260217 has 38%. And creating a blank document with window etc. shouldn't need much memory.

mDocumentViewer->InitInternal can fail if

• !mDocument but CreateAboutBlankDocumentViewer used blankDoc.
• SetNewDocument fails and that can happen for many reasons, several seem to be OOM, one is another AncestorsAreCurrent check. I'll add logging there.
• InitPresentationStuff fails but this happens only if !mPresShell and CreatePresShell doesn't seem like it can return null.

Comment 6

[Vincent Hilla [:vhilla]]

The top 5 install times have the same set of extensions, likely a single user accounting for ~80% of crashes. :farre confirmed these crash on some extension page. Though number six has no extensions in common.

Still, RecvConstructBrowser crashing means we didn't yet load the specific extension document. Maybe it's not unique to the extension but rather there are specific circumstances that lead a hidden browser to crash.

Comment 7

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox149 (nightly). However, the bug still has low severity.

:hsinyi, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 8

[Vincent Hilla [:vhilla]]

Maybe we shouldn't track it for release?

• Few installs
• Crash when loading extension documents
• If there is a spike in beta, we can still back out bug 2009931 while the larger beta population might lead someone to comment steps to reproduce.

---

Bug 2017434 gave us

• SetNewDocument rv=2147500037 in all new reports, which means NS_ERROR_FAILURE, maybe one of these JS methods.

Comment 9

[Pascal Chevrel (relman team) -> :pascalc]

This is few users but this is also nightly where we have few users by nature, I prefer that we keep tracking it until we are at 100% rollout on beta to make sure it doesn't spike.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

S3 still sounds right and would continue monitoring the situation and it's with tracking+

Comment 11

[Vincent Hilla [:vhilla]]

(Partly from Claude)

The outer window and document is brand new — no inner window, no JS wrapper yet. Considering where NS_ERROR_FAILURE comes from in SetNewDocument, we can exclude:

• JS_RefreshCrossCompartmentWrappers — NOT reachable: this is inside if (reUseInnerWindow), which is false because there's no current inner window
• TransplantObjectNukingXrayWaiver — NOT reachable: this is inside else if (GetWrapperPreserveColor()), but the outer window has no wrapper yet when SetNewDocument is first called
• InitClasses — nsJSContext::InitClasses always returns NS_OK
• ExecutionReady — can only return NS_ERROR_UNEXPECTED, NS_ERROR_NOT_AVAILABLE, or NS_ERROR_DOM_INVALID_STATE_ERR

This leaves

1. CreateNativeGlobalForInner
   - Window_Binding::Wrap(...) returns false
   - OR InitializeLegacyNetscapeObject(...) returns false
2. JS_GetProperty(cx, newInnerGlobal, "window", ...)
3. JS_GetProperty(cx, newInnerGlobal, "self", ...)

Claude suggests to do return JS_IsThrowingOutOfMemory(cx) ? NS_ERROR_OUT_OF_MEMORY : NS_ERROR_FAILURE; there as is done in Promise code already.

I looked through the new reports, all except two have NS_ERROR_FAILURE. There are only two other ones (e.g. this) and those have SetNewDocument rv=2152924174. meaning NS_ERROR_OUT_OF_MEMORY. As noted earlier, there still are few reports without high memory use percentage... my query

Comment 12

[Vincent Hilla [:vhilla]]

Attachment: Bug 2014692 - Propagate OOM during DocumentViewer creation and handle it in RecvConstructBrowser. r=#dom-core

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3da21b9ff1fc
https://hg.mozilla.org/integration/autoland/rev/1de725548fbb
Propagate OOM during DocumentViewer creation and handle it in RecvConstructBrowser. r=dom-core,hsivonen

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1de725548fbb

Comment 15

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:vhilla, 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-firefox149 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

[Vincent Hilla [:vhilla]]

I suggest we wait to see whether the crash rate goes down to verify this is fixed.

If it's indeed an OOM issue, we crash either way, so there's limited benefit to uplifting. Maybe it's useful to be sure there aren't other reasons for failing here. I'll monitor the crash volume on beta.

Comment 18

[Vincent Hilla [:vhilla]]

This depends on bug 2017434, bug 2013872 and bug 2009931. All of these landed in 149, so only this patch would need uplifting to beta.

Comment 19

[BugBot [:suhaib / :marco]]

Copying crash signatures from duplicate bugs.

Comment 20

[Vincent Hilla [:vhilla]]

There are new signatures:
IPCError-content | RecvConstructBrowser BrowserChild::Init failed (rv=NS_ERROR_NOT_AVAILABLE) 2 reports, one before the latest patch
IPCError-content | RecvConstructBrowser BrowserChild::Init failed (rv=NS_ERROR_RECURSIVE_DOCUMENT_LOAD) 2 reports, both after the latest patch.

Comment 21

[Vincent Hilla [:vhilla]]

Anyhow, I think the patch is simple enough to uplift and I want to know how many non-OOM crashes there are in beta (and release once 149 ships).

Comment 22

[Vincent Hilla [:vhilla]]

Attachment: Bug 2014692 - Propagate OOM during DocumentViewer creation and handle it in RecvConstructBrowser.

Original Revision: https://phabricator.services.mozilla.com/D285092

Comment 23

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: We won't know if / how many non-OOM crashes there are in this code.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Propagating an error instead of crashing more generically
• String changes made/needed: No
• Is Android affected?: yes

Comment 24

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/9aae3423ab41
https://hg.mozilla.org/releases/mozilla-beta/rev/93dd140f71b5