Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sync RTCDataChannelInit as per Web Specification #31701

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Sync RTCDataChannelInit as per Web Specification #31701

wants to merge 1 commit into from

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Aug 4, 2024 •
edited by webkit-early-warning-system
Loading 

Sync `RTCDataChannelInit` as per Web Specification
https://bugs.webkit.org/show_bug.cgi?id=277604

Reviewed by NOBODY (OOPS!).

This patch aligns 'RTCDataChannelInit' with Web Specification [1]:

[1] https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelinit

It adds '[EnforceRange]' to 'maxPacketLifeTime' and 'maxRetransmits'.

Additionally, I just did fly-by fix of adding link of web
specification for easier reference.

* Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:

b3ac3da

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792 Ahmad-S792 added the WebRTC For bugs in WebRTC label Aug 4, 2024
@Ahmad-S792 Ahmad-S792 self-assigned this Aug 4, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 4, 2024 •
edited
Loading

EWS run on previous version of this PR (hash e31dbdd)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792
Sync RTCDataChannelInit as per Web Specification
b3ac3da 
https://bugs.webkit.org/show_bug.cgi?id=277604

Reviewed by NOBODY (OOPS!).

This patch aligns 'RTCDataChannelInit' with Web Specification [1]:

[1] https://w3c.github.io/webrtc-pc/#dom-rtcdatachannelinit

It adds '[EnforceRange]' to 'maxPacketLifeTime' and 'maxRetransmits'.

Additionally, I just did fly-by fix of adding link of web
specification for easier reference.

* Source/WebCore/Modules/mediastream/RTCPeerConnection.idl:
@Ahmad-S792 Ahmad-S792 force-pushed the eng/Sync-RTCDataChannelInit-as-per-Web-Specification branch from e31dbdd to b3ac3da Compare August 4, 2024 12:32
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 4, 2024 •
edited
Loading

EWS run on current version of this PR (hash b3ac3da)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792 Ahmad-S792 marked this pull request as ready for review August 4, 2024 14:52
cdumez
cdumez requested changes Aug 5, 2024
View reviewed changes
Source/WebCore/Modules/mediastream/RTCPeerConnection.idl
[
Conditional=WEB_RTC,
EnabledBySetting=PeerConnectionEnabled
] dictionary RTCDataChannelInit {
boolean ordered = true;
unsigned short maxPacketLifeTime;
unsigned short maxRetransmits;
[EnforceRange] unsigned short maxPacketLifeTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is web observable and requires adding a layout test.

Source/WebCore/Modules/mediastream/RTCPeerConnection.idl
unsigned short maxPacketLifeTime;
unsigned short maxRetransmits;
[EnforceRange] unsigned short maxPacketLifeTime;
[EnforceRange] unsigned short maxRetransmits;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez requested changes

@youennf youennf Awaiting requested review from youennf

@philn philn Awaiting requested review from philn

Assignees

@Ahmad-S792 Ahmad-S792

Labels
WebRTC For bugs in WebRTC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ahmad-S792 @webkit-early-warning-system @cdumez