Skip to content

refactor: merge cross_realm_transform_* fields into one#37102

Merged
wusyong merged 2 commits intoservo:mainfrom
pewsheen:merge-cross-realm-transform-field
May 27, 2025
Merged

refactor: merge cross_realm_transform_* fields into one#37102
wusyong merged 2 commits intoservo:mainfrom
pewsheen:merge-cross-realm-transform-field

Conversation

@pewsheen
Copy link
Copy Markdown
Contributor

In #36977, when transferring TransformStream, CrossRealmTransform::Writable and CrossRealmTransform::Readable are set to different message ports. The message port will not be readable and writable at the same time when transferring the stream, so we can now merge cross_realm_transform_readable and cross_realm_transform_writable into a single field cross_realm_transform.

Testing: WPT (passed on try branch)
Fixes: #37084

@pewsheen pewsheen requested a review from gterzian as a code owner May 23, 2025 14:38
/// for the message port used by the transfered stream.
#[derive(Clone, JSTraceable, MallocSizeOf)]
#[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)]
pub(crate) enum CrossRealmTransform {
Copy link
Copy Markdown
Member

@Taym95 Taym95 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like https://streams.spec.whatwg.org/#generictransformstream, I am not sure if we should introduce a new type, what do you think @gterzian ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it applies here, otherwise the standard would have used it to specify the transfer. I think it's meant when actually making it available to script.

Copy link
Copy Markdown
Member

@gterzian gterzian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but one doc needs to be corrected.

@pewsheen pewsheen force-pushed the merge-cross-realm-transform-field branch from 9a18916 to 3e26c44 Compare May 26, 2025 14:01
@pewsheen pewsheen force-pushed the merge-cross-realm-transform-field branch from 3e26c44 to b05a3c3 Compare May 26, 2025 16:21
@wusyong wusyong added this pull request to the merge queue May 27, 2025
Merged via the queue into servo:main with commit d76b4a1 May 27, 2025
21 checks passed
@pewsheen pewsheen deleted the merge-cross-realm-transform-field branch June 16, 2025 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streams: rationalize management of cross realm transforms

4 participants