Make intervals easier and faster, add window.setInterval and clearInterval

When the browser microtask was added, zig-specific timeout functions were
added to the loop. This was necessary for two reasons:
1 - The existing functions were JS specific
2 - We wanted a different reset counter for JS and Zig

Like we did in https://github.com/lightpanda-io/browser/pull/577, the loop is
now JS-agnostic. It gets a Zig callback, and the Zig callback can execute JS
(or do whatever). An intrusive node, like with events, is used to minimize
allocations.

Also, because the microtask was recently moved to the page, there is no longer
a need for separate event counters. All timeouts are scoped to the page.

The new timeout callback can now be used to efficiently reschedule a task. This
reuses the IO.completion and Context, avoiding 2 allocations. More importantly
it makes the internal timer_id static for the lifetime of an "interval". This
is important for window.setInterval, where the callback can itself clear the
interval, which we would need to detect in the callback handler to avoid
re-scheduling. With the stable timer_id, the existing cancel mechanism works
as expected.

The loop no longer has a cbk_error. Callback code is expected to try/catch
callbacks (or use callback.tryCall) and handle errors accordingly.

src/browser/browser.zig

@@ -32,6 +32,7 @@ const Walker = @import("dom/walker.zig").WalkerDepthFirst;
 
 const Env = @import("env.zig").Env;
 const App = @import("../app.zig").App;
+const Loop = @import("../runtime/loop.zig").Loop;
 
 const URL = @import("../url.zig").URL;
 
@@ -171,8 +172,7 @@ pub const Session = struct {
 
         std.debug.assert(self.page != null);
         // Reset all existing callbacks.
-        self.browser.app.loop.resetJS();
+        self.browser.app.loop.reset();
-        self.browser.app.loop.resetZig();
         self.executor.endScope();
         self.page = null;
 
@@ -230,6 +230,8 @@ pub const Page = struct {
 
     renderer: FlatRenderer,
 
+    microtask_node: Loop.CallbackNode,
+
     window_clicked_event_node: parser.EventNode,
 
     scope: *Env.Scope,
@@ -248,6 +250,7 @@ pub const Page = struct {
             .url = URL.empty,
             .session = session,
             .renderer = FlatRenderer.init(arena),
+            .microtask_node = .{ .func = microtaskCallback },
             .window_clicked_event_node = .{ .func = windowClicked },
             .state = .{
                 .arena = arena,
@@ -264,13 +267,13 @@ pub const Page = struct {
         // load polyfills
         try polyfill.load(self.arena, self.scope);
 
-        self.microtaskLoop();
+        // _ = try session.browser.app.loop.timeout(1 * std.time.ns_per_ms, &self.microtask_node);
     }
 
-    fn microtaskLoop(self: *Page) void {
+    fn microtaskCallback(node: *Loop.CallbackNode, repeat_delay: *?u63) void {
-        const browser = self.session.browser;
+        const self: *Page = @fieldParentPtr("microtask_node", node);
-        browser.runMicrotasks();
+        self.session.browser.runMicrotasks();
-        browser.app.loop.zigTimeout(1 * std.time.ns_per_ms, *Page, self, microtaskLoop);
+        repeat_delay.* = 1 * std.time.ns_per_ms;
     }
 
     // dump writes the page content into the given file.
@@ -297,20 +300,19 @@ pub const Page = struct {
     }
 
     pub fn wait(self: *Page) !void {
-        // try catch
         var try_catch: Env.TryCatch = undefined;
         try_catch.init(self.scope);
         defer try_catch.deinit();
 
-        self.session.browser.app.loop.run() catch |err| {
+        try self.session.browser.app.loop.run();
-            if (try try_catch.err(self.arena)) |msg| {
+
-                log.info("wait error: {s}", .{msg});
+        if (try_catch.hasCaught() == false) {
-                return;
+            log.debug("wait: OK", .{});
-            } else {
+            return;
-                log.info("wait error: {any}", .{err});
+        }
-            }
+
-        };
+        const msg = (try try_catch.err(self.arena)) orelse "unknown";
-        log.debug("wait: OK", .{});
+        log.info("wait error: {s}", .{msg});
     }
 
     pub fn origin(self: *const Page, arena: Allocator) ![]const u8 {

src/browser/html/window.zig

@@ -21,6 +21,7 @@ const std = @import("std");
 const parser = @import("../netsurf.zig");
 const Callback = @import("../env.zig").Callback;
 const SessionState = @import("../env.zig").SessionState;
+const Loop = @import("../../runtime/loop.zig").Loop;
 
 const Navigator = @import("navigator.zig").Navigator;
 const History = @import("history.zig").History;
@@ -31,6 +32,8 @@ const EventTarget = @import("../dom/event_target.zig").EventTarget;
 
 const storage = @import("../storage/storage.zig");
 
+const log = std.log.scoped(.window);
+
 // https://dom.spec.whatwg.org/#interface-window-extensions
 // https://html.spec.whatwg.org/multipage/nav-history-apis.html#window
 pub const Window = struct {
@@ -45,10 +48,9 @@ pub const Window = struct {
     location: Location = .{},
     storage_shelf: ?*storage.Shelf = null,
 
-    // store a map between internal timeouts ids and pointers to uint.
+    // counter for having unique timer ids
-    // the maximum number of possible timeouts is fixed.
+    timer_id: u31 = 0,
-    timeoutid: u32 = 0,
+    timers: std.AutoHashMapUnmanaged(u32, *TimerCallback) = .{},
-    timeoutids: [512]u64 = undefined,
 
     crypto: Crypto = .{},
     console: Console = .{},
@@ -129,23 +131,93 @@ pub const Window = struct {
 
     // TODO handle callback arguments.
     pub fn _setTimeout(self: *Window, cbk: Callback, delay: ?u32, state: *SessionState) !u32 {
-        if (self.timeoutid >= self.timeoutids.len) return error.TooMuchTimeout;
+        return self.createTimeout(cbk, delay, state, false);
+    }
 
-        const ddelay: u63 = delay orelse 0;
+    // TODO handle callback arguments.
-        const id = try state.loop.timeout(ddelay * std.time.ns_per_ms, cbk);
+    pub fn _setInterval(self: *Window, cbk: Callback, delay: ?u32, state: *SessionState) !u32 {
-
+        return self.createTimeout(cbk, delay, state, true);
-        self.timeoutids[self.timeoutid] = id;
-        defer self.timeoutid += 1;
-
-        return self.timeoutid;
     }
 
     pub fn _clearTimeout(self: *Window, id: u32, state: *SessionState) !void {
-        // I do would prefer return an error in this case, but it seems some JS
+        const kv = self.timers.fetchRemove(id) orelse return;
-        // uses invalid id, in particular id 0.
+        try state.loop.cancel(kv.value.loop_id);
-        // So we silently ignore invalid id for now.
+    }
-        if (id >= self.timeoutid) return;
 
-        try state.loop.cancel(self.timeoutids[id], null);
+    pub fn _clearInterval(self: *Window, id: u32, state: *SessionState) !void {
+        const kv = self.timers.fetchRemove(id) orelse return;
+        try state.loop.cancel(kv.value.loop_id);
+    }
+
+    pub fn createTimeout(self: *Window, cbk: Callback, delay_: ?u32, state: *SessionState, comptime repeat: bool) !u32 {
+        if (self.timers.count() > 512) {
+            return error.TooManyTimeout;
+        }
+        const timer_id = self.timer_id +% 1;
+        self.timer_id = timer_id;
+
+        const arena = state.arena;
+
+        const gop = try self.timers.getOrPut(arena, timer_id);
+        if (gop.found_existing) {
+            // this can only happen if we've created 2^31 timeouts.
+            return error.TooManyTimeout;
+        }
+        errdefer _ = self.timers.remove(timer_id);
+
+        const delay: u63 = (delay_ orelse 0) * std.time.ns_per_ms;
+        const callback = try arena.create(TimerCallback);
+
+        callback.* = .{
+            .cbk = cbk,
+            .loop_id = 0, // we're going to set this to a real value shortly
+            .window = self,
+            .timer_id = timer_id,
+            .node = .{ .func = TimerCallback.run },
+            .repeat = if (repeat) delay else null,
+        };
+        callback.loop_id = try state.loop.timeout(delay, &callback.node);
+
+        gop.value_ptr.* = callback;
+        return timer_id;
+    }
+};
+
+const TimerCallback = struct {
+    // the internal loop id, need it when cancelling
+    loop_id: usize,
+
+    // the id of our timer (windows.timers key)
+    timer_id: u31,
+
+    // The JavaScript callback to execute
+    cbk: Callback,
+
+    // This is the internal data that the event loop tracks. We'll get this
+    // back in run and, from it, can get our TimerCallback instance
+    node: Loop.CallbackNode = undefined,
+
+    // if the event should be repeated
+    repeat: ?u63 = null,
+
+    window: *Window,
+
+    fn run(node: *Loop.CallbackNode, repeat_delay: *?u63) void {
+        const self: *TimerCallback = @fieldParentPtr("node", node);
+
+        var result: Callback.Result = undefined;
+        self.cbk.tryCall(.{}, &result) catch {
+            log.err("timeout callback error: {s}", .{result.exception});
+            log.debug("stack:\n{s}", .{result.stack orelse "???"});
+        };
+
+        if (self.repeat) |r| {
+            // setInterval
+            repeat_delay.* = r;
+            return;
+        }
+
+        // setTimeout
+        _ = self.window.timers.remove(self.timer_id);
     }
 };

src/main_wpt.zig

@@ -156,12 +156,11 @@ fn run(arena: Allocator, test_file: []const u8, loader: *FileLoader, err_out: *?
         var try_catch: Env.TryCatch = undefined;
         try_catch.init(runner.scope);
         defer try_catch.deinit();
-        runner.loop.run() catch |err| {
+        try runner.loop.run();
-            if (try try_catch.err(arena)) |msg| {
+
-                err_out.* = msg;
+        if (try_catch.hasCaught()) {
-            }
+            err_out.* = (try try_catch.err(arena)) orelse "unknwon error";
-            return err;
+        }
-        };
     }
 
     // Check the final test status.

src/runtime/loop.zig

@@ -37,24 +37,15 @@ pub const Loop = struct {
     alloc: std.mem.Allocator, // TODO: unmanaged version ?
     io: IO,
 
-    // both events_nb are used to track how many callbacks are to be called.
+    // Used to track how many callbacks are to be called and wait until all
-    // We use these counters to wait until all the events are finished.
+    // event are finished.
-    js_events_nb: usize,
+    events_nb: usize,
-    zig_events_nb: usize,
 
-    cbk_error: bool = false,
+    // ctx_id is incremented each time the loop is reset.
-
+    // All callbacks store an initial ctx_id and compare before execution.
-    // js_ctx_id is incremented each time the loop is reset for JS.
-    // All JS callbacks store an initial js_ctx_id and compare before execution.
     // If a ctx is outdated, the callback is ignored.
-    // This is a weak way to cancel all future JS callbacks.
+    // This is a weak way to cancel all future callbacks.
-    js_ctx_id: u32 = 0,
+    ctx_id: u32 = 0,
-
-    // zig_ctx_id is incremented each time the loop is reset for Zig.
-    // All Zig callbacks store an initial zig_ctx_id and compare before execution.
-    // If a ctx is outdated, the callback is ignored.
-    // This is a weak way to cancel all future Zig callbacks.
-    zig_ctx_id: u32 = 0,
 
     // We use this to track cancellation ids and, on the timeout callback,
     // we can can check here to see if it's been cancelled.
@@ -66,32 +57,27 @@ pub const Loop = struct {
     const Self = @This();
     pub const Completion = IO.Completion;
 
-    pub const ConnectError = IO.ConnectError;
     pub const RecvError = IO.RecvError;
     pub const SendError = IO.SendError;
+    pub const ConnectError = IO.ConnectError;
 
     pub fn init(alloc: std.mem.Allocator) !Self {
         return Self{
             .alloc = alloc,
             .cancelled = .{},
             .io = try IO.init(32, 0),
-            .js_events_nb = 0,
+            .events_nb = 0,
-            .zig_events_nb = 0,
             .timeout_pool = MemoryPool(ContextTimeout).init(alloc),
             .event_callback_pool = MemoryPool(EventCallbackContext).init(alloc),
         };
     }
 
     pub fn deinit(self: *Self) void {
-        // first disable callbacks for existing events.
+        self.reset();
-        // We don't want a callback re-create a setTimeout, it could create an
-        // infinite loop on wait for events.
-        self.resetJS();
-        self.resetZig();
 
         // run tail events. We do run the tail events to ensure all the
         // contexts are correcly free.
-        while (self.eventsNb(.js) > 0 or self.eventsNb(.zig) > 0) {
+        while (self.eventsNb() > 0) {
             self.io.run_for_ns(10 * std.time.ns_per_ms) catch |err| {
                 log.err("deinit run tail events: {any}", .{err});
                 break;
@@ -112,40 +98,24 @@ pub const Loop = struct {
     // Note that I/O events callbacks might register more I/O events
     // on the go when they are executed (ie. nested I/O events).
     pub fn run(self: *Self) !void {
-        while (self.eventsNb(.js) > 0) {
+        while (self.eventsNb() > 0) {
             try self.io.run_for_ns(10 * std.time.ns_per_ms);
             // at each iteration we might have new events registred by previous callbacks
         }
-        // TODO: return instead immediatly on the first JS callback error
-        // and let the caller decide what to do next
-        // (typically retrieve the exception through the TryCatch and
-        // continue the execution of callbacks with a new call to loop.run)
-        if (self.cbk_error) {
-            return error.JSExecCallback;
-        }
-    }
-
-    const Event = enum { js, zig };
-
-    fn eventsPtr(self: *Self, comptime event: Event) *usize {
-        return switch (event) {
-            .zig => &self.zig_events_nb,
-            .js => &self.js_events_nb,
-        };
     }
 
     // Register events atomically
     // - add 1 event and return previous value
-    fn addEvent(self: *Self, comptime event: Event) void {
+    fn addEvent(self: *Self) void {
-        _ = @atomicRmw(usize, self.eventsPtr(event), .Add, 1, .acq_rel);
+        _ = @atomicRmw(usize, &self.events_nb, .Add, 1, .acq_rel);
     }
     // - remove 1 event and return previous value
-    fn removeEvent(self: *Self, comptime event: Event) void {
+    fn removeEvent(self: *Self) void {
-        _ = @atomicRmw(usize, self.eventsPtr(event), .Sub, 1, .acq_rel);
+        _ = @atomicRmw(usize, &self.events_nb, .Sub, 1, .acq_rel);
     }
     // - get the number of current events
-    fn eventsNb(self: *Self, comptime event: Event) usize {
+    fn eventsNb(self: *Self) usize {
-        return @atomicLoad(usize, self.eventsPtr(event), .seq_cst);
+        return @atomicLoad(usize, &self.events_nb, .seq_cst);
     }
 
     // JS callbacks APIs
@@ -153,10 +123,18 @@ pub const Loop = struct {
 
     // Timeout
 
+    // The state that we add to a timeout. This is what we get back from a
+    // timeoutCallback. It contains the function to execute. The user is expected
+    // to be able to turn a reference to this into whatever state it needs,
+    // probably by inserting this node into its own stae and using @fieldParentPtr
+    pub const CallbackNode = struct {
+        func: *const fn (node: *CallbackNode, repeat: *?u63) void,
+    };
+
     const ContextTimeout = struct {
         loop: *Self,
-        js_cbk: ?JSCallback,
+        ctx_id: u32,
-        js_ctx_id: u32,
+        callback_node: ?*CallbackNode,
     };
 
     fn timeoutCallback(
@@ -164,21 +142,25 @@ pub const Loop = struct {
         completion: *IO.Completion,
         result: IO.TimeoutError!void,
     ) void {
+        var repeating = false;
         const loop = ctx.loop;
+
         defer {
-            loop.removeEvent(.js);
+            loop.removeEvent();
-            loop.timeout_pool.destroy(ctx);
+            if (repeating == false) {
-            loop.alloc.destroy(completion);
+                loop.timeout_pool.destroy(ctx);
+                loop.alloc.destroy(completion);
+            }
         }
 
         if (loop.cancelled.remove(@intFromPtr(completion))) {
             return;
         }
 
-        // If the loop's context id has changed, don't call the js callback
+        // Abort if this completion was created for a different version of the loop.
-        // function. The callback's memory has already be cleaned and the
+        if (ctx.ctx_id != loop.ctx_id) {
-        // events nb reset.
+            return;
-        if (ctx.js_ctx_id != loop.js_ctx_id) return;
+        }
 
         // TODO: return the error to the callback
         result catch |err| {
@@ -189,56 +171,51 @@ pub const Loop = struct {
             return;
         };
 
-        // js callback
+        if (ctx.callback_node) |cn| {
-        if (ctx.js_cbk) |*js_cbk| {
+            var repeat_in: ?u63 = null;
-            js_cbk.call(null) catch {
+            cn.func(cn, &repeat_in);
-                loop.cbk_error = true;
+            if (repeat_in) |r| {
-            };
+                // prevents our context and completion from being cleaned up
+                repeating = true;
+                loop.scheduleTimeout(r, ctx, completion);
+            }
         }
     }
 
-    pub fn timeout(self: *Self, nanoseconds: u63, js_cbk: ?JSCallback) !usize {
+    pub fn timeout(self: *Self, nanoseconds: u63, callback_node: ?*CallbackNode) !usize {
         const completion = try self.alloc.create(Completion);
         errdefer self.alloc.destroy(completion);
         completion.* = undefined;
 
         const ctx = try self.timeout_pool.create();
         errdefer self.timeout_pool.destroy(ctx);
-        ctx.* = ContextTimeout{
+        ctx.* = .{
             .loop = self,
-            .js_cbk = js_cbk,
+            .ctx_id = self.ctx_id,
-            .js_ctx_id = self.js_ctx_id,
+            .callback_node = callback_node,
         };
 
-        self.addEvent(.js);
+        self.scheduleTimeout(nanoseconds, ctx, completion);
-        self.io.timeout(*ContextTimeout, ctx, timeoutCallback, completion, nanoseconds);
         return @intFromPtr(completion);
     }
 
-    pub fn cancel(self: *Self, id: usize, js_cbk: ?JSCallback) !void {
+    fn scheduleTimeout(self: *Self, nanoseconds: u63, ctx: *ContextTimeout, completion: *Completion) void {
+        self.addEvent();
+        self.io.timeout(*ContextTimeout, ctx, timeoutCallback, completion, nanoseconds);
+    }
+
+    pub fn cancel(self: *Self, id: usize) !void {
         try self.cancelled.put(self.alloc, id, {});
-        if (js_cbk) |cbk| {
-            cbk.call(null) catch {
-                self.cbk_error = true;
-            };
-        }
     }
 
-    // Reset all existing JS callbacks.
+    // Reset all existing callbacks.
     // The existing events will happen and their memory will be cleanup but the
     // corresponding callbacks will not be called.
-    pub fn resetJS(self: *Self) void {
+    pub fn reset(self: *Self) void {
-        self.js_ctx_id += 1;
+        self.ctx_id += 1;
         self.cancelled.clearRetainingCapacity();
     }
 
-    // Reset all existing Zig callbacks.
-    // The existing events will happen and their memory will be cleanup but the
-    // corresponding callbacks will not be called.
-    pub fn resetZig(self: *Self) void {
-        self.zig_ctx_id += 1;
-    }
-
     // IO callbacks APIs
     // -----------------
 
@@ -256,7 +233,7 @@ pub const Loop = struct {
         const onConnect = struct {
             fn onConnect(callback: *EventCallbackContext, completion_: *Completion, res: ConnectError!void) void {
                 defer callback.loop.event_callback_pool.destroy(callback);
-                callback.loop.removeEvent(.js);
+                callback.loop.removeEvent();
                 cbk(@alignCast(@ptrCast(callback.ctx)), completion_, res);
             }
         }.onConnect;
@@ -265,7 +242,7 @@ pub const Loop = struct {
         errdefer self.event_callback_pool.destroy(callback);
         callback.* = .{ .loop = self, .ctx = ctx };
 
-        self.addEvent(.js);
+        self.addEvent();
         self.io.connect(*EventCallbackContext, callback, onConnect, completion, socket, address);
     }
 
@@ -283,7 +260,7 @@ pub const Loop = struct {
         const onSend = struct {
             fn onSend(callback: *EventCallbackContext, completion_: *Completion, res: SendError!usize) void {
                 defer callback.loop.event_callback_pool.destroy(callback);
-                callback.loop.removeEvent(.js);
+                callback.loop.removeEvent();
                 cbk(@alignCast(@ptrCast(callback.ctx)), completion_, res);
             }
         }.onSend;
@@ -292,7 +269,7 @@ pub const Loop = struct {
         errdefer self.event_callback_pool.destroy(callback);
         callback.* = .{ .loop = self, .ctx = ctx };
 
-        self.addEvent(.js);
+        self.addEvent();
         self.io.send(*EventCallbackContext, callback, onSend, completion, socket, buf);
     }
 
@@ -310,7 +287,7 @@ pub const Loop = struct {
         const onRecv = struct {
             fn onRecv(callback: *EventCallbackContext, completion_: *Completion, res: RecvError!usize) void {
                 defer callback.loop.event_callback_pool.destroy(callback);
-                callback.loop.removeEvent(.js);
+                callback.loop.removeEvent();
                 cbk(@alignCast(@ptrCast(callback.ctx)), completion_, res);
             }
         }.onRecv;
@@ -319,76 +296,9 @@ pub const Loop = struct {
         errdefer self.event_callback_pool.destroy(callback);
         callback.* = .{ .loop = self, .ctx = ctx };
 
-        self.addEvent(.js);
+        self.addEvent();
         self.io.recv(*EventCallbackContext, callback, onRecv, completion, socket, buf);
     }
-
-    // Zig timeout
-
-    const ContextZigTimeout = struct {
-        loop: *Self,
-        zig_ctx_id: u32,
-
-        context: *anyopaque,
-        callback: *const fn (
-            context: ?*anyopaque,
-        ) void,
-    };
-
-    fn zigTimeoutCallback(
-        ctx: *ContextZigTimeout,
-        completion: *IO.Completion,
-        result: IO.TimeoutError!void,
-    ) void {
-        const loop = ctx.loop;
-        defer {
-            loop.removeEvent(.zig);
-            loop.alloc.destroy(ctx);
-            loop.alloc.destroy(completion);
-        }
-
-        // If the loop's context id has changed, don't call the js callback
-        // function. The callback's memory has already be cleaned and the
-        // events nb reset.
-        if (ctx.zig_ctx_id != loop.zig_ctx_id) return;
-
-        result catch |err| {
-            switch (err) {
-                error.Canceled => {},
-                else => log.err("zig timeout callback: {any}", .{err}),
-            }
-            return;
-        };
-
-        // callback
-        ctx.callback(ctx.context);
-    }
-
-    // zigTimeout performs a timeout but the callback is a zig function.
-    pub fn zigTimeout(
-        self: *Self,
-        nanoseconds: u63,
-        comptime Context: type,
-        context: Context,
-        comptime callback: fn (context: Context) void,
-    ) void {
-        const completion = self.alloc.create(IO.Completion) catch unreachable;
-        completion.* = undefined;
-        const ctxtimeout = self.alloc.create(ContextZigTimeout) catch unreachable;
-        ctxtimeout.* = ContextZigTimeout{
-            .loop = self,
-            .zig_ctx_id = self.zig_ctx_id,
-            .context = context,
-            .callback = struct {
-                fn wrapper(ctx: ?*anyopaque) void {
-                    callback(@ptrCast(@alignCast(ctx)));
-                }
-            }.wrapper,
-        };
-
-        self.addEvent(.zig);
-        self.io.timeout(*ContextZigTimeout, ctxtimeout, zigTimeoutCallback, completion, nanoseconds);
-    }
 };
 
 const EventCallbackContext = struct {

src/runtime/testing.zig

@@ -26,7 +26,9 @@ pub const allocator = std.testing.allocator;
 // browser.Env or the browser.SessionState
 pub fn Runner(comptime State: type, comptime Global: type, comptime types: anytype) type {
     const AdjustedTypes = if (Global == void) generate.Tuple(.{ types, DefaultGlobal }) else types;
-    const Env = js.Env(State, struct {pub const Interfaces = AdjustedTypes;});
+    const Env = js.Env(State, struct {
+        pub const Interfaces = AdjustedTypes;
+    });
 
     return struct {
         env: *Env,

src/server.zig

@@ -1042,9 +1042,6 @@ pub fn run(
     // - JS callbacks events from scripts
     while (true) {
         try loop.io.run_for_ns(10 * std.time.ns_per_ms);
-        if (loop.cbk_error) {
-            log.err("JS error", .{});
-        }
     }
 }