Investigate WS_THICKFRAME impact when creating frameless windows

[deepak1556]

Continuation of #146683

Current status of investigation

The upstream Electron issue is at electron/electron#30760.
The issue was introduced in Electron v14.0.0-beta.5 and the commit that introduced the issue is electron/electron@172ac25.
The PR electron/electron#35189 fixes the issue where frameless resizable windows show Windows 7 frames during startup, but there's a larger issue that is still unresolved even with that PR. I also searched through the thousands of Chromium commits associated with that Electron commit to see whether they had anything to do with the issue, but I didn't find anything relevant even after reverting several suspicious commits.

Minimal repro

const { app, BrowserWindow } = require('electron')
function createWindow() {
  new BrowserWindow({
    width: 800,
    height: 600,
    frame: false
  })
}
app.whenReady().then(() => {
  createWindow()
})
app.on('window-all-closed', function () {
  app.quit()
})

Running <path-to-electron-exe> <path-to-js-file-above> with Electron v14.0.0-beta.5 or newer on Windows 10 reproduces the issue.

Going through the Chromium code with the PR fix for frameless resizable windows

The PR electron/electron#35189 fixes the issue for frameless resizable windows by calling set_can_resize() at an earlier stage of program initialization. Assume the window is frameless and resizable. Then, the program sets can_resize to true earlier in widget_delegate.cc, and the following occurs:

1. widget_hwnd_utils.cc:68 is not run, meaning WS_THICKFRAME is not removed from the style set in Line 62. This style parameter is passed in to a CreateWindowEx call at window_impl.cc:213.
2. hwnd_message_handler.cc:1029 is not run. Instead, the if block above is run, which adds a WS_THICKFRAME style to the style passed into the SetWindowLong call.
3. Somehow, because of these WS_THICKFRAME styles being passed in, after the window is shown, we get a single WM_DWMNCRENDERINGCHANGED message, and no WM_NCPAINT messages after. NC in this case refers to non-client area, which is the frame. DWM stands for desktop windows manager.

After adding a long setTimeout of 30 seconds to the createWindow call above, I was able to use Spy++ to log messages to that window and see the following:

<000157> 0000000000170B9C R WM_WINDOWPOSCHANGING
<000158> 0000000000170B9C S WM_GETICON nType:ICON_BIG
<000159> 0000000000170B9C R WM_GETICON hicon:00000000
<000160> 0000000000170B9C P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000161> 0000000000170B9C S WM_GETICON nType:ICON_SMALL2
<000162> 0000000000170B9C R WM_GETICON hicon:00000000
<000163> 0000000000170B9C P message:0x0400 [User-defined:WM_USER+0] wParam:00000003 lParam:00000000
<000164> 0000000000170B9C S WM_GETICON nType:ICON_SMALL
<000165> 0000000000170B9C R WM_GETICON hicon:00000000
<000166> 0000000000170B9C P message:0xC112 [Registered:"TaskbarButtonCreated"] wParam:00000000 lParam:00000000
<000167> 0000000000170B9C S WM_GETMINMAXINFO lpmmi:0000002354FFE8F0
<000168> 0000000000170B9C R WM_GETMINMAXINFO lpmmi:0000002354FFE8F0
<000169> 0000000000170B9C P WM_PAINT hdc:00000000

I have also attached full logs to the end of this comment.

The PR doesn't fix the case of frameless non-resizable windows

For frameless windows that cannot resize, even if we call set_can_resize() earlier, because the program would pass in false, for the CreateWindowEx and SetWindowLong calls above, the WS_THICKFRAME style would not be in the styles sent to the calls. As a result, what I see in Spy++ is WM_DWMNCRENDERINGCHANGED being toggled three times, and several WM_WINDOWPOSCHANGING calls that result in WM_NCPAINT messages being emitted. Using Spy++, I confirmed that the WM_WINDOWPOSCHANGING calls have the flag SWP_FRAMECHANGED set.

<000174> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000175> 0000000000130AAA S WM_GETICON nType:ICON_SMALL2
<000176> 0000000000130AAA R WM_GETICON hicon:00000000
<000177> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:False
<000178> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFE280
<000179> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000180> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFE250
<000181> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFE250
<000182> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000183> 0000000000130AAA R WM_NCPAINT
<000184> 0000000000130AAA S WM_ERASEBKGND hdc:5B01182B
<000185> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000186> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFE280
<000187> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFDB20
<000188> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000189> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFDAF0
<000190> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFDAF0
<000191> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000192> 0000000000130AAA R WM_NCPAINT
<000193> 0000000000130AAA S WM_ERASEBKGND hdc:FFFFFFFF87011810
<000194> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000195> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFDB20
<000196> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000197> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000198> 0000000000130AAA S WM_GETICON nType:ICON_SMALL
<000199> 0000000000130AAA R WM_GETICON hicon:00000000
<000200> 0000000000130AAA P message:0x0400 [User-defined:WM_USER+0] wParam:00000003 lParam:00000000
<000201> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000202> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFE280
<000203> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000204> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFE250
<000205> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFE250
<000206> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000207> 0000000000130AAA R WM_NCPAINT
<000208> 0000000000130AAA S WM_ERASEBKGND hdc:010118D7
<000209> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000210> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFE280
<000211> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFDB20
<000212> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000213> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFDAF0
<000214> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFDAF0
<000215> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000216> 0000000000130AAA R WM_NCPAINT
<000217> 0000000000130AAA S WM_ERASEBKGND hdc:5B01182B
<000218> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000219> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFDB20
<000220> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000221> 0000000000130AAA R WM_WINDOWPOSCHANGED

More information

I have searched the Chromium and Electron codebases for WS_THICKFRAME and SWP_FRAMECHANGED (as well as SWP_DRAWFRAME) to see why the frame might be rendering more often when WS_THICKFRAME is not passed in. However, I haven't found anything within the codebase itself.

Searching Windows documentation, I found the following:

• WS_THICKFRAME causes WM_GETMINMAXINFO to be sent. Reference: https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-windowposchanging. However, I'm not sure whether this could explain the difference in behaviour between resizable and non-resizable windows.
• SetWindowPos can pass in SWP_FRAMECHANGED or SWP_DRAWFRAME to send NC_CALCSIZE to the window, which is why I searched for those two constants within the codebase. Reference: https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowpos. Also, Chromium handles NC_CALCSIZE by returning WVR_REDRAW at hwnd_message_handler.cc:2353, which I assume causes the WM_NCPAINT message to occur.

Also, Electron calls SetWindowLong at native_window_views.cc:350. I noticed that if I don't call SetWindowLong there, the Windows 7 frame doesn't show up, but then the Electron window doesn't render with the proper style (see electron/electron#32981). I also noticed that when the Windows 7 frame shows up, it has the style of whatever was passed into that specific SetWindowLong call. For example, if I add WS_SYSMENU to the call, then for a second I see a Windows 7 frame with WS_SYSMENU applied.

I also tried creating a minimal project in VS to reproduce the issue but wasn't successful. The frame that rendered seemed to have a Windows 10 style rather than a Windows 7 one.

I also tried registering a listener in hwnd_message_handler.cc for the WM_DWMNCRENDERINGCHANGED messages, and it did seem to get hit, though I couldn't do anything with it. There's also the mysterious ScopedRedrawLock, but I wasn't able to apply it to resolve the issue.

Attached log files

A text file showing the good case where WM_DWMNCRENDERINGCHANGED is only called once. The window is frameless and resizable.
A text file showing the bad case where WM_DWMNCRENDERINGCHANGED is called three times. The window is frameless and not resizable.

[deepak1556]

More logging

I added some lines to log the hwnd and style passed into each relevant CreateWindowEx and SetWindowLong call. One example of the styles that show up:

[22824:0812/111914.541:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000060218 style 06cc0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_CAPTION | WS_THICKFRAME | WS_SYSMENU
[22824:0812/111914.541:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000060218 ex_style 0
[22824:0812/111914.548:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 060e0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_THICKFRAME | WS_SYSMENU | WS_MINIMIZEBOX
[22824:0812/111914.549:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 060a0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_SYSMENU | WS_MINIMIZEBOX
[22824:0812/111914.549:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000060218 style 00c30000
WS_CAPTION | WS_MINIMIZEBOX | WS_MAXIMIZEBOX
[22824:0812/111914.601:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 04c20000
WS_CLIPSIBLINGS | WS_CAPTION | WS_MINIMIZEBOX

There's also another window being created really early, but I don't think it influences the issue.

[24748:0812/115319.325:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 000000000029053C style 02cf0000
[24748:0812/115319.325:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 000000000029053C ex_style 0
WS_CLIPCHILDREN | WS_CAPTION | WS_THICKFRAME | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX

Good scenario (meaning no frame showing) with can_resize set to true and staying true

[4124:0812/124902.900:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000000C0BC8 style 2cf0000
[4124:0812/124902.900:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000000C0BC8 ex_style 0
[4124:0812/124902.996:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000060D16 style 6cc0000
[4124:0812/124902.996:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000060D16 ex_style 0
[4124:0812/124903.007:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060D16 style 60f0000
[4124:0812/124903.009:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000060D16 style c70000
[4124:0812/124903.065:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060D16 style 4c70000

Bad scenario with can_resize set to true then false

[7740:0812/124938.950:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000003A0240 style 2cf0000
[7740:0812/124938.950:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000003A0240 ex_style 0
[7740:0812/124939.033:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000740736 style 6cc0000
[7740:0812/124939.033:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000740736 ex_style 0
[7740:0812/124939.041:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 60e0000
[7740:0812/124939.043:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 60a0000
[7740:0812/124939.044:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000740736 style c30000
[7740:0812/124939.099:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 4c20000

Bad scenario with can_resize set to false and staying false

[6636:0812/125425.559:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000230986 style 2cf0000
[6636:0812/125425.559:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000230986 ex_style 0
[6636:0812/125425.662:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000001E0BBC style 6c80000
[6636:0812/125425.662:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000001E0BBC ex_style 0
[6636:0812/125425.668:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 00000000001E0BBC style 60a0000
[6636:0812/125425.671:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 00000000001E0BBC style c30000
[6636:0812/125425.730:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 00000000001E0BBC style 4c20000

The SetWindowLong call with style 60a0000 causes the Windows 7 frame to render, and its style corresponds to WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_SYSMENU | WS_MINIMIZEBOX
60e0000 is actually the same style just with WS_THICKFRAME, once again showing how mysterious the WS_THICKFRAME constant is.

Delaying the SetCanResize call

While working on electron/electron#35189, I also noticed that changing the order of the calls seemed to fix the issue for frameless non-resizable windows. In particular, on the Electron side, in native_window_views.cc, if I first called set_can_resize(true) to pretend that the window is resizable, and then called SetCanResize(resizable_) (where resizable_ = false) after the SetWindowLong call around line 365, then the issue is resolved even for frameless non-resizable windows. However, this workaround delays the SetCanResize call to hundreds of lines later, and I wouldn't be surprised if it would cause a regression elsewhere.

Another log file for the scenario above.

Just like in the log file of the good case of the previous comment, WM_DWMNCRENDERINGCHANGED only shows up once, rather than three times.

For completeness, here are the logs with the styles for this scenario, excluding the CreateWindowEx logs:

[11236:0812/145829.596:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 60e0000
[11236:0812/145829.601:ERROR:native_window_views.cc(367)] native_window_views.cc SetWindowLong hwnd 0000000000830BCA style c30000
[11236:0812/145829.601:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 4c20000
[11236:0812/145829.666:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 4c20000

In this case, the style 60a0000 never comes up!

I also tried delaying the SetCanResize(resizable_) call and calling set_can_resize(resizable_) instead of set_can_resize(true), but that doesn't work because it fails to pass WS_THICKFRAME to the CreateWindowEx call as well as the first SetWindowLong call, both of which need WS_THICKFRAME for the frame not to render:

[26344:0812/150627.310:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000170B7C style 60a0000
[26344:0812/150627.312:ERROR:native_window_views.cc(367)] native_window_views.cc SetWindowLong hwnd 0000000000170B7C style c30000
[26344:0812/150627.359:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000170B7C style 4c20000

A note on DwmSetAttribute

DwmSetAttribute can be used to change the non-client (NC) rendering policy and hence result in a WM_DWMNCRENDERINGCHANGED call. However, I did not find any references to DwmSetAttribute within the Electron and Chromium codebases that were actually called when running my minimal repro.

[rzhao271]

In Visual Studio, I got an older-looking frame to show up by calling SetWindowLong with certain styles before calling ShowWindow.

The older-looking frame kept showing up until I added WS_CAPTION. Even adding WS_BORDER didn't help.

I also noticed that 0x60a0000 included the WS_SYSMENU style. Based on https://docs.microsoft.com/en-us/windows/win32/winmsg/window-styles, when the WS_SYSMENU style is specified, the WS_CAPTION style must also be specified. However, adding WS_CAPTION to the style didn't fix the issue in Electron.

[rzhao271]

@deepak1556 thought it was strange that the styles kept changing, whereas I just took the styles changing for granted other than the 0x06e0000 style going to 0x06a0000 (which is where WS_THICKFRAME is removed from the styling due to resizable being false).

I annotated an example of the styles changing below:

Resizable = false
[22816:0816/155129.261:ERROR:window_impl.cc(211)] CreateWindow style 6c80000
[22816:0816/155129.265:ERROR:hwnd_message_handler.cc(1732)] Removing caption from 6c80000
[22816:0816/155129.282:ERROR:hwnd_message_handler.cc(1029)] HwndMessageHandler SetWindowLong old style 6080000
[22816:0816/155129.282:ERROR:hwnd_message_handler.cc(1044)] HwndMessageHandler SetWindowLong new style 60a0000 <- added 20000 = WS_MINIMIZEBOX
[22816:0816/155129.287:ERROR:native_window_views.cc(367)] native_window_views c30000
[22816:0816/155129.381:ERROR:hwnd_message_handler.cc(1029)] HwndMessageHandler SetWindowLong old style 4c30000 <- something added 4000000 = WS_CLIPSIBLINGS
[22816:0816/155129.381:ERROR:hwnd_message_handler.cc(1044)] HwndMessageHandler SetWindowLong new style 4c20000 <- removed 10000 = WS_MAXIMIZEBOX

It turns out the WS_CAPTION style is removed from the Window in HWNDMessageHandler::OnCreate, just for Electron to add it back later. Even though the style is applied early, it turns out that commenting out that if statement seems to fix the issue for both resizable and non-resizable frameless windows, even without setting can_resize to true earlier.

That if statement arises due to https://codereview.chromium.org/9372053/, with a comment that states that "[The if statement] removes the unsightly close button visible in the frame during startup and resizing". I still have to check whether commenting out that code causes that glitch to show up again, considering Electron has its own style that it applies to the window anyway. For non-resizable windows, I haven't seen any regression. I still have to test the change for resizable windows, though. The code review also links to images, but those links are dead, so I'm unable to see what the glitchy behaviour looked like.

Edit: we hit that if statement in the first place due to remove_standard_frame being set. Some code pointers:
Setting remove_standard_frame in Electron https://github.com/electron/electron/blob/1c0f1180995df357e199dcb99e6cb434f6daa32e/shell/browser/native_window_views.cc#L248
Initializing remove_standard_frame_ in Chromium https://source.chromium.org/chromium/chromium/src/+/main:ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc;l=187
Referencing remove_standard_frame_ https://source.chromium.org/chromium/chromium/src/+/main:ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc;l=763

[rzhao271]

Upstream: electron/electron#30024

[rzhao271]

TODO:

• Merge the upstream PR into Electron and backport it
• VS Code version bump to take in the new patch

Verification steps if needed:

1. Launch VS Code on Windows
2. Confirm that a Windows 7 frame doesn't show up during launch

Note that #146683 already gets rid of the Windows 7 frame showing up during launch, but this issue tracks an alternative solution for Electron that is simpler and applicable to a larger class of frameless windows.

[deepak1556]

This is now addressed.