river: properly teardown surface tree

When an xdg toplevel, layer surface, etc is destroyed, it is not
guaranteed that all the children in the surface tree have already been
destroyed. If there are still children around, destroying the root of
the tree would leave dangling pointers.

To fix this, destroy all children when destroying any node in the tree.

river/DragIcon.zig

@@ -61,6 +61,8 @@ fn handleDestroy(listener: *wl.Listener(*wlr.Drag.Icon), wlr_drag_icon: *wlr.Dra
     drag_icon.unmap.link.remove();
     drag_icon.new_subsurface.link.remove();
 
+    Subsurface.destroySubsurfaces(wlr_drag_icon.surface);
+
     util.gpa.destroy(node);
 }
 

river/LayerSurface.zig

@@ -82,6 +82,12 @@ fn handleDestroy(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface:
     self.commit.link.remove();
     self.new_subsurface.link.remove();
 
+    Subsurface.destroySubsurfaces(self.wlr_layer_surface.surface);
+    var it = wlr_layer_surface.popups.iterator(.forward);
+    while (it.next()) |wlr_xdg_popup| {
+        if (@intToPtr(?*XdgPopup, wlr_xdg_popup.base.data)) |xdg_popup| xdg_popup.destroy();
+    }
+
     const node = @fieldParentPtr(std.TailQueue(Self).Node, "data", self);
     util.gpa.destroy(node);
 }

river/Subsurface.zig

@@ -18,6 +18,7 @@
 const Subsurface = @This();
 
 const std = @import("std");
+const assert = std.debug.assert;
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
 
@@ -26,16 +27,16 @@ const util = @import("util.zig");
 
 const DragIcon = @import("DragIcon.zig");
 const LayerSurface = @import("LayerSurface.zig");
-const View = @import("View.zig");
+const XdgToplevel = @import("XdgToplevel.zig");
 
 pub const Parent = union(enum) {
-    view: *View,
+    xdg_toplevel: *XdgToplevel,
     layer_surface: *LayerSurface,
     drag_icon: *DragIcon,
 
     pub fn damageWholeOutput(parent: Parent) void {
         switch (parent) {
-            .view => |view| view.output.damage.addWhole(),
+            .xdg_toplevel => |xdg_toplevel| xdg_toplevel.view.output.damage.addWhole(),
             .layer_surface => |layer_surface| layer_surface.output.damage.addWhole(),
             .drag_icon => |drag_icon| {
                 var it = server.root.outputs.first;
@@ -50,7 +51,7 @@ parent: Parent,
 wlr_subsurface: *wlr.Subsurface,
 
 // Always active
-destroy: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleDestroy),
+subsurface_destroy: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleDestroy),
 map: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleMap),
 unmap: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleUnmap),
 new_subsurface: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init(handleNewSubsurface),
@@ -65,8 +66,10 @@ pub fn create(wlr_subsurface: *wlr.Subsurface, parent: Parent) void {
         return;
     };
     subsurface.* = .{ .wlr_subsurface = wlr_subsurface, .parent = parent };
+    assert(wlr_subsurface.data == 0);
+    wlr_subsurface.data = @ptrToInt(subsurface);
 
-    wlr_subsurface.events.destroy.add(&subsurface.destroy);
+    wlr_subsurface.events.destroy.add(&subsurface.subsurface_destroy);
     wlr_subsurface.events.map.add(&subsurface.map);
     wlr_subsurface.events.unmap.add(&subsurface.unmap);
     wlr_subsurface.surface.events.new_subsurface.add(&subsurface.new_subsurface);
@@ -85,17 +88,37 @@ pub fn handleExisting(surface: *wlr.Surface, parent: Parent) void {
     while (above_it.next()) |s| Subsurface.create(s, parent);
 }
 
-fn handleDestroy(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void {
+/// Destroy this Subsurface and all of its children
-    const subsurface = @fieldParentPtr(Subsurface, "destroy", listener);
+pub fn destroy(subsurface: *Subsurface) void {
-
+    subsurface.subsurface_destroy.link.remove();
-    subsurface.destroy.link.remove();
     subsurface.map.link.remove();
     subsurface.unmap.link.remove();
     subsurface.new_subsurface.link.remove();
 
+    Subsurface.destroySubsurfaces(subsurface.wlr_subsurface.surface);
+
+    subsurface.wlr_subsurface.data = 0;
     util.gpa.destroy(subsurface);
 }
 
+pub fn destroySubsurfaces(surface: *wlr.Surface) void {
+    var below_it = surface.subsurfaces_below.iterator(.forward);
+    while (below_it.next()) |wlr_subsurface| {
+        if (@intToPtr(?*Subsurface, wlr_subsurface.data)) |s| s.destroy();
+    }
+
+    var above_it = surface.subsurfaces_above.iterator(.forward);
+    while (above_it.next()) |wlr_subsurface| {
+        if (@intToPtr(?*Subsurface, wlr_subsurface.data)) |s| s.destroy();
+    }
+}
+
+fn handleDestroy(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void {
+    const subsurface = @fieldParentPtr(Subsurface, "subsurface_destroy", listener);
+
+    subsurface.destroy();
+}
+
 fn handleMap(listener: *wl.Listener(*wlr.Subsurface), wlr_subsurface: *wlr.Subsurface) void {
     const subsurface = @fieldParentPtr(Subsurface, "map", listener);
 

river/XdgPopup.zig

@@ -15,9 +15,10 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-const Self = @This();
+const XdgPopup = @This();
 
 const std = @import("std");
+const assert = std.debug.assert;
 const wlr = @import("wlroots");
 const wl = @import("wayland").server.wl;
 
@@ -31,7 +32,7 @@ parent: Parent,
 wlr_xdg_popup: *wlr.XdgPopup,
 
 // Always active
-destroy: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleDestroy),
+surface_destroy: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleDestroy),
 map: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleMap),
 unmap: wl.Listener(*wlr.XdgSurface) = wl.Listener(*wlr.XdgSurface).init(handleUnmap),
 new_popup: wl.Listener(*wlr.XdgPopup) = wl.Listener(*wlr.XdgPopup).init(handleNewPopup),
@@ -41,28 +42,30 @@ new_subsurface: wl.Listener(*wlr.Subsurface) = wl.Listener(*wlr.Subsurface).init
 commit: wl.Listener(*wlr.Surface) = wl.Listener(*wlr.Surface).init(handleCommit),
 
 pub fn create(wlr_xdg_popup: *wlr.XdgPopup, parent: Parent) void {
-    const self = util.gpa.create(Self) catch {
+    const xdg_popup = util.gpa.create(XdgPopup) catch {
         std.log.crit("out of memory", .{});
         wlr_xdg_popup.resource.postNoMemory();
         return;
     };
-    self.* = .{
+    xdg_popup.* = .{
         .parent = parent,
         .wlr_xdg_popup = wlr_xdg_popup,
     };
+    assert(wlr_xdg_popup.base.data == 0);
+    wlr_xdg_popup.base.data = @ptrToInt(xdg_popup);
 
     const parent_box = switch (parent) {
-        .view => |view| &view.pending.box,
+        .xdg_toplevel => |xdg_toplevel| &xdg_toplevel.view.pending.box,
         .layer_surface => |layer_surface| &layer_surface.box,
         .drag_icon => unreachable,
     };
     const output_dimensions = switch (parent) {
-        .view => |view| view.output.getEffectiveResolution(),
+        .xdg_toplevel => |xdg_toplevel| xdg_toplevel.view.output.getEffectiveResolution(),
         .layer_surface => |layer_surface| layer_surface.output.getEffectiveResolution(),
         .drag_icon => unreachable,
     };
 
-    // The output box relative to the parent of the popup
+    // The output box relative to the parent of the xdg_popup
     var box = wlr.Box{
         .x = -parent_box.x,
         .y = -parent_box.y,
@@ -71,55 +74,69 @@ pub fn create(wlr_xdg_popup: *wlr.XdgPopup, parent: Parent) void {
     };
     wlr_xdg_popup.unconstrainFromBox(&box);
 
-    wlr_xdg_popup.base.events.destroy.add(&self.destroy);
+    wlr_xdg_popup.base.events.destroy.add(&xdg_popup.surface_destroy);
-    wlr_xdg_popup.base.events.map.add(&self.map);
+    wlr_xdg_popup.base.events.map.add(&xdg_popup.map);
-    wlr_xdg_popup.base.events.unmap.add(&self.unmap);
+    wlr_xdg_popup.base.events.unmap.add(&xdg_popup.unmap);
-    wlr_xdg_popup.base.events.new_popup.add(&self.new_popup);
+    wlr_xdg_popup.base.events.new_popup.add(&xdg_popup.new_popup);
-    wlr_xdg_popup.base.surface.events.new_subsurface.add(&self.new_subsurface);
+    wlr_xdg_popup.base.surface.events.new_subsurface.add(&xdg_popup.new_subsurface);
 
     Subsurface.handleExisting(wlr_xdg_popup.base.surface, parent);
 }
 
+pub fn destroy(xdg_popup: *XdgPopup) void {
+    xdg_popup.surface_destroy.link.remove();
+    xdg_popup.map.link.remove();
+    xdg_popup.unmap.link.remove();
+    xdg_popup.new_popup.link.remove();
+    xdg_popup.new_subsurface.link.remove();
+
+    Subsurface.destroySubsurfaces(xdg_popup.wlr_xdg_popup.base.surface);
+    XdgPopup.destroyPopups(xdg_popup.wlr_xdg_popup.base);
+
+    xdg_popup.wlr_xdg_popup.base.data = 0;
+    util.gpa.destroy(xdg_popup);
+}
+
+pub fn destroyPopups(wlr_xdg_surface: *wlr.XdgSurface) void {
+    var it = wlr_xdg_surface.popups.iterator(.forward);
+    while (it.next()) |wlr_xdg_popup| {
+        if (@intToPtr(?*XdgPopup, wlr_xdg_popup.base.data)) |xdg_popup| xdg_popup.destroy();
+    }
+}
+
 fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), wlr_xdg_surface: *wlr.XdgSurface) void {
-    const self = @fieldParentPtr(Self, "destroy", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "surface_destroy", listener);
-
+    xdg_popup.destroy();
-    self.destroy.link.remove();
-    self.map.link.remove();
-    self.unmap.link.remove();
-    self.new_popup.link.remove();
-    self.new_subsurface.link.remove();
-
-    util.gpa.destroy(self);
 }
 
 fn handleMap(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void {
-    const self = @fieldParentPtr(Self, "map", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "map", listener);
 
-    self.wlr_xdg_popup.base.surface.events.commit.add(&self.commit);
+    xdg_popup.wlr_xdg_popup.base.surface.events.commit.add(&xdg_popup.commit);
-    self.parent.damageWholeOutput();
+    xdg_popup.parent.damageWholeOutput();
 }
 
 fn handleUnmap(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void {
-    const self = @fieldParentPtr(Self, "unmap", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "unmap", listener);
 
-    self.commit.link.remove();
+    xdg_popup.commit.link.remove();
-    self.parent.damageWholeOutput();
+    xdg_popup.parent.damageWholeOutput();
 }
 
 fn handleCommit(listener: *wl.Listener(*wlr.Surface), surface: *wlr.Surface) void {
-    const self = @fieldParentPtr(Self, "commit", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "commit", listener);
 
-    self.parent.damageWholeOutput();
+    xdg_popup.parent.damageWholeOutput();
 }
 
 fn handleNewPopup(listener: *wl.Listener(*wlr.XdgPopup), wlr_xdg_popup: *wlr.XdgPopup) void {
-    const self = @fieldParentPtr(Self, "new_popup", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "new_popup", listener);
 
-    Self.create(wlr_xdg_popup, self.parent);
+    XdgPopup.create(wlr_xdg_popup, xdg_popup.parent);
 }
 
 fn handleNewSubsurface(listener: *wl.Listener(*wlr.Subsurface), new_wlr_subsurface: *wlr.Subsurface) void {
-    const self = @fieldParentPtr(Self, "new_subsurface", listener);
+    const xdg_popup = @fieldParentPtr(XdgPopup, "new_subsurface", listener);
 
-    Subsurface.create(new_wlr_subsurface, self.parent);
+    Subsurface.create(new_wlr_subsurface, xdg_popup.parent);
 }

river/XdgToplevel.zig

@@ -69,7 +69,7 @@ pub fn init(self: *Self, view: *View, xdg_surface: *wlr.XdgSurface) void {
     xdg_surface.events.new_popup.add(&self.new_popup);
     xdg_surface.surface.events.new_subsurface.add(&self.new_subsurface);
 
-    Subsurface.handleExisting(xdg_surface.surface, .{ .view = view });
+    Subsurface.handleExisting(xdg_surface.surface, .{ .xdg_toplevel = self });
 }
 
 pub fn deinit(self: *Self) void {
@@ -80,6 +80,9 @@ pub fn deinit(self: *Self) void {
         self.unmap.link.remove();
         self.new_popup.link.remove();
         self.new_subsurface.link.remove();
+
+        Subsurface.destroySubsurfaces(self.xdg_surface.surface);
+        XdgPopup.destroyPopups(self.xdg_surface);
     }
 }
 
@@ -157,7 +160,6 @@ pub fn getConstraints(self: Self) View.Constraints {
     };
 }
 
-/// Called when the xdg surface is destroyed
 fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), xdg_surface: *wlr.XdgSurface) void {
     const self = @fieldParentPtr(Self, "destroy", listener);
     self.deinit();
@@ -298,12 +300,12 @@ fn handleCommit(listener: *wl.Listener(*wlr.Surface), surface: *wlr.Surface) voi
 
 fn handleNewPopup(listener: *wl.Listener(*wlr.XdgPopup), wlr_xdg_popup: *wlr.XdgPopup) void {
     const self = @fieldParentPtr(Self, "new_popup", listener);
-    XdgPopup.create(wlr_xdg_popup, .{ .view = self.view });
+    XdgPopup.create(wlr_xdg_popup, .{ .xdg_toplevel = self });
 }
 
 fn handleNewSubsurface(listener: *wl.Listener(*wlr.Subsurface), new_wlr_subsurface: *wlr.Subsurface) void {
     const self = @fieldParentPtr(Self, "new_subsurface", listener);
-    Subsurface.create(new_wlr_subsurface, .{ .view = self.view });
+    Subsurface.create(new_wlr_subsurface, .{ .xdg_toplevel = self });
 }
 
 /// Called when the client asks to be fullscreened. We always honor the request