From 1c37f820e366fcaf97fafc81056489e7cdb734e4 Mon Sep 17 00:00:00 2001 From: Alexander Dunaev Date: Tue, 24 May 2022 06:11:03 +0000 Subject: [PATCH] [linux/wayland] Added some DCHECKs in the output handling. [merge to M102] This patch adds some diagnostics for a possible issue in output handling logic. (cherry picked from commit 5aa732f7b7acdb84fc647a44461dffc16340915e) Bug: 1323635 Change-Id: I5fa34348e62a972466b68ac08c1b9b1192fb3b8e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3634462 Commit-Queue: Alexander Dunaev Reviewed-by: Maksim Sisov Cr-Original-Commit-Position: refs/heads/main@{#1001444} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3660509 Auto-Submit: Alexander Dunaev Commit-Queue: Maksim Sisov Cr-Commit-Position: refs/branch-heads/5005@{#982} Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} --- .../platform/wayland/host/wayland_surface.cc | 32 +++++++++---------- .../platform/wayland/host/wayland_window.cc | 8 +++-- .../platform/wayland/host/wayland_window.h | 9 +++--- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/ui/ozone/platform/wayland/host/wayland_surface.cc b/ui/ozone/platform/wayland/host/wayland_surface.cc index 0ff844dda1dfa3..43e6152cc2b76a 100644 --- a/ui/ozone/platform/wayland/host/wayland_surface.cc +++ b/ui/ozone/platform/wayland/host/wayland_surface.cc @@ -26,6 +26,7 @@ #include "ui/ozone/platform/wayland/host/wayland_buffer_handle.h" #include "ui/ozone/platform/wayland/host/wayland_connection.h" #include "ui/ozone/platform/wayland/host/wayland_output.h" +#include "ui/ozone/platform/wayland/host/wayland_output_manager.h" #include "ui/ozone/platform/wayland/host/wayland_subsurface.h" #include "ui/ozone/platform/wayland/host/wayland_window.h" @@ -681,10 +682,15 @@ void WaylandSurface::Enter(void* data, auto* wayland_output = static_cast(wl_output_get_user_data(output)); + + DCHECK_NE(surface->connection_->wayland_output_manager()->GetOutput( + wayland_output->output_id()), + nullptr); + surface->entered_outputs_.emplace_back(wayland_output->output_id()); if (surface->root_window_) - surface->root_window_->OnEnteredOutputIdAdded(); + surface->root_window_->OnEnteredOutput(); } // static @@ -700,27 +706,21 @@ void WaylandSurface::Leave(void* data, } void WaylandSurface::RemoveEnteredOutput(uint32_t output_id) { - if (entered_outputs().empty()) - return; - auto entered_outputs_it_ = std::find_if(entered_outputs_.begin(), entered_outputs_.end(), [&output_id](uint32_t id) { return id == output_id; }); + if (entered_outputs_it_ == entered_outputs_.end()) + return; - // The `entered_outputs_` list should be updated, - // 1. for wl_surface::leave, when a user switches physical output between two - // displays, a surface does not necessarily receive enter events immediately - // or until a user resizes/moves it. This means that switching output between - // displays in a single output mode results in leave events, but the surface - // might not have received enter event before. Thus, remove the id of the - // output that the surface leaves only if it was stored before. - // 2. for wl_registry::global_remove, when wl_output is removed by a server - // after the display is unplugged or switched off. - if (entered_outputs_it_ != entered_outputs_.end()) - entered_outputs_.erase(entered_outputs_it_); + // In certain use cases, such as switching outputs in the single output + // configuration, the compositor may move the surface from one output to + // another one, send wl_surface::leave event to it, but defer sending + // wl_surface::enter until the user moves or resizes the surface on the new + // output. + entered_outputs_.erase(entered_outputs_it_); if (root_window_) - root_window_->OnEnteredOutputIdRemoved(); + root_window_->OnLeftOutput(); } void WaylandSurface::SetOverlayPriority( diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc index 273f4b514ade79..af66f8148e6012 100644 --- a/ui/ozone/platform/wayland/host/wayland_window.cc +++ b/ui/ozone/platform/wayland/host/wayland_window.cc @@ -175,6 +175,10 @@ uint32_t WaylandWindow::GetPreferredEnteredOutputId() { auto* output_manager = connection_->wayland_output_manager(); auto* output = output_manager->GetOutput(output_id); auto* preferred_output = output_manager->GetOutput(preferred_output_id); + // crbug.com/1323635 + DCHECK(output) << " output " << output_id << " not found!"; + DCHECK(preferred_output) + << " output " << preferred_output_id << " not found!"; if (output->scale_factor() > preferred_output->scale_factor()) preferred_output_id = output_id; } @@ -632,7 +636,7 @@ WaylandWindow* WaylandWindow::GetRootParentWindow() { return parent_window_ ? parent_window_->GetRootParentWindow() : this; } -void WaylandWindow::OnEnteredOutputIdAdded() { +void WaylandWindow::OnEnteredOutput() { // Wayland does weird things for menus so instead of tracking outputs that // we entered or left, we take that from the parent window and ignore this // event. @@ -642,7 +646,7 @@ void WaylandWindow::OnEnteredOutputIdAdded() { UpdateWindowScale(true); } -void WaylandWindow::OnEnteredOutputIdRemoved() { +void WaylandWindow::OnLeftOutput() { // Wayland does weird things for menus so instead of tracking outputs that // we entered or left, we take that from the parent window and ignore this // event. diff --git a/ui/ozone/platform/wayland/host/wayland_window.h b/ui/ozone/platform/wayland/host/wayland_window.h index 52472f360ae539..5eb89b3917a183 100644 --- a/ui/ozone/platform/wayland/host/wayland_window.h +++ b/ui/ozone/platform/wayland/host/wayland_window.h @@ -278,12 +278,13 @@ class WaylandWindow : public PlatformWindow, WaylandWindow* GetTopMostChildWindow(); // Called by the WaylandSurface attached to this window when that surface - // becomes partially or fully within the scanout region of |output|. - void OnEnteredOutputIdAdded(); + // becomes partially or fully within the scanout region of an output that it + // wasn't before. + void OnEnteredOutput(); // Called by the WaylandSurface attached to this window when that surface - // becomes fully outside of the scanout region of |output|. - void OnEnteredOutputIdRemoved(); + // becomes fully outside of one of outputs that it previously resided on. + void OnLeftOutput(); // Returns true iff this window is opaque. bool IsOpaqueWindow() const;