From 7750bc94f63bd6c842d61759dd334c3bfae07406 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 13 May 2026 20:57:59 +0800 Subject: [PATCH] Apply suggestions from code review Remove no-longer needed setTimeouts in test now that messages are queued. Runner also checks ready_queue when determining doneness. Co-authored-by: Navid EMAD --- src/browser/Runner.zig | 13 ++++++++++++- src/browser/tests/net/xhr_worker.html | 6 +++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/browser/Runner.zig b/src/browser/Runner.zig index 7a343ac3..34c768f5 100644 --- a/src/browser/Runner.zig +++ b/src/browser/Runner.zig @@ -212,7 +212,18 @@ fn _tick(self: *Runner, comptime is_cdp: bool, opts: TickOpts) !CDPTickResult { }, } - if (http_active == 0 and http_client.ws_active == 0 and http_client.queue.first == null and (comptime is_cdp == false)) { + if (http_active == 0 and http_client.ws_active == 0 and http_client.queue.first == null and http_client.ready_queue.first == null and (comptime is_cdp == false)) { + // we don't need to consider http_client.intercepted here + // because is_cdp is false, and that can only be + // the case when interception isn't possible. + // + // ready_queue is also part of the check: makeRequest now + // wraps its handles.perform() in a performing=true window, + // and any synchronous libcurl callback that ends up + // calling trackConn during that window (e.g. JS creating + // a WebSocket) will append to ready_queue. Without this + // check we could observe it non-empty after + // http_client.tick returns. // we don't need to consider http_client.intercepted here // because is_cdp is false, and that can only be // the case when interception isn't possible. diff --git a/src/browser/tests/net/xhr_worker.html b/src/browser/tests/net/xhr_worker.html index bbae4e73..54cbaa3a 100644 --- a/src/browser/tests/net/xhr_worker.html +++ b/src/browser/tests/net/xhr_worker.html @@ -6,7 +6,7 @@ const state = await testing.async(); const worker = new Worker('./xhr-worker.js'); worker.onmessage = (e) => state.resolve(e.data); - setTimeout(() => worker.postMessage({ kind: 'basic' }), 100); + worker.postMessage({ kind: 'basic' }); await state.done((data) => { testing.expectTrue(data.ok, 'worker xhr error: ' + data.err); @@ -29,7 +29,7 @@ const state = await testing.async(); const worker = new Worker('./xhr-worker.js'); worker.onmessage = (e) => state.resolve(e.data); - setTimeout(() => worker.postMessage({ kind: 'arraybuffer' }), 100); + worker.postMessage({ kind: 'arraybuffer' }); await state.done((data) => { testing.expectTrue(data.ok, 'worker xhr error: ' + data.err); @@ -48,7 +48,7 @@ const state = await testing.async(); const worker = new Worker('./xhr-worker.js'); worker.onmessage = (e) => state.resolve(e.data); - setTimeout(() => worker.postMessage({ kind: 'document_unsupported' }), 100); + worker.postMessage({ kind: 'document_unsupported' }); await state.done((data) => { testing.expectTrue(data.ok, 'worker xhr error: ' + data.err);