Merge pull request #3602 from lightpanda-io/iterative-rendertree

mem: make markdown/screenshot iterative

src/browser/RenderTree.zig

@@ -83,6 +83,10 @@ pub const Slotted = struct {
     assigned: []const *Node,
     fallback: Children,
 
+    pub fn init(children_: Children) Slotted {
+        return .{ .tree = children_.tree, .assigned = &.{}, .fallback = children_ };
+    }
+
     pub fn next(self: *Slotted) ?Child {
         while (self.assigned.len > 0) {
             const node = self.assigned[0];

src/browser/markdown.zig

@@ -36,6 +36,8 @@ pub const Opts = struct {
 
 const truncation_marker = LimitedWriter.truncation_marker;
 
+const Error = error{ WriteFailed, OutOfMemory };
+
 const State = struct {
     const ListType = enum { ordered, unordered };
     const ListState = struct {
@@ -63,11 +65,31 @@ fn getAnchorLabel(el: *Element) ?[]const u8 {
     return el.getAttributeInterned("aria-label") orelse el.getAttributeInterned("title");
 }
 
+// Iterative else large trees will stackoverflow
 const Context = struct {
     state: State,
     writer: *std.Io.Writer,
     frame: *Frame,
     tree: RenderTree,
+    stack: std.ArrayList(Open) = .empty,
+
+    // Content still to render, followed by what closes the element.
+    const Open = struct {
+        iter: RenderTree.Slotted,
+        epilogue: Epilogue,
+    };
+
+    // what follows after the children
+    const Epilogue = union(enum) {
+        none,
+        element: Element.Tag,
+        block_anchor: struct { href: ?[]const u8, label: ?[]const u8 },
+        inline_anchor: struct { href: ?[]const u8, standalone: bool },
+    };
+
+    fn deinit(self: *Context) void {
+        self.stack.deinit(self.frame.local_arena);
+    }
 
     fn ensureNewline(self: *Context) !void {
         if (!self.state.last_char_was_newline) {
@@ -76,18 +98,32 @@ const Context = struct {
         }
     }
 
-    fn render(self: *Context, node: *Node) error{WriteFailed}!void {
+    fn render(self: *Context, node: *Node) Error!void {
         switch (node._type) {
-            .document, .document_fragment => try self.renderChildren(node, false),
+            .document, .document_fragment => try self.open(.init(self.tree.children(node, false)), .none),
             else => {
                 if (self.tree.classify(node, .{})) |child| {
                     try self.renderChild(child);
                 }
             },
         }
+
+        while (self.stack.items.len > 0) {
+            // renderChild can grow the stack, so we can't re-use top on continue
+            const top = &self.stack.items[self.stack.items.len - 1];
+            if (top.iter.next()) |child| {
+                try self.renderChild(child);
+                continue;
+            }
+            try self.close(self.stack.pop().?.epilogue);
+        }
     }
 
-    fn renderChild(self: *Context, child: RenderTree.Child) error{WriteFailed}!void {
+    fn open(self: *Context, iter: RenderTree.Slotted, epilogue: Epilogue) Error!void {
+        return self.stack.append(self.frame.local_arena, .{ .iter = iter, .epilogue = epilogue });
+    }
+
+    fn renderChild(self: *Context, child: RenderTree.Child) Error!void {
         switch (child.what) {
             .element => |display| {
                 const el = child.node.subtype(Node.Element);
@@ -105,28 +141,8 @@ const Context = struct {
         }
     }
 
-    fn renderChildren(self: *Context, parent: *Node, boxed: bool) error{WriteFailed}!void {
-        var it = self.tree.children(parent, boxed);
-        while (it.next()) |child| {
-            try self.renderChild(child);
-        }
-    }
-
-    fn renderContent(self: *Context, el: *Element, boxed: bool) error{WriteFailed}!void {
-        var it = self.tree.content(el, boxed);
-        while (it.next()) |child| {
-            try self.renderChild(child);
-        }
-    }
-
-    fn renderSlotContent(self: *Context, slot: *Slot) error{WriteFailed}!void {
-        var it = self.tree.slotted(slot);
-        while (it.next()) |child| {
-            try self.renderChild(child);
-        }
-    }
-
-    fn renderElement(self: *Context, el: *Element, display: StyleManager.Display) !void {
+    // write the element's opening markers and push its contents
+    fn renderElement(self: *Context, el: *Element, display: StyleManager.Display) Error!void {
         const tag = el.getTag();
         const boxed = display == .flex or display == .grid;
 
@@ -247,22 +263,17 @@ const Context = struct {
                 const label = getAnchorLabel(el);
                 const href_raw = el.getAttributeInterned("href");
 
-                if (!info.has_visible and label == null and href_raw == null) return;
+                if (!info.has_visible and label == null and href_raw == null) {
+                    return;
+                }
 
                 const href = if (href_raw) |h| URL.resolve(frame.local_arena, frame.base(), h, .{ .encoding = frame.charset }) catch h else null;
 
                 if (info.has_block) {
-                    try self.renderContent(el, boxed);
-                    if (href) |h| {
-                        if (!self.state.last_char_was_newline) try self.writer.writeByte('\n');
-                        try self.writer.writeByte('[');
-                        try self.writer.writeAll(label orelse h);
-                        try self.writer.writeAll("](");
-                        try self.writer.writeAll(h);
-                        try self.writer.writeAll(")\n");
-                        self.state.last_char_was_newline = true;
-                    }
-                    return;
+                    return self.open(.init(self.tree.content(el, boxed)), .{ .block_anchor = .{
+                        .href = href,
+                        .label = label,
+                    } });
                 }
 
                 const standalone = RenderTree.isStandaloneAnchor(el, frame);
@@ -270,23 +281,13 @@ const Context = struct {
                     if (!self.state.last_char_was_newline) try self.writer.writeByte('\n');
                 }
                 try self.writer.writeByte('[');
+
+                const epilogue: Epilogue = .{ .inline_anchor = .{ .href = href, .standalone = standalone } };
                 if (info.has_visible) {
-                    try self.renderContent(el, boxed);
-                } else {
-                    try self.writer.writeAll(label orelse "");
+                    return self.open(.init(self.tree.content(el, boxed)), epilogue);
                 }
-                try self.writer.writeAll("](");
-                if (href) |h| {
-                    try self.writer.writeAll(h);
-                }
-                try self.writer.writeByte(')');
-                if (standalone) {
-                    try self.writer.writeByte('\n');
-                    self.state.last_char_was_newline = true;
-                } else {
-                    self.state.last_char_was_newline = false;
-                }
-                return;
+                try self.writer.writeAll(label orelse "");
+                return self.close(epilogue);
             },
             .input => {
                 const type_attr = el.getAttributeInterned("type") orelse return;
@@ -297,20 +298,49 @@ const Context = struct {
                 }
                 return;
             },
-            .slot => return self.renderSlotContent(el.as(Slot)),
+            .slot => return self.open(self.tree.slotted(el.as(Slot)), .none),
             else => {},
         }
 
-        try self.renderContent(el, boxed);
+        return self.open(.init(self.tree.content(el, boxed)), .{ .element = tag });
+    }
+
+    // Finish the element after renderElement has written the children
+    fn close(self: *Context, epilogue: Epilogue) Error!void {
+        const tag = switch (epilogue) {
+            .none => return,
+            .element => |t| t,
+            .block_anchor => |anchor| {
+                const href = anchor.href orelse return;
+                try self.ensureNewline();
+                try self.writer.writeByte('[');
+                try self.writer.writeAll(anchor.label orelse href);
+                try self.writer.writeAll("](");
+                try self.writer.writeAll(href);
+                try self.writer.writeAll(")\n");
+                return;
+            },
+            .inline_anchor => |anchor| {
+                try self.writer.writeAll("](");
+                if (anchor.href) |h| {
+                    try self.writer.writeAll(h);
+                }
+                try self.writer.writeByte(')');
+                if (anchor.standalone) {
+                    try self.writer.writeByte('\n');
+                    self.state.last_char_was_newline = true;
+                } else {
+                    self.state.last_char_was_newline = false;
+                }
+                return;
+            },
+        };
 
         switch (tag) {
             .pre => {
-                if (!self.state.last_char_was_newline) {
-                    try self.writer.writeByte('\n');
-                }
+                try self.ensureNewline();
                 try self.writer.writeAll("```\n");
                 self.state.pre_node = null;
-                self.state.last_char_was_newline = true;
             },
             .code => {
                 if (self.state.pre_node == null) {
@@ -425,7 +455,9 @@ pub fn dump(state: RenderTree.State, opts: Opts, writer: *std.Io.Writer, frame:
             .frame = frame,
             .tree = .{ .frame = frame, .state = state },
         };
+        defer ctx.deinit();
         ctx.render(node) catch |err| switch (err) {
+            error.OutOfMemory => return err,
             error.WriteFailed => {
                 if (!lw.truncated) return err;
                 try writer.writeAll(truncation_marker);
@@ -444,6 +476,7 @@ pub fn dump(state: RenderTree.State, opts: Opts, writer: *std.Io.Writer, frame:
         .frame = frame,
         .tree = .{ .frame = frame, .state = state },
     };
+    defer ctx.deinit();
     try ctx.render(node);
     if (!ctx.state.last_char_was_newline) {
         try writer.writeByte('\n');
@@ -927,3 +960,25 @@ test "browser.markdown: declarative shadow DOM renders through piercing" {
 
     try testing.expectString("\nshadow content\n", aw.written());
 }
+
+test "browser.markdown: deep nesting doesn't overflow the native stack" {
+    const frame = try testing.createFrame();
+    defer testing.test_session.closeAllPages();
+
+    const depth = 50_000;
+    const doc = frame.window._document;
+    var top = (try doc.createElement("i", null, frame)).asNode();
+    for (1..depth) |_| {
+        const parent = (try doc.createElement("i", null, frame)).asNode();
+        _ = try parent.appendChild(top, frame);
+        top = parent;
+    }
+
+    var aw: std.Io.Writer.Allocating = .init(testing.allocator);
+    defer aw.deinit();
+    try dump(.{ .root = top }, .{}, &aw.writer, frame);
+
+    // Every <i> opens and closes with a '*', then dump's trailing newline.
+    try testing.expectEqual(depth * 2 + 1, aw.written().len);
+    try testing.expectString("**", aw.written()[0..2]);
+}

src/browser/screenshot.zig

@@ -567,11 +567,15 @@ extern "c" fn lp_layout_new(
 extern "c" fn lp_layout_free(h: *LayoutHandle) void;
 extern "c" fn lp_render_abi(out: *LpAbi) void;
 
+// Iterative else large trees will stackoverflow
 const Builder = struct {
     frame: *Frame,
     arena: Allocator,
     tree: RenderTree,
 
+    // Content still to render, followed by what closes the element.
+    stack: std.ArrayList(Open) = .empty,
+
     blocks: std.ArrayList(LpBlock) = .empty,
 
     // The block being built.
@@ -618,6 +622,30 @@ const Builder = struct {
         index: u32,
     };
 
+    const Open = struct {
+        iter: RenderTree.Slotted,
+        epilogue: Epilogue,
+    };
+
+    // what follows after the children
+    const Epilogue = union(enum) {
+        none,
+        block,
+        pre: ?*Node,
+        list: bool,
+        list_item: bool,
+        blockquote,
+        anchor: Anchor,
+        cell: bool,
+        element: Element.Tag,
+
+        const Anchor = struct {
+            linked: bool,
+            prev_href: []const u8,
+            standalone: bool,
+        };
+    };
+
     const Error = Allocator.Error;
 
     fn currentFlags(self: *const Builder) u32 {
@@ -758,9 +786,23 @@ const Builder = struct {
 
     fn render(self: *Builder, node: *Node) Error!void {
         switch (node._type) {
-            .document, .document_fragment => try self.renderChildren(node, false),
+            .document, .document_fragment => try self.open(.init(self.tree.children(node, false)), .none),
             else => if (self.tree.classify(node, .{})) |child| try self.renderChild(child),
         }
+
+        while (self.stack.items.len > 0) {
+            // renderChild can grow the stack, so we can't re-use top on continue
+            const top = &self.stack.items[self.stack.items.len - 1];
+            if (top.iter.next()) |child| {
+                try self.renderChild(child);
+                continue;
+            }
+            try self.close(self.stack.pop().?.epilogue);
+        }
+    }
+
+    fn open(self: *Builder, iter: RenderTree.Slotted, epilogue: Epilogue) Error!void {
+        return self.stack.append(self.arena, .{ .iter = iter, .epilogue = epilogue });
     }
 
     fn renderChild(self: *Builder, child: RenderTree.Child) Error!void {
@@ -771,21 +813,6 @@ const Builder = struct {
         }
     }
 
-    fn renderChildren(self: *Builder, parent: *Node, boxed: bool) Error!void {
-        var it = self.tree.children(parent, boxed);
-        while (it.next()) |child| try self.renderChild(child);
-    }
-
-    fn renderContent(self: *Builder, el: *Element, boxed: bool) Error!void {
-        var it = self.tree.content(el, boxed);
-        while (it.next()) |child| try self.renderChild(child);
-    }
-
-    fn renderSlotContent(self: *Builder, slot: *Slot) Error!void {
-        var it = self.tree.slotted(slot);
-        while (it.next()) |child| try self.renderChild(child);
-    }
-
     fn renderText(self: *Builder, text: []const u8) Error!void {
         if (text.len == 0) return;
 
@@ -824,16 +851,13 @@ const Builder = struct {
                     else => 6,
                 };
                 try self.openBlock(.heading, level);
-                try self.renderContent(el, boxed);
-                return self.closeBlock();
+                return self.open(.init(self.tree.content(el, boxed)), .block);
             },
             .pre => {
                 try self.openBlock(.pre, 0);
                 const prev = self.pre_node;
                 self.pre_node = el.asNode();
-                try self.renderContent(el, boxed);
-                self.pre_node = prev;
-                return self.closeBlock();
+                return self.open(.init(self.tree.content(el, boxed)), .{ .pre = prev });
             },
             .hr => {
                 try self.openBlock(.rule, 0);
@@ -854,10 +878,7 @@ const Builder = struct {
                     self.list_stack[self.list_depth] = .{ .ordered = tag == .ol, .index = 1 };
                     self.list_depth += 1;
                 }
-                try self.renderContent(el, boxed);
-                try self.closeBlock();
-                if (pushed) self.list_depth -= 1;
-                return;
+                return self.open(.init(self.tree.content(el, boxed)), .{ .list = pushed });
             },
             .li => {
                 try self.closeBlock();
@@ -871,19 +892,12 @@ const Builder = struct {
                 } else {
                     self.pending_marker = "•";
                 }
-                try self.renderContent(el, boxed);
-                try self.closeBlock();
-                self.pending_marker = "";
-                if (stray) self.list_depth = 0;
-                return;
+                return self.open(.init(self.tree.content(el, boxed)), .{ .list_item = stray });
             },
             .blockquote => {
                 try self.closeBlock();
                 self.quote_depth +|= 1;
-                try self.renderContent(el, boxed);
-                try self.closeBlock();
-                self.quote_depth -= 1;
-                return;
+                return self.open(.init(self.tree.content(el, boxed)), .blockquote);
             },
             .img => {
                 const alt = el.getAttributeInterned("alt") orelse return;
@@ -926,24 +940,19 @@ const Builder = struct {
                     // point outside the document.
                     self.href = URL.resolve(self.arena, self.frame.base(), h, .{ .encoding = self.frame.charset }) catch h;
                 }
+
+                const epilogue: Epilogue = .{ .anchor = .{
+                    .linked = href != null,
+                    .prev_href = prev_href,
+                    .standalone = standalone,
+                } };
                 if (info.has_visible) {
-                    try self.renderContent(el, boxed);
-                } else {
-                    try self.renderText(label.?);
+                    return self.open(.init(self.tree.content(el, boxed)), epilogue);
                 }
-                if (href != null) {
-                    self.link -= 1;
-                    self.href = prev_href;
-                }
-                if (standalone) {
-                    try self.closeBlock();
-                    self.tight -= 1;
-                } else {
-                    self.after_anchor = true;
-                }
-                return;
+                try self.renderText(label.?);
+                return self.close(epilogue);
             },
-            .slot => return self.renderSlotContent(el.as(Slot)),
+            .slot => return self.open(self.tree.slotted(el.as(Slot)), .none),
             .td, .th => {
                 if (self.has_content) {
                     self.pending_space = true;
@@ -953,19 +962,12 @@ const Builder = struct {
                     self.pending_space = true;
                 }
                 if (tag == .th) self.bold += 1;
-                try self.renderContent(el, boxed);
-                if (tag == .th) self.bold -= 1;
-                self.pending_space = true;
-                return;
+                return self.open(.init(self.tree.content(el, boxed)), .{ .cell = tag == .th });
             },
             else => {},
         }
 
-        const block = tag.isBlock() or switch (tag) {
-            .tr, .dt, .dd, .details, .summary, .caption, .legend, .option, .textarea => true,
-            else => false,
-        };
-        if (block) try self.closeBlock();
+        if (isBlockTag(tag)) try self.closeBlock();
 
         switch (tag) {
             .b, .strong => self.bold += 1,
@@ -975,22 +977,70 @@ const Builder = struct {
             .code => self.mono += 1,
             else => {},
         }
-        try self.renderContent(el, boxed);
-        switch (tag) {
-            .b, .strong => self.bold -= 1,
-            .i, .em, .dfn => self.italic -= 1,
-            .ins => self.underline -= 1,
-            .s, .del => self.strike -= 1,
-            .code => self.mono -= 1,
-            else => {},
-        }
+        return self.open(.init(self.tree.content(el, boxed)), .{ .element = tag });
+    }
 
-        if (block) {
-            try self.closeBlock();
+    // Finish the element after renderElement has written the children
+    fn close(self: *Builder, epilogue: Epilogue) Error!void {
+        switch (epilogue) {
+            .none => {},
+            .block => try self.closeBlock(),
+            .pre => |prev| {
+                self.pre_node = prev;
+                try self.closeBlock();
+            },
+            .list => |pushed| {
+                try self.closeBlock();
+                if (pushed) self.list_depth -= 1;
+            },
+            .list_item => |stray| {
+                try self.closeBlock();
+                self.pending_marker = "";
+                if (stray) self.list_depth = 0;
+            },
+            .blockquote => {
+                try self.closeBlock();
+                self.quote_depth -= 1;
+            },
+            .anchor => |anchor| {
+                if (anchor.linked) {
+                    self.link -= 1;
+                    self.href = anchor.prev_href;
+                }
+                if (anchor.standalone) {
+                    try self.closeBlock();
+                    self.tight -= 1;
+                } else {
+                    self.after_anchor = true;
+                }
+            },
+            .cell => |header| {
+                if (header) self.bold -= 1;
+                self.pending_space = true;
+            },
+            .element => |tag| {
+                switch (tag) {
+                    .b, .strong => self.bold -= 1,
+                    .i, .em, .dfn => self.italic -= 1,
+                    .ins => self.underline -= 1,
+                    .s, .del => self.strike -= 1,
+                    .code => self.mono -= 1,
+                    else => {},
+                }
+                if (isBlockTag(tag)) try self.closeBlock();
+            },
         }
     }
 };
 
+// Tags that get a block of their own, beyond the ones CSS calls blocks.
+fn isBlockTag(tag: Element.Tag) bool {
+    return tag.isBlock() or switch (tag) {
+        .tr, .dt, .dd, .details, .summary, .caption, .legend, .option, .textarea => true,
+        else => false,
+    };
+}
+
 const testing = @import("../testing.zig");
 test "browser.screenshot: rust abi matches" {
     var got: LpAbi = undefined;
@@ -1535,3 +1585,27 @@ test "browser.screenshot: collect honours strip flags" {
     try testing.expectEqual(1, stripped.len);
     try testing.expectEqual("Body text", try S.text(stripped[0], arena));
 }
+
+test "browser.screenshot: deep nesting doesn't overflow the native stack" {
+    defer testing.test_session.closeAllPages();
+    const frame = try testing.createFrame();
+
+    // <div>, not an inline tag: the inline style counters are u8, and nesting
+    // one 256 deep overflows them long before the walk is the problem.
+    const depth = 50_000;
+    const doc = frame.window._document;
+    const innermost = (try doc.createElement("div", null, frame)).asNode();
+    _ = try innermost.appendChild(try doc.createTextNode("deep"), frame);
+
+    var top = innermost;
+    for (1..depth) |_| {
+        const parent = (try doc.createElement("div", null, frame)).asNode();
+        _ = try parent.appendChild(top, frame);
+        top = parent;
+    }
+
+    const blocks = try collect(testing.arena_allocator, .{ .root = top }, frame);
+    try testing.expectEqual(1, blocks.len);
+    try testing.expectEqual(1, blocks[0].spans_len);
+    try testing.expectEqual("deep", blocks[0].spans[0].text[0..blocks[0].spans[0].len]);
+}