| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Install the DOM abort primitives as globals in every isolate, modeled on Node's internal/abort_controller.js: AbortController, and AbortSignal with the abort/timeout/any statics, onabort with HTML event-handler semantics, and WebIDL-shaped interfaces (enumerable members, Symbol.toStringTag, brand-checked accessors). The builtin (internal/abort-signal.js) runs from Events::Init right after the Event/EventTarget builtin it is layered on. Deviations from Node, documented in docs/abort-signal.md: no DOMException (default reasons are Error instances with name patched to AbortError/TimeoutError, the same stand-in performance.js and structured-clone.js use) and no WeakRef bookkeeping (a timeout() timer holds its signal until it fires; any() links source -> dependent strongly and unlinks as soon as either side aborts). Adds RangeError and NumberIsInteger to primordials and the eslint restriction lists, and a 20-spec Jasmine suite. Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
Match Node's memory behavior: internal references never keep an unobservable signal alive and never drop an observable abort. - timeout() timers close over a WeakRef; a FinalizationRegistry cancels the pending native timer when the signal is collected. - any() links are WeakRefs in both directions with prune registries, so per-request composites never accumulate on a long-lived source and a composite whose sources all died stops being retained. - A gcPersistentSignals set strong-holds exactly the signals whose abort someone can still observe: live timeout signals and non-empty composites while they have abort listeners, plus timeout sources a composite follows until their timer fires. The listener accounting comes from a new symbol-keyed listener-mutation hook in events.js, called from every listener-list mutation path (add, remove, once-splice during dispatch) and handed to the abort builtin in a one-shot through its binding, so it cannot be bypassed via a captured EventTarget.prototype.addEventListener. Adds WeakRef/FinalizationRegistry captures to primordials and the eslint restriction lists, and 8 GC specs driven by __collect() plus a finalization-registry substrate canary. Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
Add a sixth fixed wrapper parameter, `internals`: one plain per-isolate
object (stored in the BuiltinRealm per-runtime state) handed identically
to every builtin and reachable from nowhere else. Producers publish
during their init, consumers read during theirs, so the PrepareV8Runtime
ordering is the dependency graph and a missing key fails loudly at init.
Both existing ad-hoc channels migrate onto it: events.js publishes the
kListenerChanged hook key (read by abort-signal.js, previously a one-shot
relayed through the abort builtin's binding) and setListenerErrorReporter
(called by error-events.js, previously the _installListenerErrorReporter
one-shot on the app-reachable global target). No capability ever sits on
an app-reachable object anymore, even transiently.
Documented in the js README as an interim mechanism: if cross-builtin
needs outgrow one shared object, migrate to a Node-style private
internal-module tier (require("internal/...") resolved for builtins
only) and fold internals into it.
Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 409e9f79-88df-46fe-923f-9a4fcb54bc07 📥 CommitsReviewing files that changed from the base of the PR and between d1fc925 and 074a8e5. 📒 Files selected for processing (15)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 Walkthrough WalkthroughChangesThe runtime adds global AbortController and AbortSignal implementations. It integrates them with EventTarget, per-isolate builtin internals, weak-reference GC handling, runtime initialization, documentation, and functional and asynchronous tests. AbortController and AbortSignal
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to 074a8 The PR adds AbortController and AbortSignal runtime support without any identified concrete correctness, security, availability, or integration issue; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant App
participant AbortController
participant AbortSignal
participant EventTarget
App->>AbortController: create controller
AbortController->>AbortSignal: expose stable signal
App->>AbortController: abort(reason)
AbortController->>AbortSignal: set aborted state and reason
AbortSignal->>EventTarget: dispatch abort event
EventTarget-->>App: invoke abort listeners
Suggested reviewers: nathanwalker Poem 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Mirrors NativeScript/ios#447 on Android (same three commits, same file shapes).
What
Installs the DOM Standard's abort primitives as globals in every isolate (main and workers), modeled on Node's internal/abort_controller.js:
How
The builtin (test-app/runtime/src/main/cpp/js/abort-signal.js) runs from Events::Init immediately after the Event/EventTarget builtin it is layered on.
This PR also introduces a sixth fixed wrapper parameter, internals: one plain per-isolate object (a BuiltinRealm slot in the per-runtime state) handed identically to every builtin and reachable from nowhere app code can see — the private channel for cross-builtin capabilities. Both previously ad-hoc channels now ride it: the kListenerChanged hook key (events → abort-signal) and setListenerErrorReporter (error-events → events), replacing the _installListenerErrorReporter one-shot that transiently sat on the app-reachable global target. Documented in the js README as an interim mechanism, with the intended end-state being a Node-style private internal-module tier (require("internal/…") for builtins only). RangeError, NumberIsInteger, WeakRef, and FinalizationRegistry are added to primordials (and the eslint restriction lists).
GC contract (Node-equivalent, documented in docs/abort-signal.md)
Internal references never keep an unobservable signal alive and never drop an observable abort:
Deviation from Node
No DOMException in this runtime: default reasons are Error instances with name patched to "AbortError" / "TimeoutError", the same stand-in performance.js and structured-clone.js use.
Tests
The spec files are byte-identical to the iOS suite; the only source divergence from iOS is one comment line naming the Android init path (PrepareV8Runtime).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests