Post-review cleanup of the goto-waituntil branch:
- Recorder: emit the plain goto immediately and keep only a rewind offset +
pre-scrubbed dcl line; a readiness wait recorded next rewinds and swaps the
line. bytes() is infallible again, isEmpty() is gone, and the Agent.zig
save-path changes revert. Shared renderCall() replaces the duplicated
await/page. prologue; ObjectMap.clone + @tagName replace the hand-rolled
clone loop and string literal.
- tools: Tool.waitsForReadiness as an exhaustive predicate (waitForScript was
silently missing from the recorder's inline set); GotoParams.waitUntil is
non-optional with the default resolved once; performGoto takes an options
struct instead of two trailing nullables; the goto schema enum comes from
Config.tagJsonArray like waitForState's.
- actions: comment states the invariant (polling needs a parsed document)
rather than the symptom.
The recorder holds each goto one step: if the next recorded call is
waitForSelector/waitForState, the emitted line carries
waitUntil=domcontentloaded (the follow-up wait covers readiness); anything
else, including extract and comments, flushes the plain load-wait form. An
explicit waitUntil is never rewritten. bytes() flushes a trailing held goto
and became fallible; isEmpty() replaces the len check in synthesizeSave (#3138).
waitForSelector and waitForScript pre-waited for .load before polling, which
re-waited the late-script tail a waitUntil=domcontentloaded navigation
deliberately skipped. Parsing done is enough for the poll to start; the poll
itself keeps ticking the page. On an ad-heavy apnews article this is the
difference between the dcl goto helping (8.8s -> 3.4s debug build) and not
helping at all (#3138).
Optional param on the goto tool (load | domcontentloaded | networkalmostidle |
networkidle), default unchanged (load). Threaded through performGoto, startGoto
and the script runtime's pending-goto driver, which previously hard-coded
.load. Closes half of #3138.
Huge max-age/Age header values previously either panicked in the
i64 casts when storing metadata, or were silently truncated to their
first 8 digits by the fixed-size lowercase buffer in
CacheControl.parse. Cap them at 2^31 as RFC 9111 §1.2.2 prescribes
and compare directives case-insensitively without truncation.
Adding headers to an HTTP request was a bit awkward due to my desire to avoid
having an intermediate representation (e.g. an ArrayList(Header)). Going
straight to a curl slist avoids double-copying the headers (first to Zig, then
to curl).
But the CORS work (https://github.com/lightpanda-io/browser/pull/3002) showcases
that this micro-optimization simply isn't worth it, since it needs that
intermediate representation anyways.
And, this change isn't just for CORS. Headers have been a silly pain in the past
like unclear ownership, and messy APIs used in _a lot_ of places (WebBotAuth,
WebSocket, Fetch, ...)
This new approach stores headers on the transfer in an ArrayList. The API is:
```
const transfer = try client.newRequest(.{...}, owner);
{
errdefer transfer.deinit();
try transfer.addHeader("Over", "9000", .{});
}
try transfer.submit();
```
This:
1 - Eliminates ambiguity about errdefer cleanup responsibility
2 - Eliminates a bunch of stringZ concat that Frame, Config, CDP were doing
3 - Transfer.arena is now available for headers
Alignment allowing (1) this reduces Node size by 8 bytes. It also aligns with
previous cdata changes with the end-goal to have the entire Node chain adopt
bare tags.
(1) string.String is 16-byte aligned, so Text doesn't shrink with this change.
Hopefully this can be addressed separately.
This does 2 scheduler-related changes. The first is that the high/low priority
queues are gone. These were misleading and hinted at some type of ordering. The
point was strictly to help us decide when do we consider a page "done" and that
some tasks SHOULD hold up the page (high priority) and some pages SHOULD NOT
(low priority).
So, when adding a task, `low_priority == true` is now `blocks_done == false`.
But it isn't just a rename:
1 - There two queues are merged, and we keep a blocking count
2 - Past a certain timer depth, blocks_done == true, so it protects against more
cases than just the previous RAF
The other change relates to the Scheduler's implementation. While low/high have
been merged into a single PriorityQueue, "immediate" and "front" tasks get their
own distinct queue (so we now have 3). This is an optimization for the very
common case where run_at = 0 - it can be a plain FIFO.
Rather than comparing tasks on run_at (u64) and sequence (u64), the key (u64)
merges the two.
Cacheable for ScriptManager means something very specific (should the js.Context
cache the module for subsequent loads). With HTTP caching coming, I'm removing
this field from the ScriptManager logs because (a) it isn't very useful anyways
and (b) it doesn't mean what most people who see it would think it means.
This is a classic (for us) issue. Session.initiateRootNavigation errdefer a
number of steps BUT, if the frame.navigate() call reaches libcurl's event-loop,
then those errdefer can conflict with the `frameErrorCallback`. Who's
responsible for cleaning up? It depends where the failure happened.
So the cleanup is now more defensive. Page.destroying guards against multiple
calls to Session.queuePageDestruction and initiateRootNavigation no longer
assumes that the new Page is in the pages collection.
I believe this is the cause of the "release overflow" for `Selection` that is
infrequent but has been seen for a long time.
Adds a proper QuoteExceededError WebApi.
Handles more invalid parameters.
The most significant change is that types which return a promise don't return
an error, they reject. This is an issue that we run into a lot, and this
commit has started to work on a more generic (i.e. in the bridge) solution.
This is a new API, but it's in all browser (nightly in Firefox). It's
essentially a more powerful api than setTimeout, it has priority and
cancellation.
Continues the work started in https://github.com/lightpanda-io/browser/pull/3070
Removes the _proto field from intermediary (non-leaf) elements and uses the
relative positioning from the contiguous allocation. With the work done in
previous commit, this is pretty mechanical.
This essentially saves 8 bytes per type. Pretty small, but now Node->* is
consistently designed 1 way. Empty leaf nodes are still an issue, and the _proto
remains in them (for now). It's an issue because, if it's truly empty, the leaf
can have the same address as the parent or a sibling. This is something we
used to have problems with (hence some types have a _pad: bool). Solvable, but
separately.
Adrià Arrufat