Commit 0fea19b0 authored by Zakhar Voit's avatar Zakhar Voit Committed by Chromium LUCI CQ

[Lacros] Fix tooltip window crash on mixed DPI

WaylandAuxiliaryWindow is not provided with a correct parent and
buffer_scale upon creation. That causes the DCHECK at
https://source.chromium.org/chromium/chromium/src/+/master:ui/ozone/platform/wayland/common/wayland_util.cc;l=271
being triggered.

This CL updates buffer_scale before creating the tooltip subsurface
and makes sure that WaylandAuxiliaryWindow doesn't receive scale
updates when a display changes (just like we already do with menus).

BUG=1152569

Change-Id: I0a0357fe1151915d5ed9142c395b4f76099abe3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560588
Commit-Queue: Zakhar Voit <zvoit@igalia.com>
Reviewed-by: default avatarMaksim Sisov (GMT+2) <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#835376}
parent 56ad5563
......@@ -46,7 +46,7 @@ void WaylandAuxiliaryWindow::SetBounds(const gfx::Rect& bounds) {
auto old_bounds = GetBounds();
WaylandWindow::SetBounds(bounds);
if (old_bounds == bounds || !parent_window())
if (old_bounds == bounds || !parent_window() || !subsurface_)
return;
auto subsurface_bounds_dip =
......@@ -58,22 +58,11 @@ void WaylandAuxiliaryWindow::SetBounds(const gfx::Rect& bounds) {
}
void WaylandAuxiliaryWindow::CreateSubsurface() {
auto* parent = parent_window();
if (!parent) {
// wl_subsurface can be used for several purposes: tooltips and drag arrow
// windows. If we are in a drag process, use the entered window. Otherwise,
// it must be a tooltip.
if (connection()->IsDragInProgress()) {
parent = connection()->data_drag_controller()->entered_window();
set_parent_window(parent);
} else {
// If Aura does not not provide a reference parent window, needed by
// Wayland, we get the current focused window to place and show the
// tooltips.
parent =
connection()->wayland_window_manager()->GetCurrentFocusedWindow();
}
}
auto* parent = FindParentWindow();
set_parent_window(parent);
// We need to make sure that buffer scale matches the parent window.
UpdateBufferScale(true);
// Tooltip and drag arrow creation is an async operation. By the time Aura
// actually creates them, it is possible that the user has already moved the
......@@ -101,14 +90,30 @@ void WaylandAuxiliaryWindow::CreateSubsurface() {
connection()->wayland_window_manager()->NotifyWindowConfigured(this);
}
WaylandWindow* WaylandAuxiliaryWindow::FindParentWindow() {
// wl_subsurface can be used for several purposes: tooltips and drag arrow
// windows. If we are in a drag process, use the entered window. Otherwise,
// it must be a tooltip.
if (connection()->IsDragInProgress())
return connection()->data_drag_controller()->entered_window();
// We get the current focused window to place and show the
// tooltips.
return connection()->wayland_window_manager()->GetCurrentFocusedWindow();
}
bool WaylandAuxiliaryWindow::OnInitialize(
PlatformWindowInitProperties properties) {
DCHECK(!parent_window());
// If we do not have parent window provided, we must always use a focused
// window or a window that entered drag whenever the subsurface is created.
if (properties.parent_widget == gfx::kNullAcceleratedWidget)
if (properties.parent_widget == gfx::kNullAcceleratedWidget) {
// Need to set the possible parent window here, so the initial scale will be
// calculated correctly.
set_parent_window(FindParentWindow());
return true;
}
set_parent_window(
connection()->wayland_window_manager()->FindParentForNewWindow(
......
......@@ -31,6 +31,9 @@ class WaylandAuxiliaryWindow : public WaylandWindow {
// Creates (if necessary) and shows a subsurface window.
void CreateSubsurface();
// Finds an appropriate parent window.
WaylandWindow* FindParentWindow();
wl::Object<wl_subsurface> subsurface_;
};
......
......@@ -418,7 +418,7 @@ void WaylandWindow::AddEnteredOutputId(struct wl_output* output) {
// 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.
if (wl::IsMenuType(type()))
if (wl::IsMenuType(type()) || type() == ui::PlatformWindowType::kTooltip)
return;
const uint32_t entered_output_id =
......
......@@ -1437,6 +1437,63 @@ TEST_P(WaylandWindowTest, ToplevelWindowUpdateBufferScale) {
EXPECT_EQ(gfx::Rect(0, 0, 1600, 1200), window_->GetBounds());
}
TEST_P(WaylandWindowTest, AuxiliaryWindowUpdateBufferScale) {
VerifyAndClearExpectations();
// Creating an output with scale 1.
wl::TestOutput* output1 = server_.CreateAndInitializeOutput();
output1->SetRect(gfx::Rect(0, 0, 1920, 1080));
output1->SetScale(1);
Sync();
// Creating an output with scale 2.
wl::TestOutput* output2 = server_.CreateAndInitializeOutput();
output2->SetRect(gfx::Rect(0, 0, 1920, 1080));
output2->SetScale(2);
Sync();
// Send the window to |output1|.
wl::MockSurface* surface = server_.GetObject<wl::MockSurface>(
window_->root_surface()->GetSurfaceId());
ASSERT_TRUE(surface);
wl_surface_send_enter(surface->resource(), output1->resource());
Sync();
// Creating a tooltip on |window_|.
window_->SetPointerFocus(true);
gfx::Rect subsurface_bounds(15, 15, 10, 10);
std::unique_ptr<WaylandWindow> auxiliary_window =
CreateWaylandWindowWithParams(PlatformWindowType::kTooltip,
gfx::kNullAcceleratedWidget,
subsurface_bounds, &delegate_);
EXPECT_TRUE(auxiliary_window);
auxiliary_window->Show(false);
// |auxiliary_window| should inherit its buffer scale from the focused window.
EXPECT_EQ(1, auxiliary_window->buffer_scale());
EXPECT_EQ(subsurface_bounds, auxiliary_window->GetBounds());
auxiliary_window->Hide();
// Send the window to |output2|.
wl_surface_send_enter(surface->resource(), output2->resource());
wl_surface_send_leave(surface->resource(), output1->resource());
Sync();
EXPECT_EQ(2, window_->buffer_scale());
auxiliary_window->Show(false);
// |auxiliary_window|'s scale and bounds must change whenever its parents
// scale is changed.
EXPECT_EQ(2, window_->buffer_scale());
EXPECT_EQ(2, auxiliary_window->buffer_scale());
EXPECT_EQ(gfx::ScaleToRoundedRect(subsurface_bounds, 2),
auxiliary_window->GetBounds());
auxiliary_window->Hide();
window_->SetPointerFocus(false);
}
// Tests WaylandWindow repositions menu windows to be relative to parent window
// in a right way.
TEST_P(WaylandWindowTest, AdjustPopupBounds) {
......@@ -1662,23 +1719,16 @@ TEST_P(WaylandWindowTest, OnCloseRequest) {
TEST_P(WaylandWindowTest, AuxiliaryWindowSimpleParent) {
VerifyAndClearExpectations();
std::unique_ptr<WaylandWindow> second_window = CreateWaylandWindowWithParams(
PlatformWindowType::kWindow, gfx::kNullAcceleratedWidget,
gfx::Rect(0, 0, 640, 480), &delegate_);
EXPECT_TRUE(second_window);
// Test case 1: if the subsurface is provided with a parent widget, it must
// always use that as a parent.
// Auxiliary window must ignore the parent provided by aura and should always
// use focused window instead.
gfx::Rect subsurface_bounds(gfx::Point(15, 15), gfx::Size(10, 10));
std::unique_ptr<WaylandWindow> auxiliary_window =
CreateWaylandWindowWithParams(PlatformWindowType::kTooltip,
window_->GetWidget(), subsurface_bounds,
&delegate_);
gfx::kNullAcceleratedWidget,
subsurface_bounds, &delegate_);
EXPECT_TRUE(auxiliary_window);
// The subsurface mustn't take the focused window as a parent, but use the
// provided one.
second_window->SetPointerFocus(true);
window_->SetPointerFocus(true);
auxiliary_window->Show(false);
Sync();
......@@ -1698,47 +1748,7 @@ TEST_P(WaylandWindowTest, AuxiliaryWindowSimpleParent) {
->resource();
EXPECT_EQ(parent_resource, test_subsurface->parent_resource());
// Test case 2: the subsurface must use the focused window as its parent.
auxiliary_window = CreateWaylandWindowWithParams(
PlatformWindowType::kTooltip, gfx::kNullAcceleratedWidget,
subsurface_bounds, &delegate_);
EXPECT_TRUE(auxiliary_window);
// The tooltip must take the focused window.
second_window->SetPointerFocus(true);
auxiliary_window->Show(false);
Sync();
// Get new surface after recreating the WaylandAuxiliaryWindow.
mock_surface_subsurface = server_.GetObject<wl::MockSurface>(
auxiliary_window->root_surface()->GetSurfaceId());
test_subsurface = mock_surface_subsurface->sub_surface();
auto* second_parent_resource =
server_
.GetObject<wl::MockSurface>(
second_window->root_surface()->GetSurfaceId())
->resource();
EXPECT_EQ(second_parent_resource, test_subsurface->parent_resource());
auxiliary_window->Hide();
Sync();
// The subsurface must take the focused window.
second_window->SetPointerFocus(false);
window_->SetPointerFocus(true);
auxiliary_window->Show(false);
Sync();
// The subsurface is invalidated on each Hide call.
test_subsurface = mock_surface_subsurface->sub_surface();
// The |window_|'s resource must be the parent resource.
EXPECT_EQ(parent_resource, test_subsurface->parent_resource());
window_->SetPointerFocus(false);
}
......@@ -1897,6 +1907,7 @@ TEST_P(WaylandWindowTest, AuxiliaryWindowNestedParent) {
EXPECT_TRUE(menu_window);
VerifyAndClearExpectations();
menu_window->SetPointerFocus(true);
gfx::Rect subsurface_bounds(gfx::Point(15, 15), gfx::Size(10, 10));
std::unique_ptr<WaylandWindow> auxiliary_window =
......@@ -1907,8 +1918,6 @@ TEST_P(WaylandWindowTest, AuxiliaryWindowNestedParent) {
VerifyAndClearExpectations();
menu_window->SetPointerFocus(true);
auxiliary_window->Show(false);
Sync();
......
......@@ -933,6 +933,8 @@ TEST_P(WaylandBufferManagerTest, TestCommitBufferConditionsAckConfigured) {
DCHECK(mock_surface->xdg_surface());
ActivateSurface(mock_surface->xdg_surface());
} else {
// WaylandAuxiliaryWindow uses the focused window as a parent.
window_->SetPointerFocus(true);
// See the comment near Show() call above.
temp_window->Show(false);
}
......@@ -943,6 +945,7 @@ TEST_P(WaylandBufferManagerTest, TestCommitBufferConditionsAckConfigured) {
Sync();
window_->SetPointerFocus(false);
temp_window.reset();
DestroyBufferAndSetTerminateExpectation(widget, kDmabufBufferId,
false /*fail*/);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment