Unblock async http request

src/app.zig

@@ -52,7 +52,8 @@ pub const App = struct {
             .telemetry = undefined,
             .app_dir_path = app_dir_path,
             .notification = notification,
-            .http_client = try HttpClient.init(allocator, 5, .{
+            .http_client = try HttpClient.init(allocator, .{
+                .max_concurrent = 3,
                 .http_proxy = config.http_proxy,
                 .tls_verify_host = config.tls_verify_host,
             }),

src/browser/html/window.zig

@@ -163,50 +163,33 @@ pub const Window = struct {
         return &self.performance;
     }
 
-    // Tells the browser you wish to perform an animation. It requests the browser to call a user-supplied callback function before the next repaint.
-    // fn callback(timestamp: f64)
-    // Returns the request ID, that uniquely identifies the entry in the callback list.
-    pub fn _requestAnimationFrame(
-        self: *Window,
-        callback: Function,
-    ) !u32 {
-        // We immediately execute the callback, but this may not be correct TBD.
-        // Since: When multiple callbacks queued by requestAnimationFrame() begin to fire in a single frame, each receives the same timestamp even though time has passed during the computation of every previous callback's workload.
-        var result: Function.Result = undefined;
-        callback.tryCall(void, .{self.performance._now()}, &result) catch {
-            log.debug(.user_script, "callback error", .{
-                .err = result.exception,
-                .stack = result.stack,
-                .source = "requestAnimationFrame",
-            });
-        };
-        return 99; // not unique, but user cannot make assumptions about it. cancelAnimationFrame will be too late anyway.
+    pub fn _requestAnimationFrame(self: *Window, cbk: Function, page: *Page) !u32 {
+        return self.createTimeout(cbk, 5, page, .{ .animation_frame = true });
     }
 
-    // Cancels an animation frame request previously scheduled through requestAnimationFrame().
-    // This is a no-op since _requestAnimationFrame immediately executes the callback.
-    pub fn _cancelAnimationFrame(_: *Window, request_id: u32) void {
-        _ = request_id;
+    pub fn _cancelAnimationFrame(self: *Window, id: u32, page: *Page) !void {
+        const kv = self.timers.fetchRemove(id) orelse return;
+        return page.loop.cancel(kv.value.loop_id);
     }
 
     // TODO handle callback arguments.
     pub fn _setTimeout(self: *Window, cbk: Function, delay: ?u32, page: *Page) !u32 {
-        return self.createTimeout(cbk, delay, page, false);
+        return self.createTimeout(cbk, delay, page, .{});
     }
 
     // TODO handle callback arguments.
     pub fn _setInterval(self: *Window, cbk: Function, delay: ?u32, page: *Page) !u32 {
-        return self.createTimeout(cbk, delay, page, true);
+        return self.createTimeout(cbk, delay, page, .{ .repeat = true });
     }
 
     pub fn _clearTimeout(self: *Window, id: u32, page: *Page) !void {
         const kv = self.timers.fetchRemove(id) orelse return;
-        try page.loop.cancel(kv.value.loop_id);
+        return page.loop.cancel(kv.value.loop_id);
     }
 
     pub fn _clearInterval(self: *Window, id: u32, page: *Page) !void {
         const kv = self.timers.fetchRemove(id) orelse return;
-        try page.loop.cancel(kv.value.loop_id);
+        return page.loop.cancel(kv.value.loop_id);
     }
 
     pub fn _matchMedia(_: *const Window, media: []const u8, page: *Page) !MediaQueryList {
@@ -216,10 +199,14 @@ pub const Window = struct {
         };
     }
 
-    fn createTimeout(self: *Window, cbk: Function, delay_: ?u32, page: *Page, comptime repeat: bool) !u32 {
+    const CreateTimeoutOpts = struct {
+        repeat: bool = false,
+        animation_frame: bool = false,
+    };
+    fn createTimeout(self: *Window, cbk: Function, delay_: ?u32, page: *Page, comptime opts: CreateTimeoutOpts) !u32 {
         const delay = delay_ orelse 0;
         if (delay > 5000) {
-            log.warn(.user_script, "long timeout ignored", .{ .delay = delay, .interval = repeat });
+            log.warn(.user_script, "long timeout ignored", .{ .delay = delay, .interval = opts.repeat });
             // self.timer_id is u30, so the largest value we can generate is
             // 1_073_741_824. Returning 2_000_000_000 makes sure that clients
             // can call cancelTimer/cancelInterval without breaking anything.
@@ -250,7 +237,8 @@ pub const Window = struct {
             .window = self,
             .timer_id = timer_id,
             .node = .{ .func = TimerCallback.run },
-            .repeat = if (repeat) delay_ms else null,
+            .repeat = if (opts.repeat) delay_ms else null,
+            .animation_frame = opts.animation_frame,
         };
         callback.loop_id = try page.loop.timeout(delay_ms, &callback.node);
 
@@ -300,13 +288,23 @@ const TimerCallback = struct {
     // if the event should be repeated
     repeat: ?u63 = null,
 
+    animation_frame: bool = false,
+
     window: *Window,
 
     fn run(node: *Loop.CallbackNode, repeat_delay: *?u63) void {
         const self: *TimerCallback = @fieldParentPtr("node", node);
 
         var result: Function.Result = undefined;
-        self.cbk.tryCall(void, .{}, &result) catch {
+
+        var call: anyerror!void = undefined;
+        if (self.animation_frame) {
+            call = self.cbk.tryCall(void, .{self.window.performance._now()}, &result);
+        } else {
+            call = self.cbk.tryCall(void, .{}, &result);
+        }
+
+        call catch {
             log.debug(.user_script, "callback error", .{
                 .err = result.exception,
                 .stack = result.stack,

src/browser/page.zig

@@ -113,7 +113,9 @@ pub const Page = struct {
             .cookie_jar = &session.cookie_jar,
             .microtask_node = .{ .func = microtaskCallback },
             .window_clicked_event_node = .{ .func = windowClicked },
-            .request_factory = browser.http_client.requestFactory(browser.notification),
+            .request_factory = browser.http_client.requestFactory(.{
+                .notification = browser.notification,
+            }),
             .scope = undefined,
             .module_map = .empty,
         };
@@ -205,58 +207,63 @@ pub const Page = struct {
         // redirect)
         self.url = request_url;
 
-        // load the data
-        var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true });
-        defer request.deinit();
-        request.body = opts.body;
-        request.notification = notification;
+        {
+            // block exists to limit the lifetime of the request, which holds
+            // onto a connection
+            var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true });
+            defer request.deinit();
 
-        notification.dispatch(.page_navigate, &.{
-            .opts = opts,
-            .url = &self.url,
-            .timestamp = timestamp(),
-        });
+            request.body = opts.body;
+            request.notification = notification;
 
-        var response = try request.sendSync(.{});
+            notification.dispatch(.page_navigate, &.{
+                .opts = opts,
+                .url = &self.url,
+                .timestamp = timestamp(),
+            });
 
-        // would be different than self.url in the case of a redirect
-        self.url = try URL.fromURI(arena, request.request_uri);
+            var response = try request.sendSync(.{});
 
-        const header = response.header;
-        try session.cookie_jar.populateFromResponse(&self.url.uri, &header);
+            // would be different than self.url in the case of a redirect
+            self.url = try URL.fromURI(arena, request.request_uri);
 
-        // TODO handle fragment in url.
-        try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) });
+            const header = response.header;
+            try session.cookie_jar.populateFromResponse(&self.url.uri, &header);
 
-        const content_type = header.get("content-type");
+            // TODO handle fragment in url.
+            try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) });
 
-        const mime: Mime = blk: {
-            if (content_type) |ct| {
-                break :blk try Mime.parse(arena, ct);
-            }
-            break :blk Mime.sniff(try response.peek());
-        } orelse .unknown;
+            const content_type = header.get("content-type");
 
-        log.info(.http, "navigation", .{
-            .status = header.status,
-            .content_type = content_type,
-            .charset = mime.charset,
-            .url = request_url,
-        });
+            const mime: Mime = blk: {
+                if (content_type) |ct| {
+                    break :blk try Mime.parse(arena, ct);
+                }
+                break :blk Mime.sniff(try response.peek());
+            } orelse .unknown;
+
+            log.info(.http, "navigation", .{
+                .status = header.status,
+                .content_type = content_type,
+                .charset = mime.charset,
+                .url = request_url,
+            });
+
+            if (!mime.isHTML()) {
+                var arr: std.ArrayListUnmanaged(u8) = .{};
+                while (try response.next()) |data| {
+                    try arr.appendSlice(arena, try arena.dupe(u8, data));
+                }
+                // save the body into the page.
+                self.raw_data = arr.items;
+                return;
+            }
 
-        if (mime.isHTML()) {
-            self.raw_data = null;
             try self.loadHTMLDoc(&response, mime.charset orelse "utf-8");
-            try self.processHTMLDoc();
-        } else {
-            var arr: std.ArrayListUnmanaged(u8) = .{};
-            while (try response.next()) |data| {
-                try arr.appendSlice(arena, try arena.dupe(u8, data));
-            }
-            // save the body into the page.
-            self.raw_data = arr.items;
         }
 
+        try self.processHTMLDoc();
+
         notification.dispatch(.page_navigated, &.{
             .url = &self.url,
             .timestamp = timestamp(),

src/browser/session.zig

@@ -72,7 +72,7 @@ pub const Session = struct {
 
     pub fn deinit(self: *Session) void {
         if (self.page != null) {
-            self.removePage();
+            self.removePage() catch {};
         }
         self.cookie_jar.deinit();
         self.storage_shed.deinit();
@@ -104,14 +104,35 @@ pub const Session = struct {
         return page;
     }
 
-    pub fn removePage(self: *Session) void {
+    pub fn removePage(self: *Session) !void {
         // Inform CDP the page is going to be removed, allowing other worlds to remove themselves before the main one
         self.browser.notification.dispatch(.page_remove, .{});
 
         std.debug.assert(self.page != null);
-        // Reset all existing callbacks.
-        self.browser.app.loop.reset();
+
+        // Cleanup is a bit sensitive. We could still have inflight I/O. For
+        // example, we could have an XHR request which is still in the connect
+        // phase. It's important that we clean these up, as they're holding onto
+        // limited resources (like our fixed-sized http state pool).
+        //
+        // First thing we do, is endScope() which will execute the destructor
+        // of any type that registered a destructor (e.g. XMLHttpRequest).
+        // This will shutdown any pending sockets, which begins our cleaning
+        // processed
         self.executor.endScope();
+
+        // Second thing we do is reset the loop. This increments the loop ctx_id
+        // so that any "stale" timeouts we process will get ignored. We need to
+        // do this BEFORE running the loop because, at this point, things like
+        // window.setTimeout and running microtasks should be ignored
+        self.browser.app.loop.reset();
+
+        // Finally, we run the loop. Because of the reset just above, this will
+        // ignore any timeouts. And, because of the endScope about this, it
+        // should ensure that the http requests detect the shutdown socket and
+        // release their resources.
+        try self.browser.app.loop.run();
+
         self.page = null;
 
         // clear netsurf memory arena.
@@ -143,7 +164,7 @@ pub const Session = struct {
         // the final URL, possibly following redirects)
         const url = try self.page.?.url.resolve(self.transfer_arena, url_string);
 
-        self.removePage();
+        try self.removePage();
         var page = try self.createPage();
         return page.navigate(url, opts);
     }

src/browser/xhr/xhr.zig

@@ -30,6 +30,7 @@ const Mime = @import("../mime.zig").Mime;
 const parser = @import("../netsurf.zig");
 const http = @import("../../http/client.zig");
 const Page = @import("../page.zig").Page;
+const Loop = @import("../../runtime/loop.zig").Loop;
 const CookieJar = @import("../storage/storage.zig").CookieJar;
 
 // XHR interfaces
@@ -78,6 +79,7 @@ const XMLHttpRequestBodyInit = union(enum) {
 
 pub const XMLHttpRequest = struct {
     proto: XMLHttpRequestEventTarget = XMLHttpRequestEventTarget{},
+    loop: *Loop,
     arena: Allocator,
     request: ?*http.Request = null,
 
@@ -91,6 +93,7 @@ pub const XMLHttpRequest = struct {
     sync: bool = true,
     err: ?anyerror = null,
     last_dispatch: i64 = 0,
+    request_body: ?[]const u8 = null,
 
     cookie_jar: *CookieJar,
     // the URI of the page where this request is originating from
@@ -241,12 +244,13 @@ pub const XMLHttpRequest = struct {
     pub fn constructor(page: *Page) !XMLHttpRequest {
         const arena = page.arena;
         return .{
+            .url = null,
             .arena = arena,
+            .loop = page.loop,
             .headers = Headers.init(arena),
             .response_headers = Headers.init(arena),
             .method = undefined,
             .state = .unsent,
-            .url = null,
             .origin_url = &page.url,
             .cookie_jar = page.cookie_jar,
         };
@@ -422,18 +426,31 @@ pub const XMLHttpRequest = struct {
         log.debug(.http, "request", .{ .method = self.method, .url = self.url, .source = "xhr" });
 
         self.send_flag = true;
+        if (body) |b| {
+            self.request_body = try self.arena.dupe(u8, b);
+        }
 
-        self.request = try page.request_factory.create(self.method, &self.url.?.uri);
-        var request = self.request.?;
-        errdefer request.deinit();
+        try page.request_factory.initAsync(
+            page.arena,
+            self.method,
+            &self.url.?.uri,
+            self,
+            onHttpRequestReady,
+            self.loop,
+        );
+    }
+
+    fn onHttpRequestReady(ctx: *anyopaque, request: *http.Request) !void {
+        // on error, our caller will cleanup request
+        const self: *XMLHttpRequest = @alignCast(@ptrCast(ctx));
 
         for (self.headers.list.items) |hdr| {
             try request.addHeader(hdr.name, hdr.value, .{});
         }
 
         {
             var arr: std.ArrayListUnmanaged(u8) = .{};
-            try self.cookie_jar.forRequest(&self.url.?.uri, arr.writer(page.arena), .{
+            try self.cookie_jar.forRequest(&self.url.?.uri, arr.writer(self.arena), .{
                 .navigation = false,
                 .origin_uri = &self.origin_url.uri,
             });
@@ -447,14 +464,15 @@ pub const XMLHttpRequest = struct {
         //  if the request method is GET or HEAD.
         //  https://xhr.spec.whatwg.org/#the-send()-method
         // var used_body: ?XMLHttpRequestBodyInit = null;
-        if (body) |b| {
+        if (self.request_body) |b| {
             if (self.method != .GET and self.method != .HEAD) {
-                request.body = try page.arena.dupe(u8, b);
+                request.body = b;
                 try request.addHeader("Content-Type", "text/plain; charset=UTF-8", .{});
             }
         }
 
-        try request.sendAsync(page.loop, self, .{});
+        try request.sendAsync(self.loop, self, .{});
+        self.request = request;
     }
 
     pub fn onHttpResponse(self: *XMLHttpRequest, progress_: anyerror!http.Progress) !void {
@@ -522,7 +540,7 @@ pub const XMLHttpRequest = struct {
         log.info(.http, "request complete", .{
             .source = "xhr",
             .url = self.url,
-            .status = progress.header.status,
+            .status = self.response_status,
         });
 
         // Not that the request is done, the http/client will free the request