get namespaceURI() derived the namespace from the tag name and only ever
recognised the <svg> root, so every element parsed inside an <svg> subtree
reported the XHTML namespace. get href() gates the SVGAnimatedString
reflection on the SVG namespace, so for parsed content the gate never
opened: href returned a plain string and .animVal/.baseVal were undefined,
making new URL(el.href.animVal) throw. Only createElementNS-built elements
worked, because createElementNS records _ns explicitly.
The tree builder already applies the HTML5 foreign-content rules, so
QualName.ns is correct for parsed SVG; it just was not exposed to JS. Add a
namespace_uri DOM op and read it in get namespaceURI(), keeping _ns as the
override and the old tag-name heuristic as the fallback when no element name
is recorded. Namespaces are immutable, so the result is cached like _lname.
Fixes#484
The template.content bridge (#463) routed every element through
content_source(), which called get_node().clone() to read the Copy
template_contents field. That deep-cloned each element's Node (its attrs
Vec plus a String per attribute) on every innerHTML/outerHTML/dump-html
serialization, once per non-void element.
template_contents is already destructured from the node in the serialize
loop, so read it there directly. content_source() (inner_html path) now
reads it through with_node() by reference. Serialization output is
byte-identical; ~10% faster on serialization-heavy pages.
_IframeDocument's addEventListener, removeEventListener, and dispatchEvent were
no-ops, so listeners registered on an iframe document never ran. Iframe load also
invoked the onload property directly, bypassing addEventListener("load", ...)
listeners.
Register and dispatch events on _IframeDocument, and fire the iframe load event
through the element so both the onload property and addEventListener listeners run.
Fixes#478.
Keep pumping the event loop past the general 500ms settle limit only while the dynamic-script queue remains pending, with a 3s hard cap.
Add a deterministic CDP regression for direct and nested external scripts whose fetch crosses the original settle deadline.
parentNode() inspected only the immediate parent, which caused three
spec deviations:
- With currentNode at root, the `parent !== this.root` guard did not
fire (parent is root's OWN parent), so a node OUTSIDE the walker's
subtree was returned — createTreeWalker(root).parentNode() yielded
<body>. A TreeWalker must never surface a node above its root.
- The same guard excluded root as a *result*, so when currentNode's
accepted ancestor was root itself, parentNode() returned null instead
of root.
- A skipped (or otherwise non-accepted) immediate parent ended the
search rather than continuing up to the first accepted ancestor.
Replaced with the DOM 6.1 algorithm: climb from currentNode, returning
the first ancestor that filters to FILTER_ACCEPT, with the loop's
`node !== this.root` guard keeping the walk inside root while still
letting root be returned. Reuses the three-valued `_filter` from #461.
Last of the TreeWalker mover deviations, after nextNode (#461),
previousNode (#462) and the child/sibling movers (#469).
createNodeIterator returned a TreeWalker with a flag on it. The two are
different interfaces, and the alias got two things wrong.
An iterator's pointer starts *before* its root, so the first nextNode()
returns the root itself; a TreeWalker starts *at* its root and never
returns it. So createNodeIterator(el, SHOW_ELEMENT) silently skipped
exactly the element it was rooted at:
<div id=root><a></a></div> -> ["A"] (Chrome: ["DIV","A"])
It also exposed the wrong surface: no referenceNode,
pointerBeforeReferenceNode or detach (older library code still calls
detach and hit "not a function"), while leaking currentNode, firstChild,
parentNode and the rest of TreeWalker's movers.
Implemented DOM 6.2 "traverse" directly. Keeping the pointer as
(referenceNode, pointerBeforeReferenceNode) rather than a single cursor
is what makes a direction change re-yield the current node instead of
stepping over it, and a failed traversal leaves both untouched so the
iterator stays resumable either way.
The backward step is a new `DomTree::prev_in_subtree`, mirroring
`next_in_subtree`. It can live entirely in the DOM layer because a
NodeIterator prunes nothing -- unlike the TreeWalker's backward walk
(#462), where FILTER_REJECT forces the filter to be consulted at every
step of the descent.
FILTER_REJECT still behaves as FILTER_SKIP here, which the existing test
keeps covering; its expectation gains the root node the alias used to
drop. The TreeWalker's now-dead `_rejectIsSkip` branch is removed.
firstChild/lastChild/nextSibling/previousSibling stepped straight to the
next sibling whenever a node was not accepted, so a FILTER_SKIP node hid
its children instead of exposing them:
<div id=root><section><a></a></section></div>, SECTION -> FILTER_SKIP
w.firstChild() // null (Chrome: <a>)
They also used the boolean `_accept`, so FILTER_REJECT (which must not
descend) and FILTER_SKIP (which must) were indistinguishable — the same
root cause #461 fixed for nextNode().
Replaced all four with the spec's "traverse children" and "traverse
siblings" algorithms, parameterised by direction so forward and backward
share one implementation and cannot drift: `edge` is first/last child and
`step` is next/previous sibling. Both consult the three-valued `_filter`
from #461, so SKIP descends, REJECT prunes, and the climb back out stops
at root or the node the walk started from.
parentNode() is left alone here: its deviations are different (it checks
only the immediate parent, and excludes root, which the spec permits it
to return), so it belongs in its own change.
The parser already put template children in a separate contents document
per the HTML spec (tree_sink.rs), but nothing exposed that node, so
`get content()` fabricated an empty DocumentFragment and the parsed
markup was unreachable:
document.body.innerHTML = '<template id=t><p>a</p></template>';
t.content.childNodes.length // 0 (Chrome: 1)
t.innerHTML // "" (Chrome: '<p>a</p>')
Every <template>-driven UI — custom elements cloning
`template.content`, lit, Vue runtime templates, htmx fragments, Alpine
x-if/x-for — stamped nothing, with no error raised.
Three layers were involved:
- `DomTree::template_contents` returns the contents node, allocating one
on demand so templates built via `createElement` work too. Exposed as
the `template_contents` op; `get content()` wraps it as a
DocumentFragment cached by node id, so `.content` keeps a stable
identity across reads.
- `import_node_from` cloned the element data verbatim, carrying the
*source* tree's contents NodeId into the destination tree, where it
indexed whatever unrelated node occupied that slot. That is why a
template injected via innerHTML resolved `.content` to a bogus node
rather than merely an empty one. The contents are now imported into a
fresh node and the reference remapped, queued onto the existing
worklist so nested templates stay iterative.
- The serializer emitted a template's own (always empty) children, so
contents vanished from outerHTML/innerHTML and `cloneNode(true)` —
which round-trips through outer_html — produced an empty template.
`content_source` resolves a template to its contents document for both
inner_html and outer_html.
With serialization fixed on the Rust side, the `set innerHTML` template
branch that routed to `.content` now has a matching getter for free, so
no template special case is needed on the JS hot path.
window.scrollTo/scrollBy/scroll were empty stubs, scrollX/scrollY and
pageXOffset/pageYOffset were hard-coded 0 data properties, no scroll
event was ever fired at the window, and document.scrollingElement did
not exist.
that "scroll-driven lazy loaders advance instead of throwing". But the
dominant infinite-scroll idiom is window-level:
window.scrollTo(0, document.body.scrollHeight);
if (window.scrollY + innerHeight >= body.scrollHeight) loadMore();
all of which was inert -- the call succeeded, the offset never moved, the
sentinel never tripped -- so automation that scrolls a page to trigger
loading silently got one screenful.
The offset is stored on the scrolling element rather than in separate
window state, so window.scrollY and document.scrollingElement.scrollTop
are two views of one value in both directions, which is what pages
assume when they mix the two. scrollX/scrollY/pageXOffset/pageYOffset
become read-only accessors over it, matching a real Window, and
document.scrollingElement returns documentElement (standards mode; we
never parse in quirks mode).
The scroll event is dispatched at both the document and the window.
Chrome propagates a page scroll to both, but Document.dispatchEvent here
runs only its own listeners and does not propagate, so firing once would
strand half the listeners.
As with #431 there is no layout, so the offset still cannot be clamped to
a real maximum; negative values clamp to 0 exactly as they do for
elements.
previousNode() considered exactly one candidate — the previous sibling's
deepest last child — and returned null as soon as the filter rejected it,
so a backward walk died mid-tree. This is the backward mirror of #432,
which was fixed forward-only in #447.
<div id=root><a><b></b></a><c></c></div>, SHOW_ELEMENT, B -> FILTER_SKIP
forward: ["A","C"] backward: [] (Chrome: ["A"])
Replaced with the DOM 6.1 algorithm verbatim: scan previous siblings,
descend to the deepest last descendant of each, then step up to the
parent, continuing until a node is accepted or root is reached. The
descent stops on FILTER_REJECT, which is what makes rejection prune
subtrees in the backward direction too.
Unlike nextNode this stays in JS rather than adding a DOM traversal op:
the descent has to consult the filter at every step to honour REJECT, so
there is no run of JS/native crossings for a native helper to collapse.
Depends on the three-valued `_filter` from #461.
`_accept` collapsed every non-ACCEPT filter result into a single boolean,
so FILTER_REJECT behaved exactly like FILTER_SKIP: the node was not
returned, but the walk still descended into its subtree. Per DOM 6.1,
FILTER_REJECT rejects a node *and* its descendants.
This was unreachable until #440 exposed the real NodeFilter constants,
which made the canonical prune idiom
if (node.tagName === 'SCRIPT') return NodeFilter.FILTER_REJECT;
start yielding nodes from inside the pruned subtree instead of failing
loudly — sanitizers and text extractors built on TreeWalker silently got
extra content.
Split the filter into a three-valued `_filter` (keeping `_accept` as the
boolean wrapper the sibling/child movers use) and give nextNode a second
traversal step for the reject case. That step lives in the DOM layer as
`DomTree::next_after_subtree`, mirroring how #447 put `next_in_subtree`
there: both share the ancestor climb, and REJECT costs no more JS/native
crossings than SKIP.
NodeIterator shares this walker but has no subtree pruning (DOM 6.2:
FILTER_REJECT behaves as FILTER_SKIP), so `createNodeIterator` marks the
walker `_rejectIsSkip` and keeps the plain step. Covered by its own test
so the shared implementation cannot drift.
scrollTo/scrollBy dispatch a scroll event; direct assignment did not. So the
usual lazy-load idiom
el.addEventListener('scroll', loadMore);
el.scrollTop = el.scrollHeight;
never reached the listener, and scroll-driven feeds stalled after their first
batch with no error to show for it.
Checked against a real Chrome over CDP, same probe on the same page:
Chrome el.scrollTop = 100 -> 1 scroll event
obscura (before) el.scrollTop = 100 -> 0 scroll events
obscura (after) el.scrollTop = 100 -> 1 scroll event
The setters only fire when the offset actually changes, so a loader that
rewrites the same scrollTop each frame does not re-enter. scrollTo/scrollBy
suppress the per-axis events and emit one for the whole movement, the way a
browser reports one scroll per operation rather than one per axis.
The offset stays unclamped on purpose. Without layout, any maximum is a guess,
and a guess derived from the synthetic scroll box pins scrollTop at 0, which
deadlocks the very loaders this unblocks: no scroll, no content, no scroll.
Geometry is left alone here — this is event dispatch only.
Tests: 4 of the 5 new cases fail without this change (the scrollTo coalescing
one passes either way; it guards against this change turning one event into
three). nextest -p obscura-cdp 91/91, obstacle course 33/33.
Move fragment children into the target for appendChild, insertBefore, and replaceChild while preserving their order and emptying the fragment.
Add runtime regression coverage for the DOM attachment portion of #452.
Connection threads are detached and never joined, so `save_cookies()` ran on
the main thread while they were still serving: a connection writing a
Set-Cookie after that point lost it, and the process then exited and killed the
thread mid-flight. Before the per-connection move this could not happen -- the
single processor saved on its own way out, ordered against all connection work
on one LocalSet. Draining on the live-connection counter restores that ordering.
Scope, honestly: this bounds the loss window rather than closing it. The drain
does not finish early today because a connection's frame reader is parked in a
socket read and never observes `notify_waiters`, so a client that holds a
connection open costs the full 3s deadline and then gets the warning. Making
readers wake on shutdown is a separate change and not #430's business.
Measured: 2ms to shut down with no connections (unchanged), 3005ms with one
held open, against the 10s `docker stop` allows before SIGKILL. Cookies are
persisted in both cases.
Refs #430, #333.
The test's whole value is that its fixture keeps the navigation settle loop
pumping, but nothing checked the fixture was reached. Pointing `page_url` at a
dead port passed in 0.6s instead of 1.25s, and that degraded version no longer
reproduced the #430 abort on the pre-fix tree -- so a fixture regression would
have turned this back into the `data:`-URL test it was written to replace,
silently and while staying green.
Count the requests the fixture answers and assert on it. Two per client, not
three: the engine does not fetch the `<img>`, only the document and /slow.js.
Verified the assertion kills the dead-port mutant (0 requests, fails) and that
the real run is stable across repeats.
Refs #430.
The premise was wrong. deno_core 0.350's run_event_loop is:
pub async fn run_event_loop(&mut self, opts) -> Result<(), CoreError> {
poll_fn(|cx| self.poll_event_loop(cx, opts)).await
}
so replacing `timeout(budget, run_event_loop())` with a hand-rolled
`poll_fn(poll_event_loop)` loop is the same code with the same cancellation
semantics, not safer ones. The mechanism I described cannot happen either:
`poll` is a synchronous `&mut self` call, a future can only be dropped between
polls, and the HandleScope/ContextScope live inside poll_event_loop as stack
locals that are destroyed before it returns. The isolate is exited between
polls both before and after that change.
What actually fixes#430 is confining each connection's isolates to their own
OS thread, which the next commits do. The regression test still reproduces the
abort on the pre-thread-per-connection tree and still passes here without this
commit, so nothing depended on it.
It was not merely a no-op. Two regressions it introduced:
- resolve_promises_until inverted the meaning of tick_ms. It was "pump the
event loop for up to tick_ms, returning early on idle"; it became one single
poll followed by an unconditional sleep(tick_ms). With the 1->50ms backoff
that settles at 20 event-loop turns per second, so an awaited promise chain
needing many turns crawls, and past the budget the caller silently reads a
stale __obscura_await_meta.
- drive_module_eval replaced `e?` with `let _ = poll_event_loop(...)`,
discarding event-loop errors. A module stalled on a top-level await used to
exit immediately with a logged error; it instead spun for the whole budget
logging nothing -- the exact #374 starvation the surrounding comment claims
to prevent.
Keep the drive_module_eval extraction (it genuinely dedupes load_module and
load_inline_module) but restore the original select!-based body.
Refs #430.
Each connection runs on its own OS thread with its own V8 isolates, so
connection count is what drives the server's thread footprint.
MAX_PENDING_WS_HANDOFFS bounds only the handoff queue: once a stream is handed
off, nothing limited how many could be live at once, and a client could grow
the server without limit.
Reserve a slot in the accept loop and release it from a guard that runs when
the connection thread unwinds, on every path including panics and the early
returns before the runtime is built -- a plain decrement at the end of the
closure would leak slots until the cap wedged the server shut. Connections
past the limit get an explicit 503 with X-Obscura-Reason: max-connections
rather than a bare reset, so a client can tell a refusal from a crash. The
refusal happens in the WS dispatch loop, so the HTTP control plane (#62) stays
reachable at the cap.
Default 128, matching the handoff bound and well above any real client fan-out
(Playwright/Puppeteer use one connection per browser). At the cap, with arenas
capped by the previous commit: 146 threads, 33.2 GiB of reserved address space
and 51 MiB resident -- nearly all of that reservation being V8's process-wide
sandbox, which is present at zero connections.
Threads are the honest cost here, not memory: at 100 connections each running
JS this server reserves 83.5 GiB against `main`'s 83.6 GiB, because `main`
pays the same V8 sandbox as soon as it runs any JS. What it does not pay is
118 threads against 17.
The test covers all three properties, and specifically that a closed
connection frees its slot: a leak there would only surface as a server that
silently stops accepting after N lifetime connections.
Refs #430.
glibc hands each new thread its own 64 MiB arena (up to 8x cores). With one
thread per connection that arena is the dominant per-connection memory term,
and freed chunks stay reserved in it rather than returning to the OS: resident
memory tracks the *peak* number of concurrent connections and never comes back
down, which reads as a leak although nothing is leaked and no thread is stuck.
Measured with reliability/conn-scale.py in obscura-benchmark. 100 connections
each running JS reserve 90.0 GiB of address space uncapped versus 83.5 GiB
capped; per idle connection, 65 MiB of reserved address space drops to 2.0 MiB.
`main` at that same 100-connection JS workload reserves 83.6 GiB, so with the
cap this server is level with it -- most of the total is V8's process-wide
sandbox, which `main` pays too the moment it runs any JS.
The resident-set effect is the one that bites in production. Measured in the
container image against Google Maps, four concurrent connections per round:
350 / 619 / 826 MiB over three rounds uncapped and still climbing linearly,
versus 166 / 235 / 269 MiB capped, on a decelerating curve.
Two arenas cost no measurable throughput here -- 8 concurrent connections x 12
navigations, 1.53s uncapped against 1.50s capped over three runs. V8 allocates
the JS heap through its own allocator and the Rust side is dominated by network
I/O rather than malloc traffic. Applied only from `serve`, which owns the
process, and skipped when the caller has already set MALLOC_ARENA_MAX.
Refs #430.
- Delete the now-unused CdpContext::new_full (its only caller switched to
new_with_shared_context); have _new_inner build the shared BrowserContext and
delegate, so the struct's field list lives in one place.
- Extract drive_module_eval, shared verbatim by load_module and
load_inline_module.
- Update comments that still described the deleted process-wide V8 lock and the
single-LocalSet model to the per-connection reality, drop the dangling
reference to the removed obscura-js/src/v8_lock.rs, and soften the
ISOLATE_CREATE_LOCK note now that the main-thread warmup is the real guard
against the InitializeBuiltinJSDispatchTable segfault.
Refs #430.
The per-command V8 watchdog kept a single global slot whose correctness relied
on the old process-wide lock (at most one command armed at a time). With
thread-per-connection several connections arm concurrently, so a second arm
overwrote the first's slot and left that command's isolate unbounded. Key the
armed slots by generation so each connection's command has its own; the worker
fires whichever have overrun and terminates each via its IsolateHandle.
Refs #430.
With every connection on its own thread (previous commit), the process-wide V8
lock served no safety purpose across connections yet forced all of them to
serialize on one mutex, so throughput did not scale with concurrency. Move the
lock into CdpContext so each connection has its own: a connection's nav task and
command dispatch still coordinate on one page's isolate at a time on its thread,
but different connections no longer block each other. Delete the now-unused
process-wide obscura_js::v8_lock.
Measured on a local CPU-bound fixture, page throughput stays flat (~75/s from
concurrency 1 to 8) with the global lock and scales to about 2.5x at concurrency
8 with the per-connection lock, with no crash at any concurrency. Obstacle
course 33/33, obscura-cdp 76/76.
Refs #430.
concurrent_navigations uses data: URLs, which settle instantly and so never
drove the abort. This test serves a local page with slow subresources plus a
setInterval so the navigation settle loop keeps pumping, then drives it from
four independent CDP connections at once. It aborts the process on the old
single-LocalSet server (SIGTRAP, the V8 heap->isolate() == TryGetCurrent()
fatal) and passes on the thread-per-connection server.
Refs #430.
The CDP server ran every connection's pages on one LocalSet thread, so two
pages' V8 isolates shared an OS thread. Under concurrent navigation a cancelled
event-loop future could leave one isolate entered while another page's
execute_script ran, tripping V8's heap->isolate() == Isolate::TryGetCurrent()
check and aborting the process (#430). No single-thread serialization fixes
this reliably: the cancellation-leak pattern is pervasive and partly inside
deno_core.
Give each WebSocket connection its own OS thread with its own current_thread
runtime, LocalSet, and CdpContext, so every page's isolate is confined to one
thread. V8's invariant is per-thread, so two connections' isolates can never
collide. This is the "Option 2" fix named in obscura-js/src/v8_lock.rs, and it
also lets connections run in parallel instead of serializing on one thread.
- CdpContext::new_with_shared_context reuses one Arc<BrowserContext> across all
connections, so they share one cookie jar and one HTTP client.
- A single signal watcher flips the accept flag and wakes every connection's
processor via notify_waiters(); the accept side persists cookies once on
shutdown.
- Serialize isolate construction with a process-wide lock: the leaptiering
JSDispatchTable setup is not safe to run from several threads at once.
- Warm up V8 once on the main thread before any connection thread creates an
isolate; creating the first isolate off the main thread segfaults in
InitializeBuiltinJSDispatchTable.
obstacle course 33/33, obscura-cdp 75/75 including the concurrent-navigation and
control-plane tests. Concurrency 2 and 4 against heavy pages no longer abort.
Fixes#430.
Several event-loop operations bounded their runtime with
tokio::time::timeout(budget, run_event_loop()) or a select! against a
timer. When the timer wins, tokio drops the run_event_loop future
mid-poll; deno_core enters the isolate while polling, so the drop leaves
the isolate entered on the OS thread. Under concurrent CDP work a second
page's execute_script then trips heap->isolate() == TryGetCurrent() and
aborts the process (#430).
Drive the loop with poll_event_loop inside a poll_fn instead. It creates
and drops its own isolate scope per call, so stopping between ticks never
leaves the isolate entered. Convert resolve_promises_until,
run_event_loop_bounded, resolve_promises, the mod_evaluate settle in
load_module and load_inline_module, and the two navigation settle loops
in page.rs. Add poll_event_loop_once as the cancellation-safe single-tick
primitive the settle loops use.
Keeps the obstacle course at 33/33. This is hardening toward #430; the
residual concurrency abort still needs the thread-per-runtime refactor
that v8_lock.rs calls the "Option 2" fix.
Refs #430.
Element.prototype.scrollBy, scrollTo and scroll were undefined; they only
existed as no-ops on the window object. Scroll-driven infinite-scroll code
therefore threw "scrollBy is not a function", and scrollTop/scrollLeft were
no-op stubs that always read back 0.
Track a per-element scroll offset so scrollTop/scrollLeft round-trip, add the
three methods accepting either (x, y) or a ScrollToOptions object, and fire a
scroll event so listeners that drive lazy loading can advance. There is no
layout engine, so the offset is not clamped to a real overflow maximum.
Register the new methods in the native-function list so their toString stays
masked.
Fixes#429.
Co-authored-by: sycatle <sycatle@users.noreply.github.com>
NodeFilter was defined twice: a partial live definition (SHOW_ELEMENT,
SHOW_TEXT, SHOW_ALL) and a complete one behind an `if (typeof NodeFilter
=== 'undefined')` guard that never fired because the partial one already
defined it. So NodeFilter.FILTER_ACCEPT/REJECT/SKIP and most SHOW_* values
were undefined at runtime, and the canonical filter idiom
`acceptNode() { return NodeFilter.FILTER_ACCEPT; }` returned undefined —
making TreeWalker/NodeIterator reject every node.
Make the live definition authoritative with the full standard constant set
and drop the dead duplicate. Adds tests asserting the constant values and
that a filter returning NodeFilter.FILTER_ACCEPT accepts the node.
