Bug 227571

Summary: Fix StructuredClone for streams to handle BigInt64Array / BigUint64Array
Product: WebKit Reporter: Timothy Gu <timothygu99>
Component: BindingsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, calvaris, cdumez, clopez, darin, ews-watchlist, heycam, joepeck, sam, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch cdumez: review+, cdumez: commit-queue-

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>