Dom: Re-implement `ReadableStream` Part 1 : Default `Reader` and `Controller` by gterzian · Pull Request #34064 · servo/servo

gterzian · 2024-10-30T07:53:16Z

First part of #29088

Corresponding to #32898

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix ReadableStream: finish default controller and reader impl #32898

There are tests for these changes

gterzian · 2024-10-30T09:12:49Z

@Taym95 @wusyong I have rebased, but the new CanGc stuff I intentially did "wrong", by calling CanGc::note() where it was needed, instead of making it flow down from the original call. Leaving this as a TODO as part of clearning-up this branch.

There are also a couple of more TODO's in the code we need to address(at least one constructor, and the removing of native calls part of #32898)

I've added these as TODOs to #32898

sagudev · 2024-11-01T07:21:13Z

I fixed streams feature in mozjs to properly hide readablestream things behind streams feature.

github-actions · 2024-11-01T09:02:18Z

🔨 Triggering try run (#11626946713) for Linux WPT

gterzian · 2024-11-01T09:22:28Z

I am currently debugging the "Controller must have a stream when the should pull algo is called into." CRASH of tee.any.js. See #34097

github-actions · 2024-11-01T09:50:46Z

⚠️ Try run (#11626946713) failed.

sagudev · 2024-11-01T09:52:03Z

So many results that only job summary works: https://github.com/servo/servo/actions/runs/11626946713/attempts/1#summary-32381010382

sagudev · 2024-11-01T11:29:30Z

btw we also need to use_builtin_readable_stream=False when init webidl parser: sagudev@8f71968#diff-27c57f586c93e0626e7e988aeb22181aec12a417825fd07c922250538de52cc3R23

gterzian · 2024-11-15T05:33:51Z

Current status of the readablestream suite(which does not include fetch and other suites using streams):

Note the absence of crashes.

gterzian · 2024-11-19T05:39:54Z

At the next rebase, we can add the --signoff flag to fix that one failing check.

gterzian · 2024-11-22T05:37:37Z

Nice! /streams/readable-streams/templated.any.html is not only back to where it was, but we're apparently more correct than the old SM implementation by one test.

components/script/dom/teereadrequest.rs

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

* fix error handling in cancel steps Signed-off-by: gterzian <[email protected]> * in pull steps, reject promise if pull algo throws Signed-off-by: gterzian <[email protected]> * if start algorithm fails, rethrow the error Signed-off-by: gterzian <[email protected]> * when the strategy size fails, directly get the pending exception and use it to error the stream Signed-off-by: gterzian <[email protected]> * add error handling to enqueue value with size Signed-off-by: gterzian <[email protected]> * when enqueueing a value errors, ensure we error and stream with the same error used to throw an exception Signed-off-by: gterzian <[email protected]> --------- Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

* Implement readablestreamdefaulttee Signed-off-by: Taym <[email protected]> * Create UnderlyingSourceType::Tee each stream Signed-off-by: Taym <[email protected]> * Use Dom instead of DomRoot Signed-off-by: Taym <[email protected]> * Queue a microtask for readRequest chunk steps Signed-off-by: Taym <[email protected]> * fix create_readable_stream Signed-off-by: Taym <[email protected]> * Remove unnecessary Rc Signed-off-by: Taym <[email protected]> * Use correct doc link Signed-off-by: Taym <[email protected]> * Add #[allow(crown::unrooted_must_root)] Signed-off-by: Taym <[email protected]> * Fix crash in ClosedPromiseRejectionHandler Signed-off-by: Taym <[email protected]> * reflect TeeReadRequest and TeeUnderlyingSource Signed-off-by: Taym <[email protected]> * fix can_gc Signed-off-by: Taym <[email protected]> * reflect tee source, and fix use of mutable dom for tee source and request Signed-off-by: gterzian <[email protected]> * Fix typo that resolves multiple test failures in 'Tee' tests Signed-off-by: Taym <[email protected]> * Fix readable-streams/tee.any.js test Signed-off-by: Taym <[email protected]> --------- Signed-off-by: Taym <[email protected]> Signed-off-by: gterzian <[email protected]> Co-authored-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

…in default-reader.any.js (#34531) And fix crate::DomTypeHolder usage * Align ReadableStreamDefaultReader with spec and fix additional tests in default-reader.any.js Signed-off-by: Taym <[email protected]> * make reader rooted in Constructor and acquire_default_reader Signed-off-by: Taym <[email protected]> * Remove spaces Signed-off-by: Taym <[email protected]> --------- Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

* fetch stream chunks should be uint8 arrays Signed-off-by: gterzian <[email protected]> * fix clippy Signed-off-by: Taym Haddadi <[email protected]> --------- Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym Haddadi <[email protected]> Co-authored-by: Taym Haddadi <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

… one reason (#34560) Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Signed-off-by: Taym Haddadi <[email protected]>

* Fix compositeReason for DefaultTeeUnderlyingSource Signed-off-by: Taym Haddadi <[email protected]> * Update test Signed-off-by: Taym Haddadi <[email protected]> --------- Signed-off-by: Taym Haddadi <[email protected]>

* remove now unsused from_js method of readable stream * fix documenation of error steps * return type error instread of panicking on a todo, when trying to construct a stream of type bytes Signed-off-by: gterzian <[email protected]> --------- Signed-off-by: Gregory Terzian <[email protected]>

Signed-off-by: Gregory Terzian <[email protected]>

sagudev · 2024-12-17T20:56:59Z

Rebased in hope that #34670 fixed the problem.

gterzian · 2024-12-18T04:47:58Z

@sagudev Thanks for the fix.

gterzian force-pushed the readablestream-re-implementation branch from 97316af to 13a6c4a Compare October 30, 2024 09:11

mrobinson changed the title ~~Readablestream re implementation - the Default reader and controller part~~ dom: Re-implement ReadableStream Part 1 : Default Reader and Controller Oct 30, 2024

gterzian changed the title ~~dom: Re-implement ReadableStream Part 1 : Default Reader and Controller~~ Re-implement ReadableStream Part 1 : Default Reader and Controller Oct 30, 2024

gterzian changed the title ~~Re-implement ReadableStream Part 1 : Default Reader and Controller~~ Dom Re-implement ReadableStream Part 1 : Default Reader and Controller Oct 30, 2024

gterzian changed the title ~~Dom Re-implement ReadableStream Part 1 : Default Reader and Controller~~ Dom: Re-implement ReadableStream Part 1 : Default Reader and Controller Oct 30, 2024

gterzian force-pushed the readablestream-re-implementation branch from da9c79b to b460d74 Compare November 1, 2024 08:49

gterzian added the T-linux-wpt Do a try run of the WPT label Nov 1, 2024

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Nov 1, 2024

This was referenced Nov 1, 2024

Fix already mutably borrowed crash #34105

Merged

Refactor get_in_memory_bytes to return Option<Vec<u8>> and avoid unreachable! panic #34123

Merged

Set ReadableStreamDefaultReader in start_reading #34125

Merged

Taym95 mentioned this pull request Nov 10, 2024

Fix Unhandled rejection with value: object TypeError: stream is not locked #34204

Merged

5 tasks

Taym95 mentioned this pull request Nov 15, 2024

Use JSVal for ValueWithSize::value #34259

Merged

5 tasks

Taym95 force-pushed the readablestream-re-implementation branch from 6b8567b to ff9fcc2 Compare November 18, 2024 16:04

Taym95 mentioned this pull request Nov 18, 2024

Fix UnderlyingSourceContainer::call_start_algorithm #34277

Merged

5 tasks

Taym95 force-pushed the readablestream-re-implementation branch from f0859a0 to 93fa9e9 Compare December 9, 2024 12:02

Taym95 mentioned this pull request Dec 9, 2024

Fix compilation errors on stream-related DOM methods traits #34385

Closed

Taym95 reviewed Dec 9, 2024

View reviewed changes

components/script/dom/teereadrequest.rs Outdated Show resolved Hide resolved

Taym95 added the T-linux-wpt Do a try run of the WPT label Dec 9, 2024

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Dec 9, 2024

gterzian and others added 19 commits December 17, 2024 21:54

set correct default count queuing size strategy (#34389)

dc445ba

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

use proto in stream constructor (#34441)

a228e59

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

fix edge cases in get_desired_size (#34440)

7ec2a08

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

fix native use of streams (#34468)

967596f

Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Update wpt test for ReadableStream reimplementation (#34579)

5323e7c

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Fix ignore_malloc_size_of in readablestream tee (#34578)

beed082

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Remove incorrect use of handle array, fail test safely by giving only…

fc5c9ae

… one reason (#34560) Signed-off-by: gterzian <[email protected]> Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Update more wpt test for ReadableStream reimplementation (#34598)

2715e24

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Fix doc and rename Tee to DefaultTee (#34612)

448f076

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

fix: Address review comments

edf293a

Signed-off-by: Taym <[email protected]> Signed-off-by: Taym Haddadi <[email protected]>

Update response-stream-with-broken-then.any.js.ini test expectation

c30bb4e

Signed-off-by: Taym Haddadi <[email protected]>

fix reflect_dom_object can_gc

45b532e

Signed-off-by: Taym Haddadi <[email protected]>

Fix compositeReason for DefaultTeeUnderlyingSource (#34627)

3b51a89

* Fix compositeReason for DefaultTeeUnderlyingSource Signed-off-by: Taym Haddadi <[email protected]> * Update test Signed-off-by: Taym Haddadi <[email protected]> --------- Signed-off-by: Taym Haddadi <[email protected]>

fix crown rooting related errors (#34662)

11d511d

Signed-off-by: Gregory Terzian <[email protected]>

sagudev force-pushed the readablestream-re-implementation branch from e028b12 to 11d511d Compare December 17, 2024 20:54

sagudev enabled auto-merge December 17, 2024 20:55

sagudev added this pull request to the merge queue Dec 17, 2024

Merged via the queue into main with commit 379bbb4 Dec 17, 2024

sagudev deleted the readablestream-re-implementation branch December 17, 2024 22:04

gterzian mentioned this pull request Dec 18, 2024

Implement Streams #34676

Closed

27 tasks

sagudev mentioned this pull request Dec 18, 2024

Remove jsstreams servo/mozjs#540

Merged

gterzian mentioned this pull request Nov 7, 2025

script: Registration retrieval methods for service workers #39194

Merged

Uh oh!

Conversation

gterzian commented Oct 30, 2024 • edited by Taym95 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gterzian commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagudev commented Nov 1, 2024

Uh oh!

github-actions bot commented Nov 1, 2024

Uh oh!

gterzian commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 1, 2024

Uh oh!

sagudev commented Nov 1, 2024

Uh oh!

sagudev commented Nov 1, 2024

Uh oh!

gterzian commented Nov 15, 2024

Uh oh!

gterzian commented Nov 19, 2024

Uh oh!

gterzian commented Nov 22, 2024

Uh oh!

Uh oh!

sagudev commented Dec 17, 2024

Uh oh!

gterzian commented Dec 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gterzian commented Oct 30, 2024 •

edited by Taym95

Loading

gterzian commented Oct 30, 2024 •

edited

Loading

gterzian commented Nov 1, 2024 •

edited

Loading