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".
- Use an allocating writer in `sendResponse` to handle large payloads.
- Update the main loop to tick the HTTP client and cap poll timeouts.
- Update protocol version and minify tool input schemas.
Previously the "load" event happened when all external scripts were done. In the
case that there was no external script, the "load" event would fire immediately
after parsing.
With iframes, it now waits for external script AND iframes to complete but the
no-external-script code was never updated to consider iframes and would thus
fire load events prematurely.
