From 2ecf9016ba8b14f7759331a80382f22b30f9bf62 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 29 Sep 2025 20:23:09 +0800 Subject: [PATCH] Better support for Uint8Array in ReadableStream There's always going to be ambiguity between a string and a Uint8Array. We already had TypedArray(u8) as a discriminator when _returning_ values. But now the type is also used by mapping JS values to Zig. To support this efficiently when probing the union, the typed array mapping logic was extracted into its own function (so that it can be used by the probe). --- src/browser/fetch/Request.zig | 2 +- src/browser/fetch/Response.zig | 2 +- src/browser/streams/ReadableStream.zig | 41 +++- .../ReadableStreamDefaultController.zig | 6 +- .../streams/ReadableStreamDefaultReader.zig | 6 +- src/runtime/js.zig | 178 ++++++++++-------- src/tests/streams/readable_stream.html | 17 ++ 7 files changed, 156 insertions(+), 96 deletions(-) diff --git a/src/browser/fetch/Request.zig b/src/browser/fetch/Request.zig index 0674bcea..5881b427 100644 --- a/src/browser/fetch/Request.zig +++ b/src/browser/fetch/Request.zig @@ -181,7 +181,7 @@ pub fn constructor(input: RequestInput, _options: ?RequestInit, page: *Page) !Re pub fn get_body(self: *const Request, page: *Page) !?*ReadableStream { if (self.body) |body| { const stream = try ReadableStream.constructor(null, null, page); - try stream.queue.append(page.arena, body); + try stream.queue.append(page.arena, .{ .string = body }); return stream; } else return null; } diff --git a/src/browser/fetch/Response.zig b/src/browser/fetch/Response.zig index cf67b462..d9530fb1 100644 --- a/src/browser/fetch/Response.zig +++ b/src/browser/fetch/Response.zig @@ -109,7 +109,7 @@ pub fn constructor(_input: ?ResponseBody, _options: ?ResponseOptions, page: *Pag pub fn get_body(self: *const Response, page: *Page) !*ReadableStream { const stream = try ReadableStream.constructor(null, null, page); if (self.body) |body| { - try stream.queue.append(page.arena, body); + try stream.queue.append(page.arena, .{ .string = body }); } return stream; } diff --git a/src/browser/streams/ReadableStream.zig b/src/browser/streams/ReadableStream.zig index 762050a2..6cd67655 100644 --- a/src/browser/streams/ReadableStream.zig +++ b/src/browser/streams/ReadableStream.zig @@ -19,8 +19,9 @@ const std = @import("std"); const log = @import("../../log.zig"); -const Page = @import("../page.zig").Page; +const Allocator = std.mem.Allocator; const Env = @import("../env.zig").Env; +const Page = @import("../page.zig").Page; const ReadableStream = @This(); const ReadableStreamDefaultReader = @import("ReadableStreamDefaultReader.zig"); @@ -45,16 +46,42 @@ cancel_fn: ?Env.Function = null, pull_fn: ?Env.Function = null, strategy: QueueingStrategy, -queue: std.ArrayListUnmanaged([]const u8) = .empty, +queue: std.ArrayListUnmanaged(Chunk) = .empty, + +pub const Chunk = union(enum) { + // the order matters, sorry. + uint8array: Env.TypedArray(u8), + string: []const u8, + + pub fn dupe(self: Chunk, allocator: Allocator) !Chunk { + return switch (self) { + .string => |str| .{ .string = try allocator.dupe(u8, str) }, + .uint8array => |arr| .{ .uint8array = try arr.dupe(allocator) }, + }; + } +}; pub const ReadableStreamReadResult = struct { - const ValueUnion = - union(enum) { data: []const u8, empty: void }; - - value: ValueUnion, done: bool, + value: Value = .empty, - pub fn get_value(self: *const ReadableStreamReadResult) ValueUnion { + const Value = union(enum) { + empty, + data: Chunk, + }; + + pub fn init(chunk: Chunk, done: bool) ReadableStreamReadResult { + if (done) { + return .{ .done = true, .value = .empty }; + } + + return .{ + .done = false, + .value = .{ .data = chunk }, + }; + } + + pub fn get_value(self: *const ReadableStreamReadResult) Value { return self.value; } diff --git a/src/browser/streams/ReadableStreamDefaultController.zig b/src/browser/streams/ReadableStreamDefaultController.zig index 59ee5fb9..a6a6345d 100644 --- a/src/browser/streams/ReadableStreamDefaultController.zig +++ b/src/browser/streams/ReadableStreamDefaultController.zig @@ -51,17 +51,17 @@ pub fn _close(self: *ReadableStreamDefaultController, _reason: ?[]const u8, page // to discard, must use cancel. } -pub fn _enqueue(self: *ReadableStreamDefaultController, chunk: []const u8, page: *Page) !void { +pub fn _enqueue(self: *ReadableStreamDefaultController, chunk: ReadableStream.Chunk, page: *Page) !void { const stream = self.stream; if (stream.state != .readable) { return error.TypeError; } - const duped_chunk = try page.arena.dupe(u8, chunk); + const duped_chunk = try chunk.dupe(page.arena); if (self.stream.reader_resolver) |*rr| { - try rr.resolve(ReadableStreamReadResult{ .value = .{ .data = duped_chunk }, .done = false }); + try rr.resolve(ReadableStreamReadResult.init(duped_chunk, false)); self.stream.reader_resolver = null; } diff --git a/src/browser/streams/ReadableStreamDefaultReader.zig b/src/browser/streams/ReadableStreamDefaultReader.zig index c64c2d8d..9d1d7d47 100644 --- a/src/browser/streams/ReadableStreamDefaultReader.zig +++ b/src/browser/streams/ReadableStreamDefaultReader.zig @@ -49,7 +49,7 @@ pub fn _read(self: *const ReadableStreamDefaultReader, page: *Page) !Env.Promise const data = self.stream.queue.orderedRemove(0); const resolver = page.main_context.createPromiseResolver(); - try resolver.resolve(ReadableStreamReadResult{ .value = .{ .data = data }, .done = false }); + try resolver.resolve(ReadableStreamReadResult.init(data, false)); try self.stream.pullIf(); return resolver.promise(); } else { @@ -67,9 +67,9 @@ pub fn _read(self: *const ReadableStreamDefaultReader, page: *Page) !Env.Promise if (stream.queue.items.len > 0) { const data = self.stream.queue.orderedRemove(0); - try resolver.resolve(ReadableStreamReadResult{ .value = .{ .data = data }, .done = false }); + try resolver.resolve(ReadableStreamReadResult.init(data, false)); } else { - try resolver.resolve(ReadableStreamReadResult{ .value = .empty, .done = true }); + try resolver.resolve(ReadableStreamReadResult{ .done = true }); } return resolver.promise(); diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 62f82886..7d4c7974 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -1094,88 +1094,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { } }, .slice => { - var force_u8 = false; - var array_buffer: ?v8.ArrayBuffer = null; - if (js_value.isTypedArray()) { - const buffer_view = js_value.castTo(v8.ArrayBufferView); - array_buffer = buffer_view.getBuffer(); - } else if (js_value.isArrayBufferView()) { - force_u8 = true; - const buffer_view = js_value.castTo(v8.ArrayBufferView); - array_buffer = buffer_view.getBuffer(); - } else if (js_value.isArrayBuffer()) { - force_u8 = true; - array_buffer = js_value.castTo(v8.ArrayBuffer); - } - - if (array_buffer) |buffer| { - const backing_store = v8.BackingStore.sharedPtrGet(&buffer.getBackingStore()); - const data = backing_store.getData(); - const byte_len = backing_store.getByteLength(); - - switch (ptr.child) { - u8 => { - // need this sentinel check to keep the compiler happy - if (ptr.sentinel() == null) { - if (force_u8 or js_value.isUint8Array() or js_value.isUint8ClampedArray()) { - if (byte_len == 0) return &[_]u8{}; - const arr_ptr = @as([*]u8, @ptrCast(@alignCast(data))); - return arr_ptr[0..byte_len]; - } - } - }, - i8 => { - if (js_value.isInt8Array()) { - if (byte_len == 0) return &[_]i8{}; - const arr_ptr = @as([*]i8, @ptrCast(@alignCast(data))); - return arr_ptr[0..byte_len]; - } - }, - u16 => { - if (js_value.isUint16Array()) { - if (byte_len == 0) return &[_]u16{}; - const arr_ptr = @as([*]u16, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 2]; - } - }, - i16 => { - if (js_value.isInt16Array()) { - if (byte_len == 0) return &[_]i16{}; - const arr_ptr = @as([*]i16, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 2]; - } - }, - u32 => { - if (js_value.isUint32Array()) { - if (byte_len == 0) return &[_]u32{}; - const arr_ptr = @as([*]u32, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 4]; - } - }, - i32 => { - if (js_value.isInt32Array()) { - if (byte_len == 0) return &[_]i32{}; - const arr_ptr = @as([*]i32, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 4]; - } - }, - u64 => { - if (js_value.isBigUint64Array()) { - if (byte_len == 0) return &[_]u64{}; - const arr_ptr = @as([*]u64, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 8]; - } - }, - i64 => { - if (js_value.isBigInt64Array()) { - if (byte_len == 0) return &[_]i64{}; - const arr_ptr = @as([*]i64, @ptrCast(@alignCast(data))); - return arr_ptr[0 .. byte_len / 8]; - } - }, - else => {}, + if (ptr.sentinel() == null) { + if (try self.jsValueToTypedArray(ptr.child, js_value)) |value| { + return value; } - return error.InvalidArgument; } if (ptr.child == u8) { @@ -1282,6 +1204,12 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { return try self.createFunction(js_value); } + if (@hasDecl(T, "_TYPED_ARRAY_ID_KLUDGE")) { + const VT = @typeInfo(std.meta.fieldInfo(T, .values).type).pointer.child; + const arr = (try self.jsValueToTypedArray(VT, js_value)) orelse return null; + return .{ .values = arr }; + } + if (T == String) { return .{ .string = try valueToString(self.context_arena, js_value, self.isolate, self.v8_context) }; } @@ -1320,6 +1248,90 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { return value; } + fn jsValueToTypedArray(_: *JsContext, comptime T: type, js_value: v8.Value) !?[]T { + var force_u8 = false; + var array_buffer: ?v8.ArrayBuffer = null; + if (js_value.isTypedArray()) { + const buffer_view = js_value.castTo(v8.ArrayBufferView); + array_buffer = buffer_view.getBuffer(); + } else if (js_value.isArrayBufferView()) { + force_u8 = true; + const buffer_view = js_value.castTo(v8.ArrayBufferView); + array_buffer = buffer_view.getBuffer(); + } else if (js_value.isArrayBuffer()) { + force_u8 = true; + array_buffer = js_value.castTo(v8.ArrayBuffer); + } + + const buffer = array_buffer orelse return null; + + const backing_store = v8.BackingStore.sharedPtrGet(&buffer.getBackingStore()); + const data = backing_store.getData(); + const byte_len = backing_store.getByteLength(); + + switch (T) { + u8 => { + // need this sentinel check to keep the compiler happy + if (force_u8 or js_value.isUint8Array() or js_value.isUint8ClampedArray()) { + if (byte_len == 0) return &[_]u8{}; + const arr_ptr = @as([*]u8, @ptrCast(@alignCast(data))); + return arr_ptr[0..byte_len]; + } + }, + i8 => { + if (js_value.isInt8Array()) { + if (byte_len == 0) return &[_]i8{}; + const arr_ptr = @as([*]i8, @ptrCast(@alignCast(data))); + return arr_ptr[0..byte_len]; + } + }, + u16 => { + if (js_value.isUint16Array()) { + if (byte_len == 0) return &[_]u16{}; + const arr_ptr = @as([*]u16, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 2]; + } + }, + i16 => { + if (js_value.isInt16Array()) { + if (byte_len == 0) return &[_]i16{}; + const arr_ptr = @as([*]i16, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 2]; + } + }, + u32 => { + if (js_value.isUint32Array()) { + if (byte_len == 0) return &[_]u32{}; + const arr_ptr = @as([*]u32, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 4]; + } + }, + i32 => { + if (js_value.isInt32Array()) { + if (byte_len == 0) return &[_]i32{}; + const arr_ptr = @as([*]i32, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 4]; + } + }, + u64 => { + if (js_value.isBigUint64Array()) { + if (byte_len == 0) return &[_]u64{}; + const arr_ptr = @as([*]u64, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 8]; + } + }, + i64 => { + if (js_value.isBigInt64Array()) { + if (byte_len == 0) return &[_]i64{}; + const arr_ptr = @as([*]i64, @ptrCast(@alignCast(data))); + return arr_ptr[0 .. byte_len / 8]; + } + }, + else => {}, + } + return error.InvalidArgument; + } + fn createFunction(self: *JsContext, js_value: v8.Value) !Function { // caller should have made sure this was a function std.debug.assert(js_value.isFunction()); @@ -2387,6 +2399,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { const _TYPED_ARRAY_ID_KLUDGE = true; values: []const T, + + pub fn dupe(self: TypedArray(T), allocator: Allocator) !TypedArray(T) { + return .{ .values = try allocator.dupe(T, self.values) }; + } }; } diff --git a/src/tests/streams/readable_stream.html b/src/tests/streams/readable_stream.html index a8d71d66..a8339cc5 100644 --- a/src/tests/streams/readable_stream.html +++ b/src/tests/streams/readable_stream.html @@ -1,3 +1,4 @@ + + +