Repository: electron/electron (language: cpp)

Issue:
[Backport]: interface 'wl_output' has no event 4
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

16.0.6

### What operating system are you using?

Other Linux

### Operating System Version

NixOS 21.11

### What arch are you using?

x64

### Last Known Working Electron version

None

### Expected Behavior

When electron is started using the ozone wayland platform it does not crash.

### Actual Behavior

Under the latest wlroots or sway release, any electron app crashes at start with the error message in the title.

### Testcase Gist URL

_No response_

### Additional Information

This is an upstream issue in chromium and fixed with the [following commit](https://chromium.googlesource.com/chromium/src/+/dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d) included with v99.0.4766. 

However this chromium version is only uses in electron v18 nightly so far, so it might be worth back porting it to v17 and even v16.
[Backport]: interface 'wl_output' has no event 4
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

16.0.6

### What operating system are you using?

Other Linux

### Operating System Version

NixOS 21.11

### What arch are you using?

x64

### Last Known Working Electron version

None

### Expected Behavior

When electron is started using the ozone wayland platform it does not crash.

### Actual Behavior

Under the latest wlroots or sway release, any electron app crashes at start with the error message in the title.

### Testcase Gist URL

_No response_

### Additional Information

This is an upstream issue in chromium and fixed with the [following commit](https://chromium.googlesource.com/chromium/src/+/dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d) included with v99.0.4766. 

However this chromium version is only uses in electron v18 nightly so far, so it might be worth back porting it to v17 and even v16.


Discussion / hints:
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574 (Pri: 1).

I wrote a more detailed motivation for backporting here: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c19
> This will be required when using Ozone/Wayland to prevent crashes on Wayland compositors that support version 4 of the wl_output protocol (rendering Ozone/Wayland unusable and thus requiring the use of XWayland). That version is available since Wayland 1.20.0 [0] which was released on 2021-12-09 [1]. It already affects any Wayland compositor that is based on wlroots 0.15.0 but over time more compositors should follow until it eventually affects all (actively developed) Wayland compositors. Backporting this to M98 would also help a lot for Electron apps as Electron version 17 is currently based on that version [2] (although there is now also an independent Electron request for backporting [3]).
> 
> Both fixes can be applied to M98 without any changes. For M97 I had to resolve a small merge conflict. It basically requires the following changes to dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d [Chromium commit]:
> ```diff
> @@ -233,9 +233,9 @@
>   namespace ui {
> 
>   namespace {
> --constexpr uint32_t kMaxSurfaceAugmenterVersion = 2;
> +-constexpr uint32_t kMaxSurfaceAugmenterVersion = 1;
>  +constexpr uint32_t kMinVersion = 1;
> -+constexpr uint32_t kMaxVersion = 2;
> ++constexpr uint32_t kMaxVersion = 1;
>   }
> 
>   // static
> ```
> 
> I've already tested backporting the fixes for both M97 and M98 and can confirm that it fixes the crashes (tested via Sway 1.7-rc2 which is based on wlroots 0.15).
> 
> [0]: https://gitlab.freedesktop.org/wayland/wayland/-/commit/9c05e6c475e0f67519d88f285a971b5e51455a1d
> [1]: https://lists.freedesktop.org/archives/wayland-devel/2021-December/042064.html
> [2]: https://github.com/electron/electron/releases/tag/v17.0.0-beta.4
> [3]: https://github.com/electron/electron/issues/32487
Is this the same as https://github.com/electron/electron/issues/32436?
No, unfortunately not. #32436 is another issue that I can also reproduce and it's also present on "older" Wayland compositors that don't support version 4 of the wl_outupt protocol yet.

As an update on this issue: Both fixes are now backported to M98 by upstream:
https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c30

AFAIK that means they'll automatically land in Electron version 17 so I guess the question is not just whether to backport them to Electron version 16?
Threw up a test cherry-pick: https://github.com/electron/electron/pull/32565
Looks like there are some merge conflicts. I don't currently have the time to look deeper into it, but I'd be happy for someone to take over that cherry-pick PR.
Thanks @nornagon. I did just try to backport the fixes to M96 (96.0.4664.99 to be precise) and there was luckily only a small and simple to resolve merge conflict when using Git:
```diff
diff --cc ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
index a9e723769879a,1b5585f7c18de..0000000000000
--- a/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
+++ b/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
@@@ -18,7 -19,8 +18,14 @@@
  namespace ui {

  namespace {
++<<<<<<< HEAD
 +constexpr uint32_t kMaxAuraShellVersion = 24;
++||||||| parent of dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
++constexpr uint32_t kMaxAuraShellVersion = 28;
++=======
+ constexpr uint32_t kMinVersion = 1;
+ constexpr uint32_t kMaxVersion = 28;
++>>>>>>> dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
  }

  // static
* Unmerged path ui/ozone/platform/wayland/host/surface_augmenter.cc
```

The other required change is that `ui/ozone/platform/wayland/host/surface_augmenter.cc` doesn't exist on M96 so I simply dropped that part from the patch:
```
Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

        deleted by us:   ui/ozone/platform/wayland/host/surface_augmenter.cc
        both modified:   ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
```

I've then proceeded to update your PR accordingly https://github.com/primeos/electron/commit/c5c69f16c76c0a40ae381aa19a736d68bdc8ff24 (the diff is a bit large because I backported the original changes from M99 - I could also backport the cherry-picked commits from M98, if preferred). Those patches should now apply to M96 but I haven't done any testing. Feel free to use that changes if it helps (also applies if someone else is interested in opening a PR - I could also open a PR for a CI run but I cannot test Electron, unfortunately).
@primeos if you want to throw up a PR and let our CI do the testing, that would be great!
@nornagon sure :) I've opened https://github.com/electron/electron/pull/32603 and the Linux builds already passed. However, I wasn't able to test/confirm if it works as expected (due to NixOS-specific issues / additional Electron issues). It would be awesome if you or someone else here could help with that. I've noted my results in https://github.com/electron/electron/pull/32603#issuecomment-1020537807 (plus a link to the build artifact).
I'm facing this issue too, using Arch Linux + Sway
Here as well with Chrome and Discord. Element defaults to Xwayland and with the wayland flags it gives the above error.
Same here on Artix + Sway. Bug reproduced with Brave and FreeTube. They work fine in XWayland.
Does this work for electron 13 too? Or does it need a different fix?
electron 13 is too different. I currently try to fix it there but wasn't able to build it yet.
Upstream Chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574 (Pri: 1).

I wrote a more detailed motivation for backporting here: https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c19
> This will be required when using Ozone/Wayland to prevent crashes on Wayland compositors that support version 4 of the wl_output protocol (rendering Ozone/Wayland unusable and thus requiring the use of XWayland). That version is available since Wayland 1.20.0 [0] which was released on 2021-12-09 [1]. It already affects any Wayland compositor that is based on wlroots 0.15.0 but over time more compositors should follow until it eventually affects all (actively developed) Wayland compositors. Backporting this to M98 would also help a lot for Electron apps as Electron version 17 is currently based on that version [2] (although there is now also an independent Electron request for backporting [3]).
> 
> Both fixes can be applied to M98 without any changes. For M97 I had to resolve a small merge conflict. It basically requires the following changes to dd4c3ddadbb9869f59cee201a38e9ca3b9154f4d [Chromium commit]:
> ```diff
> @@ -233,9 +233,9 @@
>   namespace ui {
> 
>   namespace {
> --constexpr uint32_t kMaxSurfaceAugmenterVersion = 2;
> +-constexpr uint32_t kMaxSurfaceAugmenterVersion = 1;
>  +constexpr uint32_t kMinVersion = 1;
> -+constexpr uint32_t kMaxVersion = 2;
> ++constexpr uint32_t kMaxVersion = 1;
>   }
> 
>   // static
> ```
> 
> I've already tested backporting the fixes for both M97 and M98 and can confirm that it fixes the crashes (tested via Sway 1.7-rc2 which is based on wlroots 0.15).
> 
> [0]: https://gitlab.freedesktop.org/wayland/wayland/-/commit/9c05e6c475e0f67519d88f285a971b5e51455a1d
> [1]: https://lists.freedesktop.org/archives/wayland-devel/2021-December/042064.html
> [2]: https://github.com/electron/electron/releases/tag/v17.0.0-beta.4
> [3]: https://github.com/electron/electron/issues/32487
Is this the same as https://github.com/electron/electron/issues/32436?
No, unfortunately not. #32436 is another issue that I can also reproduce and it's also present on "older" Wayland compositors that don't support version 4 of the wl_outupt protocol yet.

As an update on this issue: Both fixes are now backported to M98 by upstream:
https://bugs.chromium.org/p/chromium/issues/detail?id=1279574#c30

AFAIK that means they'll automatically land in Electron version 17 so I guess the question is not just whether to backport them to Electron version 16?
Threw up a test cherry-pick: https://github.com/electron/electron/pull/32565
Looks like there are some merge conflicts. I don't currently have the time to look deeper into it, but I'd be happy for someone to take over that cherry-pick PR.
Thanks @nornagon. I did just try to backport the fixes to M96 (96.0.4664.99 to be precise) and there was luckily only a small and simple to resolve merge conflict when using Git:
```diff
diff --cc ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
index a9e723769879a,1b5585f7c18de..0000000000000
--- a/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
+++ b/ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
@@@ -18,7 -19,8 +18,14 @@@
  namespace ui {

  namespace {
++<<<<<<< HEAD
 +constexpr uint32_t kMaxAuraShellVersion = 24;
++||||||| parent of dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
++constexpr uint32_t kMaxAuraShellVersion = 28;
++=======
+ constexpr uint32_t kMinVersion = 1;
+ constexpr uint32_t kMaxVersion = 28;
++>>>>>>> dd4c3ddadbb98... [linux/wayland] Fixed terminate caused by binding to wrong version.
  }

  // static
* Unmerged path ui/ozone/platform/wayland/host/surface_augmenter.cc
```

The other required change is that `ui/ozone/platform/wayland/host/surface_augmenter.cc` doesn't exist on M96 so I simply dropped that part from the patch:
```
Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

        deleted by us:   ui/ozone/platform/wayland/host/surface_augmenter.cc
        both modified:   ui/ozone/platform/wayland/host/wayland_zaura_shell.cc
```

I've then proceeded to update your PR accordingly https://github.com/primeos/electron/commit/c5c69f16c76c0a40ae381aa19a736d68bdc8ff24 (the diff is a bit large because I backported the original changes from M99 - I could also backport the cherry-picked commits from M98, if preferred). Those patches should now apply to M96 but I haven't done any testing. Feel free to use that changes if it helps (also applies if someone else is interested in opening a PR - I could also open a PR for a CI run but I cannot test Electron, unfortunately).
@primeos if you want to throw up a PR and let our CI do the testing, that would be great!
@nornagon sure :) I've opened https://github.com/electron/electron/pull/32603 and the Linux builds already passed. However, I wasn't able to test/confirm if it works as expected (due to NixOS-specific issues / additional Electron issues). It would be awesome if you or someone else here could help with that. I've noted my results in https://github.com/electron/electron/pull/32603#issuecomment-1020537807 (plus a link to the build artifact).
I'm facing this issue too, using Arch Linux + Sway
Here as well with Chrome and Discord. Element defaults to Xwayland and with the wayland flags it gives the above error.
Same here on Artix + Sway. Bug reproduced with Brave and FreeTube. They work fine in XWayland.
Does this work for electron 13 too? Or does it need a different fix?
electron 13 is too different. I currently try to fix it there but wasn't able to build it yet.

Fix patch:
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
index 448506ee016e7..534a9bad9e6a1 100644
--- a/patches/chromium/.patches
+++ b/patches/chromium/.patches
@@ -178,3 +178,4 @@ sandbox_fix_sandbox_inheritance_m96_merge.patch
 cherry-pick-dbde8795233a.patch
 cherry-pick-6bb320d134b1.patch
 m96_fileapi_move_origin_checks_in_bloburlstore_sooner.patch
+fix_wayland_bugs_related_to_bad_versioning.patch
diff --git a/patches/chromium/fix_wayland_bugs_related_to_bad_versioning.patch b/patches/chromium/fix_wayland_bugs_related_to_bad_versioning.patch
new file mode 100644
index 0000000000000..a3749fc19a7c8
--- /dev/null
+++ b/patches/chromium/fix_wayland_bugs_related_to_bad_versioning.patch
@@ -0,0 +1,76 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Robin Ebert <ebertrobin2002@gmail.com>
+Date: Thu, 27 Jan 2022 23:41:41 +0100
+Subject: Fix wayland bugs related to bad versioning
+
+Chromium tries to bind to the newest version of some wayland interfaces (output, drm, xdg-exporter). This causes issues when it doesn't actually support the version.
+To mitigate this this patch introduces some constants specifing the max supported version and ensures that this value is enforced.
+Chromium also tries to map the keymap file descriptor as read-write but new versions of at least wlroots now passes it as read-only.
+This is fixed by mapping the file read-only.
+
+diff --git a/ui/ozone/platform/wayland/host/wayland_connection.cc b/ui/ozone/platform/wayland/host/wayland_connection.cc
+index 6c0501b0f95fa50ff4fe677cff7354401fb05268..a9a366b54a7490e005357224351b2ae2b40d052e 100644
+--- a/ui/ozone/platform/wayland/host/wayland_connection.cc
++++ b/ui/ozone/platform/wayland/host/wayland_connection.cc
+@@ -69,10 +69,13 @@ constexpr uint32_t kMaxLinuxDmabufVersion = 3;
+ constexpr uint32_t kMaxSeatVersion = 5;
+ constexpr uint32_t kMaxShmVersion = 1;
+ constexpr uint32_t kMaxXdgShellVersion = 1;
++constexpr uint32_t kMaxWlOutputVersion = 3;
+ constexpr uint32_t kMaxDeviceManagerVersion = 3;
+ constexpr uint32_t kMaxWpPresentationVersion = 1;
+ constexpr uint32_t kMaxWpViewporterVersion = 1;
+ constexpr uint32_t kMaxTextInputManagerVersion = 1;
++constexpr uint32_t kMaxXdgExporterVersion = 1;
++constexpr uint32_t kMaxWlDrmVersion = 2;
+ constexpr uint32_t kMaxExplicitSyncVersion = 2;
+ constexpr uint32_t kMaxXdgDecorationVersion = 1;
+ constexpr uint32_t kMaxExtendedDragVersion = 1;
+@@ -405,7 +408,7 @@ void WaylandConnection::Global(void* data,
+       return;
+     }
+ 
+-    wl::Object<wl_output> output = wl::Bind<wl_output>(registry, name, version);
++    wl::Object<wl_output> output = wl::Bind<wl_output>(registry, name, std::min(version, kMaxWlOutputVersion));
+     if (!output) {
+       LOG(ERROR) << "Failed to bind to wl_output global";
+       return;
+@@ -502,10 +505,10 @@ void WaylandConnection::Global(void* data,
+   } else if (!connection->xdg_foreign_ &&
+              strcmp(interface, "zxdg_exporter_v1") == 0) {
+     connection->xdg_foreign_ = std::make_unique<XdgForeignWrapper>(
+-        connection, wl::Bind<zxdg_exporter_v1>(registry, name, version));
++        connection, wl::Bind<zxdg_exporter_v1>(registry, name, std::min(version, kMaxXdgExporterVersion)));
+   } else if (!connection->drm_ && (strcmp(interface, "wl_drm") == 0) &&
+              version >= kMinWlDrmVersion) {
+-    auto wayland_drm = wl::Bind<struct wl_drm>(registry, name, version);
++    auto wayland_drm = wl::Bind<struct wl_drm>(registry, name, std::min(version, kMaxWlDrmVersion));
+     connection->drm_ =
+         std::make_unique<WaylandDrm>(wayland_drm.release(), connection);
+   } else if (!connection->zaura_shell_ &&
+diff --git a/ui/ozone/platform/wayland/host/wayland_keyboard.cc b/ui/ozone/platform/wayland/host/wayland_keyboard.cc
+index 85fcd6a423b5af29a1bbd910468778afd28b8c27..462bb1f39b1194a01fd398d8c63f032f8b9d4a0b 100644
+--- a/ui/ozone/platform/wayland/host/wayland_keyboard.cc
++++ b/ui/ozone/platform/wayland/host/wayland_keyboard.cc
+@@ -7,7 +7,7 @@
+ #include <utility>
+ 
+ #include "base/files/scoped_file.h"
+-#include "base/memory/unsafe_shared_memory_region.h"
++#include "base/memory/read_only_shared_memory_region.h"
+ #include "base/unguessable_token.h"
+ #include "ui/base/buildflags.h"
+ #include "ui/events/base_event_utils.h"
+@@ -119,10 +119,10 @@ void WaylandKeyboard::Keymap(void* data,
+   base::ScopedFD fd(keymap_fd);
+   auto length = size - 1;
+   auto shmen = base::subtle::PlatformSharedMemoryRegion::Take(
+-      std::move(fd), base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
++      std::move(fd), base::subtle::PlatformSharedMemoryRegion::Mode::kReadOnly,
+       length, base::UnguessableToken::Create());
+   auto mapped_memory =
+-      base::UnsafeSharedMemoryRegion::Deserialize(std::move(shmen)).Map();
++      base::ReadOnlySharedMemoryRegion::Deserialize(std::move(shmen)).Map();
+   const char* keymap = mapped_memory.GetMemoryAs<char>();
+ 
+   if (!keymap || format != WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1)


Test patch: