Seat: rework Xwayland Override Redirect focus

Instead of stashing the active view and setting Seat.focused to the
Xwayland OR surface when a child OR surface of a currently focused
Xwayland view is given keyboard focus, keep Seat.focused set to the
Xwayland view.

Such Override Redirect surfaces are commonly used for drop down menus
and the like, and river should behave as if the parent Xwayland view
still has focus.

This ensures that the riverctl focus-view next/prev commands continue to
work as expected while a popup is open, the correct focused view title
will be sent over river status, etc.

It's also cleaner to centralize this logic in XwaylandOverrideRedirect
and keep it out of Seat.zig.

4 files changed, 44 insertions, 63 deletions

diff --git a/river/Cursor.zig b/river/Cursor.zig
index 38eb6f6..8fd669d 100644
--- a/river/Cursor.zig
+++ b/river/Cursor.zig
@@ -367,12 +367,8 @@ fn updateKeyboardFocus(self: Self, result: SurfaceAtResult) void {
             self.seat.setFocusRaw(.{ .lock_surface = lock_surface });
         },
         .xwayland_override_redirect => |override_redirect| {
-            if (!build_options.xwayland) unreachable;
-            if (override_redirect.xwayland_surface.overrideRedirectWantsFocus() and
-                override_redirect.xwayland_surface.icccmInputModel() != .none)
-            {
-                self.seat.setFocusRaw(.{ .xwayland_override_redirect = override_redirect });
-            }
+            assert(server.lock_manager.state != .unlocked);
+            override_redirect.focusIfDesired();
         },
     }
 }
@@ -663,7 +659,7 @@ const SurfaceAtResult = struct {
         view: *View,
         layer_surface: *LayerSurface,
         lock_surface: *LockSurface,
-        xwayland_override_redirect: if (build_options.xwayland) *XwaylandOverrideRedirect else void,
+        xwayland_override_redirect: if (build_options.xwayland) *XwaylandOverrideRedirect else noreturn,
     },
 };
 
diff --git a/river/Seat.zig b/river/Seat.zig
index 441ede3..baf1c74 100644
--- a/river/Seat.zig
+++ b/river/Seat.zig
@@ -80,9 +80,6 @@ focused_output: *Output,
 
 focused: FocusTarget = .none,
 
-/// Currently activated Xwayland view (used to handle override redirect menus)
-activated_xwayland_view: if (build_options.xwayland) ?*View else void = if (build_options.xwayland) null else {},
-
 /// Stack of views in most recently focused order
 /// If there is a currently focused view, it is on top.
 focus_stack: ViewStack(*View) = .{},
@@ -233,36 +230,11 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void {
         .none => null,
     };
 
-    if (build_options.xwayland) {
-        // Keep the parent top-level Xwayland view of any override redirect surface
-        // activated while that override redirect surface is focused. This ensures
-        // override redirect menus do not disappear as a result of deactivating
-        // their parent window.
-        if (new_focus == .xwayland_override_redirect and
-            self.focused == .view and
-            self.focused.view.impl == .xwayland_view and
-            self.focused.view.impl.xwayland_view.xwayland_surface.pid == new_focus.xwayland_override_redirect.xwayland_surface.pid)
-        {
-            self.activated_xwayland_view = self.focused.view;
-        } else if (self.activated_xwayland_view) |active_view| {
-            if (!(new_focus == .view and new_focus.view == active_view) and
-                !(new_focus == .xwayland_override_redirect and new_focus.xwayland_override_redirect.xwayland_surface.pid == active_view.impl.xwayland_view.xwayland_surface.pid))
-            {
-                if (active_view.pending.focus == 0) active_view.setActivated(false);
-                self.activated_xwayland_view = null;
-            }
-        }
-    }
-
     // First clear the current focus
     switch (self.focused) {
         .view => |view| {
             view.pending.focus -= 1;
-            if (view.pending.focus == 0 and
-                (!build_options.xwayland or view != self.activated_xwayland_view))
-            {
-                view.setActivated(false);
-            }
+            if (view.pending.focus == 0) view.setActivated(false);
         },
         .xwayland_override_redirect, .layer, .lock_surface, .none => {},
     }
@@ -272,11 +244,7 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void {
         .view => |target_view| {
             assert(server.lock_manager.state == .unlocked);
             assert(self.focused_output == target_view.output);
-            if (target_view.pending.focus == 0 and
-                (!build_options.xwayland or target_view != self.activated_xwayland_view))
-            {
-                target_view.setActivated(true);
-            }
+            if (target_view.pending.focus == 0) target_view.setActivated(true);
             target_view.pending.focus += 1;
             target_view.pending.urgent = false;
         },
@@ -289,7 +257,17 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void {
     }
     self.focused = new_focus;
 
-    // Send keyboard enter/leave events and handle pointer constraints
+    self.keyboardEnterOrLeave(target_surface);
+
+    // Inform any clients tracking status of the change
+    var it = self.status_trackers.first;
+    while (it) |node| : (it = node.next) node.data.sendFocusedView();
+}
+
+/// Send keyboard enter/leave events and handle pointer constraints
+/// This should never normally be called from outside of setFocusRaw(), but we make an exception for
+/// XwaylandOverrideRedirect surfaces as they don't conform to the Wayland focus model.
+pub fn keyboardEnterOrLeave(self: *Self, target_surface: ?*wlr.Surface) void {
     if (target_surface) |wlr_surface| {
         self.keyboardNotifyEnter(wlr_surface);
 
@@ -317,10 +295,6 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void {
             self.cursor.may_need_warp = true;
         }
     }
-
-    // Inform any clients tracking status of the change
-    var it = self.status_trackers.first;
-    while (it) |node| : (it = node.next) node.data.sendFocusedView();
 }
 
 fn keyboardNotifyEnter(self: *Self, wlr_surface: *wlr.Surface) void {
diff --git a/river/XwaylandOverrideRedirect.zig b/river/XwaylandOverrideRedirect.zig
index 6b61100..224ad65 100644
--- a/river/XwaylandOverrideRedirect.zig
+++ b/river/XwaylandOverrideRedirect.zig
@@ -97,10 +97,26 @@ pub fn handleMap(listener: *wl.Listener(*wlr.XwaylandSurface), xwayland_surface:
 
     xwayland_surface.surface.?.events.commit.add(&self.commit);
 
+    self.focusIfDesired();
+}
+
+pub fn focusIfDesired(self: *Self) void {
     if (self.xwayland_surface.overrideRedirectWantsFocus() and
         self.xwayland_surface.icccmInputModel() != .none)
     {
-        server.input_manager.defaultSeat().setFocusRaw(.{ .xwayland_override_redirect = self });
+        const seat = server.input_manager.defaultSeat();
+        // Keep the parent top-level Xwayland view of any override redirect surface
+        // activated while that override redirect surface is focused. This ensures
+        // override redirect menus do not disappear as a result of deactivating
+        // their parent window.
+        if (seat.focused == .view and
+            seat.focused.view.impl == .xwayland_view and
+            seat.focused.view.impl.xwayland_view.xwayland_surface.pid == self.xwayland_surface.pid)
+        {
+            seat.keyboardEnterOrLeave(self.xwayland_surface.surface);
+        } else {
+            seat.setFocusRaw(.{ .xwayland_override_redirect = self });
+        }
     }
 }
 
@@ -113,15 +129,20 @@ fn handleUnmap(listener: *wl.Listener(*wlr.XwaylandSurface), _: *wlr.XwaylandSur
 
     self.commit.link.remove();
 
-    // If the unmapped surface is currently focused, reset focus to the most
-    // appropriate view.
+    // If the unmapped surface is currently focused, pass keyboard focus
+    // to the most appropriate surface.
     var seat_it = server.input_manager.seats.first;
     while (seat_it) |seat_node| : (seat_it = seat_node.next) {
         const seat = &seat_node.data;
-        if (seat.focused == .xwayland_override_redirect and
-            seat.focused.xwayland_override_redirect == self)
-        {
-            seat.focus(null);
+        switch (seat.focused) {
+            .view => |focused| if (focused.impl == .xwayland_view and
+                focused.impl.xwayland_view.xwayland_surface.pid == self.xwayland_surface.pid and
+                seat.wlr_seat.keyboard_state.focused_surface == self.xwayland_surface.surface)
+            {
+                seat.keyboardEnterOrLeave(focused.surface.?);
+            },
+            .xwayland_override_redirect => |focused| if (focused == self) seat.focus(null),
+            .layer, .lock_surface, .none => {},
         }
     }
 
diff --git a/river/XwaylandView.zig b/river/XwaylandView.zig
index 55681e6..0eef7a4 100644
--- a/river/XwaylandView.zig
+++ b/river/XwaylandView.zig
@@ -175,16 +175,6 @@ pub fn getConstraints(self: Self) View.Constraints {
 /// Called when the xwayland surface is destroyed
 fn handleDestroy(listener: *wl.Listener(*wlr.XwaylandSurface), _: *wlr.XwaylandSurface) void {
     const self = @fieldParentPtr(Self, "destroy", listener);
-    const view = self.view;
-
-    // Ensure no seat will attempt to access this view after it is destroyed.
-    var seat_it = server.input_manager.seats.first;
-    while (seat_it) |seat_node| : (seat_it = seat_node.next) {
-        const seat = &seat_node.data;
-        if (seat.activated_xwayland_view == view) {
-            seat.activated_xwayland_view = null;
-        }
-    }
 
     // Remove listeners that are active for the entire lifetime of the view
     self.destroy.link.remove();