ungoogled-software / ungoogled-chromium-archlinux

Arch Linux packaging for ungoogled-chromium
BSD 3-Clause "New" or "Revised" License
339 stars 36 forks source link

v105 and v106 Wayland surface regression #198

Closed nikolowry closed 2 years ago

nikolowry commented 2 years ago

See: https://chromium-review.googlesource.com/c/chromium/src/+/3831855

Easiest way to reproduce the regression is to right-click on a tab's content and observing the position of the context menu dialog. I'd include a screenshot but my last build was already patched.

The fix doesn't land until v107, so it be great to include the following patch until then:

From ac7d143e1d9eca969b3f23a7fa9d022577ae6005 Mon Sep 17 00:00:00 2001
From: Alexander Dunaev <adunaev@igalia.com>
Date: Mon, 15 Aug 2022 11:12:21 +0000
Subject: [PATCH] Revert "wayland: Commit surface after configure with same
 size"

This reverts commit 32e27b753bfb2e99232c1bb303f07283ab248ea3.

Reason for revert: this change caused a major regression on mutter Wayland compositor.

Original change's description:
> wayland: Commit surface after configure with same size
>
> The configure -> ack sequence doesn't take effect on the server side
> until the associated surface is committed.
>
> Fixes a sync issue in TabDragging/* tests on lacros that reposition
> windows during test setup, where lacros and exo have different screen
> positions for a surface.
>
> Bug: 1336691, 1336706
> Change-Id: I8d34f23ece4808bb56d8bc01712d4488bf472359
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3714566
> Reviewed-by: Kramer Ge <fangzhoug@chromium.org>
> Commit-Queue: River Gilhuly <rivr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1023916}

Bug: 1336691, 1336706
Change-Id: Ia88538e5645da5a65d0c1073c672f1f3bfbc4c54
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3831855
Auto-Submit: Alexander Dunaev <adunaev@igalia.com>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1035020}
---
 ui/ozone/platform/wayland/host/wayland_window.cc             | 2 +-
 ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc
index f4e616b22beb5..fb95e498e0dfe 100644
--- a/ui/ozone/platform/wayland/host/wayland_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window.cc
@@ -950,7 +950,7 @@ void WaylandWindow::ProcessPendingBoundsDip(uint32_t serial) {
     // window has been applied.
     SetWindowGeometry(pending_bounds_dip_);
     AckConfigure(serial);
-    root_surface()->Commit();
+    connection()->ScheduleFlush();
   } else if (!pending_configures_.empty() &&
              pending_bounds_dip_.size() ==
                  pending_configures_.back().bounds_dip.size()) {
diff --git a/ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc b/ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
index 7a1a95d26f3fa..58804a06caffa 100644
--- a/ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
+++ b/ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc
@@ -1112,9 +1112,7 @@ TEST_P(WaylandBufferManagerTest, TestCommitBufferConditionsAckConfigured) {
     EXPECT_CALL(*xdg_surface, AckConfigure(_)).Times(1);
     EXPECT_CALL(*mock_surface, Attach(_, _, _)).Times(1);
     EXPECT_CALL(*mock_surface, Frame(_)).Times(1);
-    // Commit() can be called a second time as part of the configure -> ack
-    // sequence.
-    EXPECT_CALL(*mock_surface, Commit()).Times(testing::Between(1, 2));
+    EXPECT_CALL(*mock_surface, Commit()).Times(1);

     ActivateSurface(mock_surface->xdg_surface());
     Sync();
jstkdng commented 2 years ago

shouldn't this be added to arch's chromium package as well?

nikolowry commented 2 years ago

@jstkdng is that not what this repo is for? I could open a pull request, but since this is a temporary issue (v105-v106) figured it was best to open an issue first to discuss

jstkdng commented 2 years ago

uhh, I meant extra/chromium, since we just copy their stuff and add uc on top. But it's fine, the patch can be added once flac is updated since that will need a bump on the pkgrel/rebuild.

nikolowry commented 2 years ago

My bad @jstkdng, I read your earlier comment too quickly -- was just about to edit it, but I'll respond here instead.

It most definitely should be in extra/chromium, but I've never contributed or opened any issues for that package. I see the last commit was done by @foutrelis, so hopefully tagging him brings this to their attention.

nikolowry commented 2 years ago

Here's the link to the original bug reported, should have included it in my initial post: https://bugs.chromium.org/p/chromium/issues/detail?id=1350605.

Not sure if they'll port this back to v106, but it definitely won't be in any v105 releases. From the bug report link:

Given we haven't heard much of feedback and at this stage we are only considering the changes which are critical to M105 hence rejecting this change to M105. Please reach out if you have any concerns

jstkdng commented 2 years ago

the patch has been added here and in the aur, closing.