[114] Crash within WaylandWindowManager::GetCurrentPointerOrTouchFocusedWindow()

[russdill]

A recent change within ui/ozone/platform/wayland/host/wayland_window_manager.cc, 5ae87c3476 "fixup! [ozone/wayland] Harden the logic that chooses the window being dragged" causes a crash if no input devices are available.

From 0fe409ca76554825fda818df1a015412bb0d9612 Mon Sep 17 00:00:00 2001
From: Russ Dill <russ.dill@nikolamotor.com>
Date: Tue, 18 Jul 2023 07:17:24 +0000
Subject: [PATCH 7/7] WaylandWindowManager: Protect access to drag_source()

window_drag_controller() may return a NULL result.

This crash was introduced by 5ae87c3476 "fixup!
[ozone/wayland] Harden the logic that chooses the
window being dragged"
---
 ui/ozone/platform/wayland/host/wayland_window_manager.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/ozone/platform/wayland/host/wayland_window_manager.cc b/ui/ozone/platform/wayland/host/wayland_window_manager.cc
index d65cc6e3a627d..5b910ec848fd1 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_manager.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_manager.cc
@@ -101,6 +101,7 @@ WaylandWindow* WaylandWindowManager::GetCurrentPointerOrTouchFocusedWindow()
   //
   // TODO(https://crbug.com/1317063): Apply the same logic to data drag sessions
   // too?
+  if (connection_ && connection_->window_drag_controller())
   if (auto drag_source = connection_->window_drag_controller()->drag_source()) {
     return *drag_source == mojom::DragEventSource::kMouse
                ? GetCurrentPointerFocusedWindow()
-- 
2.34.1

[rakuco]

Thanks for the patch. @MaxIhlenfeldt can you take a look at this one with your upstream hat?

[MaxIhlenfeldt]

Sure! Thanks for catching this.

I don't think connection_ can be NULL here, could you please confirm that adding only if (connection_->window_drag_controller()) is enough to fix the crash? Then I can upstream the change.

[russdill]

Verified that removing the check for connection_ being NULL does not break the fix.

[MaxIhlenfeldt]

Thank you! Upstream review is live at https://crrev.com/c/4724882.

[MaxIhlenfeldt]

Relaying a comment from the reviewer: @russdill

I'd like to understand it a bit more: is it the seat or the data_device_manager that isn't available in the aforementioned environment?

[russdill]

I'm sorry, I don't know the answer to those questions. I just know we have two weston instances running, and one of them doesn't have any input devices associated with it.

[MaxIhlenfeldt]

The patch has been merged upstream, thanks again! I'll upload a PR with a backport tomorrow.