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.

5 files changed, 95 insertions, 45 deletions

diff --git a/river/DragIcon.zig b/river/DragIcon.zig
index e61d06f..bb05161 100644
--- a/river/DragIcon.zig
+++ b/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);
 }
 
diff --git a/river/LayerSurface.zig b/river/LayerSurface.zig
index 6f73c4d..68f2dc8 100644
--- a/river/LayerSurface.zig
+++ b/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);
 }
diff --git a/river/Subsurface.zig b/river/Subsurface.zig
index a5ae6e4..0880163 100644
--- a/river/Subsurface.zig
+++ b/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 {
-    const subsurface = @fieldParentPtr(Subsurface, "destroy", listener);
-
-    subsurface.destroy.link.remove();
+/// Destroy this Subsurface and all of its children
+pub fn destroy(subsurface: *Subsurface) void {
+    subsurface.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);
 
diff --git a/river/XdgPopup.zig b/river/XdgPopup.zig
index e243b4b..dce546b 100644
--- a/river/XdgPopup.zig
+++ b/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.map.add(&self.map);
-    wlr_xdg_popup.base.events.unmap.add(&self.unmap);
-    wlr_xdg_popup.base.events.new_popup.add(&self.new_popup);
-    wlr_xdg_popup.base.surface.events.new_subsurface.add(&self.new_subsurface);
+    wlr_xdg_popup.base.events.destroy.add(&xdg_popup.surface_destroy);
+    wlr_xdg_popup.base.events.map.add(&xdg_popup.map);
+    wlr_xdg_popup.base.events.unmap.add(&xdg_popup.unmap);
+    wlr_xdg_popup.base.events.new_popup.add(&xdg_popup.new_popup);
+    wlr_xdg_popup.base.surface.events.new_subsurface.add(&xdg_popup.new_subsurface);
 
     Subsurface.handleExisting(wlr_xdg_popup.base.surface, parent);
 }
 
-fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), wlr_xdg_surface: *wlr.XdgSurface) void {
-    const self = @fieldParentPtr(Self, "destroy", listener);
+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);
+}
 
-    self.destroy.link.remove();
-    self.map.link.remove();
-    self.unmap.link.remove();
-    self.new_popup.link.remove();
-    self.new_subsurface.link.remove();
+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();
+    }
+}
 
-    util.gpa.destroy(self);
+fn handleDestroy(listener: *wl.Listener(*wlr.XdgSurface), wlr_xdg_surface: *wlr.XdgSurface) void {
+    const xdg_popup = @fieldParentPtr(XdgPopup, "surface_destroy", listener);
+    xdg_popup.destroy();
 }
 
 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);
-    self.parent.damageWholeOutput();
+    xdg_popup.wlr_xdg_popup.base.surface.events.commit.add(&xdg_popup.commit);
+    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();
-    self.parent.damageWholeOutput();
+    xdg_popup.commit.link.remove();
+    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);
 }
diff --git a/river/XdgToplevel.zig b/river/XdgToplevel.zig
index bd55969..baddfb8 100644
--- a/river/XdgToplevel.zig
+++ b/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