New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: cherry-pick dd4c3ddadbb9 from chromium #32603
chore: cherry-pick dd4c3ddadbb9 from chromium #32603
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. |
Those patches are required to fix Ozone/Wayland on newer Wayland compositors. The previous behaviour is incorrect but worked as long as Wayland compositors didn't implement newer protocol versions that aren't supported yet by Ozone/Wayland (e.g. wl_output version 4). Fix electron#32487. Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574 Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
cbbcf18
to
c3d730f
Compare
Ok, so at least the Linux builds are passing now (the rest of the builds is still running). As for testing: With Electron version 16.0.7 (https://github.com/electron/electron/releases/download/v16.0.7/electron-v16.0.7-linux-x64.zip) it crashes as expected: $ electron --enable-features=UseOzonePlatform --ozone-platform=wayland https://github.com/electron/electron/releases/tag/v16.0.7
interface 'wl_output' has no event 4
^C
$ echo $?
130 And with the build artifact for this PR (https://app.circleci.com/pipelines/github/electron/electron/48705/workflows/e2e1e0d2-d04a-49df-9a44-173baef7f4ad/jobs/1107659/artifacts -> https://1107659-9384267-gh.circle-artifacts.com/0/dist.zip) I get past that but it still fails: $ result/bin/electron --enable-features=UseOzonePlatform --ozone-platform=wayland https://github.com/electron/electron/releases/tag/v16.0.7
[6798:0124/214016.725787:ERROR:power_monitor_device_source_stub.cc(11)] Not implemented reached in virtual bool base::PowerMonitorDeviceSource::IsOnBatteryPower()
[6798:0124/214016.966095:ERROR:wayland_connection.cc(611)] Not implemented reached in static void ui::WaylandConnection::Global(void *, wl_registry *, uint32_t, const char *, uint32_t)
[6798:0124/214017.359549:ERROR:cursor_loader.cc(114)] Failed to load a platform cursor of type kNull
[6798:0124/214017.360234:ERROR:wayland_window.cc(394)] Not implemented reached in virtual void ui::WaylandWindow::SetWindowIcons(const gfx::ImageSkia &, const gfx::ImageSkia &)
[6832:0124/214017.441688:ERROR:egl_util.cc(74)] Failed to load GLES library: libGLESv2.so.2: libGLESv2.so.2: cannot open shared object file: No such file or directory
[6832:0124/214017.449296:ERROR:viz_main_impl.cc(161)] Exiting GPU process due to errors during initialization
[6863:0124/214017.578882:ERROR:egl_util.cc(74)] Failed to load GLES library: libGLESv2.so.2: libGLESv2.so.2: cannot open shared object file: No such file or directory
[6863:0124/214017.590378:ERROR:viz_main_impl.cc(161)] Exiting GPU process due to errors during initialization
[6893:0124/214017.829907:ERROR:sandbox_linux.cc(376)] InitializeSandbox() called with multiple threads in process gpu-process.
[6893:0124/214017.843207:ERROR:wayland_canvas_surface.cc(244)] Not implemented reached in virtual std::unique_ptr<gfx::VSyncProvider> ui::WaylandCanvasSurface::CreateVSyncProvider()
[6798:0124/214042.334603:ERROR:object_proxy.cc(642)] Failed to call method: org.freedesktop.DBus.Properties.Get: object_path= /org/freedesktop/portal/desktop: org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.
[6798:0124/214042.334646:ERROR:select_file_dialog_impl_portal.cc(243)] Failed to read portal version property
^C[6798:0124/214116.236338:ERROR:wayland_surface.cc(257)] Not implemented reached in zwp_linux_surface_synchronization_v1 *ui::WaylandSurface::GetSurfaceSync()
[6798:0124/214116.303097:ERROR:process_posix.cc(330)] Unable to terminate process 6839: No such process (3) The |
@primeos I applied your patch into my environment to test, and I also see the I then addressed the GLES error by adding libglvnd into my environment, but it still segfaults. This is sitting on my fork of nixpkgs here: smasher164/nixpkgs@ac7d1eb. Here is the output after applying this patch and adding libglvnd:
|
I'm working on a backport for electron 13. OK, maybe mapping it readonly is not enough. It also expects that we map it with MAP_PRIVATE which needs some change in the code base. |
The segmentation fault we get confuses me:
I guess it has something to do with the build arguments :( |
Thanks for the testing, suggestions, etc. Unfortunately I don't have much time and motivation for this so if someone wants to take over that'd be welcome (feel free to open a new PR based on this one). @smasher164 thanks for testing this PR. I coincidentally also tested it with Sway on NixOS with pretty much the same approach (I didn't override @Molytho IIRC I once saw that Chromium issue regarding |
Yes it's this issue. Looks like it is fix with later versions of chromium. The difference between 2 and 3 is: Looks like Arch also suffered from this segfaults and fix it by building without -fstack-clash-protection |
@primeos is this PR still relevant? |
I think it is, see #33355 (comment) Btw, hi! :D |
Yes, if it's a goal to fix Ozone/Wayland for Electron 16 then this PR is still required (it fixes #32487). #33355 is the missing part to fix the rest of the issues/regressions. I've managed to stop using Electron since the Ozone/Wayland support regressed so I'm not really motivated to backport the other PR as well. Does anyone have time and motivation to take this over? |
Both Electron 15 and 16 are going to be discontinued when Electron 19 is released in May. This PR needs to be fixed and merged before that happens, otherwise it will just be abandoned as obsolete. |
Hey all! We're going to be releasing our final 15-x-y and 16-x-y Electron builds this morning - since it looks like tests are still failing here, and it hasn't been touched in some time, I'm so sorry, but I'm going to close this PR. Happy to accept fixes and changes to later Electron versions if the issue persists! |
Description of Change
Those patches are required to fix Ozone/Wayland on newer Wayland
compositors. The previous behaviour is incorrect but worked as long as
Wayland compositors didn't implement newer protocol versions that aren't
supported yet by Ozone/Wayland (e.g. wl_output version 4).
Fix #32487.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574
Co-authored-by: Jeremy Rose nornagon@nornagon.net
Merge to M98: [linux/wayland] Fixed terminate caused by binding to...
...wrong version.
The Ozone/Wayland implementation had a few places where the Wayland
objects were bound without proper checking for their versions. That was
part of the technical debt not addressed before, and ended up causing
the issue explained in the linked crbug: the compositor terminates the
client that binds to the protocol that it does not actually support.
This patch fixes the issue by adding the necessary checks in all places
where they were missing. Also a convenience macro for validating the
version is proposed.
(cherry picked from commit dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d)
Bug: 1279574
Change-Id: I74efa97f64b480bed47372d8d559593ae84eeb18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3337037
Reviewed-by: Maksim Sisov msisov@igalia.com
Commit-Queue: Alexander Dunaev adunaev@igalia.com
Cr-Original-Commit-Position: refs/heads/main@{#951428}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3402123
Cr-Commit-Position: refs/branch-heads/4758@{#772}
Cr-Branched-From: 4a2cf4baf90326df19c3ee70ff987960d59a386e-refs/heads/main@{#950365}
Notes: Backported fix for 1279574.