feat: structuredClone global (HTML structured clone, ArrayBuffer transfer) - #431
Merged
Conversation
…sfer)
Adds the WHATWG structuredClone(value, { transfer }) global, following
the post-context builtin architecture: all spec-level argument coercion
lives in the portable internal/structured-clone.js, and the native side
is one binding function so the Android runtime can reuse the JS
unchanged.
StructuredClone.cpp round-trips the value through v8::ValueSerializer
and v8::ValueDeserializer inside the one isolate, which is what
StructuredDeserialize(StructuredSerializeWithTransfer(...)) reduces to
when there is no second agent. Transferred buffers are validated up
front (ArrayBuffer, not detached, detachable, no duplicates) so a
rejected call never leaves a half-transferred graph, registered with the
serializer before the write, then detached and re-wrapped around their
original backing store for the deserializer.
There is no DOMException in this runtime, so clone failures throw an
Error whose name is "DataCloneError", matching how native exceptions are
surfaced. GetSharedArrayBufferId and AdoptSharedValueConveyor are
overridden purely to preserve that name: with a delegate installed V8's
defaults throw a plain Error straight onto the isolate instead of
routing through ThrowDataCloneError.
Host objects are rejected. A native/interop wrapper serialized without
its native counterpart would deserialize into a wrapper around nothing,
so WriteHostObject reports it as uncloneable.
The transfer list is converted per WebIDL sequence semantics, so any
iterable works and a string primitive does not. ArrayBuffer membership is
brand-checked through the captured byteLength getter, which also excludes
SharedArrayBuffer -- correctly, since it is not transferable.
Adds the SymbolIterator primordial and documents the surface, the
transfer semantics and the deviations in docs/structured-clone.md.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds the global ChangesStructured Clone API
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/structured-clone.md`:
- Around line 21-25: Update the cloneability description to exclude Symbol from
the supported primitive values, replacing “all primitives” with “all primitive
values except Symbol” while preserving the existing BigInt, undefined, and -0
examples.
In `@NativeScript/runtime/js/structured-clone.js`:
- Around line 61-80: Capture and validate the iterator’s next method once
immediately after creating iterator, then invoke that captured method with
FunctionPrototypeCall for each iteration. Update the loop in the transfer-list
conversion flow to stop reading iterator.next repeatedly while preserving the
existing iterator-result and transferable-item validation.
In `@TestRunner/app/shared`:
- Line 1: Merge the structured-clone tests into the TestRunner/app/shared
submodule, ensuring the resulting master contains all four structured-clone
specifications, then update the parent repository’s submodule pointer from
a7492cecc9e2be95c7eb58591a5cbbb5dbb0267a to the correct master commit
3a262b979c6b84cdfe69cd495436a7088d016505.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0999161a-9849-4c27-b8b8-3278b181edf5
📒 Files selected for processing (11)
NativeScript/runtime/Runtime.mmNativeScript/runtime/StructuredClone.cppNativeScript/runtime/StructuredClone.hNativeScript/runtime/js/primordials.jsNativeScript/runtime/js/structured-clone.jsTestRunner/app/sharedTestRunner/app/tests/index.jsdocs/README.mddocs/structured-clone.mdtools/js2c-inputs.xcfilelistv8ios.xcodeproj/project.pbxproj
Converting `options.transfer` re-read `next` off the iterator on every step. WebIDL creates the iterator record once and captures `next` with it, so a `next` that changes mid-iteration must not be observed; read it once after creating the iterator, reject a non-callable one as a TypeError, and invoke it through the captured reference. Also reconciles the cloneable-types list in the docs with the error list: symbols throw a DataCloneError, so "all primitives" was wrong.
…e and worker postMessage structuredClone and worker postMessage each carried their own serializer and deserializer delegates, so the two could drift on everything the structured clone algorithm leaves to the embedder. StructuredSerialization now owns that machinery once and both call it. SerializedValue keeps serializing and deserializing as separate halves over a neutral buffer plus its backing-store lists, because a worker message is read back on a different isolate than it was written on while structuredClone round-trips on one. Transfer-list validation, id registration and the claim-then-detach ordering move in with it, so the rule that a buffer is detached only after the value is safely written holds for both callers. Consolidating settles three inconsistencies: - SharedArrayBuffer is now shared by structuredClone rather than rejected, which is what the spec asks for and what the worker path already did. - Worker postMessage no longer ignores a failed Serialize; it returns with the exception pending instead of posting an unwritten message. - Both paths raise DataCloneError through NativeScriptException. The two mechanisms built the same object -- Error with name "DataCloneError" -- except that the worker's also carried `fullMessage`, so unifying on it keeps every property worker callers can already see today. Host objects are the one place the callers still differ, and HostObjectPolicy is now the only place that is written down: structuredClone rejects them per spec, postMessage keeps delivering an empty object as it always has. The degrade branch writes no payload and its ReadHostObject counterpart consumes none, so the stream stays balanced. Drops the Node message-port scaffolding the core supersedes along with Worker::Serialize, the pre-structured-clone JSON path whose only remaining callers were commented out.
Both postMessage entry points -- Worker.prototype.postMessage and the worker global -- accepted a second argument and dropped it, so buffers could only ever be copied into a worker. They now hand it to the shared serialization core as the transfer list, which moves each buffer's memory instead: the sender's ArrayBuffer is detached as the message is written, and the receiving isolate wraps the same backing store. The list must be a plain array; anything else is a TypeError. The WebIDL iterable-to-sequence conversion that lets structuredClone accept a Set or any iterable belongs to its JavaScript wrapper, and postMessage has none. Everything past that point -- duplicate detection, the detached and detachable checks, and detaching only after a successful write -- is the same code structuredClone runs. Documents the transfer semantics alongside structuredClone's, including the one behavior the two entry points do not share: a posted native object still arrives as an empty object rather than raising a DataCloneError.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
NativeScript/runtime/StructuredSerialization.cpp (1)
212-216: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueLeave
transferredBuffers_in a safe state after deserialization.The loop moves each
shared_ptr<BackingStore>out but keeps the vector at the same size. Every element becomes null. A secondDeserializecall then passes a null backing store toArrayBuffer::New, which dereferences it and crashes.The header documents a single call, and both current callers respect that. Make the constraint enforced rather than documented.
♻️ Proposed refactor
for (size_t i = 0; i < transferredBuffers_.size(); i++) { deserializer.TransferArrayBuffer( static_cast<uint32_t>(i), ArrayBuffer::New(isolate, std::move(transferredBuffers_[i]))); } + transferredBuffers_.clear();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@NativeScript/runtime/StructuredSerialization.cpp` around lines 212 - 216, Update the deserialization loop in StructuredSerialization so transferredBuffers_ is emptied or otherwise made unusable after its buffers are moved, preventing a second Deserialize call from passing null backing stores to ArrayBuffer::New. Enforce the single-use constraint in the implementation rather than relying only on the header documentation.NativeScript/runtime/Worker.mm (1)
296-304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared postMessage serialization block.
PostMessageToMainCallbackandPostMessageCallbacknow build the same{ data }wrapper, read the same optional transfer list, and callMessage::Serializewith the samekDegradepolicy. The two copies must stay in sync with the transfer-list contract.
NativeScript/runtime/Worker.mm#L296-L304: move the wrap-and-serialize sequence into a static helper, for examplestatic bool BuildMessage(Isolate*, Local<Context>, const FunctionCallbackInfo<Value>&, std::shared_ptr<worker::Message>&), and call it here.NativeScript/runtime/Worker.mm#L356-L365: replace this copy with a call to the same helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@NativeScript/runtime/Worker.mm` around lines 296 - 304, Extract the duplicated `{ data }` wrapping, optional transfer-list handling, and `Message::Serialize` call into a shared static helper such as `BuildMessage`, returning failure when serialization rejects the value while preserving the pending exception. Update `PostMessageToMainCallback` at NativeScript/runtime/Worker.mm:296-304 and `PostMessageCallback` at NativeScript/runtime/Worker.mm:356-365 to call the helper, with no direct change needed beyond replacing each duplicated block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/structured-clone.md`:
- Around line 46-47: Update the transfer-list documentation to state that null
and undefined are accepted as no-transfer values by SerializedValue::Serialize,
while other non-array values cause a TypeError. Preserve the existing
explanation that arrays are accepted transfer lists.
In `@NativeScript/runtime/StructuredSerialization.cpp`:
- Around line 185-191: Update the transfer loop in StructuredSerialization so
the result of buffer->Detach(Local<Value>()) is checked; when it returns
Nothing, preserve or replace the pending exception with DataCloneError and
return Nothing<bool>() instead of storing the backing store or reporting
successful serialization. Keep the existing successful transfer behavior
unchanged.
In `@NativeScript/runtime/Worker.mm`:
- Around line 296-304: Move the target runtime lookup in the message-posting
function to before the Serialize call, and return immediately if it yields
nullptr. Keep serialization and its existing pending-exception handling after
this validation so transfer-list ArrayBuffers are not detached when the message
will be dropped.
---
Nitpick comments:
In `@NativeScript/runtime/StructuredSerialization.cpp`:
- Around line 212-216: Update the deserialization loop in
StructuredSerialization so transferredBuffers_ is emptied or otherwise made
unusable after its buffers are moved, preventing a second Deserialize call from
passing null backing stores to ArrayBuffer::New. Enforce the single-use
constraint in the implementation rather than relying only on the header
documentation.
In `@NativeScript/runtime/Worker.mm`:
- Around line 296-304: Extract the duplicated `{ data }` wrapping, optional
transfer-list handling, and `Message::Serialize` call into a shared static
helper such as `BuildMessage`, returning failure when serialization rejects the
value while preserving the pending exception. Update `PostMessageToMainCallback`
at NativeScript/runtime/Worker.mm:296-304 and `PostMessageCallback` at
NativeScript/runtime/Worker.mm:356-365 to call the helper, with no direct change
needed beyond replacing each duplicated block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b49a49d4-120d-46d8-8cf4-d9b3a5f439a8
📒 Files selected for processing (11)
NativeScript/runtime/Message.cppNativeScript/runtime/Message.hppNativeScript/runtime/StructuredClone.cppNativeScript/runtime/StructuredSerialization.cppNativeScript/runtime/StructuredSerialization.hNativeScript/runtime/Worker.hNativeScript/runtime/Worker.mmNativeScript/runtime/js/structured-clone.jsTestRunner/app/shareddocs/structured-clone.mdv8ios.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- NativeScript/runtime/Message.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- TestRunner/app/shared
- NativeScript/runtime/js/structured-clone.js
The shared suite now gates itself on the global being present, so the explicit opt-in call is redundant: runAllTests() picks it up and the suite skips itself on runtimes that do not implement structuredClone. That gate would also turn this runtime losing structuredClone into a green run, so RuntimeImplementedAPIs.js gets an unguarded spec asserting the global exists. The shared suite is free to skip; this one is not.
…sage target before serializing Two ways a transfer could take a buffer's memory and give nothing back. Serialize discarded the result of ArrayBuffer::Detach, so a buffer that refused the null key was recorded as transferred while its contents stayed where they were; the receiver would have got an empty buffer and no error. The failure now propagates. Only a buffer carrying an [[ArrayBufferDetachKey]] can refuse, which nothing here produces -- script cannot set one, this runtime never calls SetDetachKey, and the WebAssembly memory buffers that have one are already turned away as non-detachable -- so the branch is unreachable and stays untested by design; the comment says so, and V8's TypeError is passed along unchanged because it names the mismatch. The worker-to-main path also looked up the target runtime after serializing and returned silently when it was gone -- by which point the caller's transfer-list buffers were already detached. The lookup moves above the serialize call, alongside the IsRunning check, so a message that cannot be delivered costs the caller nothing. The main-to-worker path already resolved its target first and is unchanged. Also documents that postMessage treats a missing, undefined or null transfer list as "transfer nothing", and rejects every other non-array.
The structuredClone suite is merged (#26), so the pointer moves off the feature branch entirely and onto master, where the work now lives alongside the Performance suite merged as #25. master's runAllTests() calls both. Nothing is lost in the move: the suite on master is byte-identical to what the branch carried. The Performance suite arrives with it and gates itself the same way this one does, so on this runtime -- which has no PerformanceObserver -- it reports a single pending spec rather than failures.
NathanWalker
approved these changes
Aug 11, 2026
edusperoni
added a commit
to NativeScript/android
that referenced
this pull request
Aug 11, 2026
…sfer) Android port of NativeScript/ios#431, in lockstep with the iOS runtime. Implements the WHATWG structuredClone(value, { transfer }) global as a post-context JS builtin: js/structured-clone.js (shared unchanged with iOS) owns the WebIDL argument coercion, and a thin native binding runs a v8::ValueSerializer -> ValueDeserializer round-trip in the one isolate. The serialization machinery is consolidated into StructuredSerialization.{h,cpp} (tns::serialization) so structuredClone and worker postMessage run on one core: delegate pair, DataCloneError construction (an Error carrying that name -- previously the worker path threw a plain Error with a message prefix), transfer-list validation, and the register->write->claim->detach ordering that V8 14.9 requires (Detach() aborts on non-detachable buffers; Release() must be claimed even after a failed write). WorkerMessage.cpp is deleted and WorkerMessage.h reduced to an alias. postMessage gains the ArrayBuffer transfer list on both entry points. Host objects stay intentionally asymmetric via HostObjectPolicy: structuredClone rejects (spec), postMessage keeps the shipped degrade-to-{} behavior. Tests: shared cross-runtime suite (common-runtime-tests-app#26, 54 specs) wired via shared.runStructuredCloneTests(), plus an unguarded canary in testRuntimeImplementedAPIs.js. Documented in docs/structured-clone.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the WHATWG
structuredClone(value, { transfer })global.Architecture
Follows the post-context JS builtin pattern, same as the performance API in #430:
NativeScript/runtime/StructuredClone.{h,cpp}exposes a one-function binding bag (clone(value, transferArray)) and runsBuiltinId::kStructuredClone. The clone is av8::ValueSerializer→v8::ValueDeserializerround-trip inside the one isolate, which is whatStructuredDeserialize(StructuredSerializeWithTransfer(...))reduces to when there is no second agent.NativeScript/runtime/js/structured-clone.jsowns the argument checks and thesequence<object>conversion fortransfer. It contains nothing iOS-specific and is intended to be reused unchanged by the Android runtime.ArrayBuffermembership in the transfer list is brand-checked from JS through the capturedArrayBuffer.prototype.byteLengthgetter (tamper-proof, and correctly excludesSharedArrayBuffer, which is not transferable); the native side re-checks withIsArrayBuffer().Adds the
SymbolIteratorprimordial.V8 14.9 notes
ArrayBuffer::Detach()on a non-detachable buffer aborts the process — it does not throw. TheIsDetachable()pre-check before detaching is therefore load-bearing, not defensive. The deprecatedvoid Detach()is avoided in favour ofDetach(Local<Value>()).DataCloneErrorname. With a delegate installed, V8's baseGetSharedArrayBufferIdandAdoptSharedValueConveyorthrow a plainErrordirectly onto the isolate and never route throughThrowDataCloneError, so without these overrides aSharedArrayBufferwould surface an unnamed error.Release()must be called and its bufferfree()d even after a failed write — the default delegate grows the buffer withrealloc(), and the memory is owned by the caller afterRelease()regardless of whether serialization succeeded.TransferArrayBuffermakes the serializer skip itswas_detachedcheck, so detaching after a successfulWriteValue(rather than before) is the correct ordering.Deviations from the specification
DataCloneErroris anErrorwith.name = "DataCloneError", not aDOMException. This runtime has noDOMException; the shape matches how native exceptions are already surfaced (seedocs/error-handling.md).ArrayBufferis the only transferable type. NoMessagePort,ImageBitmaporReadableStreamexists here, so any other value in the transfer list is aDataCloneError.SharedArrayBuffershares (a new SAB over the same backing store) and is not transferable — a SAB in the transfer list is aDataCloneError.WriteHostObjectreports it as uncloneable.Documented in
docs/structured-clone.md.Tests
Shared cross-runtime suite: NativeScript/common-runtime-tests-app#26 (54 specs — clone semantics, graph identity and cycles, transfer, SharedArrayBuffer sharing, worker message transfer,
DataCloneErrorcases). It runs fromrunAllTests()and gates itself onstructuredClonebeing present — a runtime without the API reports one visible pending spec instead of failures, so the tests PR is safe on Android'smasterimmediately. An unguarded canary inRuntimeImplementedAPIs.jsasserts the global exists on iOS, so the gate cannot hide a regression here.Full TestRunner suite green on the simulator: 1010 tests, 0 failures, 11 pre-existing skips.
Merge sequencing
Done — the tests PR (NativeScript/common-runtime-tests-app#26) is merged and the submodule pointer here references the resulting
master(which also carries the self-gating Performance suite from #25; it skips on this branch as one pending spec, verified in the full run). No remaining ordering constraints.Independent of #430 — both branch off
main. Trivial conflicts are expected in whichever merges second: theRuntime.mmpost-context init block,project.pbxproj,tools/js2c-inputs.xcfilelist,js/primordials.js, and (between the two tests PRs)shared/index.jsandRuntimeImplementedAPIs.js.Serialization consolidation (added scope)
Follow-up review moved the shared machinery into one core so
structuredCloneand workerpostMessagecan never diverge:StructuredSerialization.{h,cpp}(tns::serialization) now owns the serializer/deserializer delegate pair,DataCloneErrorconstruction, transfer-list validation (duplicates, detached/non-detachable — the pre-check is load-bearing sinceDetach()aborts on 14.9), and the register→write→claim→detach ordering.SerializedValuekeeps serialize/deserialize as separate halves because a worker message deserializes on a different isolate.Message.cppis deleted (Message.hppis a one-line alias),StructuredClone.cppis a thin same-isolate consumer, and the dead Node message-port scaffolding plus the legacy JSONWorker::Serializepath are pruned.postMessagegains the ArrayBuffer transfer list on both entry points (ArrayofArrayBuffers,TypeErrorotherwise), using the shared validation. A latent bug fixed on the way:postMessagepreviously ignoredSerializefailures and posted an unwritten buffer.HostObjectPolicy { kReject, kDegrade }.structuredClonerejects withDataCloneError(spec);postMessagekeeps the shipped degrade-to-{}behavior that the 2016 cross-runtime worker test asserts on both runtimes (the write/read sides are a matched zero-byte pair over V8's ownkHostObjecttag — balanced, not corrupt). Unifying on reject is a coordinated follow-up with the Android runtime; it will be a one-line policy change here.SharedArrayBuffernow shares throughstructuredClone(new SAB over the same backing store, both directions asserted by the suite) instead of throwing — that deviation is removed. Errors from both entry points are unified onNativeScriptExceptionwith theDataCloneErrorname, so worker-path errors keepfullMessageandstructuredCloneerrors gain it.Summary by CodeRabbit
New Features
structuredClone()support for cloning values, circular references, and transferableArrayBufferobjects.postMessage()calls.Documentation
Tests
structuredClone()is available at runtime.