script: implement ReadableByteStreamController#35410
Merged
Taym95 merged 39 commits intoservo:mainfrom Mar 10, 2025
Merged
Conversation
Taym95
commented
Feb 10, 2025
Taym95
force-pushed
the
script-implement-ReadableByteStreamController
branch
2 times, most recently
from
16ea2f9 to
f704098
Compare
gterzian
reviewed
Feb 13, 2025
Member
gterzian
left a comment
gterzian
left a comment
There was a problem hiding this comment.
Looks good, with a few comments and questions.
Taym95
commented
Feb 18, 2025
Taym95
force-pushed
the
script-implement-ReadableByteStreamController
branch
from
7b411e6 to
8f7a130
Compare
Taym95
added
T-linux-wpt
|
🔨 Triggering try run (#13396547352) for Linux (WPT) |
|
|
|
🔨 Triggering try run (#13410367448) for Linux (WPT) |
|
|
Taym95
force-pushed
the
script-implement-ReadableByteStreamController
branch
from
6e7455d to
76c7942
Compare
gterzian
reviewed
Feb 19, 2025
Member
gterzian
left a comment
gterzian
left a comment
There was a problem hiding this comment.
LGTM on the buffer source use of new js functions: I've compared their use with that of Gecko in https://searchfox.org/mozilla-central/source/dom/streams/ReadableByteStreamController.cpp and they are the same.
|
🔨 Triggering try run (#13422686696) for Linux (WPT) |
|
Test results for linux-wpt-layout-2020 from try job (#13422686696): Flaky unexpected result (22)
Stable unexpected results that are known to be intermittent (16)
|
|
✨ Try run (#13422686696) succeeded. |
|
🔨 Triggering try run (#13437586384) for Linux (WPT) |
Signed-off-by: Taym Haddadi <[email protected]>
Signed-off-by: Taym Haddadi <[email protected]>
|
Test results for linux-wpt-layout-2020 from try job (#13708650290): Flaky unexpected result (13)
Stable unexpected results that are known to be intermittent (13)
|
|
✨ Try run (#13708650290) succeeded. |
Signed-off-by: Taym Haddadi <[email protected]>
Signed-off-by: Taym Haddadi <[email protected]>
Collaborator
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#51210) with upstreamable changes. |
Member
|
This looks like some sort of rebasing mistake: https://github.com/web-platform-tests/wpt/pull/51210/files |
gterzian
approved these changes
Mar 10, 2025
Member
gterzian
left a comment
gterzian
left a comment
There was a problem hiding this comment.
Lgtm, thanks for addressing my earlier comment about how the spec does early returns.
There are two tests that seem like they should pass?
Also it seems like some rebasing error with a css test slipped through.
Comment on lines
+24
to
+25
| [templated.any.html] | ||
| [ReadableStream with byte source (empty): instances have the correct methods and properties] |
Member
There was a problem hiding this comment.
This one and the one above seems like something that should pass?
Member
Author
There was a problem hiding this comment.
Ah yes, don't knew why was not picked up when running test!
Member
Author
There was a problem hiding this comment.
This still needed, check:
Member
There was a problem hiding this comment.
So those actually should fail: "assert_equals: has a pipeThrough method expected "function" but got "undefined""
Member
Author
There was a problem hiding this comment.
Yes tests failing because pipeThrough is not implemented
|
🔨 Triggering try run (#13766454248) for Linux (WPT) |
Collaborator
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51210). |
|
Test results for linux-wpt-layout-2020 from try job (#13766454248): Flaky unexpected result (19)
Stable unexpected results that are known to be intermittent (15)
Stable unexpected results (2)
|
|
|
Signed-off-by: Taym Haddadi <[email protected]>
Taym95
force-pushed
the
script-implement-ReadableByteStreamController
branch
from
d804dbf to
d3af3b3
Compare
Collaborator
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#51210). |
Member
Author
|
@jdm do you still want to review this one? |
Member
|
I'm recovering from an illness right now so it does not make sense to wait on me. |
Member
Author
Got it. Hope you feel better soon! I'll proceed without waiting on your review. |
Collaborator
|
⛔ Failed to properly merge the upstream pull request (web-platform-tests/wpt#51210). Please address any CI issues and try to merge manually. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #34676
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors