Bug 227571 - Fix StructuredClone for streams to handle BigInt64Array / BigUint64Array
Summary: Fix StructuredClone for streams to handle BigInt64Array / BigUint64Array
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-01 00:20 PDT by Timothy Gu
Modified: 2021-07-08 17:38 PDT (History)
12 users (show)

See Also:


Attachments
Patch (26.18 KB, patch)
2021-07-08 15:14 PDT, Yusuke Suzuki
cdumez: review+
cdumez: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Gu 2021-07-01 00:20:25 PDT
See https://github.com/heycam/webidl/pull/936. A summary of changes are:

- BigInt64Array and BigUint64Array are introduced as keywords of the Web IDL language
- The ArrayBufferView and BufferSource aliases now include BigInt64Array and BigUint64Array as part of the union.

It looks like this may already have been partially implemented, since

    new Blob([new BigInt64Array([0n])]);

appears to work as expected.

Tests: https://github.com/web-platform-tests/wpt/pull/27920
Comment 1 Radar WebKit Bug Importer 2021-07-08 00:21:18 PDT
<rdar://problem/80309572>
Comment 2 Yusuke Suzuki 2021-07-08 15:14:01 PDT
Created attachment 433168 [details]
Patch
Comment 3 Chris Dumez 2021-07-08 16:50:10 PDT
Comment on attachment 433168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433168&action=review

> Source/WebCore/ChangeLog:3
> +        [WebIDL] Add BigInt64Array and BigUint64Array support to IDL bindings generator

The title is misleading, this seems to only be fixing a bug in the streams implementation?

This does not impact the bindings generator in any way. Streams is not using the IDL binding generator (unfortunately).

> Source/WebCore/Modules/streams/ReadableStreamInternals.js:460
> +                @readableStreamDefaultControllerEnqueue(teeState.branch2.@readableStreamController, shouldClone ? @structuredCloneForFetch(result.value) : result.value);

Why is this called structuredCloneForFetch? Did you mean structuredCloneForStream?
Comment 4 Yusuke Suzuki 2021-07-08 16:58:53 PDT
Comment on attachment 433168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433168&action=review

Thanks!

>> Source/WebCore/ChangeLog:3
>> +        [WebIDL] Add BigInt64Array and BigUint64Array support to IDL bindings generator
> 
> The title is misleading, this seems to only be fixing a bug in the streams implementation?
> 
> This does not impact the bindings generator in any way. Streams is not using the IDL binding generator (unfortunately).

Changed the title to reflect the patch.

>> Source/WebCore/Modules/streams/ReadableStreamInternals.js:460
>> +                @readableStreamDefaultControllerEnqueue(teeState.branch2.@readableStreamController, shouldClone ? @structuredCloneForFetch(result.value) : result.value);
> 
> Why is this called structuredCloneForFetch? Did you mean structuredCloneForStream?

I used Fetch since the test is included in wpt/fetch, but it is OK to change it to Stream. Changed.
Comment 5 Yusuke Suzuki 2021-07-08 17:38:51 PDT
Committed r279769 (239539@main): <https://commits.webkit.org/239539@main>