Most significantly, if removing from the multi fails, the connection
is added to a "dirty" list for the removal to be retried later. Looking at
the curl source code, remove fails on a recursive call, and we've struggled with
recursive calls before, so I _think_ this might be happening (it fails in other
cases, but I suspect if it _is_ happening, it's for this reason). The retry
happens _after_ `perform`, so it cannot fail for due to recursiveness. If it
fails at this point, we @panic. This is harsh, but it isn't easily recoverable
and before putting effort into it, I'd like to know that it's actually happening.
Fix potential use of undefined when a 401-407 request is received, but no
'WWW-Authenticate' or 'Proxy-Authenticate' header is received.
Don't call `curl_multi_remove_handle` on an easy that hasn't been added yet do
to error. Specifically, if `makeRequest` fails during setup, transfer_conn is
nulled so that `transfer.deinit()` doesn't try to remove the connection. And the
conn is removed from the `in_use` queue and made `available` again.
On Abort, if getting the private fails (extremely unlikely), we now still try
to remove the connection from the multi.
Added a few more fields to the famous "ScriptManager.Header recall" assertion.
This PR introduces a custom CDP domain 'LP' (Lightpanda) to expose browser-specific tools. The first method, 'LP.getMarkdown', allows retrieving a Markdown representation of the DOM or a specific node by its 'nodeId'. This is optimized for AI agents and LLM-based scraping tasks.
- Use StaticStringMap and enums for method, tool, and resource lookups.
- Implement comptime JSON minification for tool schemas.
- Refactor router and harness to use more efficient buffered polling.
- Consolidate integration tests and add synchronous unit tests.
In the previous commits, two separte crash resolution conspired to introduce
1 tick delay in request handling.
When we're in a libcurl perform, we can't re-enter libcurl. So we added a
check before processing new requests to make sure we weren't "performing" and,
if we were, we'd queue the request (hence the 1 tick delay).
But for another issue, we set the same "performing" check when manually
triggering callbacks. This extended the situations where the above check fired
thus causing the 1-tick delay to happen under more (and even common) situation.
This commit improves this - instead of relying on the global "performing" check
when processing 1 transfer explicitly, we now have a per-transfer performing
check. This prevents the transfer from being deinitialized during a callback
but does not block requests from being started immediately.
This field was recently added and is used to generate correct frameIds in CDP
messages. They remain the same during a navigation event, so calling them
page.id might cause surprises since navigation events create new pages, but
retain the original id. Hence, frame_id is more accurate and hopefully less
surprising.
(This is a small cleanup prior to doing some iframe navigation work).
- Refactor router and test harness for non-blocking I/O using buffered polling.
- Implement reliable test failure reporting from sub-threads to the main test runner.
- Encapsulate pipe management using idiomatic std.fs.File methods.
- Fix invalid JSON generation in resource streaming due to duplicate fields.
- Improve shutdown sequence for clean test exits.
There are two main ways to dispatch events, both via the EventManager: dispatch
and dispatchWithFunction. dispatchWithFunction came about from having to
dispatch to function callbacks in addition to event listeners. Specifically,
firing the window.onload callback.
Since that original design, much has changed. Most significantly, with
https://github.com/lightpanda-io/browser/pull/1524 callbacks defined via
attributes became properly (I hope) integrated with the event dispatching.
Furthermore, the number of non-tree event targets (e.g. AbortSignal) has grown
significantly. Finally, dispatching an event is DOM-based event is pretty
complex, involving multiple phases and capturing the path.
The current design is largely correct, but non-obvious. This commit attempts to
improve the ergonomics of event dispatching.
`dispatchWithFunction` has been renamed to `dispatchDirect`. This function is
meant to be used with non-DOM event targets. It is optimized for having an event
path with a single target andh no bubbling/capture phase. In addition to being
a little more streamlined, `dispatchDirect` will internally turn a
`js.Function.Global` or `js.Function.Temp` into a local. This makes the callsite
simpler, but also provides optimization opportunity - not having to create
a new scope for the common case of having no callback/listener. This lays the
groundwork for having a `hasDirect` guard clause at the callsite to avoid
unnecessary event creation (todo in a follow up commit).
`dispatch` remains unchanged. While `dispatch` is primarily meant to handle the
DOM-based EventTarget, it will forward non-DOM EventTargets to `dispatchDirect`.
This is necessary since JS code can call `signal.dispatchEvent(....)`.
Two notes:
1 - The flow of dispatchDirect is an optimization. The spec makes no distinction
between DOM and non-DOM based EventTargets.
2 - While the window (as an EventTarget) should probably be thought of as a
DOM-based EventTarget, we use `dispatchDirect with it. This is because it
sits at the root and thus can safely go through the faster `dispatchDirect`.
Fixes a separate but similar issue to
https://github.com/lightpanda-io/browser/pull/1689
Specifically, it prevents starting a request from within a libcurl handler, thus
avoiding an illegal recursive call.
(This commit also removes the failed function call debug logging for
DOMExceptions, as these aren't particularly abnormal / log-worthy)
This was already handled in most cases, but not for a body-less response. It's
safe to call transfer.abort() during a callback, so long as the performing flag
is set to true. This was set during the normal libcurl callbacks, but for a
body-less response, we manually invoke the header_done_callback and were not
setting the performing flag.
https://github.com/lightpanda-io/browser/pull/1614 improved our shutdown
behavior so that microtasks associated with a context wouldn't fire after the
context was disposed of. This involved having context-specific microtasks,
pumping the message loop, and prevent re-entry.
The shutdown code in CDP already had much of this behavior built-in, but it has
now become redundant. Most importantly the CDP shutdown logic did not prevent
re-entry.
Removing this code fixes a flaky WPT crash. I didn't seem to be tied to a
specific test, but rather a cross-context/page use-after-free that was saw
prior to 1614. I could reproduce it reliably by running `/wasm/core/`.
I'll be honest, it isn't clear to me why _removing_ the CDP cleanup helps.
Running the message loop and microtask _before_ our normal shutdown might be
unnecessary, but why would it crash? I don't know, but the CDP path is slightly
different in that it also involves Inspector shutdown. So there's still
something about this flow I don't quite understand. And, at least for this case
the current flow seems "correct".
