1986701 - Crashes [@ mozilla::layers::NativeLayerRootCA::SuspendOffMainThreadCommits ]

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

Typical crash stack:

Crashing Thread (0), Name: MainThread
Frame  Module  Signature  Source  Trust
0  libsystem_pthread.dylib  pthread_mutex_lock   context
1  libmozglue.dylib  mozilla::detail::MutexImpl::mutexLock()  mozglue/misc/Mutex_posix.cpp:94  inlined
1  libmozglue.dylib  mozilla::detail::MutexImpl::lock()  mozglue/misc/Mutex_posix.cpp:116  cfi
2  XUL  mozilla::OffTheBooksMutex::Lock()  xpcom/threads/Mutex.h:66  inlined
2  XUL  mozilla::detail::BaseAutoLock<mozilla::Mutex&>::BaseAutoLock(mozilla::Mutex&)  xpcom/threads/Mutex.h:161  inlined
2  XUL  mozilla::detail::BaseAutoLock<mozilla::Mutex&>::BaseAutoLock(mozilla::Mutex&)  xpcom/threads/Mutex.h:160  inlined
2  XUL  mozilla::layers::NativeLayerRootCA::SuspendOffMainThreadCommits()  gfx/layers/NativeLayerCA.mm:325  cfi
3  XUL  nsCocoaWindow::BackingScaleFactorChanged()  widget/cocoa/nsCocoaWindow.mm:6269  cfi
4  XUL  -[ChildView viewDidChangeBackingProperties]  widget/cocoa/nsCocoaWindow.mm:1651  cfi
5  AppKit  _NSViewHierarchyDidChangeBackingProperties   cfi
6  AppKit  -[NSView addSubview:]   cfi
7  XUL  nsCocoaWindow::Create(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::DesktopPixel> const&, mozilla::widget::InitData*)  widget/cocoa/nsCocoaWindow.mm:4617  cfi
8  XUL  nsCocoaWindow::Create(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::LayoutDevicePixel> const&, mozilla::widget::InitData*)  widget/cocoa/nsCocoaWindow.mm:4640  cfi
9  XUL  nsBaseWidget::CreateChild(mozilla::gfx::IntRectTyped<mozilla::LayoutDevicePixel> const&, mozilla::widget::InitData&)  widget/nsBaseWidget.cpp:518  cfi
10  XUL  nsView::CreateWidgetForPopup(mozilla::widget::InitData*, nsIWidget*)  view/nsView.cpp:511  cfi
11  XUL  nsMenuPopupFrame::CreateWidgetForView(nsView*)  layout/xul/nsMenuPopupFrame.cpp:333  cfi
12  XUL  nsMenuPopupFrame::InitializePopup(nsIContent*, nsIContent*, nsTSubstring<char16_t> const&, int, int, MenuPopupAnchorType, bool)  layout/xul/nsMenuPopupFrame.cpp:798  cfi
13  XUL  nsXULPopupManager::ShowMenu(nsIContent*, bool)  layout/xul/nsXULPopupManager.cpp:886  cfi
14  XUL  mozilla::dom::XULButtonElement::OpenMenuPopup(bool)::$_0::operator()() const  dom/xul/XULButtonElement.cpp:202  inlined
14  XUL  mozilla::detail::RunnableFunction<mozilla::dom::XULButtonElement::OpenMenuPopup(bool)::$_0>::Run()  xpcom/threads/nsThreadUtils.h:548  cfi
15  XUL  mozilla::RunnableTask::Run()  xpcom/threads/TaskController.cpp:703  inlined
15  XUL  mozilla::TaskController::RunTask(mozilla::Task*)  xpcom/threads/TaskController.cpp:228  inlined
15  XUL  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&)  xpcom/threads/TaskController.cpp:1310  cfi
16  XUL  mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&)  xpcom/threads/TaskController.cpp:1133  inlined
16  XUL  mozilla::TaskController::ProcessPendingMTTask(bool)  xpcom/threads/TaskController.cpp:639  inlined
16  XUL  mozilla::TaskController::TaskController()::$_0::operator()() const  xpcom/threads/TaskController.cpp:333  inlined
16  XUL  mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run()  xpcom/threads/nsThreadUtils.h:548  cfi
17  XUL  nsThread::ProcessNextEvent(bool, bool*)  xpcom/threads/nsThread.cpp:1159  inlined
17  XUL  NS_ProcessPendingEvents(nsIThread*, unsigned int)  xpcom/threads/nsThreadUtils.cpp:445  cfi
18  XUL  nsBaseAppShell::NativeEventCallback()  widget/nsBaseAppShell.cpp:87  inlined
18  XUL  nsAppShell::ProcessGeckoEvents(void*)  widget/cocoa/nsAppShell.mm:534  cfi
19  CoreFoundation  __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__   cfi
20  CoreFoundation  __CFRunLoopDoSource0   cfi
21  CoreFoundation  __CFRunLoopDoSources0   cfi
22  CoreFoundation  __CFRunLoopRun   cfi
23  CoreFoundation  CFRunLoopRunSpecific   cfi
24  HIToolbox  RunCurrentEventLoopInMode   cfi
25  HIToolbox  ReceiveNextEventCommon   cfi
26  HIToolbox  _BlockUntilNextEventMatchingListInModeWithFilter   cfi
27  AppKit  _DPSNextEvent   cfi
28  AppKit  -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]   cfi
29  XUL  -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]  widget/cocoa/nsAppShell.mm:189  cfi
30  AppKit  -[NSApplication run]   cfi
31  XUL  -[GeckoNSApplication run]  widget/cocoa/nsAppShell.mm:173  cfi
32  XUL  nsAppShell::Run()  widget/cocoa/nsAppShell.mm:864  cfi
33  XUL  nsAppStartup::Run()  toolkit/components/startup/nsAppStartup.cpp:291  cfi
34  XUL  XREMain::XRE_mainRun()  toolkit/xre/nsAppRunner.cpp:5893  cfi
35  XUL  XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6138  cfi
36  XUL  XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6211  cfi
37  firefox  do_main(int, char**, char**)  browser/app/nsBrowserApp.cpp:232  inlined
37  firefox  main  browser/app/nsBrowserApp.cpp:464  cfi
38  dyld  start   cfi

These started in FF 140, and seem to have been regressed by bug 1919165. I think they have a very simple solution. I'll say more in later comments.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

These crashes happen because nsCocoaWindow::mNativeLayerRoot is null.

This variable is initialized here. But all the crashes happen here, just before it's initialized. The solution may be as simple as putting [contentView addSubview:mChildView]; after mNativeLayerRoot->SetBackingScale(BackingScaleFactor());.

I'll try this out and let you know my results.

Comment 2

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

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

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

For more information, please visit BugBot documentation.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Potential fix:

diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ -4745,12 +4745,13 @@ nsresult nsCocoaWindow::Create(nsIWidget
       initWithFrame:mWindow.childViewFrameRectForCurrentBounds
          geckoChild:this];
   mChildView.autoresizingMask = NSViewWidthSizable | NSViewHeightSizable;
-  [contentView addSubview:mChildView];
 
   mNativeLayerRoot =
       NativeLayerRootCA::CreateForCALayer(mChildView.rootCALayer);
   mNativeLayerRoot->SetBackingScale(BackingScaleFactor());
 
+  [contentView addSubview:mChildView];
+
   [WindowDataMap.sharedWindowDataMap ensureDataForWindow:mWindow];
 
   NS_ASSERTION(!mTextInputHandler, "mTextInputHandler has already existed");

I tested this briefly in a local build and had no problems. I've started a tryserver build:

https://treeherder.mozilla.org/jobs?repo=try&revision=0ade8f8aee441ca6f4afbfc181c1e40bcd999745

Once it finishes I'll test with it more widely. Then, providing everything looks OK, I'll submit my patch via Phabricator.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Attachment: Bug 1986701 - Link mChildView only after mNativeLayerRoot is initialized. r=mstange

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Thanks Steven! the patch looks sensible to me fwiw.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a96ff7db0b9f
https://hg.mozilla.org/integration/autoland/rev/50d91cd836e6
Link mChildView only after mNativeLayerRoot is initialized. r=mstange

Comment 7

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/50d91cd836e6

Comment 8

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

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

For more information, please visit BugBot documentation.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

ESR140 would be good in addition to Beta.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Attachment: Bug 1986701 - Link mChildView only after mNativeLayerRoot is initialized.

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

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Attachment: Bug 1986701 - Link mChildView only after mNativeLayerRoot is initialized. r=mstange

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

Comment 12

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Moderately frequent crashes
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Not reproducible
• Risk associated with taking this patch: Low
• Explanation of risk level: Patch is simple and straightforward
• String changes made/needed: none
• Is Android affected?: no

Comment 13

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Moderately frequent crashes
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Not reproducible
• Risk associated with taking this patch: Low
• Explanation of risk level: Patch is simple and straightforward
• String changes made/needed: none
• Is Android affected?: no

Comment 14

[Steven Michaud [:smichaud] (Retired)]

Attachment: Bug 1986701 - Link mChildView only after mNativeLayerRoot is initialized. r=mstange

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

Comment 15

[Steven Michaud [:smichaud] (Retired)]

My patch is very simple and straightfoward. It makes sense even without the crashes. So the risk should be low.

Also, there are very few crashes on trunk. So we'll be able to verify my fix much sooner if we uplift it to beta, at least.

I hope I've done the uplift requests correctly. Please let me know if I haven't. I tried first with moz-phab. But the results seemed incorrect, so I tried again with Lando.

Comment 16

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/522836f5b839
https://hg.mozilla.org/releases/mozilla-beta/rev/1db8dc28b77c

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/156d5cbbcf1b

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Here's how to search for this bug's crashes in builds that contain my patch. None so far.

https://crash-stats.mozilla.org/search/?signature=~SuspendOffMainThreadCommits&build_id=%3E%3D20250904092115&version=144.0a1&version=143.0b&version=143.0&version=143.0.1&version=144.0b&platform=Mac%20OS%20X&date=%3E%3D2025-08-19T16%3A21%3A00.000Z&date=%3C2025-09-19T16%3A21%3A00.000Z&_facets=signature&page=1&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports

Comment 19

[Steven Michaud [:smichaud] (Retired)]

It's been almost a month since this bug's patch landed on 143 beta, and there haven't been any crashes since then in builds with the patch. I think it's safe to call my fix VERIFIED.