💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

• fix: don't overwrite prevent_default if default wasn't prevented
• feat: add app.isPackaged() method
• docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

• Follow the JavaScript, C++, and Python coding style.
• Run npm run lint locally to catch formatting errors earlier.
• Document any user-facing changes you've made following the documentation styleguide.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codebytere Seems this is not very compatible with #41769. How can we handle drop shadow when WCO is enabled?

Update: I've added support thickFrame to control whether to show window shadow on Wayland.

@codebytere I've added an implementation of GTK native WCO in 6ece5db. Would you plz have a look at it? IMO it looks more integrated with system look & feel.

PS: I copied some code from your implementation. Please don't mind that :)

BTW I found hasShadow has no effect on both X11 and Wayland. On main branch, X11 will always show shadow and Wayland will not despite the setting of hasShadow. I'm not sure if GNOME is to blame or it's a regression.

@ruihe774 Thanks so much for implementing this, this is amazing work! Could I ask you to rebase this PR against main so we can get the tests running green again? 🙂 We have another PR that is about to land (#41769) - if you wouldn't mind rebasing after that PR lands, that would be great! Also more than happy to work with you to resolve that once it lands.

We have another PR that is about to land (#41769) - if you wouldn't mind rebasing after that PR lands, that would be great!

@VerteDinde As I've pointed out earlier, I'm afraid that these two PRs are not very compatible, in that they are implementing WCO in different ways. I'd like to have a discussion about how we can deal with it, but the OP of that PR has not responded.

Could I ask you to rebase this PR against main so we can get the tests running green again?

I've done rebasing. The build fails for Windows. I'll fix it later.

@ruihe774 after some discussion myself and Deepak have come to the tentative conclusion that it's likely more
expedient in the short term to try to land my initial PR for x11. This PR combines two discrete goals:

1. window shadow for frameless window on Wayland
2. WCO on Linux

which should be split into two PRs. I understand my PR adds some duplication and extra code, but the longer-term plan would be to combine the two classes where possible. At the moment, the need for this feature outweighs the cost of some medium-term duplication. The other issue is that this class is more than two years old, and is notably lesser tested owing to its being Wayland-only thus far and being an opt-in feature even beyond that. I agree with @deepak1556's comment over on my PR that we should look to DRY out the code, but the severe drift between these changes and upstream leaves me concerned about the ability to accomplish the intended goal of prod use of WCO on linux without potentially introducing a host of new bugs from the drift. We'd like to continue working with you to land this code - in the short term, could you split out the window shadow changes?

@codebytere @deepak1556

its being Wayland-only thus far

Well, ClientFrameView can work on X11, though it is not the default.

accomplish the intended goal of prod use of WCO on linux

I wonder what is the long-term solution to implement WCO: draw by ourselves (what you've implemented), or draw by GTK (my implementation)?

could you split out the window shadow changes?

I'm OK for that. But there's still question: how can I add window shadow to your OpaqueFrameView? Do you want to implement window shadow for OpaqueFrameView yourself? BTW, giving there's no GTK drawing logic in OpaqueFrameView, I think it's hard to implement a consistent window shadow with it.

FWIW, I think we'd better pause the merge of #41769 until we come up with a consensus.

draw by GTK (my implementation)?

I think this what we want for WCO so that controls can adjust properly to the system theme. I guess the question now is the reliability of the implementations,

• ClientFrameViewLinux which has only been an opt-in on Wayland but uses gtk to draw window controls
• OpaqueFrameView which is mostly adjusted for Chrome/ChromeOS UI but has been used for X11 quite sometime in upstream. @codebytere how confident are we in removing the Chrome specific changes and have parity with FramelessView + WCO support integrated ?

Would like some more opinion from linux experts /cc @ckerr

FWIW, if WCO drawed by GTK is the ultimate goal, my another concern about #41769 is that after it is merged, the users (I mean, app developers) will get used to the self-drawing WCO, and we may break things when we ultimately switch to GTK-drawing WCO.

To be clear, I'm not against unifying code and OpaqueFrameView. I think a tentative solution is to get OpaqueFrameView merged but disable its WCO support. However, we still need to come up with a way to draw window shadow with OpaqueFrameView.

I don't think it's a good idea to use GTK just to draw the window controls. GTK does not offer stable APIs for drawing window frames and controls to external surfaces, so Chromium has to use lots of workarounds to make it happen. ClientFrameViewLinux already breaks quite often between Chromium version upgrades, even though it mostly copies code from Chromium. Chromium itself does not use GTK to draw window controls in WCO mode, so it would require more fragile code to support titleBarOverlay options such as color and symbolColor.

Supporting arbitrary GTK themes is a lot of work even when using GTK directly and GNOME developers have given up on it. When you add in the need to support overriding colors, it would not be worth it anymore. I think the best path forward for a more native look and feel is to pick a particular GTK theme like Adwaita or Yaru and use its window control icons in OpaqueFrameView. This should be much simpler, it's just a matter of converting SVGs to Chromium's custom .icon format and using the new icons in OpaqueFrameView.

OpaqueFrameView is also probably a better foundation to deduplicate the window frame drawing code as OpaqueBrowserFrameView is a common base class in Chromium. Window shadows in borderless mode can be supported the same way as BrowserFrameViewLinux. ClientFrameViewLinux can be refactored to use OpaqueFrameView by following the same pattern as BrowserFrameViewLinuxNative. This should also make it easier to keep ClientFrameViewLinux aligned with upstream.

I don't think it's a good idea to use GTK just to draw the window controls. GTK does not offer stable APIs for drawing window frames and controls to external surfaces, so Chromium has to use lots of workarounds to make it happen.

I shall make it clear that it is not GTK that is actually drawing window frames. The actual stuff behind the scene is that WindowFrameProviderGtk obtains bitmaps from GTK and draw it by itself. Although there are workarounds, it work fine for now.

so it would require more fragile code to support titleBarOverlay options such as color and symbolColor.

Electron on macOS does not support color and symbolColor, either. I don't think it is a thing that needs to be considered for now.

color is the background of WCO, which, on Linux and macOS, are always transparent. App developers can easily put background underneath. This is also what they do on macOS.

Supporting arbitrary GTK themes is a lot of work even when using GTK directly and GNOME developers have given up on it. When you add in the need to support overriding colors, it would not be worth it anymore.

"Supporting arbitrary GTK themes" is not my goal in this PR.

I think the best path forward for a more native look and feel is to pick a particular GTK theme like Adwaita or Yaru and use its window control icons in OpaqueFrameView. This should be much simpler, it's just a matter of converting SVGs to Chromium's custom .icon format and using the new icons in OpaqueFrameView.

I cannot agree with "this should be much simpler". Anyway, this should be done for OpaqueFrameView. Before that, I think the immature implementation of WCO in #41769 should not be shipped.

IMO, I think this is acceptable:

• Merge OpaqueFrameView, disabling its WCO support.
• Use my implementation of window shadow in ClientFrameView as a transitional solution.
• Use my WCO implementation in ClientFrameView as a transitional solution (optional).
• Implement a self-drawing native look & feel WCO and window shadow in OpaqueFrameView as the long-term solution and switch to it after it is finished.

Having read both PRs, both look well-written. And any volunteer who has the interest, skill, and patience to dig into Electron's + Chromium's codebase and patching system is someone we should encourage! So @ruihe774 however this gets resolved, I hope you feel welcome to stick around and submit future PRs.

Right now I am leaning more towards 41769, not because of any issues with the code in the PR, but more out of uncertainty in our pre-existing Wayland codebase and the way that 41769 builds on Chrome code instead. This is a bigger-picture problem than either PR; as a project we should improve our Wayland testing.

If there are other maintainers who feel strongly about either PR, TBH I could be talked into either one.

Right now I am leaning more towards 41769. This is a bigger-picture problem than either PR; as a project we should improve our Wayland testing.

I am also leaning towards the idea of 41769; but not its implementation of WCO. The self-drawing WCO looks alien in Linux platform. I'd prefer a WCO with native look & feel. For forward compatibility, I am against the shipping of the WCO implementation in 41769.

@ruihe774 revisiting the WCO story, given our custom titlebar implementation matches Chrome it seems better to borrow the WCO implementation. Although the rendering is not native it seems to have worked well for upstream, so I am in favor of merging #41769.

This PR addressing the window shadow issue should still be accepted, if you can isolate the change that would be great.

@ruihe774 revisiting the WCO story, given our custom titlebar implementation matches Chrome it seems better to borrow the WCO implementation. Although the rendering is not native it seems to have worked well for upstream, so I am in favor of merging #41769.

I'm happy to see a WCO implementation is landed.

This PR addressing the window shadow issue should still be accepted, if you can isolate the change that would be great.

Maybe I can try later.