fix: calling of X11 functions when running under Wayland#33355
fix: calling of X11 functions when running under Wayland#33355jkleinsc merged 17 commits intoelectron:mainfrom
Conversation
|
💖 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:
Things that will help get your PR across the finish line:
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. |
|
This also addresses #31382 cc @deepak1556 |
dsanders11
left a comment
There was a problem hiding this comment.
Added some comments. I haven't tested this, just adding some suggested changes.
[1]: clean e init --bootstrap testing followed by e test I failing for me with Running "ninja -j 200 node:headers" in /home/p2004a/Workspace/electron/e/src/out/Testing ninja: error: unknown target 'node:headers', did you mean 'node_headers_1'?, I can't make it work :(, I was testing functions with my own draft app. I hope CI run as part of this PR will handle this omission.
Thanks for mentioning that! Fixing it over in electron/build-tools#351.
|
I've rebased the branch because there was a merge conflict. Thank you @dsanders11 for taking a look and fixing build tools! I've run the tests and I've got exactly the same result on the |
deepak1556
left a comment
There was a problem hiding this comment.
Thanks for working on this, really appreciate it!
Reviews are based on first pass on the PR,
-
Can you also remove the
USE_X11define fromLine 562 in 8ad1470
-
Can you apply the changes from https://chromium-review.googlesource.com/c/chromium/src/+/2289772/ and https://chromium-review.googlesource.com/c/chromium/src/+/2317185 to
GlobalMenuBarX11which better aligns with upstream. Should end up in a state similar to https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/BUILD.gn;l=3873-3882
Done, I also had to cleanup usage in
I would prefer to not do it as part of this PR, there are many more places where X11 code needs to dropped and replaced with proper ozone abstractions. |
deepak1556
left a comment
There was a problem hiding this comment.
@p2004a can you rebase to resolve the conflicts, thanks!
|
@deepak1556 has manually backported this PR to "18-x-y", please check out #33498 |
|
@deepak1556 has manually backported this PR to "17-x-y", please check out #33499 |
|
ISTM this still has to be backported to 16.x.x |
|
The PR has not been backported to |
) * fix: don't call X11 functions in file dialog and message box * refactor: remove unused GtkUiPlatform declaration * fix: set gtk darktheme only when running under X11 * fix: replace X11 window state watcher with implementation using ozone * fix: make sure global menu barr is used only when supported * fix: don't call X11 function in native window views under wayland * style: fix lint issues * fix: use GtkUiPlatform::ShowGtkWindow instead of gtk_window_present directly * refactor: extract CreateGlobalMenuBar into separate function * refactor: move checking for WaylandWindowDecorations inside class * fix: check if we run under X11 only in ozone build * refactor: drop including unused ui/base/ui_base_features.h header * fix: modify ui_gtk_public_header.patch to also export gtk_ui.h * fix: refactor guarding of X11 calls - Introduce patch exposing new electron_can_call_x11 property - Replace defined(USE_OZONE) with BUILDFLAG(OZONE_PLATFORM_X11) flags * fix: remove the last remaining usage of USE_X11 * fix: usage of BUILDFLAG(OZONE_PLATFORM_X11) not building on non ozone * fix: call UpdateWindowState from OnBoundsChanged only under X11
) * fix: don't call X11 functions in file dialog and message box * refactor: remove unused GtkUiPlatform declaration * fix: set gtk darktheme only when running under X11 * fix: replace X11 window state watcher with implementation using ozone * fix: make sure global menu barr is used only when supported * fix: don't call X11 function in native window views under wayland * style: fix lint issues * fix: use GtkUiPlatform::ShowGtkWindow instead of gtk_window_present directly * refactor: extract CreateGlobalMenuBar into separate function * refactor: move checking for WaylandWindowDecorations inside class * fix: check if we run under X11 only in ozone build * refactor: drop including unused ui/base/ui_base_features.h header * fix: modify ui_gtk_public_header.patch to also export gtk_ui.h * fix: refactor guarding of X11 calls - Introduce patch exposing new electron_can_call_x11 property - Replace defined(USE_OZONE) with BUILDFLAG(OZONE_PLATFORM_X11) flags * fix: remove the last remaining usage of USE_X11 * fix: usage of BUILDFLAG(OZONE_PLATFORM_X11) not building on non ozone * fix: call UpdateWindowState from OnBoundsChanged only under X11
Description of Change
TLDR: I primarily try to address the #33325 and #32436 issues with this PR.
In #30814 there was fe40088 committed that removed all
if (!features::IsUsingOzonePlatform()) {blocks. Those blocks were guarding calls to X11 functions under wayland and were added as part of #26022. Calling X11 functions under wayland can cause the application to crash (as visible in #33325 and #32436).This PR:
Replaces
USE_X11withUSE_OZONEblocks as ozone is the only build of chrome nowadays (Ref: [Bug]: segfault on startup at gpu_init.cc under native Wayland #32436 (comment)) and only call X11 code only when running under X11. I believe the functionality hidden on non-x11 platform is not critical and this is overall net positive to current state.Replaces
WindowStateWatcherwith ozone native implementation inElectronDesktopWindowTreeHostLinux: it is always instantiated under ozone, but the wayland decorations remain guarded by theWaylandWindowDecorationsfeature flag (As was added in fix: Add support for Wayland window decorations #29618).This makes the
maximize,enter-full-screen, etc. events ofBrowserWindowwork under wayland.Eliminates the X11 calls in file and message dialogs. Frankly I'm not sure if call to
gtk_window_presentis needed at all: under my system it's noop, but maybe there are DEs where it matters, so keeping it.Checklist
npm testpasses: kind ofI can't make the tests even start running [1]Release Notes
notes: Fixed multiple issues when running under Wayland caused by calling X11 functions.
[1]: cleane init --bootstrap testingfollowed bye testI failing for me withRunning "ninja -j 200 node:headers" in /home/p2004a/Workspace/electron/e/src/out/Testing ninja: error: unknown target 'node:headers', did you mean 'node_headers_1'?, I can't make it work :(, I was testing functions with my own draft app. I hope CI run as part of this PR will handle this omission.