Fix crash when killing WM with windows active

In commit 75308a04, I added an assertion to Output.destroy() to verify
that all of its windows had been removed already. The issue is, that
only happened when Output received a .removed event, but that never
came when killing the WM directly. Instead, I simplified the handling
for the .removed event and moved most of that into destroy().

I also changed where structs have their wl_list.link removed (to inside
of their destroys) to make it more consistent. Finally,
XkbBinding.sendWindowToOutput asserts that the current output is actually
focused. Users *should* never have a focused window if there's not also
a focused output.

src/Context.zig

@@ -125,7 +125,6 @@ pub fn manage(context: *Context) void {
         // Destroy all existing bindings
         var it = context.xkb_bindings.bindings.safeIterator(.forward);
         while (it.next()) |binding| {
-            binding.link.remove();
             binding.destroy();
         }
 

src/Output.zig

@@ -111,8 +111,46 @@ pub fn create(context: *Context, river_output_v1: *river.OutputV1) !*Output {
 }
 
 pub fn destroy(output: *Output) void {
-    // All of windows should've been removed from this output when handling the .removed event
+    const context = output.context;
-    assert(output.windows.first() == null);
+    const wm = context.wm;
+
+    // Before we destroy the output, we wnant to take care of any window that are on it
+    // We chooe to move windows to the previous output in the WM's output list
+    // If this was the only output, windows go to the WM's orphan window list instead
+    const prev_output: ?*Output = if (wm.prevOutput(output)) |prev| blk: {
+        if (prev == output) break :blk null; // Only output; wrapped to itself
+        break :blk prev;
+    } else unreachable;
+
+    const window_pending_output: Window.PendingManage.PendingOutput =
+        if (prev_output) |prev|
+            .{ .output = prev }
+        else
+            .clear_output;
+
+    // Update each window's output before moving the list
+    var it = output.windows.iterator(.forward);
+    while (it.next()) |window| {
+        window.pending_manage.pending_output = window_pending_output;
+    }
+
+    // Finally, move the windows to their new output (or the orphan list)
+    const dest_list = if (prev_output) |prev| &prev.windows else &wm.orphan_windows;
+    dest_list.appendList(&output.windows);
+
+    seat_blk: {
+        // If the output was focused, move focus to the output that took this one's windows
+        // (and its first window, if any).
+        const seat = wm.seats.first() orelse break :seat_blk;
+        if (seat.focused_output != output) break :seat_blk;
+
+        seat.pending_manage.output = if (prev_output) |prev| prev_blk: {
+            if (prev.windows.first()) |window| {
+                seat.pending_manage.window = .{ .window = window };
+            }
+            break :prev_blk .{ .output = prev };
+        } else .clear_focus;
+    }
 
     // Deinit optional surfaces
     if (output.bar) |*bar| bar.deinit();
@@ -128,6 +166,7 @@ pub fn destroy(output: *Output) void {
     output.river_output_v1.destroy();
     if (output.wl_output) |wl_output| wl_output.release();
 
+    output.link.remove();
     utils.gpa.destroy(output);
 }
 
@@ -166,54 +205,6 @@ fn riverOutputListener(river_output_v1: *river.OutputV1, event: river.OutputV1.E
     assert(output.river_output_v1 == river_output_v1);
     switch (event) {
         .removed => {
-            const context = output.context;
-            const wm = context.wm;
-
-            // Move windows to the previous output in the list.
-            // If this was the only output, windows become orphans.
-            const prev_output: ?*Output = if (wm.prevOutput(output)) |prev| blk: {
-                if (prev == output) break :blk null; // Only output; wrapped to itself
-                break :blk prev; // We got the previous list
-            } else unreachable;
-
-            const window_pending_output: Window.PendingManage.PendingOutput =
-                if (prev_output) |prev|
-                    .{ .output = prev }
-                else
-                    .clear_output;
-
-            // Update each window's output before moving the list
-            var it = output.windows.iterator(.forward);
-            while (it.next()) |window| {
-                window.pending_manage.pending_output = window_pending_output;
-            }
-
-            // Move windows to new destination
-            const dest_list = if (prev_output) |prev| &prev.windows else &wm.orphan_windows;
-            dest_list.appendList(&output.windows);
-
-            blk: {
-                // If the removed output was focused, move focus to the next
-                // available output (and its first window, if any).
-                const seat = wm.seats.first() orelse break :blk;
-                if (seat.focused_output != output) break :blk;
-
-                const next_output = wm.nextOutput(output);
-                if (next_output == output or next_output == null) {
-                    // This was the last output; clear focus
-                    seat.pending_manage.output = .clear_focus;
-                    seat.pending_manage.window = .clear_focus;
-                    break :blk;
-                }
-                const o = next_output.?;
-
-                seat.pending_manage.output = .{ .output = o };
-                if (o.windows.first()) |window| {
-                    seat.pending_manage.window = .{ .window = window };
-                }
-            }
-
-            output.link.remove();
             output.destroy();
         },
         .wl_output => |ev| {

src/Seat.zig

@@ -88,6 +88,7 @@ pub fn destroy(seat: *Seat) void {
     if (seat.resize_pointer_binding) |binding| binding.destroy();
     seat.river_seat_v1.destroy();
     seat.river_layer_shell_seat_v1.destroy();
+    seat.link.remove();
     utils.gpa.destroy(seat);
 }
 

src/Window.zig

@@ -126,6 +126,7 @@ pub fn destroy(window: *Window) void {
     window.river_node_v1.destroy();
     window.river_window_v1.destroy();
 
+    window.link.remove();
     utils.gpa.destroy(window);
 }
 
@@ -156,7 +157,6 @@ fn windowListener(river_window_v1: *river.WindowV1, event: river.WindowV1.Event,
             }
             window.nextFocus();
 
-            window.link.remove();
             window.destroy();
         },
         .dimensions => |ev| {

src/WindowManager.zig

@@ -41,21 +41,18 @@ pub fn destroy(wm: *WindowManager) void {
     {
         var it = wm.outputs.safeIterator(.forward);
         while (it.next()) |output| {
-            output.link.remove();
             output.destroy();
         }
     }
     {
         var it = wm.seats.safeIterator(.forward);
         while (it.next()) |seat| {
-            seat.link.remove();
             seat.destroy();
         }
     }
     {
         var it = wm.orphan_windows.safeIterator(.forward);
         while (it.next()) |window| {
-            window.link.remove();
             window.destroy();
         }
     }

src/XkbBindings.zig

@@ -120,19 +120,15 @@ const XkbBinding = struct {
 
     pub fn destroy(xkb_binding: *XkbBinding) void {
         xkb_binding.xkb_binding_v1.destroy();
+        xkb_binding.link.remove();
         utils.gpa.destroy(xkb_binding);
     }
 
     fn xkbBindingListener(river_xkb_binding_v1: *river.XkbBindingV1, event: river.XkbBindingV1.Event, xkb_binding: *XkbBinding) void {
         assert(xkb_binding.xkb_binding_v1 == river_xkb_binding_v1);
         switch (event) {
-            .pressed => {
+            .pressed => xkb_binding.executeCommand(),
-                xkb_binding.executeCommand();
+            else => {},
-            },
-            .released => {},
-            else => |ev| {
-                log.debug("unhandled event: {s}", .{@tagName(ev)});
-            },
         }
     }
 
@@ -375,20 +371,20 @@ const XkbBinding = struct {
         }
     }
 
+    /// This function requires that the window is currently on an output
+    /// AND that the output is the currently focused output on the first seat.
     fn sendWindowToOutput(context: *Context, direction: FocusDirection) void {
         const wm = context.wm;
         const seat = wm.seats.first() orelse return;
         const window = seat.focused_window orelse return;
-        assert(window.output == seat.focused_output);
 
-        const pending_output = if (seat.focused_output) |current|
+        assert(window.output == seat.focused_output);
-            switch (direction) {
+        assert(seat.focused_output != null);
-                .next => wm.nextOutput(current),
+
-                .prev => wm.prevOutput(current),
+        const current_output = seat.focused_output.?;
-            }
+        const pending_output = switch (direction) {
-        else switch (direction) {
+            .next => wm.nextOutput(current_output),
-            .next => wm.outputs.first(),
+            .prev => wm.prevOutput(current_output),
-            .prev => wm.outputs.last(),
         };
 
         if (pending_output) |output| blk: {

src/XkbConfig.zig

@@ -42,8 +42,8 @@ const Keyboard = struct {
     }
 
     fn destroy(keyboard: *Keyboard) void {
-        keyboard.link.remove();
         keyboard.xkb_keyboard_v1.destroy();
+        keyboard.link.remove();
         utils.gpa.destroy(keyboard);
     }