Add File / FileReader polyfill

[bkaradzic-microsoft]

The File polyfill in BabylonNative currently lives as a JS shim (Apps/Playground/Scripts/file_polyfill.js, ~308 lines) that is LoadScript-ed from the Playground's AppContext (see BabylonJS/BabylonNative#1706). Per @bghgary's review on that PR, it should live in JsRuntimeHost alongside Blob, URL, WebSocket, XMLHttpRequest, TextDecoder, AbortController, etc., so every JsRuntimeHost consumer -- not only the BabylonNative Playground -- gets File and FileReader.

This PR adds a native C++ Polyfills/File/ target that implements the WHATWG File and FileReader web APIs, plus 29 new Mocha unit tests (15 File + 14 FileReader) under Tests/UnitTests/Scripts/tests.ts.

Surface area

• File(parts, name, options) constructor.
• File instance accessors: size, type, name, lastModified.
• File instance methods: arrayBuffer(), text(), bytes().
• File extends Blob: the prototype chain is wired so new File(...) instanceof Blob === true (Babylon.js core branches on instanceof Blob in fileTools, Offline/database, abstractEngine, thinNativeEngine).
• FileReader with state constants EMPTY/LOADING/DONE (exposed both on the constructor and on instances, per WHATWG).
• FileReader readonly attributes readyState, result, error and the onloadstart/onprogress/onload/onabort/onerror/onloadend EventHandler attributes.
• FileReader events delivered via both the onX handler slots and addEventListener / removeEventListener / dispatchEvent.
• FileReader.readAsText / readAsArrayBuffer / readAsDataURL / abort.

Implementation notes

• File delegates its byte storage to the existing Blob polyfill via env.Global().Get("Blob"), reusing Blob's BlobPart handling (ArrayBuffer, typed array, string, Blob). Blob::Initialize must run before File::Initialize.
• FileReader's readonly attributes (readyState/result/error) and the on* handler slots are backed by C++ state surfaced through prototype accessors, not by writable JS properties, so script can neither overwrite them nor fool the state-machine checks. result/error are boxed on a persistent holder object rather than Napi::Reference<Value> slots, because napi_create_reference rejects primitive values on the real N-API backends (V8/JSC) and readAsText/readAsDataURL produce primitive string results.
• readAsDataURL base64-encodes via the base-n dependency (added as a top-level FetchContent), matching the encoder already used by other polyfills.
• An in-flight read keeps the FileReader alive (per spec) by anchoring its JS wrapper in a shared_ptr<Napi::ObjectReference> captured by the promise reactions -- externally owned, released automatically when the promise settles. A monotonic read id lets abort() / restart invalidate a stale continuation so a late-resolving arrayBuffer() promise cannot dispatch a phantom load after a user-initiated abort.
• The new JSRUNTIMEHOST_POLYFILL_FILE CMake option is on by default and gates building the target.

Notable JSC-specific care

Two pieces of the implementation are shaped specifically to work correctly on JavaScriptCore:

1. FileReader.EMPTY/LOADING/DONE are registered via StaticValue and InstanceValue descriptors inside the DefineClass property list, not via func.Get("prototype").Set(...) after class creation. On JSC the napi shim's ConstructorInfo::Create defaults the constructor's .prototype to Object.prototype, so the latter pattern would pollute Object.prototype with EMPTY/LOADING/DONE keys and break for..in over plain objects throughout the runtime. The regression test "does not pollute Object.prototype with EMPTY/LOADING/DONE" pins this behaviour.

2. The Blob-dependency guard in File::Initialize uses IsUndefined() rather than IsFunction(). Some JSC builds (notably libjavascriptcoregtk-4.1 on Linux) classify constructors created via JSObjectMakeConstructor as typeof 'object', not 'function', so napi_typeof returns napi_object for them and an IsFunction() guard would silently early-return on those engines. IsUndefined() matches the guard used by Blob and works on V8, JSI, Chakra, iOS-JSC, Android-JSC, and Linux JSC.

Testing

Locally UnitTests.exe (Win32 x64 Chakra Debug) passes all 180 JS tests (151 pre-existing + 29 new) and the native gtests, including the Object.prototype-pollution regression canary and the abort / late-resolve continuation guards. CI is green across all engines/platforms (V8, Chakra, JSI/JSC, iOS, Android, UWP, macOS, Ubuntu, sanitizers).

After this merges

BabylonJS/BabylonNative#1706 re-pins CMakeLists.txt's GIT_REPOSITORY/GIT_TAG to the merged SHA and removes the JS shim.

[Copilot]

Pull request overview

This PR moves File / FileReader support from a BabylonNative Playground-only JS shim into JsRuntimeHost as a native C++ polyfill, so all JsRuntimeHost consumers get the WHATWG-style APIs alongside existing polyfills (Blob, URL, XHR, etc.).

Changes:

• Add a new Polyfills/File/ C++ target implementing File and FileReader (including event handler slots + EventTarget-style listeners).
• Wire the polyfill into the build via a new JSRUNTIMEHOST_POLYFILL_FILE CMake option (ON by default) and link it into unit tests.
• Add new Mocha unit tests covering File and FileReader behaviors (construction, reads, events, abort, and a JSC regression around Object.prototype pollution).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
CMakeLists.txt	Adds JSRUNTIMEHOST_POLYFILL_FILE option (ON by default).
Polyfills/CMakeLists.txt	Conditionally includes the new File polyfill subdirectory.
Polyfills/File/CMakeLists.txt	Defines the File polyfill library target and its sources.
Polyfills/File/Include/Babylon/Polyfills/File.h	Public API entrypoint for initializing the polyfill.
Polyfills/File/Readme.md	Documents behavior and prerequisites (Blob must be initialized first).
Polyfills/File/Source/File.h	Declares the internal File ObjectWrap implementation.
Polyfills/File/Source/File.cpp	Implements File over the existing Blob polyfill and initializes FileReader.
Polyfills/File/Source/FileReader.h	Declares the internal FileReader ObjectWrap implementation.
Polyfills/File/Source/FileReader.cpp	Implements async reads, base64 DataURL generation, events, and abort logic.
Tests/UnitTests/CMakeLists.txt	Links the new File target into the UnitTests executable.
Tests/UnitTests/Shared/Shared.cpp	Initializes the File polyfill in the unit test JS environment.
Tests/UnitTests/Scripts/tests.ts	Adds new Mocha tests for File and FileReader, including a JSC regression canary.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bkaradzic-microsoft]

@bghgary friendly ping — this is the upstream half of BabylonJS/BabylonNative#1706 (File / FileReader polyfill, which re-enables 19 GLTF/OBJ serializer tests in BN). Same flow as #171 (TextEncoder) which you helped me close out earlier today: the polyfill was moved here from the BN PR per your review ("this should be a proper C++ polyfill under Polyfills/ so every consumer gets it").

State just refreshed:

• Rebased onto current upstream/main (resolved a trivial overlapping-describe() conflict in tests.ts against the just-merged Add TextEncoder polyfill (WHATWG Encoding Standard) #171).
• 5 inline Copilot review comments all have my replies with fixes in commit 52f6338. Notable bug-fix: readAsBinaryString now produces Latin-1 byte-per-code-unit per spec (was emitting UTF-8 of code points ≥ 0x80).
• File library builds standalone clean (cmake --build build-notests --target File).

Polyfills/File/ sits next to Polyfills/Blob/ and follows the same shape as TextDecoder / TextEncoder / XMLHttpRequest / AbortController / URL / WebSocket. No changes to existing files outside the standard wiring (CMakeLists.txt option, Polyfills/CMakeLists.txt add_subdirectory, Tests/UnitTests/{CMakeLists, Shared/Shared.cpp, Scripts/tests.ts} link + Initialize + tests).

Once this lands I'll bump BN #1706's pin back to BabylonJS/JsRuntimeHost and that PR is one tiny commit away from merge.

[ryantrem]

Similar to my comment on the other PR, we may want to do a minimal implementation of the File polyfill that is just enough to support BJS usage (not sure if that is already the case).

[bkaradzic-microsoft]

@ryantrem on the minimization point — I surveyed Babylon.js usage and the only thing that wasn't already minimal is FileReader.readAsBinaryString (zero call sites across packages/dev, packages/tools, etc.). Dropped it in 4480ad8.

API	BJS usage
new File(parts, name, opts)	inspector-v2, viewer, smartAssetSerializer, etc.
instanceof File	modelLoader, smartAssetSerializer, configurator
instanceof Blob	fileTools, Offline/database, abstractEngine, thinNativeEngine
FileReader.readAsArrayBuffer	fileTools, database
FileReader.readAsText	fileTools, smartAssetSerializer
FileReader.readAsDataURL	stringTools, nodeEditor, inspector
FileReader.readAsBinaryString	zero call sites — removed

Dropping readAsBinaryString also makes the deferred Latin-1/UTF-8 follow-up unnecessary (the one that previously crashed Chakra during a combined-edit attempt — see the earlier thread).

Full set of changes in 4480ad8:

1. Drop readAsBinaryString + its enum case + its test.
2. File inherits Blob via prototype-chain wire-up + new instanceof Blob test.
3. Replace shared_ptr<ObjectReference> with member Napi::ObjectReference m_selfRef slot (matches the WebSocket/XHR pattern in this repo).

Local Win32 Chakra File target builds clean.

[bkaradzic-microsoft]

CI on 35bfa81 is fully green (22/22 jobs across V8, Chakra, JSI, JSC on Linux/macOS/iOS/Android/Windows/UWP, plus Sanitizers + TSan).

The prototype-chain wire-up for File extends Blob went through three iterations:

1. 4480ad8 (initial review-fix commit): direct Object.setPrototypeOf(func.prototype, blobProto) in C++. Passed V8/Chakra; failed JSC because JSC's napi_define_class (which wraps JSObjectMakeConstructor) returns Object.prototype from func.Get("prototype") rather than the real napi-internal prototype — same quirk the FileReader constants block documents. Result: setPrototypeOf tried to mutate Object.prototype's [[Prototype]] and threw TypeError.
2. ac19d77: temp-instance trick — Object.getPrototypeOf(new File(...)) returns JSC's real napi-internal prototype, and that one accepts setPrototypeOf. Fixed JSC; broke Chakra (reason still unclear from the log alone — Chakra-specific behavior somewhere between JsConstructObject and Object.getPrototypeOf).
3. 9e7b333 (C++ dual-path direct + verify + fallback): regressed JSC because the JSC napi shim escapes the direct-path TypeError as [Uncaught Error] instead of capturing it via IsExceptionPending, so the C++ catch never gets the chance to fall through to the fallback.
4. 35bfa81 (current): move both paths into a small JS shim wrapped in try/catch. JS-level catch reliably traps the TypeError on every engine; direct path covers V8/Chakra; probe path (instanceof Blob check + getPrototypeOf + setPrototypeOf on the real napi-internal prototype) covers JSC. The shim is run once at Initialize via env.RunScript (Shared N-API) or Napi::Eval (JSI N-API).

Ball back in your court for re-review.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

Recommend landing #172 first. The JSC napi-shim bug it describes is the root cause of several workarounds in this PR — JS_PROTOTYPE_CHAIN_SHIM collapses to one direct setPrototypeOf call, the Napi::Eval / env.RunScript dispatch goes away, and the 14-line FileReader dual-exposure comment shrinks to one line. Quite a bit of code evaporates before merge if #172 lands first.

Other concerns inline.

[bkaradzic-microsoft]

Filed #174 with the JSC napi shim fix for #172. It replaces the old // BEGIN TODO ... END TODO[[Prototype]]-kludge in ConstructorInfo::Create with a proper .prototype property setup, and stops napi_define_class from going through napi_get_prototype (which per spec returns [[Prototype]], not .prototype). Includes a Blob-based regression test that asserts no instance members leak onto the global Object.prototype.

Once #174 lands, I'll rebase this branch onto main and:

• Collapse JS_PROTOTYPE_CHAIN_SHIM + the Napi::Eval / env.RunScript dispatch into a single direct C++ Object.setPrototypeOf(File.prototype, Blob.prototype) call (your Android NDK25 #51 + Support for POST requests and setting request headers #12 asks).
• Drop the 14-line FileReader StaticValue/InstanceValue comment block per Web polyfill constants: add missing InstanceValue exposure on WebSocket and XMLHttpRequest #173 (your Small inclusion fix for <exception> #109 ask).
• Address the remaining inline asks: dangling comment at FileReader.cpp:60 (your UWP isn't currently able to work with V8 #60), InstanceAccessor getters/setters for readyState/result/error + on* handlers (your JSC fix utf8 string creation #140, Fix WorkQueue destructor deadlock #147), early-out + throw-on-missing-Blob reordering in File::Initialize (your Expose napi_add_finalizer #75), promote azawadzki/base-n from BabylonNative to JsRH for base64 (your Build instructions for Android #15), file a Chakra throw-from-constructor tracker issue (your #1479).

Will respond on the shared_ptr thread separately — your UAF argument is right; reverting the m_selfRef simplification.

Re: splitting tests.ts per-polyfill (your #1400) — happy to do it in this PR or as a follow-up; let me know which you prefer.

[bkaradzic-microsoft]

Update: closing #174 without merging — the JSObjectMakeConstructor.prototype slot is ReadOnly per JSObjectRef.cpp:128, so JSObjectSetProperty calls put() on it and throws on every JSC backend. The naive workaround JSObjectMakeFunctionWithCallback produces a non-constructable result. The actually-correct fix needs to switch to JSObjectMake + JSClassDefinition with both callAsConstructor and callAsFunction, plus a refactor of the NativeInfo::Link / Query sentinel pattern — too big to bundle with this PR.

So #169 keeps its JS-shim wire-up (35bfa81). I'll address your remaining inline asks against the current shape:

• UWP isn't currently able to work with V8 #60 dangling FileReader.cpp comment — fix as suggested.
• Small inclusion fix for <exception> #109 drop the 14-line dual-exposure comment block per Web polyfill constants: add missing InstanceValue exposure on WebSocket and XMLHttpRequest #173 (Web polyfill constants: add missing InstanceValue exposure on WebSocket and XMLHttpRequest #173 is fine to land independently as the WHATWG pattern; the JSC pitfall stays, just keep one line that points at it).
• JSC fix utf8 string creation #140 / Fix WorkQueue destructor deadlock #147readyState/result/error/on* → InstanceAccessor getters/setters with C++-side state. This is the meatiest refactor; will require touching 7 jsThis.Set + 4 jsThis.Get sites.
• Build instructions for Android #15 promote azawadzki/base-n from BabylonNative to JsRH; FileReader consumes it. (Saves a third base64 impl; allows BN to drop the direct dep in a follow-up.)
• Expose napi_add_finalizer #75 reorder File::Initialize (early-out first, throw on missing Blob).
• Support for POST requests and setting request headers #12 clean up the misleading BABYLON_POLYFILL_USE_NAPI_JSI_EVAL macro (pick env.RunScript or Napi::Eval, drop the #if).
• #1479 file a JRH tracker for the Chakra throw-from-constructor limitation; reference it in the TODO.
• shared_ptr UAF revert — you're right, restoring std::shared_ptr<Napi::ObjectReference> per-lambda + dropping m_selfRef.
• #1400 tests.ts split — happy to do either way; defaulting to follow-up unless you'd rather see it here.

[bkaradzic-microsoft]

Pushed 3de01db addressing the non-controversial mechanical nits from the review. CI run 26855651151 — 22/22 green.

Changes in this commit:

• File.cpp: drop the misleading BABYLON_POLYFILL_USE_NAPI_JSI_EVAL macro and __has_include(<napi/env.h>) guard. Napi::Eval is declared on every backend (Shared N-API has it via Core/Node-API/Source/env.cc as a thin wrapper around env.RunScript; JSI N-API declares it directly in <napi/env.h>). The include is also already pulled in transitively via <Babylon/Polyfills/File.h>, so the guard always succeeded. Now a single unconditional Napi::Eval(...) call.
• File.cpp Initialize: reorder so the "already provided" check runs first (cheap no-op, common path on hosts with a native File), and throw Napi::Error on missing Blob instead of silently returning. Consumers wiring up the polyfill expect it to be installed — silent failures are hard to diagnose.
• FileReader.cpp MakeEvent: drop the dangling "JS polyfill that this C++ implementation replaces" reference (there's no such JS polyfill in this PR); replaced with a one-liner describing the actual ProgressEventloaded/total contract.
• FileReader.cpp DefineClass: collapse the 14-line dual-StaticValue/InstanceValue block down to one line pointing at Web polyfill constants: add missing InstanceValue exposure on WebSocket and XMLHttpRequest #173, since per Web polyfill constants: add missing InstanceValue exposure on WebSocket and XMLHttpRequest #173 that's the correct WHATWG IDL const member exposure pattern, not a workaround needing in-line justification.
• tests.ts: re-point the Chakra throw-from-constructor TODO to Chakra napi shim: throws from class constructors are swallowed (don't surface to JS) #175 (filed today) so the disabled expect(() => new (File as any)()).to.throw() test can be re-enabled atomically when the Chakra shim limitation is fixed.

Still to come (separate commits for review-clarity, in priority order):

• shared_ptr<Napi::ObjectReference> UAF revert in FileReader.cpp (your #r3344177855 ask) — restoring the per-lambda shared_ptr capture pattern that survives abort → drop → GC → engine-owned promise settles → onResolve fires.
• InstanceAccessor refactor for readyState / result / error / on* handlers (your JSC fix utf8 string creation #140 + Fix WorkQueue destructor deadlock #147 asks) — add C++ state members + Napi::FunctionReference slots, replace all the jsThis.Set/.Get sites.
• base-n promotion from BN to JRH (your Build instructions for Android #15 ask) — once it lands here, FileReader can drop the inline EncodeBase64 and BN's direct dependency on the same lib goes away in a follow-up.
• tests.ts per-polyfill split (your #1400 note that said "your call") — happy to defer unless you'd rather see it now.

#172 / #174 update: as noted in the follow-up comment on #174, the proper JSC .prototype fix needs a JSClassDefinition-based callable+constructable rewrite that's too big to bundle here. The JS shim approach in this PR stays as-is.

[bkaradzic-microsoft]

Rebased onto current main (now includes #177 + #179). fe457e5.

Both asks from your latest review addressed:

1. Chakra napi_wrap mutates instance [[Prototype]] chain #178 / isPrototypeOf workaround in tests.ts:1292-1297 — auto-resolved by the rebase. Stop mutating instance [[Prototype]] chain in napi_wrap on Chakra #179 already replaced the workaround block with the depth-1 Object.getPrototypeOf(blob) === Blob.prototype check on main, and rebasing pulled it in directly. grep -n 'isPrototypeOf\|#178' in tests.ts returns no matches.

2. File.cpp historical paragraph nit — applied the suggestion verbatim. The setPrototypeOf comment now sticks to why (WHATWG subtype + BJS-core instanceof Blob branches) and drops the how-we-got-here paragraph.

Rebase was clean — 8 commits replayed cleanly on top of a128e68, no conflicts. CI on the new head should be picking up shortly; will follow up once it lands.

[bkaradzic-microsoft]

Addressed the remaining review asks (pushed eaeaf20, a6155da):

• readyState / result / error are now InstanceAccessor getters backed by m_readyState / m_result / m_error. State-machine checks read the C++ members directly, so they can't be tampered with from JS, and the constructor no longer stamps instance data props.
• on* handlers collapsed to a single InstanceAccessor get/set pair (accessor data carries the event type) backed by a Napi::FunctionReference map that Dispatch consults directly.
• base64: removed the bespoke encoder. Promoted header-only azawadzki/base-n into JRH as a FetchContent INTERFACE lib (gated on JSRUNTIMEHOST_POLYFILL_FILE, pinned to the same 7573e77c commit BabylonNative uses), linked PRIVATE into File. EncodeBase64 now delegates to bn::encode_b64 + RFC 4648 padding; output is byte-for-byte identical. BN's direct base-n dep can be dropped as a follow-up.
• shared_ptr thread: already addressed by the member m_selfRef (corrected my earlier note).
• tests.ts split: filed Split Tests/UnitTests/Scripts/tests.ts into per-polyfill spec files #180 as a standalone follow-up to keep this diff focused.

All 18 review threads are now resolved.

[bkaradzic-microsoft]

CI on 14122f1 is green across every engine. The earlier 7 FileReader async-read failures were a real regression from the InstanceAccessor refactor: readAsText / readAsDataURL produce a stringresult, and the previous Napi::Reference<Napi::Value> stored it via napi_create_reference, which only accepts heap values (object/function/symbol) on real V8/JSC N-API. That threw on those backends (timeout on V8, uncaught C++ exception → SIGABRT on JSC/Linux) while Chakra's shim tolerated it. Fixed by boxing result/error as properties on a persistent holder object (Napi::ObjectReference m_state), which is always a valid reference target; state stays C++-owned and tamper-proof.

Verified green (all FileReader tests pass — 179 passing):

• V8: Win32_x64_V8, Android_V8, UWP_x64_V8
• JSC: Ubuntu_clang, Ubuntu_gcc, macOS_Xcode164_Sanitizers, iOS, Android_JSC
• Chakra: Win32_x86, UWP_x64
• JSI: Win32_x64_JSI, UWP_x64_JSI, UWP_arm64_JSI
• Sanitizers: Ubuntu ASAN/UBSan + ThreadSanitizer, macOS ThreadSanitizer

The only red job is Win32_x64_Chakra, failing solely on the unrelated, pre-existing flaky WebSocket > should connect correctly with one websocket connection test (2000 ms timeout against the external wss://ws.postman-echo.com/raw echo server; the sibling multi-connection test passes). All FileReader tests pass there too.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

[bkaradzic-microsoft]

@bghgary thanks for the careful review — all of your inline concerns are addressed in 61d9924:

• m_selfRef self-anchor is gone. Replaced with an externally-anchored shared_ptr<Napi::ObjectReference> captured by both promise reactions, matching the WebSocket lambda-owned-state convention. No self-cycle, no manual Reset() on terminal paths (all four deleted), and no dead-this race — the wrapper is guaranteed alive whenever a reaction runs. m_readId is kept solely as the abort/restart guard. Details + the one spec nuance (an in-flight read must keep the reader alive, so the wrapper can't simply be allowed to die) are in the resolved inline thread.

I also refreshed the PR description to match the current implementation (it had drifted): C++-backed readonly accessors for readyState/result/error and the on* slots, boxed result/error for primitive string results on real N-API, base-n for readAsDataURL (no more inlined encoder), and updated test counts (29 new: 15 File + 14 FileReader; 180/180 JS tests).

Verified Win32-x64 Chakra Debug green (180 JS + native gtests), and full CI is green across all engines/platforms. PTAL.

[bghgary]

[Reviewed by Copilot on behalf of @bghgary]

LGTM. All concerns addressed; the FileReader in-flight lifecycle/UAF fix is verified in 61d9924 — shared_ptr<ObjectReference> anchor captured by both promise reactions, no self-ref member, no manual Reset().