I did some more research, usually the window creation steps is
1. Create window with some flags
2. Appkit calls displayIfNeeded
3. XUL Parsing, sets some attributes
Internally the function _implicitlyAllowsFullScreenPrimary gets called to decide if the window can be fullscreen. This function in our setup looks like is non-deterministic when _NXIsBackgroundOnly=YES. With identical observable window properties (bgOnly=1, isTitled=1, isResizable=1, isSheet=0, level=0), the function returns result=1 on some calls and result=0 on others, if this behaviour happens inside the first displayIfNeeded calls, it can cause the crash.
From the swizzle trace on the crashing window 0x16593af00:
```
# During initWithContentRect and first displayIfNeeded result=1 (stable)
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
→ ecb=0x8a4 (has FullScreenPrimary 0x80 from implicit path)
# During second displayIfNeeded result FLIPS to 0
FELT_ECB displayIfNeeded.enter window=0x16593af00 cb=0x0 ecb=0x8a4 HAS_PRIMARY
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=0 bgOnly=1 ...
→ ecb changes to 0x80a4 (0x8000 bit added, FullScreenPrimary drops to button heuristic path)
→ _cacheAndSetPropertiesForCollectionBehavior detects change
→ _updateButtons called
→ _NSThemeFullScreenButton added to NSTitlebarView
→ CRASH: array mutated during NSViewUpdateVibrancyForSubtree enumeration
```
I have been trying to trace whether some enterprise code is causing this, I thought it was actually our special LSUIElement setup in [here](https://searchfox.org/enterprise-main/source/toolkit/xre/nsAppRunner.cpp#5292) and [here](https://searchfox.org/enterprise-main/rev/53a1918e52a50baf60bef562928fd31bfcf45d48/toolkit/xre/nsAppRunner.cpp#4527) but turns out without that the issue also shows up.
I am currently checking if setting ```LSUIElement``` could make this unstable.
My proposed change would be to add something similar to this diff below, to prevent to go into the unstable function ```_implicitlyAllowsFullScreenPrimary```
```
diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ @@
} else {
// Non-popup windows are always opaque.
mWindow.opaque = YES;
+
+ if (mWindowType == WindowType::TopLevel &&
+ (features & NSWindowStyleMaskTitled)) {
+ mWindow.collectionBehavior = mWindow.collectionBehavior |
+ NSWindowCollectionBehaviorFullScreenPrimary;
+ }
}
```
Bug 2031249 Comment 17 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
I did some more research, usually the window creation steps are
1. Create window with some flags
2. Appkit calls displayIfNeeded
3. XUL Parsing, sets some attributes
Internally the function _implicitlyAllowsFullScreenPrimary gets called to decide if the window can be fullscreen. This function in our setup looks like is non-deterministic when _NXIsBackgroundOnly=YES. With identical observable window properties (bgOnly=1, isTitled=1, isResizable=1, isSheet=0, level=0), the function returns result=1 on some calls and result=0 on others, if this behaviour happens inside the first displayIfNeeded calls, it can cause the crash.
From the swizzle trace on the crashing window 0x16593af00:
```
# During initWithContentRect and first displayIfNeeded result=1 (stable)
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
→ ecb=0x8a4 (has FullScreenPrimary 0x80 from implicit path)
# During second displayIfNeeded result FLIPS to 0
FELT_ECB displayIfNeeded.enter window=0x16593af00 cb=0x0 ecb=0x8a4 HAS_PRIMARY
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=0 bgOnly=1 ...
→ ecb changes to 0x80a4 (0x8000 bit added, FullScreenPrimary drops to button heuristic path)
→ _cacheAndSetPropertiesForCollectionBehavior detects change
→ _updateButtons called
→ _NSThemeFullScreenButton added to NSTitlebarView
→ CRASH: array mutated during NSViewUpdateVibrancyForSubtree enumeration
```
I have been trying to trace whether some enterprise code is causing this, I thought it was actually our special LSUIElement setup in [here](https://searchfox.org/enterprise-main/source/toolkit/xre/nsAppRunner.cpp#5292) and [here](https://searchfox.org/enterprise-main/rev/53a1918e52a50baf60bef562928fd31bfcf45d48/toolkit/xre/nsAppRunner.cpp#4527) but turns out without that the issue also shows up.
I am currently checking if setting ```LSUIElement``` could make this unstable.
My proposed change would be to add something similar to this diff below, to prevent to go into the unstable function ```_implicitlyAllowsFullScreenPrimary```
```
diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ @@
} else {
// Non-popup windows are always opaque.
mWindow.opaque = YES;
+
+ if (mWindowType == WindowType::TopLevel &&
+ (features & NSWindowStyleMaskTitled)) {
+ mWindow.collectionBehavior = mWindow.collectionBehavior |
+ NSWindowCollectionBehaviorFullScreenPrimary;
+ }
}
```
I did some more research, usually the window creation steps are
1. Create window with some flags
2. Appkit calls displayIfNeeded
3. XUL Parsing, sets some attributes
Internally the function _implicitlyAllowsFullScreenPrimary gets called to decide if the window can be fullscreen. This function in our setup looks like is non-deterministic when _NXIsBackgroundOnly=YES. With identical observable window properties (bgOnly=1, isTitled=1, isResizable=1, isSheet=0, level=0), the function returns result=1 on some calls and result=0 on others, if this behaviour happens inside the first displayIfNeeded calls, it can cause the crash.
From the swizzle trace on the crashing window 0x16593af00:
```
# During initWithContentRect and first displayIfNeeded result=1 (stable)
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=1 bgOnly=1 ...
→ ecb=0x8a4 (has FullScreenPrimary 0x80 from implicit path)
# During second displayIfNeeded result FLIPS to 0
FELT_ECB displayIfNeeded.enter window=0x16593af00 cb=0x0 ecb=0x8a4 HAS_PRIMARY
FELT_IMPLICITLY_ALLOWS_FS_PRIMARY window=0x16593af00 result=0 bgOnly=1 ...
→ ecb changes to 0x80a4 (0x8000 bit added, FullScreenPrimary drops to button heuristic path)
→ _cacheAndSetPropertiesForCollectionBehavior detects change
→ _updateButtons called
→ _NSThemeFullScreenButton added to NSTitlebarView
→ CRASH: array mutated during NSViewUpdateVibrancyForSubtree enumeration
```
I have been trying to trace whether some enterprise code is causing this, I thought it was actually our special LSUIElement setup in [here](https://searchfox.org/enterprise-main/source/toolkit/xre/nsAppRunner.cpp#5292) and [here](https://searchfox.org/enterprise-main/rev/53a1918e52a50baf60bef562928fd31bfcf45d48/toolkit/xre/nsAppRunner.cpp#4527) but turns out without that the issue also shows up.
I am currently checking if setting ```LSUIElement``` could make this unstable.
edit: Just checked and without
```
#ifdef MOZ_ENTERPRISE
<key>LSUIElement</key>
<true/>
#endif
```
the issue also happens.
My proposed change would be to add something similar to this diff below, to prevent to go into the unstable function ```_implicitlyAllowsFullScreenPrimary```
```
diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ @@
} else {
// Non-popup windows are always opaque.
mWindow.opaque = YES;
+
+ if (mWindowType == WindowType::TopLevel &&
+ (features & NSWindowStyleMaskTitled)) {
+ mWindow.collectionBehavior = mWindow.collectionBehavior |
+ NSWindowCollectionBehaviorFullScreenPrimary;
+ }
}
```